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 RCS1211 #1095

Merged
merged 10 commits into from
Jul 12, 2023
1 change: 1 addition & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,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;
}
}
");
}
}