-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Inline Method Refactoring - Fix bug for field with multiple declarations #78988
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
Inline Method Refactoring - Fix bug for field with multiple declarations #78988
Conversation
src/Features/Core/Portable/InlineMethod/AbstractInlineMethodRefactoringProvider.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/CSharpTest/CodeActions/InlineMethod/CSharpInlineMethodTests.cs
Outdated
Show resolved
Hide resolved
{ | ||
return fieldSymbol; | ||
foreach (var declarator in node.DescendantNodes().OfType<SyntaxNode>() | ||
.Where(n => _syntaxFacts.IsVariableDeclarator(n))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this is a case where you want the descendent nodes to not descend in once you've hit an IsVariableDeclarator node.
Imagine you have something like Action f = () => { var v = methodInvocation(); }
. You'll hit both the f = ...
declarator and the v = ...
declarator.
Technically, not the end of the world here as you validate you're getting an IFieldSymbol. But it's subtle and potentially a problem in the future. so better is to not descend any further once you hit the first layer of variable declarators.
does that make sense?
Fixes #76656