-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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 broken 'convert if to switch' case with recursive analysis #76317
Fix broken 'convert if to switch' case with recursive analysis #76317
Conversation
@alrz can you let me know if this meets your expectations here? |
...Core/Portable/ConvertIfToSwitch/AbstractConvertIfToSwitchCodeRefactoringProvider.Analyzer.cs
Outdated
Show resolved
Hide resolved
…ToSwitchCodeRefactoringProvider.Analyzer.cs
// If we're in the initial state, it's fine for there to be many operations that follow. We're just | ||
// trying to check if the first one completes our analysis (and we'll not touch the ones that | ||
// follow). However, if we're actually in one of the recursive calls, it's *not* ok to ignore the | ||
// following ops as those may impact if the higher call into us is ok. So in that case, we do not | ||
// allow the parsing to succeed if we have more than one operation left. | ||
return topLevel |
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.
The topLevel distinction makes complete sense. I don't have the mental model on the inner workings of this right now but I might have missed other places where this matters.
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.
Thanks!
Fixes #71295