From 60e0270ad30d2c77c94af1133d29394deb088067 Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Mon, 20 Mar 2023 22:44:22 +0000 Subject: [PATCH 01/13] with type fixes; RCS1208 also check case pattern match vars --- ...nnecessaryBracesInSwitchSectionAnalyzer.cs | 16 +- ...PattenMatchingVariableDeclarationHelper.cs | 53 +++++ .../ReduceIfNestingAnalysis.cs | 35 ++- ...veUnnecessaryBracesInSwitchSectionTests.cs | 63 ++++- .../RCS1208ReduceIfNestingTests.cs | 220 +++++++++++++++++- ...nMatchingVariableDeclarationHelperTests.cs | 184 +++++++++++++++ 6 files changed, 563 insertions(+), 8 deletions(-) create mode 100644 src/CSharp/PattenMatchingVariableDeclarationHelper.cs create mode 100644 src/Tests/CSharp.Tests/PatternMatchingVariableDeclarationHelperTests.cs diff --git a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs index 59e4ae08cf..8f686f3d6d 100644 --- a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs @@ -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) @@ -137,5 +146,4 @@ private static bool LocallyDeclaredVariablesOverlapWithAnyOtherSwitchSections(Sw return false; } - } diff --git a/src/CSharp/PattenMatchingVariableDeclarationHelper.cs b/src/CSharp/PattenMatchingVariableDeclarationHelper.cs new file mode 100644 index 0000000000..47fdfeb1c5 --- /dev/null +++ b/src/CSharp/PattenMatchingVariableDeclarationHelper.cs @@ -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 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(); + var propertyVars = propertyPatternClause?.Subpatterns.SelectMany(p=>GetVariablesDeclared(p.Pattern)) ?? Array.Empty(); + var positionalVars = positionalPatternClause?.Subpatterns.SelectMany(p => GetVariablesDeclared(p.Pattern)) ?? Array.Empty(); + 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(); + } + + public static IEnumerable 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; + } + } + +} \ No newline at end of file diff --git a/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs b/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs index 68484accf7..88f16a681c 100644 --- a/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs +++ b/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs @@ -1,5 +1,6 @@ // 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.Diagnostics; using System.Linq; using System.Threading; @@ -137,20 +138,50 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parent, semanticModel)) return Fail(parent); + var parentBody = parentKind switch + { + SyntaxKind.ConstructorDeclaration => ((ConstructorDeclarationSyntax)parent).Body, + SyntaxKind.DestructorDeclaration => ((DestructorDeclarationSyntax)parent).Body, + SyntaxKind.SetAccessorDeclaration => ((AccessorDeclarationSyntax)parent).Body, + SyntaxKind.AddAccessorDeclaration => ((AccessorDeclarationSyntax)parent).Body, + SyntaxKind.RemoveAccessorDeclaration => ((AccessorDeclarationSyntax)parent).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 = parentKind switch + { + SyntaxKind.OperatorDeclaration => ((OperatorDeclarationSyntax)parent).Body, + SyntaxKind.ConversionOperatorDeclaration => ((ConversionOperatorDeclarationSyntax)parent).Body, + _ => throw new NotImplementedException() + }; + + if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parentBody, semanticModel)) return Fail(parent); return Success(jumpKind, parent); } + case SyntaxKind.GetAccessorDeclaration: + { + var accessorDeclaration = (AccessorDeclarationSyntax)parent; + if (jumpKind == SyntaxKind.None) + return Fail(parent); + + if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, accessorDeclaration.Body, semanticModel)) + return Fail(parent); + + return Success(jumpKind, parent); + } case SyntaxKind.MethodDeclaration: { var methodDeclaration = (MethodDeclarationSyntax)parent; diff --git a/src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs b/src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs index e77d79ce4b..495f0caecb 100644 --- a/src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs @@ -237,7 +237,6 @@ void M() } "); } - [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.RemoveUnnecessaryBracesInSwitchSection)] public async Task TestNoDiagnostic_WhenOverlappingLocalVariableDeclaration() { @@ -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; + } + } + } +} +"); + } +} + + diff --git a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs index c8a3df99c2..1bc6f28b2a 100644 --- a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs @@ -13,7 +13,90 @@ public class RCS1208ReduceIfNestingTests : AbstractCSharpDiagnosticVerifier + { + [|if|] (p) + { + M2(); + } + }; + } + + void M2() + { + } +} +", @" +class C +{ + void M(bool p) + { + var f = () => + { + if (!p) + { + return; + } + + M2(); + } +; + } + + void M2() + { + } +} "); } diff --git a/src/Tests/CSharp.Tests/PatternMatchingVariableDeclarationHelperTests.cs b/src/Tests/CSharp.Tests/PatternMatchingVariableDeclarationHelperTests.cs new file mode 100644 index 0000000000..d6d59f2cee --- /dev/null +++ b/src/Tests/CSharp.Tests/PatternMatchingVariableDeclarationHelperTests.cs @@ -0,0 +1,184 @@ +using System.Collections.Generic; +using System.Linq; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Roslynator.CSharp.Analysis; +using Xunit; + +namespace Roslynator.Testing.CSharp; + +public class PatternMatchingVariableDeclarationHelperTests +{ + [Fact] + public void SingleVariableDesignation() + { + VariableDesignationSyntax designationSyntax = SyntaxFactory.SingleVariableDesignation( + SyntaxFactory.Identifier("x") + ); + var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(designationSyntax); + Assert.True(1==variables.Count()); + Assert.True("x"== variables.First()); + } + + [Fact] + public void ParenthesizedVariableDesignation() + { + VariableDesignationSyntax designationSyntax = SyntaxFactory.ParenthesizedVariableDesignation( + SyntaxFactory.SeparatedList(new List + { + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")), + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("y")) + }) + ); + + var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(designationSyntax); + Assert.True(2 == variables.Count()); + Assert.True(variables.Contains("x")); + Assert.True(variables.Contains("y")); + } + + [Fact] + public void DiscardDesignation() + { + VariableDesignationSyntax designationSyntax = SyntaxFactory.DiscardDesignation(); + var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(designationSyntax); + Assert.True(!variables.Any()); + } + + [Fact] + public void NullTest() + { + VariableDesignationSyntax designationSyntax = null; + var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(designationSyntax); + Assert.True(!variables.Any()); + + } + + [Fact] + public void DeclarationPattern() + { + PatternSyntax patternSyntax = SyntaxFactory.DeclarationPattern( + SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.IntKeyword)), + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) + ); + var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(patternSyntax); + Assert.True(1==variables.Count()); + Assert.True("x"== variables.First()); + } + + [Fact] + public void RecursivePattern_WithPositional() + { + PatternSyntax patternSyntax = SyntaxFactory.RecursivePattern( + SyntaxFactory.IdentifierName("TypeA"), + positionalPatternClause: SyntaxFactory.PositionalPatternClause( + SyntaxFactory.SeparatedList(new List + { + SyntaxFactory.Subpattern( + SyntaxFactory.DeclarationPattern( + SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.IntKeyword)), + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("p1")) + ) + ), + SyntaxFactory.Subpattern( + SyntaxFactory.DeclarationPattern( + SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.IntKeyword)), + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("p2")) + ) + ) + }) + ), + propertyPatternClause: default, + designation: default + ); + var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(patternSyntax); + Assert.True(2==variables.Count()); + Assert.True(variables.Contains("p1")); + Assert.True(variables.Contains("p2")); + } + + [Fact] + public void RecursivePattern_WithProperty() + { + PatternSyntax patternSyntax = SyntaxFactory.RecursivePattern( + SyntaxFactory.IdentifierName("TypeA"), + positionalPatternClause: default, + propertyPatternClause: SyntaxFactory.PropertyPatternClause( + SyntaxFactory.SeparatedList(new List + { + SyntaxFactory.Subpattern( + SyntaxFactory.NameColon(SyntaxFactory.IdentifierName("PropertyName")), + SyntaxFactory.VarPattern( + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) + ) + ), + }) + ), + designation: default + ); + var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(patternSyntax); + Assert.True(1==variables.Count()); + Assert.True(variables.Contains("x")); + } + + [Fact] + public void RecursivePattern_WithDesignation() + { + PatternSyntax patternSyntax = SyntaxFactory.RecursivePattern( + SyntaxFactory.IdentifierName("TypeA"), + positionalPatternClause: default, + propertyPatternClause: SyntaxFactory.PropertyPatternClause( + SyntaxFactory.SeparatedList(new List + { + SyntaxFactory.Subpattern( + SyntaxFactory.NameColon(SyntaxFactory.IdentifierName("PropertyName")), + SyntaxFactory.ConstantPattern(SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(42))) + ), + }) + ), + designation: SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) + ); + var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(patternSyntax); + Assert.True(1==variables.Count()); + Assert.True(variables.Contains("x")); + + } + + [Fact] + public void VarPattern() + { + PatternSyntax patternSyntax = SyntaxFactory.VarPattern( + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) + ); + var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(patternSyntax); + Assert.True(1==variables.Count()); + Assert.True("x"==variables.First()); + + } + + [Fact] + public void BinaryPattern() + { + PatternSyntax patternSyntax = SyntaxFactory.BinaryPattern( + SyntaxKind.AndPattern, + SyntaxFactory.ConstantPattern(SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(42))), + SyntaxFactory.ConstantPattern(SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(99))) + ); + var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(patternSyntax); + Assert.True(!variables.Any()); + } + + [Fact] + public void ParenthesizedPattern() + { + PatternSyntax patternSyntax = SyntaxFactory.ParenthesizedPattern( + SyntaxFactory.VarPattern( + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) + ) + ); + var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(patternSyntax); + Assert.True(1==variables.Count()); + Assert.True("x"==variables.First()); + } + +} \ No newline at end of file From cde590bff13e3b9b501563172d39f860e51ca53c Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Mon, 20 Mar 2023 23:08:46 +0000 Subject: [PATCH 02/13] self review changes --- .../ReduceIfNestingAnalysis.cs | 18 +- .../RCS1208ReduceIfNestingTests.cs | 255 ++++++++++++++---- 2 files changed, 207 insertions(+), 66 deletions(-) diff --git a/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs b/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs index 88f16a681c..245ea71509 100644 --- a/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs +++ b/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs @@ -135,9 +135,6 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( return Fail(parent); } - if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parent, semanticModel)) - return Fail(parent); - var parentBody = parentKind switch { SyntaxKind.ConstructorDeclaration => ((ConstructorDeclarationSyntax)parent).Body, @@ -155,7 +152,8 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( } case SyntaxKind.OperatorDeclaration: case SyntaxKind.ConversionOperatorDeclaration: - { + case SyntaxKind.GetAccessorDeclaration: + { if (jumpKind == SyntaxKind.None) return Fail(parent); @@ -163,6 +161,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( { SyntaxKind.OperatorDeclaration => ((OperatorDeclarationSyntax)parent).Body, SyntaxKind.ConversionOperatorDeclaration => ((ConversionOperatorDeclarationSyntax)parent).Body, + SyntaxKind.GetAccessorDeclaration => ((AccessorDeclarationSyntax)parent).Body, _ => throw new NotImplementedException() }; @@ -171,17 +170,6 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( return Success(jumpKind, parent); } - case SyntaxKind.GetAccessorDeclaration: - { - var accessorDeclaration = (AccessorDeclarationSyntax)parent; - if (jumpKind == SyntaxKind.None) - return Fail(parent); - - if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, accessorDeclaration.Body, semanticModel)) - return Fail(parent); - - return Success(jumpKind, parent); - } case SyntaxKind.MethodDeclaration: { var methodDeclaration = (MethodDeclarationSyntax)parent; diff --git a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs index 1bc6f28b2a..d0cdab7268 100644 --- a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs @@ -50,6 +50,49 @@ void M2() "); } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] + public async Task Test_WhenParentIsConversionOperator() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + static bool b=false; + public static implicit operator bool(C c) + { + [|if|] (b) + { + return M2(); + } + return false; + } + + static bool M2() + { + return true; + } +} +", @" +class C +{ + static bool b=false; + public static implicit operator bool(C c) + { + if (!b) + { + return false; + } + + return M2(); + } + + static bool M2() + { + return true; + } +} +"); + } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] public async Task Test_WhenParentIsGetAccessor() { @@ -96,17 +139,20 @@ static bool M2() } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] - public async Task Test_WhenParentIsMethod() + public async Task Test_WhenParentIsLambda() { await VerifyDiagnosticAndFixAsync(@" class C { void M(bool p) { - [|if|] (p) + var f = () => { - M2(); - } + [|if|] (p) + { + M2(); + } + }; } void M2() @@ -118,12 +164,16 @@ class C { void M(bool p) { - if (!p) - { - return; - } + var f = () => + { + if (!p) + { + return; + } - M2(); + M2(); + } +; } void M2() @@ -133,66 +183,66 @@ void M2() "); } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] - public async Task Test_WhenParentIsConversionOperator() + public async Task Test_WhenParentIsLocalFunction() { await VerifyDiagnosticAndFixAsync(@" class C { - static bool b=false; - public static implicit operator bool(C c) + void M(bool p) { - [|if|] (b) + void M3() { - return M2(); + [|if|] (p) + { + M2(); + } + } - return false; + M3(); } - static bool M2() + void M2() { - return true; } } ", @" class C { - static bool b=false; - public static implicit operator bool(C c) + void M(bool p) { - if (!b) + void M3() { - return false; - } + if (!p) + { + return; + } - return M2(); + M2(); + } + M3(); } - static bool M2() + void M2() { - return true; } } "); } - + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] - public async Task Test_WhenParentIsLocalFunction() + public async Task Test_WhenParentIsMethod() { await VerifyDiagnosticAndFixAsync(@" class C { void M(bool p) { - void M3() + [|if|] (p) { - [|if|] (p) - { - M2(); - } - + M2(); } - M3(); } void M2() @@ -204,16 +254,41 @@ class C { void M(bool p) { - void M3() + if (!p) { - if (!p) - { - return; - } + return; + } + + M2(); + } + + void M2() + { + } +} +"); + } + + + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] + public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsConstructor() + { + await VerifyNoDiagnosticAsync(@" +class C +{ + C(bool p) + { + if (p) + { + var s = 1; + } + if (p) + { + var s = 2; M2(); } - M3(); } void M2() @@ -222,19 +297,85 @@ void M2() } "); } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] + public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsConversionOperator() + { + await VerifyNoDiagnosticAsync(@" +class C +{ + static bool b=false; + public static implicit operator bool(C c) + { + if (b) + { + var s = 1; + } + if (b) + { + var s = 2; + return M2(); + } + return false; + } + static bool M2() + { + return true; + } +} +"); + } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] - public async Task Test_WhenParentIsLambda() + public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsGetAccessor() { - await VerifyDiagnosticAndFixAsync(@" + await VerifyNoDiagnosticAsync(@" +class C +{ + static bool b=false; + public static bool s { + get { + if (b) + { + var s = 1; + } + + if (b) + { + var s = 2; + return M2(); + } + return false; + } + } + + static bool M2() + { + return true; + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] + public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsLambda() + { + await VerifyNoDiagnosticAsync(@" class C { void M(bool p) { var f = () => { - [|if|] (p) + if (p) + { + var s = 1; + } + + if (p) { + var s = 2; M2(); } }; @@ -244,21 +385,33 @@ void M2() { } } -", @" +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] + public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsLocalFunction() + { + await VerifyNoDiagnosticAsync(@" class C { void M(bool p) { - var f = () => + void M3() + { + + if (p) { - if (!p) - { - return; - } + var s = 1; + } + if (p) + { + var s = 2; M2(); } -; + + } + M3(); } void M2() @@ -269,7 +422,7 @@ void M2() } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] - public async Task TestNoDiagnostic_OverlappingLocalVariables() + public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsMethod() { await VerifyNoDiagnosticAsync(@" class C From b98c4de54fc71cbab0f1ec38b2a86bbad5c6873e Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Mon, 20 Mar 2023 23:59:08 +0000 Subject: [PATCH 03/13] add loops and switch handling --- .../ReduceIfNestingAnalysis.cs | 61 +++++++++++++------ 1 file changed, 44 insertions(+), 17 deletions(-) diff --git a/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs b/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs index 245ea71509..c64d45cd22 100644 --- a/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs +++ b/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.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.Generic; +using System.Collections.Immutable; using System.Diagnostics; using System.Linq; using System.Threading; @@ -109,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); @@ -135,13 +146,11 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( return Fail(parent); } - var parentBody = parentKind switch + var parentBody = parent switch { - SyntaxKind.ConstructorDeclaration => ((ConstructorDeclarationSyntax)parent).Body, - SyntaxKind.DestructorDeclaration => ((DestructorDeclarationSyntax)parent).Body, - SyntaxKind.SetAccessorDeclaration => ((AccessorDeclarationSyntax)parent).Body, - SyntaxKind.AddAccessorDeclaration => ((AccessorDeclarationSyntax)parent).Body, - SyntaxKind.RemoveAccessorDeclaration => ((AccessorDeclarationSyntax)parent).Body, + ConstructorDeclarationSyntax constructorDeclarationSyntax => constructorDeclarationSyntax.Body, + DestructorDeclarationSyntax destructorDeclarationSyntax => destructorDeclarationSyntax.Body, + AccessorDeclarationSyntax accessorDeclarationSyntax => accessorDeclarationSyntax.Body, _ => throw new NotImplementedException() }; @@ -157,11 +166,11 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( if (jumpKind == SyntaxKind.None) return Fail(parent); - var parentBody = parentKind switch + var parentBody = parent switch { - SyntaxKind.OperatorDeclaration => ((OperatorDeclarationSyntax)parent).Body, - SyntaxKind.ConversionOperatorDeclaration => ((ConversionOperatorDeclarationSyntax)parent).Body, - SyntaxKind.GetAccessorDeclaration => ((AccessorDeclarationSyntax)parent).Body, + OperatorDeclarationSyntax operatorDeclarationSyntax => operatorDeclarationSyntax.Body, + ConversionOperatorDeclarationSyntax conversionOperatorDeclarationSyntax => conversionOperatorDeclarationSyntax.Body, + AccessorDeclarationSyntax accessorDeclarationSyntax=> accessorDeclarationSyntax.Body, _ => throw new NotImplementedException() }; @@ -313,13 +322,31 @@ SemanticModel semanticModel if (ifVariablesDeclared.IsEmpty) return false; - var parentStatementDeclared = semanticModel.AnalyzeDataFlow(parent)! - .VariablesDeclared; - + IEnumerable parentVariablesDeclared; + if (parent is SwitchSectionSyntax switchSectionSyntax) + { + var allDeclaredVariables = new List(); + 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) From 849bb8f515f325d5e22a2db9d5b99ff07bda86b6 Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Tue, 21 Mar 2023 00:11:08 +0000 Subject: [PATCH 04/13] update change log --- ChangeLog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.md b/ChangeLog.md index 956bbeb0c7..a7fe6f5c3a 100644 --- a/ChangeLog.md +++ b/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/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)). ## [4.2.0] - 2022-11-27 From f65b5b5638f9a0865eaa27ae2eab5343f7b770f2 Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Mon, 3 Apr 2023 20:56:03 +0100 Subject: [PATCH 05/13] also fix RCS1061 --- .../Analysis/MergeIfWithNestedIfAnalyzer.cs | 7 +- ...nnecessaryBracesInSwitchSectionAnalyzer.cs | 13 +-- .../IfLocalVariableAnalysis.cs | 104 ++++++++++++++++++ .../ReduceIfNestingAnalysis.cs | 84 ++------------ .../RCS1061MergeIfWithNestedIfTests.cs | 68 ++++++++++++ 5 files changed, 191 insertions(+), 85 deletions(-) create mode 100644 src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs create mode 100644 src/Tests/Analyzers.Tests/RCS1061MergeIfWithNestedIfTests.cs diff --git a/src/Analyzers/CSharp/Analysis/MergeIfWithNestedIfAnalyzer.cs b/src/Analyzers/CSharp/Analysis/MergeIfWithNestedIfAnalyzer.cs index 43e07fdc73..c325d2e79e 100644 --- a/src/Analyzers/CSharp/Analysis/MergeIfWithNestedIfAnalyzer.cs +++ b/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); } diff --git a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs index 8f686f3d6d..82e20ed73b 100644 --- a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs @@ -133,14 +133,13 @@ private static bool LocallyDeclaredVariablesOverlapWithAnyOtherSwitchSections(Sw 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) + foreach (var statement in otherSection.Statements) { - if (sectionDeclaredVariablesNames.Contains(v.Name)) - return true; + foreach (var v in semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared) + { + if (sectionDeclaredVariablesNames.Contains(v.Name)) + return true; + } } } return false; diff --git a/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs b/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs new file mode 100644 index 0000000000..b1348a222b --- /dev/null +++ b/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs @@ -0,0 +1,104 @@ +using System.Collections.Generic; +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 + ) + { + if (!TryGetOuterScope(ifStatement, out var outerScope)) + return true; + + var ifVariablesDeclared = semanticModel.AnalyzeDataFlow(ifStatement)! + .VariablesDeclared; + + if (ifVariablesDeclared.IsEmpty) + return false; + + IEnumerable parentVariablesDeclared; + if (outerScope is SwitchSectionSyntax switchSectionSyntax) + { + var allDeclaredVariables = new List(); + foreach (var statement in switchSectionSyntax.Statements) + { + allDeclaredVariables.AddRange(semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared); + } + + parentVariablesDeclared = allDeclaredVariables; + } + else + { + parentVariablesDeclared = semanticModel.AnalyzeDataFlow(outerScope)! + .VariablesDeclared; + } + + // 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 bool TryGetOuterScope(IfStatementSyntax ifStatement, out SyntaxNode outerScope) + { + switch (ifStatement.Parent) + { + case BlockSyntax blockSyntax: + outerScope = blockSyntax; + return true; + case ForStatementSyntax forStatementSyntax: + outerScope = forStatementSyntax.Statement; + return true; + case ForEachStatementSyntax forEachStatementSyntax: + outerScope = forEachStatementSyntax.Statement; + return true; + case DoStatementSyntax doStatementSyntax: + outerScope = doStatementSyntax.Statement; + return true; + case WhileStatementSyntax whileStatementSyntax: + outerScope = whileStatementSyntax.Statement; + return true; + case SwitchSectionSyntax switchSectionSyntax: + outerScope = switchSectionSyntax; + return true; + case ConstructorDeclarationSyntax constructorDeclarationSyntax: + outerScope = constructorDeclarationSyntax.Body; + return true; + case DestructorDeclarationSyntax destructorDeclarationSyntax: + outerScope = destructorDeclarationSyntax.Body; + return true; + case AccessorDeclarationSyntax accessorDeclarationSyntax: + outerScope = accessorDeclarationSyntax.Body; + return true; + case OperatorDeclarationSyntax operatorDeclarationSyntax: + outerScope = operatorDeclarationSyntax.Body; + return true; + case ConversionOperatorDeclarationSyntax conversionOperatorDeclarationSyntax: + outerScope = conversionOperatorDeclarationSyntax.Body; + return true; + case MethodDeclarationSyntax methodDeclarationSyntax: + outerScope = methodDeclarationSyntax.Body; + return true; + case LocalFunctionStatementSyntax localFunctionStatementSyntax: + outerScope = localFunctionStatementSyntax.Body; + return true; + case AnonymousFunctionExpressionSyntax anonymousFunctionExpressionSyntax: + outerScope = anonymousFunctionExpressionSyntax.Block; + return true; + default: + outerScope = null; + return false; + } + + } +} \ No newline at end of file diff --git a/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs b/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs index c64d45cd22..e0d2cb06c7 100644 --- a/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs +++ b/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs @@ -1,10 +1,6 @@ // 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; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -75,7 +71,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); @@ -110,17 +106,8 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( { return Fail(parent); } - - 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)) + + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel)) return Fail(parent); return Success(jumpKind, parent); @@ -146,15 +133,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( return Fail(parent); } - var parentBody = parent switch - { - ConstructorDeclarationSyntax constructorDeclarationSyntax => constructorDeclarationSyntax.Body, - DestructorDeclarationSyntax destructorDeclarationSyntax => destructorDeclarationSyntax.Body, - AccessorDeclarationSyntax accessorDeclarationSyntax => accessorDeclarationSyntax.Body, - _ => throw new NotImplementedException() - }; - - if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parentBody, semanticModel)) + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel)) return Fail(parent); return Success(jumpKind, parent); @@ -166,15 +145,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( if (jumpKind == SyntaxKind.None) return Fail(parent); - var parentBody = parent switch - { - OperatorDeclarationSyntax operatorDeclarationSyntax => operatorDeclarationSyntax.Body, - ConversionOperatorDeclarationSyntax conversionOperatorDeclarationSyntax => conversionOperatorDeclarationSyntax.Body, - AccessorDeclarationSyntax accessorDeclarationSyntax=> accessorDeclarationSyntax.Body, - _ => throw new NotImplementedException() - }; - - if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parentBody, semanticModel)) + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel)) return Fail(parent); return Success(jumpKind, parent); @@ -183,7 +154,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) @@ -217,7 +188,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) @@ -251,7 +222,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) @@ -310,45 +281,6 @@ 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; - - IEnumerable parentVariablesDeclared; - if (parent is SwitchSectionSyntax switchSectionSyntax) - { - var allDeclaredVariables = new List(); - 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. - 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) { options |= ReduceIfNestingOptions.AllowNestedFix; diff --git a/src/Tests/Analyzers.Tests/RCS1061MergeIfWithNestedIfTests.cs b/src/Tests/Analyzers.Tests/RCS1061MergeIfWithNestedIfTests.cs new file mode 100644 index 0000000000..5fa6b88f23 --- /dev/null +++ b/src/Tests/Analyzers.Tests/RCS1061MergeIfWithNestedIfTests.cs @@ -0,0 +1,68 @@ +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Roslynator.CSharp.CodeFixes; +using Roslynator.Testing.CSharp; +using Xunit; + +namespace Roslynator.CSharp.Analysis.Tests; + +public class RCS061MergeIfWithNestedIfTests : AbstractCSharpDiagnosticVerifier +{ + public override DiagnosticDescriptor Descriptor => DiagnosticRules.MergeIfWithNestedIf; + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.MergeIfWithNestedIf)] + public async Task Test_MergeIfStatement() + { + await VerifyDiagnosticAndFixAsync(@" +using System.Collections.Generic; +class C +{ + public static void M(Dictionary settings, string name) + { + [|if (name == ""name1"") + { + if (settings.TryGetValue(""name1"", out var v1)) + { + } + }|] + } +}", +@" +using System.Collections.Generic; +class C +{ + public static void M(Dictionary settings, string name) + { + if (name == ""name1"" && settings.TryGetValue(""name1"", out var v1)) + { + } + } +}"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.MergeIfWithNestedIf)] + public async Task TestNoDiagnostic_WhenLocalVariablesOverlap() + { + await VerifyNoDiagnosticAsync(@" +using System.Collections.Generic; +class C +{ + public static void M(Dictionary settings, string name) + { + if (name == ""name1"") + { + if (settings.TryGetValue(""name1"", out var v1)) + { + } + } + + if (name == ""name2"") + { + if (settings.TryGetValue(""name2"", out var v1)) + { + } + } + } +}"); + } +} From 8acacf92feeefeb08f806058cc7ecc90f2769c77 Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Mon, 3 Apr 2023 21:04:47 +0100 Subject: [PATCH 06/13] update change log --- ChangeLog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.md b/ChangeLog.md index 0460452d85..a304ad427b 100644 --- a/ChangeLog.md +++ b/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),[#1058](https://github.com/JosefPihrt/Roslynator/pull/1058)). +- 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, [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)). From ca0328833bbe4208b3eb8f84a01692f2b31e7556 Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Mon, 3 Apr 2023 21:08:56 +0100 Subject: [PATCH 07/13] update change log --- ChangeLog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.md b/ChangeLog.md index a304ad427b..6ff8833b9c 100644 --- a/ChangeLog.md +++ b/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, [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)). +- 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)). From b7e24344b0392c8fcf0e354347eb78e2df1143e3 Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Mon, 17 Apr 2023 22:13:29 +0100 Subject: [PATCH 08/13] CR comments --- ...PattenMatchingVariableDeclarationHelper.cs | 39 ++++ ...nnecessaryBracesInSwitchSectionAnalyzer.cs | 4 +- ...PattenMatchingVariableDeclarationHelper.cs | 53 ----- .../IfLocalVariableAnalysis.cs | 93 +++----- .../RCS1061MergeIfWithNestedIfTests.cs | 2 + ...nMatchingVariableDeclarationHelperTests.cs | 207 ++++++++++++++++++ ...nMatchingVariableDeclarationHelperTests.cs | 184 ---------------- 7 files changed, 281 insertions(+), 301 deletions(-) create mode 100644 src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PattenMatchingVariableDeclarationHelper.cs delete mode 100644 src/CSharp/PattenMatchingVariableDeclarationHelper.cs create mode 100644 src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs delete mode 100644 src/Tests/CSharp.Tests/PatternMatchingVariableDeclarationHelperTests.cs diff --git a/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PattenMatchingVariableDeclarationHelper.cs b/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PattenMatchingVariableDeclarationHelper.cs new file mode 100644 index 0000000000..aa0d95489a --- /dev/null +++ b/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 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 variableNames) + { + return designation switch + { + SingleVariableDesignationSyntax singleVariableDesignation => variableNames.Contains(singleVariableDesignation.Identifier.ValueText), + ParenthesizedVariableDesignationSyntax parenthesizedVariableDesignation => parenthesizedVariableDesignation.Variables.Any(variable => AnyDeclaredVariablesMatch(variable, variableNames)), + DiscardDesignationSyntax _ => false, + _ => false + }; + } + +} \ No newline at end of file diff --git a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs index ac24ca8eff..7fbbb24053 100644 --- a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs @@ -126,10 +126,10 @@ private static bool LocallyDeclaredVariablesOverlapWithAnyOtherSwitchSections(Sw foreach (var label in otherSection.Labels) { - if (label is not CasePatternSwitchLabelSyntax casePatternSwitchLabelSyntax) + if (label is not CasePatternSwitchLabelSyntax casePatternSwitchLabel) continue; - if (PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(casePatternSwitchLabelSyntax.Pattern).Intersect(sectionDeclaredVariablesNames).Any()) + if (PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(casePatternSwitchLabel.Pattern,sectionDeclaredVariablesNames)) return true; } diff --git a/src/CSharp/PattenMatchingVariableDeclarationHelper.cs b/src/CSharp/PattenMatchingVariableDeclarationHelper.cs deleted file mode 100644 index 47fdfeb1c5..0000000000 --- a/src/CSharp/PattenMatchingVariableDeclarationHelper.cs +++ /dev/null @@ -1,53 +0,0 @@ -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 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(); - var propertyVars = propertyPatternClause?.Subpatterns.SelectMany(p=>GetVariablesDeclared(p.Pattern)) ?? Array.Empty(); - var positionalVars = positionalPatternClause?.Subpatterns.SelectMany(p => GetVariablesDeclared(p.Pattern)) ?? Array.Empty(); - 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(); - } - - public static IEnumerable 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; - } - } - -} \ No newline at end of file diff --git a/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs b/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs index b1348a222b..dd119fe4cf 100644 --- a/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs +++ b/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs @@ -1,4 +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.Generic; +using System.Diagnostics; using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -13,32 +16,29 @@ public static bool DoDeclaredVariablesOverlapWithOuterScope( SemanticModel semanticModel ) { - if (!TryGetOuterScope(ifStatement, out var outerScope)) - return true; var ifVariablesDeclared = semanticModel.AnalyzeDataFlow(ifStatement)! .VariablesDeclared; if (ifVariablesDeclared.IsEmpty) return false; + + var outerScope = GetOuterScope(ifStatement); + + if (outerScope == null) + return true; - IEnumerable parentVariablesDeclared; - if (outerScope is SwitchSectionSyntax switchSectionSyntax) + IList parentVariablesDeclared; + if (outerScope is SwitchSectionSyntax switchSection) { - var allDeclaredVariables = new List(); - foreach (var statement in switchSectionSyntax.Statements) - { - allDeclaredVariables.AddRange(semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared); - } - - parentVariablesDeclared = allDeclaredVariables; + parentVariablesDeclared = switchSection.Statements.SelectMany(s => semanticModel.AnalyzeDataFlow(s)!.VariablesDeclared).ToList(); } else { - parentVariablesDeclared = semanticModel.AnalyzeDataFlow(outerScope)! - .VariablesDeclared; + 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) { @@ -49,56 +49,25 @@ SemanticModel semanticModel return false; } - private static bool TryGetOuterScope(IfStatementSyntax ifStatement, out SyntaxNode outerScope) + private static SyntaxNode GetOuterScope(IfStatementSyntax ifStatement) { - switch (ifStatement.Parent) + return ifStatement.Parent switch { - case BlockSyntax blockSyntax: - outerScope = blockSyntax; - return true; - case ForStatementSyntax forStatementSyntax: - outerScope = forStatementSyntax.Statement; - return true; - case ForEachStatementSyntax forEachStatementSyntax: - outerScope = forEachStatementSyntax.Statement; - return true; - case DoStatementSyntax doStatementSyntax: - outerScope = doStatementSyntax.Statement; - return true; - case WhileStatementSyntax whileStatementSyntax: - outerScope = whileStatementSyntax.Statement; - return true; - case SwitchSectionSyntax switchSectionSyntax: - outerScope = switchSectionSyntax; - return true; - case ConstructorDeclarationSyntax constructorDeclarationSyntax: - outerScope = constructorDeclarationSyntax.Body; - return true; - case DestructorDeclarationSyntax destructorDeclarationSyntax: - outerScope = destructorDeclarationSyntax.Body; - return true; - case AccessorDeclarationSyntax accessorDeclarationSyntax: - outerScope = accessorDeclarationSyntax.Body; - return true; - case OperatorDeclarationSyntax operatorDeclarationSyntax: - outerScope = operatorDeclarationSyntax.Body; - return true; - case ConversionOperatorDeclarationSyntax conversionOperatorDeclarationSyntax: - outerScope = conversionOperatorDeclarationSyntax.Body; - return true; - case MethodDeclarationSyntax methodDeclarationSyntax: - outerScope = methodDeclarationSyntax.Body; - return true; - case LocalFunctionStatementSyntax localFunctionStatementSyntax: - outerScope = localFunctionStatementSyntax.Body; - return true; - case AnonymousFunctionExpressionSyntax anonymousFunctionExpressionSyntax: - outerScope = anonymousFunctionExpressionSyntax.Block; - return true; - default: - outerScope = null; - return false; - } - + 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 + }; } } \ No newline at end of file diff --git a/src/Tests/Analyzers.Tests/RCS1061MergeIfWithNestedIfTests.cs b/src/Tests/Analyzers.Tests/RCS1061MergeIfWithNestedIfTests.cs index 5fa6b88f23..4458d43a87 100644 --- a/src/Tests/Analyzers.Tests/RCS1061MergeIfWithNestedIfTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1061MergeIfWithNestedIfTests.cs @@ -1,3 +1,5 @@ +// 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; diff --git a/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs b/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs new file mode 100644 index 0000000000..848dd03e1e --- /dev/null +++ b/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs @@ -0,0 +1,207 @@ +// 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.Collections.Immutable; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Roslynator.CSharp.Analysis; +using Xunit; + +namespace Roslynator.Analyzers.Tests.UnitTests; + +public class PatternMatchingVariableDeclarationHelperTests +{ + [Fact] + public void SingleVariableDesignation() + { + VariableDesignationSyntax designation = SyntaxFactory.SingleVariableDesignation( + SyntaxFactory.Identifier("x") + ); + var vars = new HashSet() { "x" }.ToImmutableHashSet(); + Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + + vars = new HashSet() { "z" }.ToImmutableHashSet(); + Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + } + + [Fact] + public void ParenthesizedVariableDesignation() + { + VariableDesignationSyntax designation = SyntaxFactory.ParenthesizedVariableDesignation( + SyntaxFactory.SeparatedList(new List + { + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")), + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("y")) + }) + ); + + var vars = new HashSet() { "x" }.ToImmutableHashSet(); + Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + + vars = new HashSet() { "y" }.ToImmutableHashSet(); + Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + + vars = new HashSet() { "z" }.ToImmutableHashSet(); + Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + + } + + [Fact] + public void DiscardDesignation() + { + VariableDesignationSyntax designation = SyntaxFactory.DiscardDesignation(); + var vars = new HashSet() { "z" }.ToImmutableHashSet(); + Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + } + + [Fact] + public void NullTest() + { + VariableDesignationSyntax designation = null; + var vars = new HashSet() { "x" }.ToImmutableHashSet(); + Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + + } + + [Fact] + public void DeclarationPattern() + { + PatternSyntax pattern = SyntaxFactory.DeclarationPattern( + SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.IntKeyword)), + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) + ); + var vars = new HashSet() { "x" }.ToImmutableHashSet(); + Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + + vars = new HashSet() { "z" }.ToImmutableHashSet(); + Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + } + + [Fact] + public void RecursivePattern_WithPositional() + { + PatternSyntax pattern = SyntaxFactory.RecursivePattern( + SyntaxFactory.IdentifierName("TypeA"), + positionalPatternClause: SyntaxFactory.PositionalPatternClause( + SyntaxFactory.SeparatedList(new List + { + SyntaxFactory.Subpattern( + SyntaxFactory.DeclarationPattern( + SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.IntKeyword)), + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) + ) + ), + SyntaxFactory.Subpattern( + SyntaxFactory.DeclarationPattern( + SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.IntKeyword)), + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("y")) + ) + ) + }) + ), + propertyPatternClause: default, + designation: default + ); + + var vars = new HashSet() { "x" }.ToImmutableHashSet(); + Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + + vars = new HashSet() { "y" }.ToImmutableHashSet(); + Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + + vars = new HashSet() { "z" }.ToImmutableHashSet(); + Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + } + + [Fact] + public void RecursivePattern_WithProperty() + { + PatternSyntax pattern = SyntaxFactory.RecursivePattern( + SyntaxFactory.IdentifierName("TypeA"), + positionalPatternClause: default, + propertyPatternClause: SyntaxFactory.PropertyPatternClause( + SyntaxFactory.SeparatedList(new List + { + SyntaxFactory.Subpattern( + SyntaxFactory.NameColon(SyntaxFactory.IdentifierName("PropertyName")), + SyntaxFactory.VarPattern( + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) + ) + ), + }) + ), + designation: default + ); + var vars = new HashSet() { "x" }.ToImmutableHashSet(); + Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + + vars = new HashSet() { "z" }.ToImmutableHashSet(); + Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + } + + [Fact] + public void RecursivePattern_WithDesignation() + { + PatternSyntax pattern = SyntaxFactory.RecursivePattern( + SyntaxFactory.IdentifierName("TypeA"), + positionalPatternClause: default, + propertyPatternClause: SyntaxFactory.PropertyPatternClause( + SyntaxFactory.SeparatedList(new List + { + SyntaxFactory.Subpattern( + SyntaxFactory.NameColon(SyntaxFactory.IdentifierName("PropertyName")), + SyntaxFactory.ConstantPattern(SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(42))) + ), + }) + ), + designation: SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) + ); + var vars = new HashSet() { "x" }.ToImmutableHashSet(); + Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + + vars = new HashSet() { "z" }.ToImmutableHashSet(); + Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + + } + + [Fact] + public void VarPattern() + { + PatternSyntax pattern = SyntaxFactory.VarPattern( + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) + ); + var vars = new HashSet() { "x" }.ToImmutableHashSet(); + Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + + vars = new HashSet() { "z" }.ToImmutableHashSet(); + Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + + } + + [Fact] + public void BinaryPattern() + { + PatternSyntax pattern = SyntaxFactory.BinaryPattern( + SyntaxKind.AndPattern, + SyntaxFactory.ConstantPattern(SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(42))), + SyntaxFactory.ConstantPattern(SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(99))) + ); + var vars = new HashSet() { "z" }.ToImmutableHashSet(); + Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + } + + [Fact] + public void ParenthesizedPattern() + { + PatternSyntax pattern = SyntaxFactory.ParenthesizedPattern( + SyntaxFactory.VarPattern( + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) + ) + ); + var vars = new HashSet() { "x" }.ToImmutableHashSet(); + Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + + vars = new HashSet() { "z" }.ToImmutableHashSet(); + Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + } +} \ No newline at end of file diff --git a/src/Tests/CSharp.Tests/PatternMatchingVariableDeclarationHelperTests.cs b/src/Tests/CSharp.Tests/PatternMatchingVariableDeclarationHelperTests.cs deleted file mode 100644 index d6d59f2cee..0000000000 --- a/src/Tests/CSharp.Tests/PatternMatchingVariableDeclarationHelperTests.cs +++ /dev/null @@ -1,184 +0,0 @@ -using System.Collections.Generic; -using System.Linq; -using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.CSharp.Syntax; -using Roslynator.CSharp.Analysis; -using Xunit; - -namespace Roslynator.Testing.CSharp; - -public class PatternMatchingVariableDeclarationHelperTests -{ - [Fact] - public void SingleVariableDesignation() - { - VariableDesignationSyntax designationSyntax = SyntaxFactory.SingleVariableDesignation( - SyntaxFactory.Identifier("x") - ); - var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(designationSyntax); - Assert.True(1==variables.Count()); - Assert.True("x"== variables.First()); - } - - [Fact] - public void ParenthesizedVariableDesignation() - { - VariableDesignationSyntax designationSyntax = SyntaxFactory.ParenthesizedVariableDesignation( - SyntaxFactory.SeparatedList(new List - { - SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")), - SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("y")) - }) - ); - - var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(designationSyntax); - Assert.True(2 == variables.Count()); - Assert.True(variables.Contains("x")); - Assert.True(variables.Contains("y")); - } - - [Fact] - public void DiscardDesignation() - { - VariableDesignationSyntax designationSyntax = SyntaxFactory.DiscardDesignation(); - var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(designationSyntax); - Assert.True(!variables.Any()); - } - - [Fact] - public void NullTest() - { - VariableDesignationSyntax designationSyntax = null; - var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(designationSyntax); - Assert.True(!variables.Any()); - - } - - [Fact] - public void DeclarationPattern() - { - PatternSyntax patternSyntax = SyntaxFactory.DeclarationPattern( - SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.IntKeyword)), - SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) - ); - var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(patternSyntax); - Assert.True(1==variables.Count()); - Assert.True("x"== variables.First()); - } - - [Fact] - public void RecursivePattern_WithPositional() - { - PatternSyntax patternSyntax = SyntaxFactory.RecursivePattern( - SyntaxFactory.IdentifierName("TypeA"), - positionalPatternClause: SyntaxFactory.PositionalPatternClause( - SyntaxFactory.SeparatedList(new List - { - SyntaxFactory.Subpattern( - SyntaxFactory.DeclarationPattern( - SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.IntKeyword)), - SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("p1")) - ) - ), - SyntaxFactory.Subpattern( - SyntaxFactory.DeclarationPattern( - SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.IntKeyword)), - SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("p2")) - ) - ) - }) - ), - propertyPatternClause: default, - designation: default - ); - var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(patternSyntax); - Assert.True(2==variables.Count()); - Assert.True(variables.Contains("p1")); - Assert.True(variables.Contains("p2")); - } - - [Fact] - public void RecursivePattern_WithProperty() - { - PatternSyntax patternSyntax = SyntaxFactory.RecursivePattern( - SyntaxFactory.IdentifierName("TypeA"), - positionalPatternClause: default, - propertyPatternClause: SyntaxFactory.PropertyPatternClause( - SyntaxFactory.SeparatedList(new List - { - SyntaxFactory.Subpattern( - SyntaxFactory.NameColon(SyntaxFactory.IdentifierName("PropertyName")), - SyntaxFactory.VarPattern( - SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) - ) - ), - }) - ), - designation: default - ); - var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(patternSyntax); - Assert.True(1==variables.Count()); - Assert.True(variables.Contains("x")); - } - - [Fact] - public void RecursivePattern_WithDesignation() - { - PatternSyntax patternSyntax = SyntaxFactory.RecursivePattern( - SyntaxFactory.IdentifierName("TypeA"), - positionalPatternClause: default, - propertyPatternClause: SyntaxFactory.PropertyPatternClause( - SyntaxFactory.SeparatedList(new List - { - SyntaxFactory.Subpattern( - SyntaxFactory.NameColon(SyntaxFactory.IdentifierName("PropertyName")), - SyntaxFactory.ConstantPattern(SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(42))) - ), - }) - ), - designation: SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) - ); - var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(patternSyntax); - Assert.True(1==variables.Count()); - Assert.True(variables.Contains("x")); - - } - - [Fact] - public void VarPattern() - { - PatternSyntax patternSyntax = SyntaxFactory.VarPattern( - SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) - ); - var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(patternSyntax); - Assert.True(1==variables.Count()); - Assert.True("x"==variables.First()); - - } - - [Fact] - public void BinaryPattern() - { - PatternSyntax patternSyntax = SyntaxFactory.BinaryPattern( - SyntaxKind.AndPattern, - SyntaxFactory.ConstantPattern(SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(42))), - SyntaxFactory.ConstantPattern(SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(99))) - ); - var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(patternSyntax); - Assert.True(!variables.Any()); - } - - [Fact] - public void ParenthesizedPattern() - { - PatternSyntax patternSyntax = SyntaxFactory.ParenthesizedPattern( - SyntaxFactory.VarPattern( - SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) - ) - ); - var variables = PattenMatchingVariableDeclarationHelper.GetVariablesDeclared(patternSyntax); - Assert.True(1==variables.Count()); - Assert.True("x"==variables.First()); - } - -} \ No newline at end of file From d0ce2501ac51e39b664e8906fcd83de02a12e4f2 Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Tue, 18 Apr 2023 23:04:48 +0100 Subject: [PATCH 09/13] more CR comments --- ...tternMatchingVariableDeclarationHelper.cs} | 2 +- ...nnecessaryBracesInSwitchSectionAnalyzer.cs | 6 +-- .../IfLocalVariableAnalysis.cs | 16 ++++++- ...nMatchingVariableDeclarationHelperTests.cs | 42 +++++++++---------- 4 files changed, 39 insertions(+), 27 deletions(-) rename src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/{PattenMatchingVariableDeclarationHelper.cs => PatternMatchingVariableDeclarationHelper.cs} (97%) diff --git a/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PattenMatchingVariableDeclarationHelper.cs b/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs similarity index 97% rename from src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PattenMatchingVariableDeclarationHelper.cs rename to src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs index aa0d95489a..b360c54ac6 100644 --- a/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PattenMatchingVariableDeclarationHelper.cs +++ b/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs @@ -5,7 +5,7 @@ namespace Roslynator.CSharp.Analysis; -public static class PattenMatchingVariableDeclarationHelper +public static class PatternMatchingVariableDeclarationHelper { public static bool AnyDeclaredVariablesMatch(PatternSyntax pattern, ImmutableHashSet variableNames) { diff --git a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs index 7fbbb24053..b953ebef02 100644 --- a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs @@ -129,15 +129,15 @@ private static bool LocallyDeclaredVariablesOverlapWithAnyOtherSwitchSections(Sw if (label is not CasePatternSwitchLabelSyntax casePatternSwitchLabel) continue; - if (PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(casePatternSwitchLabel.Pattern,sectionDeclaredVariablesNames)) + if (PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(casePatternSwitchLabel.Pattern,sectionDeclaredVariablesNames)) return true; } foreach (var statement in otherSection.Statements) { - foreach (var v in semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared) + foreach (var symbol in semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared) { - if (sectionDeclaredVariablesNames.Contains(v.Name)) + if (sectionDeclaredVariablesNames.Contains(symbol.Name)) return true; } } diff --git a/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs b/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs index dd119fe4cf..1c4a7d6dc7 100644 --- a/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs +++ b/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.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.Generic; +using System.Collections.Immutable; using System.Diagnostics; using System.Linq; using Microsoft.CodeAnalysis; @@ -31,14 +32,25 @@ SemanticModel semanticModel IList parentVariablesDeclared; if (outerScope is SwitchSectionSyntax switchSection) { - parentVariablesDeclared = switchSection.Statements.SelectMany(s => semanticModel.AnalyzeDataFlow(s)!.VariablesDeclared).ToList(); + List allDeclaredVariables = null; + foreach (StatementSyntax statement in switchSection.Statements) + { + ImmutableArray variables = semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared; + + if (variables.Any()) + (allDeclaredVariables ??= new List()).AddRange(variables); + } + + parentVariablesDeclared = allDeclaredVariables; } else { parentVariablesDeclared = semanticModel.AnalyzeDataFlow(outerScope)!.VariablesDeclared; } - Debug.Assert(parentVariablesDeclared.Count >= ifVariablesDeclared.Length); + if (parentVariablesDeclared.Count <= ifVariablesDeclared.Length) + return false; + // 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) { diff --git a/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs b/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs index 848dd03e1e..254c154578 100644 --- a/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs +++ b/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs @@ -18,10 +18,10 @@ public void SingleVariableDesignation() SyntaxFactory.Identifier("x") ); var vars = new HashSet() { "x" }.ToImmutableHashSet(); - Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); vars = new HashSet() { "z" }.ToImmutableHashSet(); - Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); } [Fact] @@ -36,13 +36,13 @@ public void ParenthesizedVariableDesignation() ); var vars = new HashSet() { "x" }.ToImmutableHashSet(); - Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); vars = new HashSet() { "y" }.ToImmutableHashSet(); - Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); vars = new HashSet() { "z" }.ToImmutableHashSet(); - Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); } @@ -51,7 +51,7 @@ public void DiscardDesignation() { VariableDesignationSyntax designation = SyntaxFactory.DiscardDesignation(); var vars = new HashSet() { "z" }.ToImmutableHashSet(); - Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); } [Fact] @@ -59,7 +59,7 @@ public void NullTest() { VariableDesignationSyntax designation = null; var vars = new HashSet() { "x" }.ToImmutableHashSet(); - Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); } @@ -71,10 +71,10 @@ public void DeclarationPattern() SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) ); var vars = new HashSet() { "x" }.ToImmutableHashSet(); - Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); vars = new HashSet() { "z" }.ToImmutableHashSet(); - Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); } [Fact] @@ -104,13 +104,13 @@ public void RecursivePattern_WithPositional() ); var vars = new HashSet() { "x" }.ToImmutableHashSet(); - Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); vars = new HashSet() { "y" }.ToImmutableHashSet(); - Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); vars = new HashSet() { "z" }.ToImmutableHashSet(); - Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); } [Fact] @@ -133,10 +133,10 @@ public void RecursivePattern_WithProperty() designation: default ); var vars = new HashSet() { "x" }.ToImmutableHashSet(); - Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); vars = new HashSet() { "z" }.ToImmutableHashSet(); - Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); } [Fact] @@ -157,10 +157,10 @@ public void RecursivePattern_WithDesignation() designation: SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) ); var vars = new HashSet() { "x" }.ToImmutableHashSet(); - Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); vars = new HashSet() { "z" }.ToImmutableHashSet(); - Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); } @@ -171,10 +171,10 @@ public void VarPattern() SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) ); var vars = new HashSet() { "x" }.ToImmutableHashSet(); - Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); vars = new HashSet() { "z" }.ToImmutableHashSet(); - Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); } @@ -187,7 +187,7 @@ public void BinaryPattern() SyntaxFactory.ConstantPattern(SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(99))) ); var vars = new HashSet() { "z" }.ToImmutableHashSet(); - Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); } [Fact] @@ -199,9 +199,9 @@ public void ParenthesizedPattern() ) ); var vars = new HashSet() { "x" }.ToImmutableHashSet(); - Assert.True(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); vars = new HashSet() { "z" }.ToImmutableHashSet(); - Assert.False(PattenMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); } } \ No newline at end of file From ce738a04a6432a6904a6bb92028836d5277e16bc Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Tue, 18 Apr 2023 23:25:30 +0100 Subject: [PATCH 10/13] format --- .../Analysis/MergeIfWithNestedIfAnalyzer.cs | 6 +- ...atternMatchingVariableDeclarationHelper.cs | 10 +- ...nnecessaryBracesInSwitchSectionAnalyzer.cs | 4 +- .../CSharp/SyntaxLogicalInverter.cs | 28 ++-- .../IfLocalVariableAnalysis.cs | 8 +- .../ReduceIfNestingAnalysis.cs | 12 +- ...veUnnecessaryBracesInSwitchSectionTests.cs | 2 +- .../RCS1208ReduceIfNestingTests.cs | 146 ++++++++++-------- ...nMatchingVariableDeclarationHelperTests.cs | 42 ++--- 9 files changed, 137 insertions(+), 121 deletions(-) diff --git a/src/Analyzers/CSharp/Analysis/MergeIfWithNestedIfAnalyzer.cs b/src/Analyzers/CSharp/Analysis/MergeIfWithNestedIfAnalyzer.cs index c325d2e79e..068ee31854 100644 --- a/src/Analyzers/CSharp/Analysis/MergeIfWithNestedIfAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/MergeIfWithNestedIfAnalyzer.cs @@ -71,10 +71,10 @@ private static void AnalyzeIfStatement(SyntaxNodeAnalysisContext context) if (!CheckTrivia(ifStatement, nestedIf.IfStatement)) return; - - if(IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, context.SemanticModel)) + + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, context.SemanticModel)) return; - + ReportDiagnostic(context, ifStatement, nestedIf.IfStatement); } diff --git a/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs b/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs index b360c54ac6..ff34037464 100644 --- a/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs +++ b/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs @@ -11,12 +11,12 @@ public static bool AnyDeclaredVariablesMatch(PatternSyntax pattern, ImmutableHas { return pattern switch { - RecursivePatternSyntax { PositionalPatternClause: var positionalPatternClause, PropertyPatternClause: var propertyPatternClause, Designation: var designation } => - (designation != null && AnyDeclaredVariablesMatch(designation, variableNames)) + 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) + BinaryPatternSyntax binaryPattern => + AnyDeclaredVariablesMatch(binaryPattern.Left, variableNames) || AnyDeclaredVariablesMatch(binaryPattern.Right, variableNames), ParenthesizedPatternSyntax parenthesizedPattern => AnyDeclaredVariablesMatch(parenthesizedPattern.Pattern, variableNames), DeclarationPatternSyntax { Designation: var variableDesignation } => AnyDeclaredVariablesMatch(variableDesignation, variableNames), @@ -24,7 +24,7 @@ public static bool AnyDeclaredVariablesMatch(PatternSyntax pattern, ImmutableHas _ => false }; } - + internal static bool AnyDeclaredVariablesMatch(VariableDesignationSyntax designation, ImmutableHashSet variableNames) { return designation switch diff --git a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs index b953ebef02..050437eadd 100644 --- a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs @@ -129,10 +129,10 @@ private static bool LocallyDeclaredVariablesOverlapWithAnyOtherSwitchSections(Sw if (label is not CasePatternSwitchLabelSyntax casePatternSwitchLabel) continue; - if (PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(casePatternSwitchLabel.Pattern,sectionDeclaredVariablesNames)) + if (PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(casePatternSwitchLabel.Pattern, sectionDeclaredVariablesNames)) return true; } - + foreach (var statement in otherSection.Statements) { foreach (var symbol in semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared) diff --git a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs index d42090810f..3c3a6914d9 100644 --- a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs +++ b/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()}'"); diff --git a/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs b/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs index 1c4a7d6dc7..0459e7e170 100644 --- a/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs +++ b/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs @@ -17,15 +17,15 @@ public static bool DoDeclaredVariablesOverlapWithOuterScope( SemanticModel semanticModel ) { - + var ifVariablesDeclared = semanticModel.AnalyzeDataFlow(ifStatement)! .VariablesDeclared; - + if (ifVariablesDeclared.IsEmpty) return false; - + var outerScope = GetOuterScope(ifStatement); - + if (outerScope == null) return true; diff --git a/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs b/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs index 8608f21772..7600486abb 100644 --- a/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs +++ b/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs @@ -107,7 +107,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( { return Fail(parent); } - + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel)) return Fail(parent); @@ -133,7 +133,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( { return Fail(parent); } - + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel)) return Fail(parent); @@ -142,10 +142,10 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( case SyntaxKind.OperatorDeclaration: case SyntaxKind.ConversionOperatorDeclaration: case SyntaxKind.GetAccessorDeclaration: - { + { if (jumpKind == SyntaxKind.None) return Fail(parent); - + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel)) return Fail(parent); @@ -154,7 +154,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( case SyntaxKind.MethodDeclaration: { var methodDeclaration = (MethodDeclarationSyntax)parent; - + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel)) return Fail(parent); @@ -188,7 +188,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( case SyntaxKind.LocalFunctionStatement: { var localFunction = (LocalFunctionStatementSyntax)parent; - + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel)) return Fail(parent); diff --git a/src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs b/src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs index c992a33e36..8fc2f578da 100644 --- a/src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs @@ -295,7 +295,7 @@ void M() } "); } - + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.RemoveUnnecessaryBracesInSwitchSection)] public async Task TestNoDiagnostic_WhenOverlappingLocalVariableWithRecursivePatternMatchDeclaration() { diff --git a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs index b59e9b73fe..c52c659d55 100644 --- a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs @@ -183,7 +183,7 @@ void M2() "); } - + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] public async Task Test_WhenParentIsLocalFunction() { @@ -230,7 +230,7 @@ void M2() } "); } - + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] public async Task Test_WhenParentIsMethod() { @@ -268,7 +268,7 @@ void M2() } "); } - + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsConstructor() { @@ -334,81 +334,58 @@ void M2() "); } - [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] - public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsConversionOperator() + public async Task Test_InvertingCoalesceToTrue() { - await VerifyNoDiagnosticAsync(@" + await VerifyDiagnosticAndFixAsync(@" class C { - static bool b=false; - public static implicit operator bool(C c) + void M(bool? p) { - if (b) - { - var s = 1; - } - if (b) + [|if|] (p??true) { - var s = 2; - return M2(); + M2(); } - return false; } - static bool M2() + void M2() { - return true; } } -"); - } - - [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] - public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsGetAccessor() - { - await VerifyNoDiagnosticAsync(@" +", @" class C { - static bool b=false; - public static bool s { - get { - if (b) - { - var s = 1; - } + void M(bool? p) + { + if (p == false) + { + return; + } - if (b) - { - var s = 2; - return M2(); - } - return false; - } + M2(); } - static bool M2() + void M2() { - return true; } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] - public async Task Test_InvertingCoalesceToTrue() + public async Task Test_InvertingCoalesceToUnknown() { await VerifyDiagnosticAndFixAsync(@" class C { + bool b { get; set; } void M(bool? p) { - [|if|] (p??true) + [|if|] (p??b) { M2(); } } - void M2() { } @@ -416,67 +393,105 @@ void M2() ", @" class C { + bool b { get; set; } void M(bool? p) { - if (p == false) + if (!(p ?? b)) { return; } M2(); } - void M2() { } } "); } - + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] - public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsLambda() + public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsConversionOperator() { await VerifyNoDiagnosticAsync(@" class C { - void M(bool p) + static bool b=false; + public static implicit operator bool(C c) { - var f = () => + if (b) { - if (p) + var s = 1; + } + if (b) + { + var s = 2; + return M2(); + } + return false; + } + + static bool M2() + { + return true; + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] + public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsGetAccessor() + { + await VerifyNoDiagnosticAsync(@" +class C +{ + static bool b=false; + public static bool s { + get { + if (b) { var s = 1; } - if (p) + if (b) { var s = 2; - M2(); + return M2(); } - }; + return false; + } } - void M2() + static bool M2() { + return true; } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] - public async Task Test_InvertingCoalesceToUnknown() + public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsLambda() { - await VerifyDiagnosticAndFixAsync(@" + await VerifyNoDiagnosticAsync(@" class C { - bool b { get; set; } - - void M(bool? p) + void M(bool p) { - [|if|] (p??b) + var f = () => { - M2(); - } + if (p) + { + var s = 1; + } + + if (p) + { + var s = 2; + M2(); + } + }; } void M2() @@ -485,7 +500,8 @@ void M2() } "); } - + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsLocalFunction() { @@ -518,7 +534,7 @@ void M2() } "); } - + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsMethod() { diff --git a/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs b/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs index 254c154578..c1d83fb032 100644 --- a/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs +++ b/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs @@ -19,11 +19,11 @@ public void SingleVariableDesignation() ); var vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); - + vars = new HashSet() { "z" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); } - + [Fact] public void ParenthesizedVariableDesignation() { @@ -40,7 +40,7 @@ public void ParenthesizedVariableDesignation() vars = new HashSet() { "y" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); - + vars = new HashSet() { "z" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); @@ -53,16 +53,16 @@ public void DiscardDesignation() var vars = new HashSet() { "z" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); } - + [Fact] public void NullTest() { VariableDesignationSyntax designation = null; var vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); - + } - + [Fact] public void DeclarationPattern() { @@ -72,11 +72,11 @@ public void DeclarationPattern() ); var vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); - + vars = new HashSet() { "z" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); } - + [Fact] public void RecursivePattern_WithPositional() { @@ -102,17 +102,17 @@ public void RecursivePattern_WithPositional() propertyPatternClause: default, designation: default ); - + var vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); vars = new HashSet() { "y" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); - + vars = new HashSet() { "z" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); } - + [Fact] public void RecursivePattern_WithProperty() { @@ -134,11 +134,11 @@ public void RecursivePattern_WithProperty() ); var vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); - + vars = new HashSet() { "z" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); } - + [Fact] public void RecursivePattern_WithDesignation() { @@ -158,12 +158,12 @@ public void RecursivePattern_WithDesignation() ); var vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); - + vars = new HashSet() { "z" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); - + } - + [Fact] public void VarPattern() { @@ -172,12 +172,12 @@ public void VarPattern() ); var vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); - + vars = new HashSet() { "z" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); - + } - + [Fact] public void BinaryPattern() { @@ -189,7 +189,7 @@ public void BinaryPattern() var vars = new HashSet() { "z" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); } - + [Fact] public void ParenthesizedPattern() { @@ -200,7 +200,7 @@ public void ParenthesizedPattern() ); var vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); - + vars = new HashSet() { "z" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); } From a0ca029c07d9ed559ae7d16b4de2fbf8a90f7cf2 Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Wed, 19 Apr 2023 00:02:55 +0100 Subject: [PATCH 11/13] diagnostic fixes --- .../PatternMatchingVariableDeclarationHelper.cs | 9 ++++----- .../Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs | 3 +-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs b/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs index ff34037464..afb5062c36 100644 --- a/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs +++ b/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs @@ -12,9 +12,9 @@ public static bool AnyDeclaredVariablesMatch(PatternSyntax pattern, ImmutableHas 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))), + (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), @@ -35,5 +35,4 @@ internal static bool AnyDeclaredVariablesMatch(VariableDesignationSyntax designa _ => false }; } - -} \ No newline at end of file +} diff --git a/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs b/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs index 0459e7e170..b9d448f5a0 100644 --- a/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs +++ b/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs @@ -17,7 +17,6 @@ public static bool DoDeclaredVariablesOverlapWithOuterScope( SemanticModel semanticModel ) { - var ifVariablesDeclared = semanticModel.AnalyzeDataFlow(ifStatement)! .VariablesDeclared; @@ -26,7 +25,7 @@ SemanticModel semanticModel var outerScope = GetOuterScope(ifStatement); - if (outerScope == null) + if (outerScope is null) return true; IList parentVariablesDeclared; From 74b1b88eaf48c389a468da7ad7bf329f69bdd913 Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Wed, 19 Apr 2023 20:48:42 +0100 Subject: [PATCH 12/13] CR comment --- .../IfLocalVariableAnalysis.cs | 67 +++---------------- 1 file changed, 11 insertions(+), 56 deletions(-) diff --git a/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs b/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs index b9d448f5a0..a834c9f1a1 100644 --- a/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs +++ b/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs @@ -1,9 +1,5 @@ // 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.Collections.Immutable; -using System.Diagnostics; -using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -22,63 +18,22 @@ SemanticModel semanticModel if (ifVariablesDeclared.IsEmpty) return false; - - var outerScope = GetOuterScope(ifStatement); - - if (outerScope is null) - return true; - - IList parentVariablesDeclared; - if (outerScope is SwitchSectionSyntax switchSection) + + foreach (StatementSyntax statement in SyntaxInfo.StatementListInfo(ifStatement).Statements) { - List allDeclaredVariables = null; - foreach (StatementSyntax statement in switchSection.Statements) - { - ImmutableArray variables = semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared; + if (statement == ifStatement) + continue; - if (variables.Any()) - (allDeclaredVariables ??= new List()).AddRange(variables); + foreach (ISymbol parentVariable in semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared) + { + foreach (ISymbol ifVariable in ifVariablesDeclared) + { + if (ifVariable.Name == parentVariable.Name) + return true; + } } - - parentVariablesDeclared = allDeclaredVariables; - } - else - { - parentVariablesDeclared = semanticModel.AnalyzeDataFlow(outerScope)!.VariablesDeclared; - } - - if (parentVariablesDeclared.Count <= ifVariablesDeclared.Length) - return false; - - // 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 - }; - } } \ No newline at end of file From 94323c7f4d5321da7942435a8007b9eaeadb566e Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Fri, 21 Apr 2023 13:44:45 +0200 Subject: [PATCH 13/13] Fix code style --- ...atternMatchingVariableDeclarationHelper.cs | 6 ++-- ...nnecessaryBracesInSwitchSectionAnalyzer.cs | 6 ++-- .../IfLocalVariableAnalysis.cs | 7 ++-- ...veUnnecessaryBracesInSwitchSectionTests.cs | 2 -- .../RCS1208ReduceIfNestingTests.cs | 3 -- ...nMatchingVariableDeclarationHelperTests.cs | 36 +++++++++---------- 6 files changed, 26 insertions(+), 34 deletions(-) diff --git a/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs b/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs index afb5062c36..6949bb4f6f 100644 --- a/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs +++ b/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs @@ -13,11 +13,11 @@ public static bool AnyDeclaredVariablesMatch(PatternSyntax pattern, ImmutableHas { 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), + || (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), + || AnyDeclaredVariablesMatch(binaryPattern.Right, variableNames), ParenthesizedPatternSyntax parenthesizedPattern => AnyDeclaredVariablesMatch(parenthesizedPattern.Pattern, variableNames), DeclarationPatternSyntax { Designation: var variableDesignation } => AnyDeclaredVariablesMatch(variableDesignation, variableNames), VarPatternSyntax { Designation: var variableDesignation } => AnyDeclaredVariablesMatch(variableDesignation, variableNames), diff --git a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs index 050437eadd..a0c3b184e9 100644 --- a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs @@ -124,7 +124,7 @@ private static bool LocallyDeclaredVariablesOverlapWithAnyOtherSwitchSections(Sw if (otherSection.Span.Contains(switchBlock.Span)) continue; - foreach (var label in otherSection.Labels) + foreach (SwitchLabelSyntax label in otherSection.Labels) { if (label is not CasePatternSwitchLabelSyntax casePatternSwitchLabel) continue; @@ -133,9 +133,9 @@ private static bool LocallyDeclaredVariablesOverlapWithAnyOtherSwitchSections(Sw return true; } - foreach (var statement in otherSection.Statements) + foreach (StatementSyntax statement in otherSection.Statements) { - foreach (var symbol in semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared) + foreach (ISymbol symbol in semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared) { if (sectionDeclaredVariablesNames.Contains(symbol.Name)) return true; diff --git a/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs b/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs index a834c9f1a1..0563f2a6ca 100644 --- a/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs +++ b/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs @@ -1,5 +1,6 @@ // 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; @@ -13,12 +14,12 @@ public static bool DoDeclaredVariablesOverlapWithOuterScope( SemanticModel semanticModel ) { - var ifVariablesDeclared = semanticModel.AnalyzeDataFlow(ifStatement)! + ImmutableArray ifVariablesDeclared = semanticModel.AnalyzeDataFlow(ifStatement)! .VariablesDeclared; if (ifVariablesDeclared.IsEmpty) return false; - + foreach (StatementSyntax statement in SyntaxInfo.StatementListInfo(ifStatement).Statements) { if (statement == ifStatement) @@ -36,4 +37,4 @@ SemanticModel semanticModel return false; } -} \ No newline at end of file +} diff --git a/src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs b/src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs index 8fc2f578da..285a796582 100644 --- a/src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs @@ -328,5 +328,3 @@ void M() "); } } - - diff --git a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs index c52c659d55..e10929fed9 100644 --- a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs @@ -183,7 +183,6 @@ void M2() "); } - [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] public async Task Test_WhenParentIsLocalFunction() { @@ -410,7 +409,6 @@ void M2() "); } - [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsConversionOperator() { @@ -501,7 +499,6 @@ void M2() "); } - [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsLocalFunction() { diff --git a/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs b/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs index c1d83fb032..6f7fc53e67 100644 --- a/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs +++ b/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs @@ -17,7 +17,7 @@ public void SingleVariableDesignation() VariableDesignationSyntax designation = SyntaxFactory.SingleVariableDesignation( SyntaxFactory.Identifier("x") ); - var vars = new HashSet() { "x" }.ToImmutableHashSet(); + ImmutableHashSet vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); vars = new HashSet() { "z" }.ToImmutableHashSet(); @@ -28,14 +28,14 @@ public void SingleVariableDesignation() public void ParenthesizedVariableDesignation() { VariableDesignationSyntax designation = SyntaxFactory.ParenthesizedVariableDesignation( - SyntaxFactory.SeparatedList(new List + SyntaxFactory.SeparatedList(new List() { SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")), SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("y")) }) ); - var vars = new HashSet() { "x" }.ToImmutableHashSet(); + ImmutableHashSet vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); vars = new HashSet() { "y" }.ToImmutableHashSet(); @@ -43,14 +43,13 @@ public void ParenthesizedVariableDesignation() vars = new HashSet() { "z" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); - } [Fact] public void DiscardDesignation() { VariableDesignationSyntax designation = SyntaxFactory.DiscardDesignation(); - var vars = new HashSet() { "z" }.ToImmutableHashSet(); + ImmutableHashSet vars = new HashSet() { "z" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); } @@ -58,9 +57,8 @@ public void DiscardDesignation() public void NullTest() { VariableDesignationSyntax designation = null; - var vars = new HashSet() { "x" }.ToImmutableHashSet(); + ImmutableHashSet vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); - } [Fact] @@ -70,7 +68,7 @@ public void DeclarationPattern() SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.IntKeyword)), SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) ); - var vars = new HashSet() { "x" }.ToImmutableHashSet(); + ImmutableHashSet vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); vars = new HashSet() { "z" }.ToImmutableHashSet(); @@ -83,7 +81,7 @@ public void RecursivePattern_WithPositional() PatternSyntax pattern = SyntaxFactory.RecursivePattern( SyntaxFactory.IdentifierName("TypeA"), positionalPatternClause: SyntaxFactory.PositionalPatternClause( - SyntaxFactory.SeparatedList(new List + SyntaxFactory.SeparatedList(new List() { SyntaxFactory.Subpattern( SyntaxFactory.DeclarationPattern( @@ -103,7 +101,7 @@ public void RecursivePattern_WithPositional() designation: default ); - var vars = new HashSet() { "x" }.ToImmutableHashSet(); + ImmutableHashSet vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); vars = new HashSet() { "y" }.ToImmutableHashSet(); @@ -120,7 +118,7 @@ public void RecursivePattern_WithProperty() SyntaxFactory.IdentifierName("TypeA"), positionalPatternClause: default, propertyPatternClause: SyntaxFactory.PropertyPatternClause( - SyntaxFactory.SeparatedList(new List + SyntaxFactory.SeparatedList(new List() { SyntaxFactory.Subpattern( SyntaxFactory.NameColon(SyntaxFactory.IdentifierName("PropertyName")), @@ -132,7 +130,7 @@ public void RecursivePattern_WithProperty() ), designation: default ); - var vars = new HashSet() { "x" }.ToImmutableHashSet(); + ImmutableHashSet vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); vars = new HashSet() { "z" }.ToImmutableHashSet(); @@ -146,7 +144,7 @@ public void RecursivePattern_WithDesignation() SyntaxFactory.IdentifierName("TypeA"), positionalPatternClause: default, propertyPatternClause: SyntaxFactory.PropertyPatternClause( - SyntaxFactory.SeparatedList(new List + SyntaxFactory.SeparatedList(new List() { SyntaxFactory.Subpattern( SyntaxFactory.NameColon(SyntaxFactory.IdentifierName("PropertyName")), @@ -156,12 +154,11 @@ public void RecursivePattern_WithDesignation() ), designation: SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) ); - var vars = new HashSet() { "x" }.ToImmutableHashSet(); + ImmutableHashSet vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); vars = new HashSet() { "z" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); - } [Fact] @@ -170,12 +167,11 @@ public void VarPattern() PatternSyntax pattern = SyntaxFactory.VarPattern( SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) ); - var vars = new HashSet() { "x" }.ToImmutableHashSet(); + ImmutableHashSet vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); vars = new HashSet() { "z" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); - } [Fact] @@ -186,7 +182,7 @@ public void BinaryPattern() SyntaxFactory.ConstantPattern(SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(42))), SyntaxFactory.ConstantPattern(SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(99))) ); - var vars = new HashSet() { "z" }.ToImmutableHashSet(); + ImmutableHashSet vars = new HashSet() { "z" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); } @@ -198,10 +194,10 @@ public void ParenthesizedPattern() SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) ) ); - var vars = new HashSet() { "x" }.ToImmutableHashSet(); + ImmutableHashSet vars = new HashSet() { "x" }.ToImmutableHashSet(); Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); vars = new HashSet() { "z" }.ToImmutableHashSet(); Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); } -} \ No newline at end of file +}