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 'invert if' refactor to properly enclose #region/#endregion blocks #74145

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

GiovanniBraconi
Copy link
Contributor

@GiovanniBraconi GiovanniBraconi commented Jun 25, 2024

PR Description

Fixes #73917

Context:

While this fix targets a specific case (InvertIfStyle.IfWithoutElse_MoveSubsequentStatementsToIfBody) of GetRootWithInvertIfStatement(), it's likely that similar issues exist in other cases within this method. As I'm new to the repository i wanted to start with this specific case to ensure the approach is correct before making broader changes.

@GiovanniBraconi GiovanniBraconi requested a review from a team as a code owner June 25, 2024 14:54
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Jun 25, 2024
@dotnet-policy-service dotnet-policy-service bot added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Jun 25, 2024
}
}
""");
}
Copy link
Member

Choose a reason for hiding this comment

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

can you have a test where the parent scope is a case block in a switch?

also a test where we're inverting an if-statement as a top level statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the switch case test 😃.

I also checked for the top level if-statement but the refactor is not suggested. Am i missing something?

@CyrusNajmabadi
Copy link
Member

This looks good. Just would like some extra tests. Thanks!

@GiovanniBraconi
Copy link
Contributor Author

Thanks for the review @CyrusNajmabadi 😄. I'll follow your suggestions ASAP.

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. untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Invert if" does not properly enclose an entire #region/#endregion
2 participants