Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Go To Base for a symbol with a single metadata location #58965

Merged
merged 2 commits into from
Jan 21, 2022

Conversation

davidwengier
Copy link
Contributor

Fixes #58964

Not sure when this broke, but it works in 17.0.4 and not in 17.1 Preview 2.

#55891 is too early and #58617 is too late, and it still repro'd when I reverted #58051

Nothing else jumped out at me as a possible culprit ¯\_(ツ)_/¯

@davidwengier davidwengier requested a review from a team as a code owner January 20, 2022 06:16
@ryzngard
Copy link
Contributor

LGTM, PR does exactly what was described.

Nothing else jumped out at me as a possible culprit ¯_(ツ)_/¯

Is it possible there's a non Roslyn change between 17.0 and 17.1 that impacts this behavior?

@davidwengier davidwengier changed the title Switch to the UI thread before talking to the platform Fix Go To Base for a symbol with a single metadata source.3 Jan 20, 2022
@davidwengier davidwengier changed the title Fix Go To Base for a symbol with a single metadata source.3 Fix Go To Base for a symbol with a single metadata source Jan 20, 2022
@davidwengier davidwengier changed the title Fix Go To Base for a symbol with a single metadata source Fix Go To Base for a symbol with a single metadata location Jan 20, 2022
@davidwengier
Copy link
Contributor Author

@sharwell for all of my UI thread needs

@Cosifne
Copy link
Member

Cosifne commented Jan 20, 2022

Is it possible to have an integration test to cover this scenario? (Not blocking this PR)
Also, do we want to consider taking it to 17.1?

@davidwengier
Copy link
Contributor Author

Is it possible to have an integration test to cover this scenario? (Not blocking this PR)

Yes, definitely.

Also, do we want to consider taking it to 17.1?

My gut feeling is that since this only affects metadata, and only if there is a single metadata result, probably not a big worry. I doubt it has high usage.

@davidwengier
Copy link
Contributor Author

davidwengier commented Jan 21, 2022

Added an integration test... enabling auto-merge..

@davidwengier davidwengier enabled auto-merge (squash) January 21, 2022 06:00
@davidwengier davidwengier merged commit bd1a381 into dotnet:main Jan 21, 2022
@ghost ghost added this to the Next milestone Jan 21, 2022
@davidwengier davidwengier deleted the FixGoToBase branch January 21, 2022 07:35
333fred added a commit to 333fred/roslyn that referenced this pull request Jan 21, 2022
* upstream/main: (1035 commits)
  Add missing header
  Mark IVSTypeScriptFormattingServiceImplementation as optional, but require it in the constructor
  Fix Go To Base for a symbol with a single metadata location (dotnet#58965)
  [EnC] Store entire spans instead of line deltas (dotnet#58831)
  Delete CodeAnalysisRules.ruleset (dotnet#58968)
  Allow xml docs to still be created when 'emit metadata only' is on. (dotnet#57667)
  Fix ParseVBErrorOrWarning (dotnet#47833)
  Update parameter nullability to match implementation
  Ensure CSharpUseParameterNullCheckingDiagnosticAnalyzer works with nullable parameters
  Add tests for issues fixed by previous PR (dotnet#58764)
  Update src/Features/CSharp/Portable/Completion/CompletionProviders/ExplicitInterfaceMemberCompletionProvider.CompletionSymbolDisplay.cs
  Disallow null checking discard parameters (dotnet#58952)
  Add extension method
  Escape type arguments
  Few fixes
  Update tests.
  Add Analyzers layer to CODEOWNERS
  Add formatting analyzer test for param nullchecking (dotnet#58936)
  Move reading HideAdvancedMembers option up (dotnet#58747)
  List patterns: Slice value is assumed to be never null (dotnet#57457)
  ...
@RikkiGibson RikkiGibson modified the milestones: Next, 17.2.P1 Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Go To Base on a member with a single base in metadata fails to navigate
4 participants