Stop RCS1031, RCS1208 and RCS1211 from being applied if the local variables overlap.

ChangeLog.md

@@ -17,6 +17,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Do not remove parameterless empty constructor in a struct with field initializers ([RCS1074](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1074.md)) ([#1021](https://github.com/josefpihrt/roslynator/pull/1021)).
 - Do not suggest to use generic event handler ([RCS1159](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1159.md)) ([#1022](https://github.com/josefpihrt/roslynator/pull/1022)).
 - Fix ([RCS1077](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1077.md)) ([#1023](https://github.com/josefpihrt/roslynator/pull/1023)).
+- Fix ([RCS1031](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1013.md)) ([#1039](https://github.com/JosefPihrt/Roslynator/pull/1039)).
+- Fix ([RCS1211](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1211.md)) ([#1039](https://github.com/JosefPihrt/Roslynator/pull/1039)).
+- Fix ([RCS1208](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1208.md)) ([#1039](https://github.com/JosefPihrt/Roslynator/pull/1039)).
 
 ## [4.2.0] - 2022-11-27
 

src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs

@@ -1,6 +1,7 @@
 // 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 System.Linq;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -90,6 +91,11 @@ public static void AnalyzerSwitchSection(SyntaxNodeAnalysisContext context)
         if (!AnalyzeTrivia(closeBrace.TrailingTrivia))
             return;
 
+        // If any of the other case blocks contain a definition for the same local variables then removing the brackets would introduce a new error.
+        if (switchSection.Parent is SwitchStatementSyntax switchStatement &&
+            LocalDeclaredVariablesOverlapWithAnyOtherSwitchSections(switchStatement, switchSection, context.SemanticModel))
+            return;
+
         DiagnosticHelpers.ReportDiagnostic(context, DiagnosticRules.RemoveUnnecessaryBracesInSwitchSection, openBrace);
         DiagnosticHelpers.ReportDiagnostic(context, DiagnosticRules.RemoveUnnecessaryBracesInSwitchSectionFadeOut, closeBrace);
 
@@ -98,4 +104,27 @@ static bool AnalyzeTrivia(SyntaxTriviaList trivia)
             return trivia.All(f => f.IsKind(SyntaxKind.WhitespaceTrivia, SyntaxKind.EndOfLineTrivia, SyntaxKind.SingleLineCommentTrivia));
         }
     }
+
+    private static bool LocalDeclaredVariablesOverlapWithAnyOtherSwitchSections(SwitchStatementSyntax switchStatement, SwitchSectionSyntax switchSection, SemanticModel semanticModel)
+    {
+        var sectionVariablesDeclared = semanticModel.AnalyzeDataFlow(switchSection)!
+            .VariablesDeclared;
+
+        if (sectionVariablesDeclared.IsEmpty)
+            return false;
+
+        var sectionDeclaredVariablesName = sectionVariablesDeclared
+            .Select(s => s.Name)
+            .ToImmutableHashSet();
+
+        return switchStatement.Sections.Any(otherSection =>
+        {
+            var otherSectionVariablesDeclared = semanticModel.AnalyzeDataFlow(otherSection)!.VariablesDeclared;
+            if (otherSectionVariablesDeclared.IsEmpty)
+                return false;
+
+            return otherSectionVariablesDeclared.Any(s => sectionDeclaredVariablesName.Contains(s.Name));
+        });
+    }
+
 }

src/Analyzers/CSharp/Analysis/RemoveUnnecessaryElseAnalyzer.cs

@@ -1,6 +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.Immutable;
+using System.Linq;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -38,13 +40,13 @@ public static void Analyze(SyntaxNodeAnalysisContext context)
         if (elseClause.ContainsDiagnostics)
             return;
 
-        if (!IsFixable(elseClause))
+        if (!IsFixable(elseClause, context.SemanticModel))
             return;
 
         DiagnosticHelpers.ReportDiagnostic(context, DiagnosticRules.RemoveUnnecessaryElse, elseClause.ElseKeyword);
     }
 
-    public static bool IsFixable(ElseClauseSyntax elseClause)
+    private static bool IsFixable(ElseClauseSyntax elseClause, SemanticModel semanticModel)
     {
         if (elseClause.Statement?.IsKind(SyntaxKind.IfStatement) != false)
             return false;
@@ -55,12 +57,39 @@ public static bool IsFixable(ElseClauseSyntax elseClause)
         if (!ifStatement.IsTopmostIf())
             return false;
 
-        StatementSyntax statement = ifStatement.Statement;
+        StatementSyntax ifStatementStatement = ifStatement.Statement;
 
-        if (statement is BlockSyntax block)
-            statement = block.Statements.LastOrDefault();
+        if (ifStatementStatement is not BlockSyntax ifBlock)
+            return CSharpFacts.IsJumpStatement(ifStatementStatement.Kind());
+
+        if (elseClause.Statement is BlockSyntax elseBlock && LocalDeclaredVariablesOverlap(elseBlock, ifBlock, semanticModel))
+            return false;
+
+        var lastStatementInIf = ifBlock.Statements.LastOrDefault();
+
+        return lastStatementInIf is not null
+               && CSharpFacts.IsJumpStatement(lastStatementInIf.Kind());
+    }
+
+    private static bool LocalDeclaredVariablesOverlap(BlockSyntax elseBlock, BlockSyntax ifBlock, SemanticModel semanticModel)
+    {        
+        var elseVariablesDeclared = semanticModel.AnalyzeDataFlow(elseBlock)!
+            .VariablesDeclared;
+
+        if (elseVariablesDeclared.IsEmpty)
+            return false;
+
+        var ifVariablesDeclared = semanticModel.AnalyzeDataFlow(ifBlock)!
+            .VariablesDeclared;
+
+        if (ifVariablesDeclared.IsEmpty)
+            return false;
 
-        return statement is not null
-            && CSharpFacts.IsJumpStatement(statement.Kind());
+        var elseVariableNames = elseVariablesDeclared
+            .Select(s => s.Name)
+            .ToImmutableHashSet();
+
+        return ifVariablesDeclared
+            .Any(s => elseVariableNames.Contains(s.Name));
     }
 }

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

@@ -1,6 +1,7 @@
 // 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.Diagnostics;
+using System.Linq;
 using System.Threading;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
@@ -71,6 +72,11 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
                 return Fail(switchSection);
             }
 
+            if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, switchSection, semanticModel))
+            {
+                return Fail(switchSection);
+            }
+
             return Success(jumpKind, switchSection);
         }
 
@@ -104,6 +110,11 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
                 return Fail(parent);
             }
 
+            if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parent, semanticModel))
+            {
+                return Fail(parent);
+            }
+
             return Success(jumpKind, parent);
         }
 
