Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix ([RCS1206](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1206.md)) ([#1049](https://github.com/JosefPihrt/Roslynator/pull/1049)).
- Prevent possible recursion in ([RCS1235](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1235.md)) ([#1054](https://github.com/JosefPihrt/Roslynator/pull/1054)).
- Fix ([RCS1223](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1223.md)) ([#1051](https://github.com/JosefPihrt/Roslynator/pull/1051)).
- Do not remove braces in the cases where there are overlapping local variables. ([RCS1031](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1013.md), [RCS1211](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1211.md), [RCS1208](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1208.md)) ([#1039](https://github.com/JosefPihrt/Roslynator/pull/1039)).
- Do not remove braces in the cases where there are overlapping local variables. ([RCS1031](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1013.md), [RCS1211](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1211.md), [RCS1208](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1208.md)) ([#1039](https://github.com/JosefPihrt/Roslynator/pull/1039),[#1058](https://github.com/JosefPihrt/Roslynator/pull/1058)).
- [CLI] Analyze command does not create the XML output file and returns incorrect exit code when only compiler diagnostics are reported ([#1056](https://github.com/JosefPihrt/Roslynator/pull/1056) by @PeterKaszab).
- [CLI] Fix exit code when multiple projects are processed ([#1061](https://github.com/JosefPihrt/Roslynator/pull/1061) by @PeterKaszab).
- Fix code fix for CS0164 ([#1031](https://github.com/JosefPihrt/Roslynator/pull/1031)).


## [4.2.0] - 2022-11-27

### Added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,20 @@ private static bool LocallyDeclaredVariablesOverlapWithAnyOtherSwitchSections(Sw

foreach (var otherSection in switchStatement.Sections)
{
// If the other section is not a block then we do not need to check as if there were overlapping variables then there would already be a error.
if (otherSection.Statements.SingleOrDefault(shouldThrow: false) is not BlockSyntax otherBlock)
if (otherSection.Span.Contains(switchBlock.Span))
continue;

if (otherBlock.Span == switchBlock.Span)
foreach (var label in otherSection.Labels)
{
if (label is not CasePatternSwitchLabelSyntax casePatternSwitchLabelSyntax)
continue;

if (PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(casePatternSwitchLabelSyntax.Pattern).Intersect(sectionDeclaredVariablesNames).Any())
return true;
}

// If the other section is not a block then we do not need to check as if there were overlapping variables then there would already be a error.
if (otherSection.Statements.SingleOrDefault(shouldThrow: false) is not BlockSyntax otherBlock)
continue;

foreach (var v in semanticModel.AnalyzeDataFlow(otherBlock)!.VariablesDeclared)
Expand All @@ -137,5 +146,4 @@ private static bool LocallyDeclaredVariablesOverlapWithAnyOtherSwitchSections(Sw
return false;
}


}
53 changes: 53 additions & 0 deletions src/CSharp/PattenMatchingVariableDeclarationHelper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace Roslynator.CSharp.Analysis;

public static class PattenMatchingVariableDeclarationHelper
{
public static IEnumerable<string> GetVariablesDeclared(PatternSyntax patternSyntax)
{
switch (patternSyntax)
{
case DeclarationPatternSyntax { Designation: var variableDesignationSyntax}:
return GetVariablesDeclared(variableDesignationSyntax);
case RecursivePatternSyntax { PositionalPatternClause: var positionalPatternClause, PropertyPatternClause: var propertyPatternClause, Designation: var designation }:
var designationVars = designation != null ? GetVariablesDeclared(designation):Array.Empty<string>();
var propertyVars = propertyPatternClause?.Subpatterns.SelectMany(p=>GetVariablesDeclared(p.Pattern)) ?? Array.Empty<string>();
var positionalVars = positionalPatternClause?.Subpatterns.SelectMany(p => GetVariablesDeclared(p.Pattern)) ?? Array.Empty<string>();
return designationVars.Concat(propertyVars).Concat(positionalVars);
case VarPatternSyntax { Designation: var variableDesignationSyntax}:
return GetVariablesDeclared(variableDesignationSyntax);
case BinaryPatternSyntax binaryPatternSyntax:
return GetVariablesDeclared(binaryPatternSyntax.Left)
.Concat(GetVariablesDeclared(binaryPatternSyntax.Right));
case ParenthesizedPatternSyntax parenthesizedPatternSyntax:
return GetVariablesDeclared(parenthesizedPatternSyntax.Pattern);
}
return Array.Empty<string>();
}

public static IEnumerable<string> GetVariablesDeclared(VariableDesignationSyntax? designationSyntax)
{
switch (designationSyntax)
{
case SingleVariableDesignationSyntax singleVariableDesignationSyntax:
yield return singleVariableDesignationSyntax.Identifier.ValueText;
break;
case ParenthesizedVariableDesignationSyntax parenthesizedVariableDesignationSyntax:
foreach (var variable in parenthesizedVariableDesignationSyntax.Variables)
{
foreach (var v in GetVariablesDeclared(variable))
{
yield return v;
}
}
break;
case DiscardDesignationSyntax _:
yield break;
}
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// Copyright (c) Josef Pihrt and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using System.Threading;
Expand Down Expand Up @@ -108,7 +111,16 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
return Fail(parent);
}

if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parent, semanticModel))
var parentBody = parent switch
{
ForStatementSyntax forStatementSyntax => forStatementSyntax.Statement,
ForEachStatementSyntax forEachStatementSyntax => forEachStatementSyntax.Statement,
DoStatementSyntax doStatementSyntax => doStatementSyntax.Statement,
WhileStatementSyntax whileStatementSyntax => whileStatementSyntax.Statement,
_ => throw new ArgumentOutOfRangeException()
};

if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parentBody, semanticModel))
return Fail(parent);

return Success(jumpKind, parent);
Expand All @@ -134,19 +146,35 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
return Fail(parent);
}

if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parent, semanticModel))
var parentBody = parent switch
{
ConstructorDeclarationSyntax constructorDeclarationSyntax => constructorDeclarationSyntax.Body,
DestructorDeclarationSyntax destructorDeclarationSyntax => destructorDeclarationSyntax.Body,
AccessorDeclarationSyntax accessorDeclarationSyntax => accessorDeclarationSyntax.Body,
_ => throw new NotImplementedException()
};

if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parentBody, semanticModel))
return Fail(parent);

return Success(jumpKind, parent);
}
case SyntaxKind.OperatorDeclaration:
case SyntaxKind.ConversionOperatorDeclaration:
case SyntaxKind.GetAccessorDeclaration:
{
{
if (jumpKind == SyntaxKind.None)
return Fail(parent);

if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parent, semanticModel))
var parentBody = parent switch
{
OperatorDeclarationSyntax operatorDeclarationSyntax => operatorDeclarationSyntax.Body,
ConversionOperatorDeclarationSyntax conversionOperatorDeclarationSyntax => conversionOperatorDeclarationSyntax.Body,
AccessorDeclarationSyntax accessorDeclarationSyntax=> accessorDeclarationSyntax.Body,
_ => throw new NotImplementedException()
};

if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parentBody, semanticModel))
return Fail(parent);

return Success(jumpKind, parent);
Expand Down Expand Up @@ -294,13 +322,31 @@ SemanticModel semanticModel
if (ifVariablesDeclared.IsEmpty)
return false;

var parentStatementDeclared = semanticModel.AnalyzeDataFlow(parent)!
.VariablesDeclared;

IEnumerable<ISymbol> parentVariablesDeclared;
if (parent is SwitchSectionSyntax switchSectionSyntax)
{
var allDeclaredVariables = new List<ISymbol>();
foreach (var statement in switchSectionSyntax.Statements)
{
allDeclaredVariables.AddRange(semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared);
}

parentVariablesDeclared = allDeclaredVariables;
}
else
{
parentVariablesDeclared = semanticModel.AnalyzeDataFlow(parent)!
.VariablesDeclared;
}

// The parent's declared variables will include those from the if and so we have to check for any symbols occurring twice.
return ifVariablesDeclared.Any(variable =>
parentStatementDeclared.Count(s => s.Name == variable.Name) > 1
);
foreach (var variable in ifVariablesDeclared)
{
if (parentVariablesDeclared.Count(s => s.Name == variable.Name) > 1)
return true;
}

return false;
}

private static bool IsNestedFix(SyntaxNode node, SemanticModel semanticModel, ReduceIfNestingOptions options, CancellationToken cancellationToken)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ void M()
}
");
}

[Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.RemoveUnnecessaryBracesInSwitchSection)]
public async Task TestNoDiagnostic_WhenOverlappingLocalVariableDeclaration()
{
Expand Down Expand Up @@ -267,4 +266,66 @@ void M()
}
");
}

[Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.RemoveUnnecessaryBracesInSwitchSection)]
public async Task TestNoDiagnostic_WhenOverlappingLocalVariableWithPatternMatchDeclaration()
{
await VerifyNoDiagnosticAsync(@"
using System;

class C
{
void M()
{
object o = null;

switch (o)
{
case string s:
var x = 1;
break;
default:
{
var s = 1;
break;
}
}
}
}
");
}

[Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.RemoveUnnecessaryBracesInSwitchSection)]
public async Task TestNoDiagnostic_WhenOverlappingLocalVariableWithRecursivePatternMatchDeclaration()
{
await VerifyNoDiagnosticAsync(@"
using System;

class C
{
class Wrapper
{
public string S;
}
void M()
{
object o = null;

switch (o)
{
case Wrapper { S: var s }:
var x = 1;
break;
default:
{
var s = 1;
break;
}
}
}
}
");
}
}


Loading