Skip to content

Commit

Permalink
Fix RCS1211 (dotnet#1095)
Browse files Browse the repository at this point in the history
Co-authored-by: Josef Pihrt <[email protected]>
  • Loading branch information
2 people authored and JochemHarmes committed Oct 30, 2023
1 parent 370b5aa commit c19dacc
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 41 deletions.
1 change: 1 addition & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix [RCS1216](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1216.md) ([#1094](https://github.com/JosefPihrt/Roslynator/pull/1094)).
- Fix [RCS1146](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1146.md) ([#1098](https://github.com/JosefPihrt/Roslynator/pull/1098)).
- Fix [RCS1154](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1154.md) ([#1105](https://github.com/JosefPihrt/Roslynator/pull/1105)).
- 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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -107,42 +107,4 @@ static bool AnalyzeTrivia(SyntaxTriviaList trivia)
}
}

private static bool LocallyDeclaredVariablesOverlapWithAnyOtherSwitchSections(SwitchStatementSyntax switchStatement, BlockSyntax switchBlock, SemanticModel semanticModel)
{
ImmutableArray<ISymbol> sectionVariablesDeclared = semanticModel.AnalyzeDataFlow(switchBlock)!
.VariablesDeclared;

if (sectionVariablesDeclared.IsEmpty)
return false;

ImmutableHashSet<string> 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;
}
}
13 changes: 11 additions & 2 deletions src/Analyzers/CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,17 @@ 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();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace Roslynator.CSharp.Analysis;

internal static class SwitchLocallyDeclaredVariablesHelper
{
internal static bool BlockDeclaredVariablesOverlapWithOtherSwitchSections(BlockSyntax block, SwitchStatementSyntax switchStatement, SemanticModel semanticModel)
{
ImmutableArray<ISymbol> sectionVariablesDeclared = semanticModel.AnalyzeDataFlow(block)!
.VariablesDeclared;

if (sectionVariablesDeclared.IsEmpty)
return false;

ImmutableHashSet<string> 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;
}
}
35 changes: 35 additions & 0 deletions src/Tests/Analyzers.Tests/RCS1211RemoveUnnecessaryElseTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
");
}
}

0 comments on commit c19dacc

Please sign in to comment.