Fix/RCS1031, RCS1208 and RCS1061

ChangeLog.md

@@ -27,7 +27,7 @@ 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/RCS1031.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), [RCS1061](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1061.md)) ([#1039](https://github.com/JosefPihrt/Roslynator/pull/1039),[#1062](https://github.com/JosefPihrt/Roslynator/pull/1062)).
 - [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)).

src/Analyzers/CSharp/Analysis/MergeIfWithNestedIfAnalyzer.cs

@@ -7,7 +7,7 @@
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Text;
-using Roslynator.CSharp;
+using Roslynator.CSharp.Analysis.ReduceIfNesting;
 using Roslynator.CSharp.Syntax;
 
 namespace Roslynator.CSharp.Analysis;
@@ -71,7 +71,10 @@ private static void AnalyzeIfStatement(SyntaxNodeAnalysisContext context)
 
         if (!CheckTrivia(ifStatement, nestedIf.IfStatement))
             return;
-
+
+        if(IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, context.SemanticModel))
+            return;
+
         ReportDiagnostic(context, ifStatement, nestedIf.IfStatement);
     }
 

src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PattenMatchingVariableDeclarationHelper.cs

@@ -0,0 +1,39 @@
+// 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.Collections.Immutable;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+
+namespace Roslynator.CSharp.Analysis;
+
+public static class PattenMatchingVariableDeclarationHelper
+{
+    public static bool AnyDeclaredVariablesMatch(PatternSyntax pattern, ImmutableHashSet<string> variableNames)
+    {
+        return pattern switch
+        {
+            RecursivePatternSyntax { PositionalPatternClause: var positionalPatternClause, PropertyPatternClause: var propertyPatternClause, Designation: var designation } => 
+                (designation != null && AnyDeclaredVariablesMatch(designation, variableNames)) 
+                || (propertyPatternClause != null && propertyPatternClause.Subpatterns.Any(p => AnyDeclaredVariablesMatch(p.Pattern, variableNames)))
+                || (positionalPatternClause != null && positionalPatternClause.Subpatterns.Any(p => AnyDeclaredVariablesMatch(p.Pattern, variableNames))),
+            BinaryPatternSyntax binaryPattern => 
+                AnyDeclaredVariablesMatch(binaryPattern.Left, variableNames) 
+                || AnyDeclaredVariablesMatch(binaryPattern.Right, variableNames),
+            ParenthesizedPatternSyntax parenthesizedPattern => AnyDeclaredVariablesMatch(parenthesizedPattern.Pattern, variableNames),
+            DeclarationPatternSyntax { Designation: var variableDesignation } => AnyDeclaredVariablesMatch(variableDesignation, variableNames),
+            VarPatternSyntax { Designation: var variableDesignation } => AnyDeclaredVariablesMatch(variableDesignation, variableNames),
+            _ => false
+        };
+    }
+
+    internal static bool AnyDeclaredVariablesMatch(VariableDesignationSyntax designation, ImmutableHashSet<string> variableNames)
+    {
+        return designation switch
+        {
+            SingleVariableDesignationSyntax singleVariableDesignation => variableNames.Contains(singleVariableDesignation.Identifier.ValueText),
+            ParenthesizedVariableDesignationSyntax parenthesizedVariableDesignation => parenthesizedVariableDesignation.Variables.Any(variable => AnyDeclaredVariablesMatch(variable, variableNames)),
+            DiscardDesignationSyntax _ => false,
+            _ => false
+        };
+    }
+
+}

src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs

@@ -121,18 +121,26 @@ private static bool LocallyDeclaredVariablesOverlapWithAnyOtherSwitchSections(Sw
 
         foreach (SwitchSectionSyntax 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)
-                continue;
-
-            foreach (ISymbol v in semanticModel.AnalyzeDataFlow(otherBlock)!.VariablesDeclared)
+            foreach (var label in otherSection.Labels)
             {
-                if (sectionDeclaredVariablesNames.Contains(v.Name))
+                if (label is not CasePatternSwitchLabelSyntax casePatternSwitchLabel)
+                    continue;
+
+                if (PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(casePatternSwitchLabel.Pattern,sectionDeclaredVariablesNames))
                     return true;
             }
+
+            foreach (var statement in otherSection.Statements)
+            {
+                foreach (var v in semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared)
+                {
+                    if (sectionDeclaredVariablesNames.Contains(v.Name))
+                        return true;
+                }
+            }
         }
 
         return false;

src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs

