Fix S1871 FN: Support single line conditional block #9408
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| } | ||
|
|
||
| // https://github.com/SonarSource/sonar-dotnet/issues/1255 | ||
| public void ExceptionOfException(int a) |
Contributor
There was a problem hiding this comment.
Let's add also a test case exactly like this one but with an else clause at the end.
Same for VB and the switch cases (add a default clause).
sebastien-marichal
force-pushed
the
sma/1871-fn
branch
from
b787554 to
04bec24
Compare
martin-strecker-sonarsource
requested changes
Jun 6, 2024
analyzers/src/SonarAnalyzer.CSharp/Rules/ConditionalStructureSameImplementation.cs
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/ConditionalStructureSameImplementation.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/ConditionalStructureSameImplementation.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/ConditionalStructureSameImplementation.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/ConditionalStructureSameImplementation.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.VisualBasic/Rules/ConditionalStructureSameImplementation.cs
Show resolved
Hide resolved
analyzers/its/expected/akka.net/S1871-Akka.Remote.TestKit-netstandard2.0.json
Show resolved
Hide resolved
analyzers/its/expected/akka.net/S1871-Akka.Cluster.Sharding-netstandard2.0.json
Show resolved
Hide resolved
analyzers/its/expected/Ember-MM/S1871-generic.EmberCore.Notifications.json
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/ConditionalStructureSameImplementation.cs
Outdated
Show resolved
Hide resolved
sebastien-marichal
force-pushed
the
sma/1871-fn
branch
from
168c83e to
b2af07b
Compare
sebastien-marichal
requested review from
Tim-Pohlmann and
martin-strecker-sonarsource
Tim-Pohlmann
requested changes
Jun 10, 2024
Comment on lines
+111
to
+108
| ? switchSection.Statements.OfType<BlockSyntax>().SelectMany(x => x.Statements) | ||
| .Union(switchSection.Statements.Where(x => !x.IsKind(SyntaxKind.Block))) |
Contributor
There was a problem hiding this comment.
This looks wild. Can it be simplified? Otherwise, it needs a comment explaining what's going on.
Contributor
There was a problem hiding this comment.
Good comment! Now that I understand it, I can propose a simplification :D
Suggested change
| ? switchSection.Statements.OfType<BlockSyntax>().SelectMany(x => x.Statements) | |
| .Union(switchSection.Statements.Where(x => !x.IsKind(SyntaxKind.Block))) | |
| ? switchSection.Statements.SelectMany(x => x is BlockSyntax block ? block.Statements : x) |
analyzers/src/SonarAnalyzer.CSharp/Rules/ConditionalStructureSameImplementation.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/ConditionalStructureSameImplementation.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/ConditionalStructureSameImplementation.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/ConditionalStructureSameImplementation.cs
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.VisualBasic/Rules/ConditionalStructureSameImplementation.cs
Show resolved
Hide resolved
| [TestMethod] | ||
| public void ConditionalStructureSameImplementation_Switch_CSharp() => | ||
| builderCS.AddPaths("ConditionalStructureSameImplementation_Switch.cs").Verify(); | ||
| builderCS.AddPaths("ConditionalStructureSameImplementation_Switch.cs").WithOptions(ParseOptionsHelper.FromCSharp8).Verify(); |
Contributor
There was a problem hiding this comment.
The tests should follow Proposal B from the test case circle.
sebastien-marichal
force-pushed
the
sma/1871-fn
branch
2 times, most recently
from
817ece2 to
d05150d
Compare
sebastien-marichal
force-pushed
the
sma/1871-fn
branch
from
d05150d to
747168d
Compare
|
|
Tim-Pohlmann
approved these changes
Jun 11, 2024
Comment on lines
+111
to
+108
| ? switchSection.Statements.OfType<BlockSyntax>().SelectMany(x => x.Statements) | ||
| .Union(switchSection.Statements.Where(x => !x.IsKind(SyntaxKind.Block))) |
Contributor
There was a problem hiding this comment.
Good comment! Now that I understand it, I can propose a simplification :D
Suggested change
| ? switchSection.Statements.OfType<BlockSyntax>().SelectMany(x => x.Statements) | |
| .Union(switchSection.Statements.Where(x => !x.IsKind(SyntaxKind.Block))) | |
| ? switchSection.Statements.SelectMany(x => x is BlockSyntax block ? block.Statements : x) |
martin-strecker-sonarsource
approved these changes
Jun 12, 2024
Contributor
martin-strecker-sonarsource
left a comment
martin-strecker-sonarsource
left a comment
There was a problem hiding this comment.
LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1255