diff --git a/ChangeLog.md b/ChangeLog.md index 00c5077e66..690fdc4920 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/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)). diff --git a/src/Analyzers/CSharp/Analysis/MergeIfWithNestedIfAnalyzer.cs b/src/Analyzers/CSharp/Analysis/MergeIfWithNestedIfAnalyzer.cs index 43e07fdc73..068ee31854 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; @@ -72,6 +72,9 @@ private static void AnalyzeIfStatement(SyntaxNodeAnalysisContext context) if (!CheckTrivia(ifStatement, nestedIf.IfStatement)) return; + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, context.SemanticModel)) + return; + ReportDiagnostic(context, ifStatement, nestedIf.IfStatement); } diff --git a/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs b/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs new file mode 100644 index 0000000000..6949bb4f6f --- /dev/null +++ b/src/Analyzers/CSharp/Analysis/PattenMatchingAnalysis/PatternMatchingVariableDeclarationHelper.cs @@ -0,0 +1,38 @@ +// Copyright (c) Josef Pihrt and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace Roslynator.CSharp.Analysis; + +public static class PatternMatchingVariableDeclarationHelper +{ + public static bool AnyDeclaredVariablesMatch(PatternSyntax pattern, ImmutableHashSet variableNames) + { + return pattern switch + { + RecursivePatternSyntax { PositionalPatternClause: var positionalPatternClause, PropertyPatternClause: var propertyPatternClause, Designation: var designation } => + (designation is not null && AnyDeclaredVariablesMatch(designation, variableNames)) + || (propertyPatternClause?.Subpatterns.Any(p => AnyDeclaredVariablesMatch(p.Pattern, variableNames)) ?? false) + || (positionalPatternClause?.Subpatterns.Any(p => AnyDeclaredVariablesMatch(p.Pattern, variableNames)) ?? false), + BinaryPatternSyntax binaryPattern => + AnyDeclaredVariablesMatch(binaryPattern.Left, variableNames) + || AnyDeclaredVariablesMatch(binaryPattern.Right, variableNames), + ParenthesizedPatternSyntax parenthesizedPattern => AnyDeclaredVariablesMatch(parenthesizedPattern.Pattern, variableNames), + DeclarationPatternSyntax { Designation: var variableDesignation } => AnyDeclaredVariablesMatch(variableDesignation, variableNames), + VarPatternSyntax { Designation: var variableDesignation } => AnyDeclaredVariablesMatch(variableDesignation, variableNames), + _ => false + }; + } + + internal static bool AnyDeclaredVariablesMatch(VariableDesignationSyntax designation, ImmutableHashSet variableNames) + { + return designation switch + { + SingleVariableDesignationSyntax singleVariableDesignation => variableNames.Contains(singleVariableDesignation.Identifier.ValueText), + ParenthesizedVariableDesignationSyntax parenthesizedVariableDesignation => parenthesizedVariableDesignation.Variables.Any(variable => AnyDeclaredVariablesMatch(variable, variableNames)), + DiscardDesignationSyntax _ => false, + _ => false + }; + } +} diff --git a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs index 1dec09e2a7..a0c3b184e9 100644 --- a/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RemoveUnnecessaryBracesInSwitchSectionAnalyzer.cs @@ -121,18 +121,26 @@ private static bool LocallyDeclaredVariablesOverlapWithAnyOtherSwitchSections(Sw foreach (SwitchSectionSyntax otherSection in switchStatement.Sections) { - // If the other section is not a block then we do not need to check as if there were overlapping variables then there would already be a error. - if (otherSection.Statements.SingleOrDefault(shouldThrow: false) is not BlockSyntax otherBlock) + if (otherSection.Span.Contains(switchBlock.Span)) continue; - if (otherBlock.Span == switchBlock.Span) - continue; - - foreach (ISymbol v in semanticModel.AnalyzeDataFlow(otherBlock)!.VariablesDeclared) + foreach (SwitchLabelSyntax label in otherSection.Labels) { - if (sectionDeclaredVariablesNames.Contains(v.Name)) + if (label is not CasePatternSwitchLabelSyntax casePatternSwitchLabel) + continue; + + if (PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(casePatternSwitchLabel.Pattern, sectionDeclaredVariablesNames)) return true; } + + foreach (StatementSyntax statement in otherSection.Statements) + { + foreach (ISymbol symbol in semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared) + { + if (sectionDeclaredVariablesNames.Contains(symbol.Name)) + return true; + } + } } return false; 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 new file mode 100644 index 0000000000..0563f2a6ca --- /dev/null +++ b/src/Common/CSharp/Analysis/ReduceIfNesting/IfLocalVariableAnalysis.cs @@ -0,0 +1,40 @@ +// Copyright (c) Josef Pihrt and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace Roslynator.CSharp.Analysis.ReduceIfNesting; + +internal static class IfStatementLocalVariableAnalysis +{ + public static bool DoDeclaredVariablesOverlapWithOuterScope( + IfStatementSyntax ifStatement, + SemanticModel semanticModel + ) + { + ImmutableArray ifVariablesDeclared = semanticModel.AnalyzeDataFlow(ifStatement)! + .VariablesDeclared; + + if (ifVariablesDeclared.IsEmpty) + return false; + + foreach (StatementSyntax statement in SyntaxInfo.StatementListInfo(ifStatement).Statements) + { + if (statement == ifStatement) + continue; + + foreach (ISymbol parentVariable in semanticModel.AnalyzeDataFlow(statement)!.VariablesDeclared) + { + foreach (ISymbol ifVariable in ifVariablesDeclared) + { + if (ifVariable.Name == parentVariable.Name) + return true; + } + } + } + + return false; + } +} diff --git a/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs b/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs index e76d87239c..7600486abb 100644 --- a/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs +++ b/src/Common/CSharp/Analysis/ReduceIfNesting/ReduceIfNestingAnalysis.cs @@ -2,7 +2,6 @@ using System.Collections.Immutable; using System.Diagnostics; -using System.Linq; using System.Threading; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -73,7 +72,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( return Fail(switchSection); } - if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, switchSection, semanticModel)) + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel)) return Fail(switchSection); return Success(jumpKind, switchSection); @@ -109,7 +108,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( return Fail(parent); } - if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parent, semanticModel)) + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel)) return Fail(parent); return Success(jumpKind, parent); @@ -135,7 +134,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( return Fail(parent); } - if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parent, semanticModel)) + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel)) return Fail(parent); return Success(jumpKind, parent); @@ -147,7 +146,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( if (jumpKind == SyntaxKind.None) return Fail(parent); - if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, parent, semanticModel)) + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel)) return Fail(parent); return Success(jumpKind, parent); @@ -156,7 +155,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( { var methodDeclaration = (MethodDeclarationSyntax)parent; - if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, methodDeclaration.Body, semanticModel)) + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel)) return Fail(parent); if (jumpKind != SyntaxKind.None) @@ -190,7 +189,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( { var localFunction = (LocalFunctionStatementSyntax)parent; - if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, localFunction.Body, semanticModel)) + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel)) return Fail(parent); if (jumpKind != SyntaxKind.None) @@ -224,7 +223,7 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( { var anonymousFunction = (AnonymousFunctionExpressionSyntax)parent; - if (LocallyDeclaredVariablesOverlapWithOuterScope(ifStatement, anonymousFunction.Block, semanticModel)) + if (IfStatementLocalVariableAnalysis.DoDeclaredVariablesOverlapWithOuterScope(ifStatement, semanticModel)) return Fail(parent); if (jumpKind != SyntaxKind.None) @@ -283,27 +282,6 @@ private static ReduceIfNestingAnalysisResult AnalyzeCore( return Fail(parent); } - private static bool LocallyDeclaredVariablesOverlapWithOuterScope( - IfStatementSyntax ifStatement, - SyntaxNode parent, - SemanticModel semanticModel - ) - { - ImmutableArray ifVariablesDeclared = semanticModel.AnalyzeDataFlow(ifStatement)! - .VariablesDeclared; - - if (ifVariablesDeclared.IsEmpty) - return false; - - ImmutableArray parentStatementDeclared = semanticModel.AnalyzeDataFlow(parent)! - .VariablesDeclared; - - // The parent's declared variables will include those from the if and so we have to check for any symbols occurring twice. - return ifVariablesDeclared.Any(variable => - parentStatementDeclared.Count(s => s.Name == variable.Name) > 1 - ); - } - private static bool IsNestedFix(SyntaxNode node, SemanticModel semanticModel, ReduceIfNestingOptions options, CancellationToken cancellationToken) { options |= ReduceIfNestingOptions.AllowNestedFix; diff --git a/src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs b/src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs index 90e693d538..285a796582 100644 --- a/src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1031RemoveUnnecessaryBracesInSwitchSectionTests.cs @@ -265,6 +265,66 @@ void M() } } } +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.RemoveUnnecessaryBracesInSwitchSection)] + public async Task TestNoDiagnostic_WhenOverlappingLocalVariableWithPatternMatchDeclaration() + { + await VerifyNoDiagnosticAsync(@" +using System; + +class C +{ + void M() + { + object o = null; + + switch (o) + { + case string s: + var x = 1; + break; + default: + { + var s = 1; + break; + } + } + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.RemoveUnnecessaryBracesInSwitchSection)] + public async Task TestNoDiagnostic_WhenOverlappingLocalVariableWithRecursivePatternMatchDeclaration() + { + await VerifyNoDiagnosticAsync(@" +using System; + +class C +{ + class Wrapper + { + public string S; + } + void M() + { + object o = null; + + switch (o) + { + case Wrapper { S: var s }: + var x = 1; + break; + default: + { + var s = 1; + break; + } + } + } +} "); } } diff --git a/src/Tests/Analyzers.Tests/RCS1061MergeIfWithNestedIfTests.cs b/src/Tests/Analyzers.Tests/RCS1061MergeIfWithNestedIfTests.cs new file mode 100644 index 0000000000..4458d43a87 --- /dev/null +++ b/src/Tests/Analyzers.Tests/RCS1061MergeIfWithNestedIfTests.cs @@ -0,0 +1,70 @@ +// Copyright (c) Josef Pihrt and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Roslynator.CSharp.CodeFixes; +using Roslynator.Testing.CSharp; +using Xunit; + +namespace Roslynator.CSharp.Analysis.Tests; + +public class 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)) + { + } + } + } +}"); + } +} diff --git a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs index 9a91c5a142..e10929fed9 100644 --- a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs @@ -13,7 +13,225 @@ public class RCS1208ReduceIfNestingTests : AbstractCSharpDiagnosticVerifier + { + [|if|] (p) + { + M2(); + } + }; + } + + void M2() + { + } +} +", @" +class C +{ + void M(bool p) + { + var f = () => + { + if (!p) + { + return; + } + + M2(); + } +; + } + + void M2() + { + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] + public async Task Test_WhenParentIsLocalFunction() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + void M(bool p) + { + void M3() + { + [|if|] (p) + { + M2(); + } + + } + M3(); + } + + void M2() + { + } +} +", @" +class C +{ + void M(bool p) + { + void M3() + { + if (!p) + { + return; + } + + M2(); + } + M3(); + } + + void M2() + { + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] + public async Task Test_WhenParentIsMethod() { await VerifyDiagnosticAndFixAsync(@" class C @@ -49,8 +267,34 @@ 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(); + } + } + + void M2() + { + } +} +"); + } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] public async Task Test_InvertingCoalesceToFalse() { @@ -88,8 +332,7 @@ void M2() } "); } - - + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] public async Task Test_InvertingCoalesceToTrue() { @@ -135,7 +378,6 @@ await VerifyDiagnosticAndFixAsync(@" class C { bool b { get; set; } - void M(bool? p) { [|if|] (p??b) @@ -143,7 +385,6 @@ void M(bool? p) M2(); } } - void M2() { } @@ -152,7 +393,6 @@ void M2() class C { bool b { get; set; } - void M(bool? p) { if (!(p ?? b)) @@ -162,6 +402,128 @@ void M(bool? p) M2(); } + 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 TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsGetAccessor() + { + 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) + { + var s = 1; + } + + if (p) + { + var s = 2; + M2(); + } + }; + } + + void M2() + { + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] + public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsLocalFunction() + { + await VerifyNoDiagnosticAsync(@" +class C +{ + void M(bool p) + { + void M3() + { + + if (p) + { + var s = 1; + } + + if (p) + { + var s = 2; + M2(); + } + + } + M3(); + } void M2() { @@ -171,7 +533,7 @@ void M2() } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] - public async Task TestNoDiagnostic_OverlappingLocalVariables() + public async Task TestNoDiagnostic_OverlappingLocalVariables_WhenParentIsMethod() { await VerifyNoDiagnosticAsync(@" class C diff --git a/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs b/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs new file mode 100644 index 0000000000..6f7fc53e67 --- /dev/null +++ b/src/Tests/Analyzers.Tests/UnitTests/PatternMatchingVariableDeclarationHelperTests.cs @@ -0,0 +1,203 @@ +// 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") + ); + ImmutableHashSet 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() + { + VariableDesignationSyntax designation = SyntaxFactory.ParenthesizedVariableDesignation( + SyntaxFactory.SeparatedList(new List() + { + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")), + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("y")) + }) + ); + + ImmutableHashSet vars = new HashSet() { "x" }.ToImmutableHashSet(); + Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + + vars = new HashSet() { "y" }.ToImmutableHashSet(); + Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + + vars = new HashSet() { "z" }.ToImmutableHashSet(); + Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + } + + [Fact] + public void DiscardDesignation() + { + VariableDesignationSyntax designation = SyntaxFactory.DiscardDesignation(); + ImmutableHashSet vars = new HashSet() { "z" }.ToImmutableHashSet(); + Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + } + + [Fact] + public void NullTest() + { + VariableDesignationSyntax designation = null; + ImmutableHashSet vars = new HashSet() { "x" }.ToImmutableHashSet(); + Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(designation, vars)); + } + + [Fact] + public void DeclarationPattern() + { + PatternSyntax pattern = SyntaxFactory.DeclarationPattern( + SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.IntKeyword)), + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) + ); + ImmutableHashSet 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() + { + 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 + ); + + ImmutableHashSet 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() + { + 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 + ); + ImmutableHashSet 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() + { + 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")) + ); + ImmutableHashSet 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() + { + PatternSyntax pattern = SyntaxFactory.VarPattern( + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) + ); + ImmutableHashSet 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() + { + PatternSyntax pattern = SyntaxFactory.BinaryPattern( + SyntaxKind.AndPattern, + SyntaxFactory.ConstantPattern(SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(42))), + SyntaxFactory.ConstantPattern(SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(99))) + ); + ImmutableHashSet vars = new HashSet() { "z" }.ToImmutableHashSet(); + Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + } + + [Fact] + public void ParenthesizedPattern() + { + PatternSyntax pattern = SyntaxFactory.ParenthesizedPattern( + SyntaxFactory.VarPattern( + SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier("x")) + ) + ); + ImmutableHashSet vars = new HashSet() { "x" }.ToImmutableHashSet(); + Assert.True(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + + vars = new HashSet() { "z" }.ToImmutableHashSet(); + Assert.False(PatternMatchingVariableDeclarationHelper.AnyDeclaredVariablesMatch(pattern, vars)); + } +}