@@ -0,0 +1,73 @@
+// 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.Collections.Generic;
+using System.Diagnostics;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+
+namespace Roslynator.CSharp.Analysis.ReduceIfNesting;
+
+internal static class IfStatementLocalVariableAnalysis
+{
+    public static bool DoDeclaredVariablesOverlapWithOuterScope(
+        IfStatementSyntax ifStatement,
+        SemanticModel semanticModel
+    )
+    {
+
+        var ifVariablesDeclared = semanticModel.AnalyzeDataFlow(ifStatement)!
+            .VariablesDeclared;
+
+        if (ifVariablesDeclared.IsEmpty)
+            return false;
+
+        var outerScope = GetOuterScope(ifStatement);
+
+        if (outerScope == null)
+            return true;
+
+        IList<ISymbol> parentVariablesDeclared;
+        if (outerScope is SwitchSectionSyntax switchSection)
+        {
+            parentVariablesDeclared = switchSection.Statements.SelectMany(s => semanticModel.AnalyzeDataFlow(s)!.VariablesDeclared).ToList();
+        }
+        else
+        {
+            parentVariablesDeclared = semanticModel.AnalyzeDataFlow(outerScope)!.VariablesDeclared;
+        }
+
+        Debug.Assert(parentVariablesDeclared.Count >= ifVariablesDeclared.Length);
+        // The parent's declared variables will include those from the if and so we have to check for any symbols occurring twice.
+        foreach (var variable in ifVariablesDeclared)
+        {
+            if (parentVariablesDeclared.Count(s => s.Name == variable.Name) > 1)
+                return true;
+        }
+
+        return false;
+    }
+
+    private static SyntaxNode GetOuterScope(IfStatementSyntax ifStatement)
+    {
+        return ifStatement.Parent switch
+        {
+            BlockSyntax block => block,
+            ForStatementSyntax forStatement => forStatement.Statement,
+            CommonForEachStatementSyntax forEachStatement => forEachStatement.Statement,
+            DoStatementSyntax doStatement => doStatement.Statement,
+            WhileStatementSyntax whileStatement => whileStatement.Statement,
+            SwitchSectionSyntax switchSection => switchSection,
+            ConstructorDeclarationSyntax constructorDeclaration => constructorDeclaration.Body,
+            DestructorDeclarationSyntax destructorDeclaration => destructorDeclaration.Body,
+            AccessorDeclarationSyntax accessorDeclaration => accessorDeclaration.Body,
+            OperatorDeclarationSyntax operatorDeclaration => operatorDeclaration.Body,
+            ConversionOperatorDeclarationSyntax conversionOperatorDeclaration => conversionOperatorDeclaration.Body,
+            MethodDeclarationSyntax methodDeclaration => methodDeclaration.Body,
+            LocalFunctionStatementSyntax localFunctionStatement => localFunctionStatement.Body,
+            AnonymousFunctionExpressionSyntax anonymousFunctionExpression => anonymousFunctionExpression.Block,
+            _ => null
+        };
+    }
+}

src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs

@@ -2,7 +2,6 @@
 
 using System.Collections.Immutable;
 using System.Diagnostics;
-using System.Linq;
 using System.Threading;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
@@ -73,7 +72,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
                 return Fail(switchSection);
             }
 
-            if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, switchSection, semanticModel))
+            if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel))
                 return Fail(switchSection);
 
             return Success(jumpKind, switchSection);
@@ -108,8 +107,8 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
             {
                 return Fail(parent);
             }
-
-            if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parent, semanticModel))
+            
+            if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel))
                 return Fail(parent);
 
             return Success(jumpKind, parent);
@@ -134,29 +133,29 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
                     {
                         return Fail(parent);
                     }
-
-                    if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parent, semanticModel))
+                    
+                    if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, 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))
+                    
+                    if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel))
                         return Fail(parent);
 
                     return Success(jumpKind, parent);
                 }
             case SyntaxKind.MethodDeclaration:
                 {
                     var methodDeclaration = (MethodDeclarationSyntax)parent;
-
-                    if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, methodDeclaration.Body, semanticModel))
+                    
+                    if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel))
                         return Fail(parent);
 
                     if (jumpKind != SyntaxKind.None)
@@ -189,8 +188,8 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
             case SyntaxKind.LocalFunctionStatement:
                 {
                     var localFunction = (LocalFunctionStatementSyntax)parent;
-
-                    if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, localFunction.Body, semanticModel))
+                    
+                    if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel))
                         return Fail(parent);
 
                     if (jumpKind != SyntaxKind.None)
@@ -224,7 +223,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
                 {
                     var anonymousFunction = (AnonymousFunctionExpressionSyntax)parent;
 
-                    if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, anonymousFunction.Block, semanticModel))
+                    if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel))
                         return Fail(parent);
 
                     if (jumpKind != SyntaxKind.None)
@@ -283,27 +282,6 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
         return Fail(parent);
     }
 
-    private static bool LocallyDeclaredVariablesOverlapWithOuterScope(
-        IfStatementSyntax ifStatement,
-        SyntaxNode parent,
-        SemanticModel semanticModel
-    )
-    {
-        ImmutableArray<ISymbol> ifVariablesDeclared = semanticModel.AnalyzeDataFlow(ifStatement)!
-            .VariablesDeclared;
-
-        if (ifVariablesDeclared.IsEmpty)
-            return false;
-
-        ImmutableArray<ISymbol> parentStatementDeclared = 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
-        );
-    }
-
     private static bool IsNestedFix(SyntaxNode node, SemanticModel semanticModel, ReduceIfNestingOptions options, CancellationToken cancellationToken)
     {
         options |= ReduceIfNestingOptions.AllowNestedFix;

src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs

@@ -267,4 +267,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;
+                }
+        }
+    }
+}
+");
+    }
+}
+
+