@@ -126,6 +137,9 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
                     {
                         return Fail(parent);
                     }
+
+                    if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parent, semanticModel))
+                        return Fail(parent);
 
                     return Success(jumpKind, parent);
                 }
@@ -135,12 +149,18 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
                 {
                     if (jumpKind == SyntaxKind.None)
                         return Fail(parent);
-
+
+                    if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parent, semanticModel))
+                        return Fail(parent);
+
                     return Success(jumpKind, parent);
                 }
             case SyntaxKind.MethodDeclaration:
                 {
                     var methodDeclaration = (MethodDeclarationSyntax)parent;
+
+                    if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, methodDeclaration.Body, semanticModel))
+                        return Fail(parent);
 
                     if (jumpKind != SyntaxKind.None)
                         return Success(jumpKind, parent);
@@ -172,6 +192,9 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
             case SyntaxKind.LocalFunctionStatement:
                 {
                     var localFunction = (LocalFunctionStatementSyntax)parent;
+
+                    if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, localFunction.Body, semanticModel))
+                        return Fail(parent);
 
                     if (jumpKind != SyntaxKind.None)
                         return Success(jumpKind, parent);
@@ -204,6 +227,9 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
                 {
                     var anonymousFunction = (AnonymousFunctionExpressionSyntax)parent;
 
+                    if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, anonymousFunction.Block, semanticModel))
+                        return Fail(parent);
+
                     if (jumpKind != SyntaxKind.None)
                         return Success(jumpKind, parent);
 
@@ -260,6 +286,26 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
         return Fail(parent);
     }
 
+    private static bool LocallyDeclaredVariablesOverlapWithOuterScope(
+        IfStatementSyntax ifStatement,
+        SyntaxNode parent,
+        SemanticModel semanticModel
+    )
+    {
+        var ifVariablesDeclared = semanticModel.AnalyzeDataFlow(ifStatement)!
+            .VariablesDeclared;
+        if (ifVariablesDeclared.IsEmpty)
+            return false;
+
+        var 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 occuring twice.
+        return ifVariablesDeclared.Any(variable =>
+            parentStatementDeclared.Count(pVar => pVar.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

@@ -184,6 +184,35 @@ void M()
         }
     }
 }
+");
+    }
+    [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.RemoveUnnecessaryBracesInSwitchSection)]
+    public async Task TestNoDiagnostic_WhenOverlappingLocalVariableDeclaration()
+    {
+        await VerifyNoDiagnosticAsync(@"
+using System;
+
+class C
+{
+    void M()
+    {
+        string s = null;
+
+        switch (s)
+        {
+            case """":
+                {
+                    var x = 1;
+                    break;
+                }
+            default:
+                {
+                    var x = 1;
+                    break;
+                }
+        }
+    }
+}
 ");
     }
 }

src/Tests/Analyzers.Tests/RCS1211RemoveUnnecessaryElseTests.cs

@@ -0,0 +1,71 @@
+// 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.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+using Roslynator.CSharp.CodeFixes;
+using Roslynator.Testing.CSharp;
+using Xunit;
+
+namespace Roslynator.CSharp.Analysis.Tests;
+
+public class RCS1211RemoveUnnecessaryElseTests : AbstractCSharpDiagnosticVerifier<RemoveUnnecessaryElseAnalyzer, RemoveUnnecessaryElseCodeFixProvider>
+{
+    public override DiagnosticDescriptor Descriptor { get; } = DiagnosticRules.RemoveUnnecessaryElse;
+
+    [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.RemoveUnnecessaryElse)]
+    public async Task Test_UnnecessaryElse_Removed()
+    {
+        await VerifyDiagnosticAndFixAsync(@"
+class C
+{
+    int M(bool flag)
+    {
+        if(flag)
+        {
+            return 1;
+        }
+        [|else|]
+        {
+            return 0;
+        }
+    }
+}
+", @"
+class C
+{
+    int M(bool flag)
+    {
+        if(flag)
+        {
+            return 1;
+        }
+
+        return 0;
+    }
+}
+");
+    }
+    [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.RemoveUnnecessaryElse)]
+    public async Task TestNoDiagnostic_OverlappingLocalVariables()
+    {
+        await VerifyNoDiagnosticAsync(@"
+class C
+{
+    int M(bool flag)
+    {
+        if(flag)
+        {
+            var z = 1;
+            return z;
+        }
+        else
+        {
+            var z = 0;
+            return z;
+        }
+    }
+}
+");
+    }
+
+}