From 0dc36e95cff3f2fdadcb66e0a6d4e727264159fd Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Tue, 6 Jun 2023 22:19:20 +0100 Subject: [PATCH 1/7] fix and test --- ...nnecessaryBracesInSwitchSectionAnalyzer.cs | 40 +-------------- .../Analysis/RemoveUnnecessaryElseAnalyzer.cs | 11 ++++- .../SwitchLocallyDeclaredVariablesHelper.cs | 49 +++++++++++++++++++ .../RCS1211RemoveUnnecessaryElseTests.cs | 35 +++++++++++++ 4 files changed, 94 insertions(+), 41 deletions(-) create mode 100644 src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs diff --git a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs index a0c3b184e9..024aeb9822 100644 --- a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs @@ -93,7 +93,7 @@ public static void AnalyzerSwitchSection(SyntaxNodeAnalysisContext context) // If any of the other case blocks contain a definition for the same local variables then removing the braces would introduce a new error. if (switchSection.Parent is SwitchStatementSyntax switchStatement - && LocallyDeclaredVariablesOverlapWithAnyOtherSwitchSections(switchStatement, block, context.SemanticModel)) + && SwitchLocallyDeclaredVariablesHelper.BlockDeclaredVariablesOverlapWithOtherSwitchSections(block, switchStatement, context.SemanticModel)) { return; } @@ -107,42 +107,4 @@ static bool AnalyzeTrivia(SyntaxTriviaList trivia) } } - private static bool LocallyDeclaredVariablesOverlapWithAnyOtherSwitchSections(SwitchStatementSyntax switchStatement, BlockSyntax switchBlock, SemanticModel semanticModel) - { - ImmutableArray sectionVariablesDeclared = semanticModel.AnalyzeDataFlow(switchBlock)! - .VariablesDeclared; - - if (sectionVariablesDeclared.IsEmpty) - return false; - - ImmutableHashSet sectionDeclaredVariablesNames = sectionVariablesDeclared - .Select(s => s.Name) - .ToImmutableHashSet(); - - foreach (SwitchSectionSyntax otherSection in switchStatement.Sections) - { - if (otherSection.Span.Contains(switchBlock.Span)) - continue; - - foreach (SwitchLabelSyntax label in otherSection.Labels) - { - if (label is not CasePatternSwitchLabelSyntax casePatternSwitchLabel) - continue; - - if (PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(casePatternSwitchLabel.Pattern, sectionDeclaredVariablesNames)) - return true; - } - - foreach (StatementSyntax statement in otherSection.Statements) - { - foreach (ISymbol symbol in semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared) - { - if (sectionDeclaredVariablesNames.Contains(symbol.Name)) - return true; - } - } - } - - return false; - } } diff --git a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs index 3abab1c3f4..caeca59689 100644 --- a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs @@ -61,8 +61,15 @@ private static bool IsFixable(ElseClauseSyntax elseClause, SemanticModel semanti if (ifStatementStatement is not BlockSyntax ifBlock) return CSharpFacts.IsJumpStatement(ifStatementStatement.Kind()); - if (elseClause.Statement is BlockSyntax elseBlock && LocalDeclaredVariablesOverlap(elseBlock, ifBlock, semanticModel)) - return false; + if (elseClause.Statement is BlockSyntax elseBlock) + { + if(LocalDeclaredVariablesOverlap(elseBlock, ifBlock, semanticModel)) + return false; + + if (ifStatement.Parent is SwitchSectionSyntax { Parent: SwitchStatementSyntax switchStatement } + && SwitchLocallyDeclaredVariablesHelper.BlockDeclaredVariablesOverlapWithOtherSwitchSections(elseBlock, switchStatement, semanticModel)) + return false; + } StatementSyntax lastStatementInIf = ifBlock.Statements.LastOrDefault(); diff --git a/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs b/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs new file mode 100644 index 0000000000..a063d5a7e6 --- /dev/null +++ b/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs @@ -0,0 +1,49 @@ +using System.Collections.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace Roslynator.CSharp.Analysis; + +public static class SwitchLocallyDeclaredVariablesHelper +{ + internal static bool BlockDeclaredVariablesOverlapWithOtherSwitchSections(BlockSyntax block, SwitchStatementSyntax switchStatement, SemanticModel semanticModel) + { + ImmutableArray sectionVariablesDeclared = semanticModel.AnalyzeDataFlow(block)! + .VariablesDeclared; + + if (sectionVariablesDeclared.IsEmpty) + return false; + + ImmutableHashSet sectionDeclaredVariablesNames = sectionVariablesDeclared + .Select(s => s.Name) + .ToImmutableHashSet(); + + foreach (SwitchSectionSyntax otherSection in switchStatement.Sections) + { + if (otherSection.Span.Contains(block.Span)) + continue; + + foreach (SwitchLabelSyntax label in otherSection.Labels) + { + if (label is not CasePatternSwitchLabelSyntax casePatternSwitchLabel) + continue; + + if (PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(casePatternSwitchLabel.Pattern, sectionDeclaredVariablesNames)) + return true; + } + + foreach (StatementSyntax statement in otherSection.Statements) + { + foreach (ISymbol symbol in semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared) + { + if (sectionDeclaredVariablesNames.Contains(symbol.Name)) + return true; + } + } + } + + return false; + } + +} \ No newline at end of file diff --git a/src/Tests/Analyzers.Tests/RCS1211RemoveUnnecessaryElseTests.cs b/src/Tests/Analyzers.Tests/RCS1211RemoveUnnecessaryElseTests.cs index 902a6a2a01..2034e5037b 100644 --- a/src/Tests/Analyzers.Tests/RCS1211RemoveUnnecessaryElseTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1211RemoveUnnecessaryElseTests.cs @@ -66,6 +66,41 @@ int M(bool flag) } } } +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.RemoveUnnecessaryElse)] + public async Task TestNoDiagnostic_OverlappingLocalVariablesWithSwitch() + { + await VerifyNoDiagnosticAsync(@" +class C +{ + int M(int i, bool flag) + { + switch(i) + { + case 0: + if(flag) + { + var z = 1; + return z; + } + break; + case 1: + if(flag) + { + var y = 1; + return y; + } + else + { + var z = 1; + return z; + } + } + return 2; + } +} "); } } From 61fd2ede681fc200ace372f730d1c58c92d37396 Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Tue, 6 Jun 2023 22:22:29 +0100 Subject: [PATCH 2/7] formatting --- .../CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs | 4 ++-- .../CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs | 2 -- .../Analyzers.Tests/RCS1211RemoveUnnecessaryElseTests.cs | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs index caeca59689..5bde622074 100644 --- a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs @@ -63,10 +63,10 @@ private static bool IsFixable(ElseClauseSyntax elseClause, SemanticModel semanti if (elseClause.Statement is BlockSyntax elseBlock) { - if(LocalDeclaredVariablesOverlap(elseBlock, ifBlock, semanticModel)) + if (LocalDeclaredVariablesOverlap(elseBlock, ifBlock, semanticModel)) return false; - if (ifStatement.Parent is SwitchSectionSyntax { Parent: SwitchStatementSyntax switchStatement } + if (ifStatement.Parent is SwitchSectionSyntax { Parent: SwitchStatementSyntax switchStatement } && SwitchLocallyDeclaredVariablesHelper.BlockDeclaredVariablesOverlapWithOtherSwitchSections(elseBlock, switchStatement, semanticModel)) return false; } diff --git a/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs b/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs index a063d5a7e6..2e42d526ae 100644 --- a/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs +++ b/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs @@ -42,8 +42,6 @@ internal static bool BlockDeclaredVariablesOverlapWithOtherSwitchSections(BlockS } } } - return false; } - } \ No newline at end of file diff --git a/src/Tests/Analyzers.Tests/RCS1211RemoveUnnecessaryElseTests.cs b/src/Tests/Analyzers.Tests/RCS1211RemoveUnnecessaryElseTests.cs index 2034e5037b..a96f4d595d 100644 --- a/src/Tests/Analyzers.Tests/RCS1211RemoveUnnecessaryElseTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1211RemoveUnnecessaryElseTests.cs @@ -72,7 +72,7 @@ int M(bool flag) [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.RemoveUnnecessaryElse)] public async Task TestNoDiagnostic_OverlappingLocalVariablesWithSwitch() { - await VerifyNoDiagnosticAsync(@" + await VerifyNoDiagnosticAsync(@" class C { int M(int i, bool flag) From 2efa0a5b07f4a5176e253fcaff2f91d14b502dfc Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Tue, 6 Jun 2023 22:29:42 +0100 Subject: [PATCH 3/7] update change log --- ChangeLog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog.md b/ChangeLog.md index c490bcb686..bda0db64bc 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Improve inversion of logical expressions to handling additional cases ([#1086](https://github.com/josefpihrt/roslynator/pull/1086)). - Fix [RCS1084](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1084.md) ([#1085](https://github.com/josefpihrt/roslynator/pull/1085)). - Fix [RCS1169](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1169.md) ([#1092](https://github.com/JosefPihrt/Roslynator/pull/1092)). +- Fix [RCS1211](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1211.md) ([#1095](https://github.com/JosefPihrt/Roslynator/pull/1095)). ## [4.3.0] - 2023-04-24 From 0b48de7f4ed0f1b05ef57c17da232357a796d078 Mon Sep 17 00:00:00 2001 From: James Hargreaves Date: Wed, 12 Jul 2023 18:08:46 +0200 Subject: [PATCH 4/7] Update src/Analyzers/CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs Co-authored-by: Josef Pihrt --- src/Analyzers/CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs index 5bde622074..f1d104c584 100644 --- a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs @@ -68,7 +68,9 @@ private static bool IsFixable(ElseClauseSyntax elseClause, SemanticModel semanti if (ifStatement.Parent is SwitchSectionSyntax { Parent: SwitchStatementSyntax switchStatement } && SwitchLocallyDeclaredVariablesHelper.BlockDeclaredVariablesOverlapWithOtherSwitchSections(elseBlock, switchStatement, semanticModel)) + { return false; + } } StatementSyntax lastStatementInIf = ifBlock.Statements.LastOrDefault(); From fb251d276d4a8c6b475568f768121df18f0cead2 Mon Sep 17 00:00:00 2001 From: James Hargreaves Date: Wed, 12 Jul 2023 18:08:52 +0200 Subject: [PATCH 5/7] Update src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs Co-authored-by: Josef Pihrt --- .../CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs b/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs index 2e42d526ae..5650dd8890 100644 --- a/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs +++ b/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs @@ -5,7 +5,7 @@ namespace Roslynator.CSharp.Analysis; -public static class SwitchLocallyDeclaredVariablesHelper +internal static class SwitchLocallyDeclaredVariablesHelper { internal static bool BlockDeclaredVariablesOverlapWithOtherSwitchSections(BlockSyntax block, SwitchStatementSyntax switchStatement, SemanticModel semanticModel) { From dadd6166498a6dc1580b980da7b2938fc2614d60 Mon Sep 17 00:00:00 2001 From: James Hargreaves Date: Wed, 12 Jul 2023 18:08:58 +0200 Subject: [PATCH 6/7] Update src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs Co-authored-by: Josef Pihrt --- .../CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs b/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs index 5650dd8890..f40128acd3 100644 --- a/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs +++ b/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs @@ -42,6 +42,7 @@ internal static bool BlockDeclaredVariablesOverlapWithOtherSwitchSections(BlockS } } } + return false; } } \ No newline at end of file From 47b6e1006c2e954ca61754e54568a7077556f6a7 Mon Sep 17 00:00:00 2001 From: James Hargreaves Date: Wed, 12 Jul 2023 18:09:06 +0200 Subject: [PATCH 7/7] Update src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs Co-authored-by: Josef Pihrt --- .../CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs b/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs index f40128acd3..af19591b7e 100644 --- a/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs +++ b/src/Analyzers/CSharp/Analysis/SwitchLocallyDeclaredVariablesHelper.cs @@ -45,4 +45,4 @@ internal static bool BlockDeclaredVariablesOverlapWithOtherSwitchSections(BlockS return false; } -} \ No newline at end of file +}