Skip to content

Fixed simple signature help bugs#59969

Merged
CyrusNajmabadi merged 17 commits intodotnet:mainfrom
DoctorKrolic:signature-help-simple-fixes
Mar 17, 2022
Merged

Fixed simple signature help bugs#59969
CyrusNajmabadi merged 17 commits intodotnet:mainfrom
DoctorKrolic:signature-help-simple-fixes

Conversation

@DoctorKrolic
Copy link
Copy Markdown
Contributor

Fixes: #33549, #23133

@DoctorKrolic DoctorKrolic requested a review from a team as a code owner March 5, 2022 18:39
@ghost ghost added Community The pull request was submitted by a contributor who is not a Microsoft employee. Area-IDE and removed Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Mar 5, 2022
@DoctorKrolic
Copy link
Copy Markdown
Contributor Author

A bit of context: I can't run integration tests on my PC because of a weird error (System.ArgumentException : Parameter is adjusted wrong. (Exception in HRESULT: 0x80070057 (E_INVALIDARG))) and repeating actions of tests in my VS produce different results, so I need to guess what valid results are, even though I fully understand why they began to fail after my changes

@jinujoseph jinujoseph added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Mar 8, 2022
@DoctorKrolic
Copy link
Copy Markdown
Contributor Author

@allisonchou I need help with integration tests. As mentioned earlyer, I can't run them on my PC because of an error. If I try to mimick what tests are doing on the stage object.Equals| pressing tab I get generated Equals method base:
devenv_NyHfrluVdG
So I can't fully mimick integration tests behaviour here. Tests broke because they are not fully correct. Previously, when invoking signature help on object.Equals($$), you got 2 suggestions: object.Equals(object obj) and object.Equals(object objA, object objB). The first one is incorrect because it is a signature of an instance, but not of a static method. So now the first and only signature help you get is object.Equals(object objA, object objB), but currently broken integration tests relied on previous incorrect variant. I tried to fix them being "blind", but it seems for me, that I've tried all possible combinations there without reaching success.

@DoctorKrolic
Copy link
Copy Markdown
Contributor Author

Closing due to merge conflicts and my inability to fix integration tests

@DoctorKrolic DoctorKrolic deleted the signature-help-simple-fixes branch March 11, 2022 13:34
@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

hey @DoctorKrolic i'm happy to help out with this :) We're jsut a little slammed right now getting some release stuff out. Can we revisit this next week? Thanks!

@DoctorKrolic DoctorKrolic restored the signature-help-simple-fixes branch March 11, 2022 15:39
@DoctorKrolic
Copy link
Copy Markdown
Contributor Author

@CyrusNajmabadi Ok)

@DoctorKrolic DoctorKrolic reopened this Mar 11, 2022
Copy link
Copy Markdown
Contributor

@allisonchou allisonchou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DoctorKrolic lgtm except for small nit comments. I took a look at the failing integration tests. The CPS failure is a known flaky test and should hopefully pass on re-run. I'm happy to update the rest for you in a bit

Copy link
Copy Markdown
Contributor

@CyrusNajmabadi CyrusNajmabadi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@CyrusNajmabadi CyrusNajmabadi merged commit d40bc9c into dotnet:main Mar 17, 2022
@ghost ghost added this to the Next milestone Mar 17, 2022
@DoctorKrolic DoctorKrolic deleted the signature-help-simple-fixes branch March 17, 2022 20:49
@allisonchou allisonchou modified the milestones: Next, 17.2.P3 Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Instance member signatures are shown in a static member context

4 participants