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

Show warning annotation in MakeMemberStatic code fix preview if all r… #1989

Merged
merged 3 commits into from Jan 9, 2019

Conversation

mavasani
Copy link
Member

@mavasani mavasani commented Jan 8, 2019

…eferences are not updated

Fixes #1987

@mavasani mavasani requested review from genlu, heejaechang and a team January 8, 2019 23:33
Copy link
Member

@genlu genlu left a comment

Choose a reason for hiding this comment

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

Thanks!

{
cancellationToken.ThrowIfCancellationRequested();

var references = await SymbolFinder.FindReferencesAsync(symbol, solution, cancellationToken).ConfigureAwait(false);
if (references.Count() != 1)
{
return solution;
return (newSolution: solution, allReferencesFixed: true);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this still be false when references.Count() > 1?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Fixing that uncovered that the current implementation did not work for properties, as find all references returned references for cascaded accessors, and we were bailing out. I have fixed it along with unit tests in the latest commit.

Copy link
Member

Choose a reason for hiding this comment

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

Seems another argument for dotnet/roslyn#25628

@mavasani mavasani merged commit 2ce6d7c into dotnet:master Jan 9, 2019
@mavasani mavasani deleted the WarningAnnotation branch January 9, 2019 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants