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;
@@ -72,6 +72,9 @@ 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/PatternMatchingVariableDeclarationHelper.cs

@@ -0,0 +1,38 @@
+// 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 PatternMatchingVariableDeclarationHelper
+{
+ public static bool AnyDeclaredVariablesMatch(PatternSyntax pattern, ImmutableHashSet<string> variableNames)
+ {
+ return pattern switch
+ {
+ RecursivePatternSyntax { PositionalPatternClause: var positionalPatternClause, PropertyPatternClause: var propertyPatternClause, Designation: var designation } =>
+ (designation is not null && AnyDeclaredVariablesMatch(designation, variableNames))
+ || (propertyPatternClause?.Subpatterns.Any(p => AnyDeclaredVariablesMatch(p.Pattern, variableNames)) ?? false)
+ || (positionalPatternClause?.Subpatterns.Any(p => AnyDeclaredVariablesMatch(p.Pattern, variableNames)) ?? false),
+ 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 (SwitchLabelSyntax label in otherSection.Labels)
  {
- if (sectionDeclaredVariablesNames.Contains(v.Name))
+ 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;

src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs

@@ -209,22 +209,22 @@ private ExpressionSyntax LogicallyInvertImpl(
  return DefaultInvert(expression);
  }
  case SyntaxKind.CoalesceExpression:
- {
- var binaryExpression = (BinaryExpressionSyntax)expression;
- if (binaryExpression.Right.IsKind(SyntaxKind.FalseLiteralExpression))
- {
- // !(x ?? false) === (x != true)
- return NotEqualsExpression(binaryExpression.Left, TrueLiteralExpression());
- }
-
- if (binaryExpression.Right.IsKind(SyntaxKind.TrueLiteralExpression))
  {
- // !(x ?? true) === (x == false)
- return EqualsExpression(binaryExpression.Left, FalseLiteralExpression());
- }
+ var binaryExpression = (BinaryExpressionSyntax)expression;
+ if (binaryExpression.Right.IsKind(SyntaxKind.FalseLiteralExpression))
+ {
+ // !(x ?? false) === (x != true)
+ return NotEqualsExpression(binaryExpression.Left, TrueLiteralExpression());
+ }
+
+ if (binaryExpression.Right.IsKind(SyntaxKind.TrueLiteralExpression))
+ {
+ // !(x ?? true) === (x == false)
+ return EqualsExpression(binaryExpression.Left, FalseLiteralExpression());
+ }
 
- return DefaultInvert(expression);
- }
+  return DefaultInvert(expression);
+  }
  }
 
  Debug.Fail($"Logical inversion of unknown kind '{expression.Kind()}'");

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

@@ -0,0 +1,40 @@
+// 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;
+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
+ )
+ {
+ ImmutableArray<ISymbol> ifVariablesDeclared = semanticModel.AnalyzeDataFlow(ifStatement)!
+ .VariablesDeclared;
+
+ if (ifVariablesDeclared.IsEmpty)
+ return false;
+
+ foreach (StatementSyntax statement in SyntaxInfo.StatementListInfo(ifStatement).Statements)
+ {
+ if (statement == ifStatement)
+ continue;
+
+ foreach (ISymbol parentVariable in semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared)
+ {
+ foreach (ISymbol ifVariable in ifVariablesDeclared)
+ {
+ if (ifVariable.Name == parentVariable.Name)
+ return true;
+ }
+ }
+ }
+
+ return false;
+ }
+}

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);
@@ -109,7 +108,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
  return Fail(parent);
  }
 
- if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parent, semanticModel))
+ if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel))
  return Fail(parent);
 
  return Success(jumpKind, parent);
@@ -135,7 +134,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
  return Fail(parent);
  }
 
- if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parent, semanticModel))
+ if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel))
  return Fail(parent);
 
  return Success(jumpKind, parent);
@@ -147,7 +146,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
  if (jumpKind == SyntaxKind.None)
  return Fail(parent);
 
- if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parent, semanticModel))
+ if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel))
  return Fail(parent);
 
  return Success(jumpKind, parent);
@@ -156,7 +155,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
  {
  var methodDeclaration = (MethodDeclarationSyntax)parent;
 
- if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, methodDeclaration.Body, semanticModel))
+ if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel))
  return Fail(parent);
 
  if (jumpKind != SyntaxKind.None)
@@ -190,7 +189,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore(
  {
  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

@@ -265,6 +265,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;
+ }
+ }
+ }
+}
 ");
  }
 }