From 61d3000723246933d3c36145a8942c307343a6fa Mon Sep 17 00:00:00 2001 From: Pavel Mikula Date: Thu, 27 Jun 2024 10:38:47 +0200 Subject: [PATCH] File-scoped NS --- .../Rules/CastShouldNotBeDuplicated.cs | 347 +++++++++--------- .../Rules/CastShouldNotBeDuplicatedTest.cs | 85 +++-- 2 files changed, 215 insertions(+), 217 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/CastShouldNotBeDuplicated.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/CastShouldNotBeDuplicated.cs index 7e66194edfb..7d2661ea659 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/CastShouldNotBeDuplicated.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/CastShouldNotBeDuplicated.cs @@ -18,228 +18,227 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -namespace SonarAnalyzer.Rules.CSharp +namespace SonarAnalyzer.Rules.CSharp; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class CastShouldNotBeDuplicated : SonarDiagnosticAnalyzer { - [DiagnosticAnalyzer(LanguageNames.CSharp)] - public sealed class CastShouldNotBeDuplicated : SonarDiagnosticAnalyzer - { - private const string DiagnosticId = "S3247"; - private const string MessageFormat = "{0}"; - private const string ReplaceWithAsAndNullCheckMessage = "Replace this type-check-and-cast sequence with an 'as' and a null check."; - private const string RemoveRedundantCastAnotherVariableMessage = "Remove this cast and use the appropriate variable."; - private const string RemoveRedundantCastMessage = "Remove this redundant cast."; + private const string DiagnosticId = "S3247"; + private const string MessageFormat = "{0}"; + private const string ReplaceWithAsAndNullCheckMessage = "Replace this type-check-and-cast sequence with an 'as' and a null check."; + private const string RemoveRedundantCastAnotherVariableMessage = "Remove this cast and use the appropriate variable."; + private const string RemoveRedundantCastMessage = "Remove this redundant cast."; - private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat); + private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat); - public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Rule); + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Rule); - protected override void Initialize(SonarAnalysisContext context) - { - context.RegisterNodeAction(IsExpression, SyntaxKind.IsExpression); - context.RegisterNodeAction(IsPatternExpression, SyntaxKindEx.IsPatternExpression); - context.RegisterNodeAction(SwitchExpressionArm, SyntaxKindEx.SwitchExpressionArm); - context.RegisterNodeAction(CasePatternSwitchLabel, SyntaxKindEx.CasePatternSwitchLabel); - } + protected override void Initialize(SonarAnalysisContext context) + { + context.RegisterNodeAction(IsExpression, SyntaxKind.IsExpression); + context.RegisterNodeAction(IsPatternExpression, SyntaxKindEx.IsPatternExpression); + context.RegisterNodeAction(SwitchExpressionArm, SyntaxKindEx.SwitchExpressionArm); + context.RegisterNodeAction(CasePatternSwitchLabel, SyntaxKindEx.CasePatternSwitchLabel); + } - private static void CasePatternSwitchLabel(SonarSyntaxNodeReportingContext analysisContext) + private static void CasePatternSwitchLabel(SonarSyntaxNodeReportingContext analysisContext) + { + var casePatternSwitch = (CasePatternSwitchLabelSyntaxWrapper)analysisContext.Node; + if (casePatternSwitch.SyntaxNode.GetFirstNonParenthesizedParent().GetFirstNonParenthesizedParent() is not SwitchStatementSyntax parentSwitchStatement) { - var casePatternSwitch = (CasePatternSwitchLabelSyntaxWrapper)analysisContext.Node; - if (casePatternSwitch.SyntaxNode.GetFirstNonParenthesizedParent().GetFirstNonParenthesizedParent() is not SwitchStatementSyntax parentSwitchStatement) - { - return; - } - ProcessPatternExpression(analysisContext, - casePatternSwitch.Pattern, - parentSwitchStatement.Expression, - parentSwitchStatement); + return; } + ProcessPatternExpression(analysisContext, + casePatternSwitch.Pattern, + parentSwitchStatement.Expression, + parentSwitchStatement); + } - private static void SwitchExpressionArm(SonarSyntaxNodeReportingContext analysisContext) - { - var isSwitchExpression = (SwitchExpressionArmSyntaxWrapper)analysisContext.Node; - var parent = isSwitchExpression.SyntaxNode.GetFirstNonParenthesizedParent(); - - if (!parent.IsKind(SyntaxKindEx.SwitchExpression)) - { - return; - } - var switchExpression = (SwitchExpressionSyntaxWrapper)parent; - ProcessPatternExpression(analysisContext, - isSwitchExpression.Pattern, - switchExpression.GoverningExpression, - isSwitchExpression); - } + private static void SwitchExpressionArm(SonarSyntaxNodeReportingContext analysisContext) + { + var isSwitchExpression = (SwitchExpressionArmSyntaxWrapper)analysisContext.Node; + var parent = isSwitchExpression.SyntaxNode.GetFirstNonParenthesizedParent(); - private static void IsPatternExpression(SonarSyntaxNodeReportingContext analysisContext) + if (!parent.IsKind(SyntaxKindEx.SwitchExpression)) { - var isPatternExpression = (IsPatternExpressionSyntaxWrapper)analysisContext.Node; - if (isPatternExpression.SyntaxNode.GetFirstNonParenthesizedParent() is not IfStatementSyntax parentIfStatement) - { - return; - } - ProcessPatternExpression(analysisContext, - isPatternExpression.Pattern, - isPatternExpression.Expression, - parentIfStatement.Statement); + return; } + var switchExpression = (SwitchExpressionSyntaxWrapper)parent; + ProcessPatternExpression(analysisContext, + isSwitchExpression.Pattern, + switchExpression.GoverningExpression, + isSwitchExpression); + } - private static void IsExpression(SonarSyntaxNodeReportingContext analysisContext) + private static void IsPatternExpression(SonarSyntaxNodeReportingContext analysisContext) + { + var isPatternExpression = (IsPatternExpressionSyntaxWrapper)analysisContext.Node; + if (isPatternExpression.SyntaxNode.GetFirstNonParenthesizedParent() is not IfStatementSyntax parentIfStatement) { - var isExpression = (BinaryExpressionSyntax)analysisContext.Node; - - if (isExpression.Right is not TypeSyntax castType - || isExpression.GetFirstNonParenthesizedParent() is not IfStatementSyntax parentIfStatement - || analysisContext.SemanticModel.GetSymbolInfo(castType).Symbol is not INamedTypeSymbol castTypeSymbol - || castTypeSymbol.TypeKind == TypeKind.Struct) - { - return; - } - - ReportPatternAtMainVariable(analysisContext, isExpression.Left, isExpression.GetLocation(), parentIfStatement.Statement, castType, ReplaceWithAsAndNullCheckMessage); + return; } + ProcessPatternExpression(analysisContext, + isPatternExpression.Pattern, + isPatternExpression.Expression, + parentIfStatement.Statement); + } - private static List GetDuplicatedCastLocations(SonarSyntaxNodeReportingContext context, SyntaxNode parentStatement, TypeSyntax castType, SyntaxNode typedVariable) - { - var typeExpressionSymbol = context.SemanticModel.GetSymbolInfo(typedVariable).Symbol - ?? context.SemanticModel.GetDeclaredSymbol(typedVariable); - - return typeExpressionSymbol is null - ? [] - : parentStatement - .DescendantNodes() - .OfType() - .Where(x => x.Type.WithoutTrivia().IsEquivalentTo(castType.WithoutTrivia()) - && IsCastOnSameSymbol(x) - && !CSharpFacade.Instance.Syntax.IsInExpressionTree(context.SemanticModel, x)) // see https://github.com/SonarSource/sonar-dotnet/issues/8735#issuecomment-1943419398 - .Select(x => x.GetLocation()).ToList(); - - bool IsCastOnSameSymbol(CastExpressionSyntax castExpression) => - Equals(context.SemanticModel.GetSymbolInfo(castExpression.Expression).Symbol, typeExpressionSymbol); - } + private static void IsExpression(SonarSyntaxNodeReportingContext analysisContext) + { + var isExpression = (BinaryExpressionSyntax)analysisContext.Node; - private static void ProcessPatternExpression(SonarSyntaxNodeReportingContext analysisContext, - SyntaxNode isPattern, - SyntaxNode mainVariableExpression, - SyntaxNode parentStatement) + if (isExpression.Right is not TypeSyntax castType + || isExpression.GetFirstNonParenthesizedParent() is not IfStatementSyntax parentIfStatement + || analysisContext.SemanticModel.GetSymbolInfo(castType).Symbol is not INamedTypeSymbol castTypeSymbol + || castTypeSymbol.TypeKind == TypeKind.Struct) { - var objectToPattern = new Dictionary(); - PatternExpressionObjectToPatternMapping.MapObjectToPattern((ExpressionSyntax)mainVariableExpression.RemoveParentheses(), isPattern.RemoveParentheses(), objectToPattern); - foreach (var expressionPatternPair in objectToPattern) - { - var pattern = expressionPatternPair.Value; - var leftVariable = expressionPatternPair.Key; - var targetTypes = GetTypesFromPattern(pattern); - var rightPartsToCheck = new Dictionary>(); - foreach (var subPattern in pattern.DescendantNodesAndSelf().Where(x => x.IsAnyKind(SyntaxKindEx.DeclarationPattern, SyntaxKindEx.RecursivePattern))) - { - if (DeclarationPatternSyntaxWrapper.IsInstance(subPattern) && (DeclarationPatternSyntaxWrapper)subPattern is var declarationPattern) - { - rightPartsToCheck.Add(declarationPattern.Designation.SyntaxNode, new Tuple(declarationPattern.Type, subPattern.GetLocation())); - } - else if ((RecursivePatternSyntaxWrapper)subPattern is { Designation.SyntaxNode: { }, Type: { }} recursivePattern) - { - rightPartsToCheck.Add(recursivePattern.Designation.SyntaxNode, new Tuple(recursivePattern.Type, subPattern.GetLocation())); - } - } + return; + } - var mainVarMsg = rightPartsToCheck.Any() - ? RemoveRedundantCastAnotherVariableMessage - : RemoveRedundantCastMessage; - foreach (var targetType in targetTypes) - { - ReportPatternAtMainVariable(analysisContext, leftVariable, leftVariable.GetLocation(), parentStatement, targetType, mainVarMsg); - } + ReportPatternAtMainVariable(analysisContext, isExpression.Left, isExpression.GetLocation(), parentIfStatement.Statement, castType, ReplaceWithAsAndNullCheckMessage); + } - foreach (var variableTypePair in rightPartsToCheck) - { - ReportPatternAtCastLocation(analysisContext, variableTypePair.Key, variableTypePair.Value.Item2, parentStatement, variableTypePair.Value.Item1, RemoveRedundantCastMessage); - } - } - } + private static List GetDuplicatedCastLocations(SonarSyntaxNodeReportingContext context, SyntaxNode parentStatement, TypeSyntax castType, SyntaxNode typedVariable) + { + var typeExpressionSymbol = context.SemanticModel.GetSymbolInfo(typedVariable).Symbol + ?? context.SemanticModel.GetDeclaredSymbol(typedVariable); + + return typeExpressionSymbol is null + ? [] + : parentStatement + .DescendantNodes() + .OfType() + .Where(x => x.Type.WithoutTrivia().IsEquivalentTo(castType.WithoutTrivia()) + && IsCastOnSameSymbol(x) + && !CSharpFacade.Instance.Syntax.IsInExpressionTree(context.SemanticModel, x)) // see https://github.com/SonarSource/sonar-dotnet/issues/8735#issuecomment-1943419398 + .Select(x => x.GetLocation()).ToList(); + + bool IsCastOnSameSymbol(CastExpressionSyntax castExpression) => + Equals(context.SemanticModel.GetSymbolInfo(castExpression.Expression).Symbol, typeExpressionSymbol); + } - private static IEnumerable GetTypesFromPattern(SyntaxNode pattern) + private static void ProcessPatternExpression(SonarSyntaxNodeReportingContext analysisContext, + SyntaxNode isPattern, + SyntaxNode mainVariableExpression, + SyntaxNode parentStatement) + { + var objectToPattern = new Dictionary(); + PatternExpressionObjectToPatternMapping.MapObjectToPattern((ExpressionSyntax)mainVariableExpression.RemoveParentheses(), isPattern.RemoveParentheses(), objectToPattern); + foreach (var expressionPatternPair in objectToPattern) { - var targetTypes = new HashSet(); - if (RecursivePatternSyntaxWrapper.IsInstance(pattern) && ((RecursivePatternSyntaxWrapper)pattern is { PositionalPatternClause.SyntaxNode: { } } recursivePattern)) + var pattern = expressionPatternPair.Value; + var leftVariable = expressionPatternPair.Key; + var targetTypes = GetTypesFromPattern(pattern); + var rightPartsToCheck = new Dictionary>(); + foreach (var subPattern in pattern.DescendantNodesAndSelf().Where(x => x.IsAnyKind(SyntaxKindEx.DeclarationPattern, SyntaxKindEx.RecursivePattern))) { - foreach (var subpattern in recursivePattern.PositionalPatternClause.Subpatterns) + if (DeclarationPatternSyntaxWrapper.IsInstance(subPattern) && (DeclarationPatternSyntaxWrapper)subPattern is var declarationPattern) { - AddPatternType(subpattern.Pattern, targetTypes); + rightPartsToCheck.Add(declarationPattern.Designation.SyntaxNode, new Tuple(declarationPattern.Type, subPattern.GetLocation())); } - } - else if (BinaryPatternSyntaxWrapper.IsInstance(pattern) && (BinaryPatternSyntaxWrapper)pattern is { } binaryPattern) - { - AddPatternType(binaryPattern.Left, targetTypes); - AddPatternType(binaryPattern.Right, targetTypes); - } - else if (ListPatternSyntaxWrapper.IsInstance(pattern) && (ListPatternSyntaxWrapper)pattern is { } listPattern) - { - foreach (var subpattern in listPattern.Patterns) + else if ((RecursivePatternSyntaxWrapper)subPattern is { Designation.SyntaxNode: { }, Type: { }} recursivePattern) { - AddPatternType(subpattern, targetTypes); + rightPartsToCheck.Add(recursivePattern.Designation.SyntaxNode, new Tuple(recursivePattern.Type, subPattern.GetLocation())); } } - else + + var mainVarMsg = rightPartsToCheck.Any() + ? RemoveRedundantCastAnotherVariableMessage + : RemoveRedundantCastMessage; + foreach (var targetType in targetTypes) { - AddPatternType(pattern, targetTypes); + ReportPatternAtMainVariable(analysisContext, leftVariable, leftVariable.GetLocation(), parentStatement, targetType, mainVarMsg); } - return targetTypes; - static void AddPatternType(SyntaxNode pattern, ISet targetTypes) + foreach (var variableTypePair in rightPartsToCheck) { - if (GetType(pattern) is { } patternType) - { - targetTypes.Add(patternType); - } + ReportPatternAtCastLocation(analysisContext, variableTypePair.Key, variableTypePair.Value.Item2, parentStatement, variableTypePair.Value.Item1, RemoveRedundantCastMessage); } } + } - private static TypeSyntax GetType(SyntaxNode pattern) + private static IEnumerable GetTypesFromPattern(SyntaxNode pattern) + { + var targetTypes = new HashSet(); + if (RecursivePatternSyntaxWrapper.IsInstance(pattern) && ((RecursivePatternSyntaxWrapper)pattern is { PositionalPatternClause.SyntaxNode: { } } recursivePattern)) { - if (ConstantPatternSyntaxWrapper.IsInstance(pattern)) - { - return ((ConstantPatternSyntaxWrapper)pattern).Expression as TypeSyntax; - } - else if (DeclarationPatternSyntaxWrapper.IsInstance(pattern)) + foreach (var subpattern in recursivePattern.PositionalPatternClause.Subpatterns) { - return ((DeclarationPatternSyntaxWrapper)pattern).Type; + AddPatternType(subpattern.Pattern, targetTypes); } - else if (RecursivePatternSyntaxWrapper.IsInstance(pattern)) + } + else if (BinaryPatternSyntaxWrapper.IsInstance(pattern) && (BinaryPatternSyntaxWrapper)pattern is { } binaryPattern) + { + AddPatternType(binaryPattern.Left, targetTypes); + AddPatternType(binaryPattern.Right, targetTypes); + } + else if (ListPatternSyntaxWrapper.IsInstance(pattern) && (ListPatternSyntaxWrapper)pattern is { } listPattern) + { + foreach (var subpattern in listPattern.Patterns) { - return ((RecursivePatternSyntaxWrapper)pattern).Type; + AddPatternType(subpattern, targetTypes); } - return null; } + else + { + AddPatternType(pattern, targetTypes); + } + return targetTypes; - private static void ReportPatternAtMainVariable(SonarSyntaxNodeReportingContext context, - SyntaxNode variableExpression, - Location mainLocation, - SyntaxNode parentStatement, - TypeSyntax castType, - string message) + static void AddPatternType(SyntaxNode pattern, ISet targetTypes) { - var duplicatedCastLocations = GetDuplicatedCastLocations(context, parentStatement, castType, variableExpression); - if (duplicatedCastLocations.Any()) + if (GetType(pattern) is { } patternType) { - context.ReportIssue(Rule, mainLocation, duplicatedCastLocations.ToSecondary(), message); + targetTypes.Add(patternType); } } + } - private static void ReportPatternAtCastLocation(SonarSyntaxNodeReportingContext context, - SyntaxNode variableExpression, - Location patternLocation, - SyntaxNode parentStatement, - TypeSyntax castType, - string message) + private static TypeSyntax GetType(SyntaxNode pattern) + { + if (ConstantPatternSyntaxWrapper.IsInstance(pattern)) + { + return ((ConstantPatternSyntaxWrapper)pattern).Expression as TypeSyntax; + } + else if (DeclarationPatternSyntaxWrapper.IsInstance(pattern)) + { + return ((DeclarationPatternSyntaxWrapper)pattern).Type; + } + else if (RecursivePatternSyntaxWrapper.IsInstance(pattern)) { - if (context.SemanticModel.GetSymbolInfo(castType).Symbol is INamedTypeSymbol castTypeSymbol - && castTypeSymbol.TypeKind != TypeKind.Struct) + return ((RecursivePatternSyntaxWrapper)pattern).Type; + } + return null; + } + + private static void ReportPatternAtMainVariable(SonarSyntaxNodeReportingContext context, + SyntaxNode variableExpression, + Location mainLocation, + SyntaxNode parentStatement, + TypeSyntax castType, + string message) + { + var duplicatedCastLocations = GetDuplicatedCastLocations(context, parentStatement, castType, variableExpression); + if (duplicatedCastLocations.Any()) + { + context.ReportIssue(Rule, mainLocation, duplicatedCastLocations.ToSecondary(), message); + } + } + + private static void ReportPatternAtCastLocation(SonarSyntaxNodeReportingContext context, + SyntaxNode variableExpression, + Location patternLocation, + SyntaxNode parentStatement, + TypeSyntax castType, + string message) + { + if (context.SemanticModel.GetSymbolInfo(castType).Symbol is INamedTypeSymbol castTypeSymbol + && castTypeSymbol.TypeKind != TypeKind.Struct) + { + var duplicatedCastLocations = GetDuplicatedCastLocations(context, parentStatement, castType, variableExpression); + foreach (var castLocation in duplicatedCastLocations) { - var duplicatedCastLocations = GetDuplicatedCastLocations(context, parentStatement, castType, variableExpression); - foreach (var castLocation in duplicatedCastLocations) - { - context.ReportIssue(Rule, castLocation, [patternLocation.ToSecondary()], message); - } + context.ReportIssue(Rule, castLocation, [patternLocation.ToSecondary()], message); } } } diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/CastShouldNotBeDuplicatedTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/CastShouldNotBeDuplicatedTest.cs index 0e9dd100bb7..eb5bc8809b1 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/CastShouldNotBeDuplicatedTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/CastShouldNotBeDuplicatedTest.cs @@ -20,59 +20,58 @@ using SonarAnalyzer.Rules.CSharp; -namespace SonarAnalyzer.Test.Rules +namespace SonarAnalyzer.Test.Rules; + +[TestClass] +public class CastShouldNotBeDuplicatedTest { - [TestClass] - public class CastShouldNotBeDuplicatedTest - { - private static readonly VerifierBuilder Builder = new VerifierBuilder(); - public TestContext TestContext { get; set; } + private static readonly VerifierBuilder Builder = new VerifierBuilder(); + public TestContext TestContext { get; set; } - [TestMethod] - public void CastShouldNotBeDuplicated() => - Builder.AddPaths("CastShouldNotBeDuplicated.cs").Verify(); + [TestMethod] + public void CastShouldNotBeDuplicated() => + Builder.AddPaths("CastShouldNotBeDuplicated.cs").Verify(); #if NET - [TestMethod] - public void CastShouldNotBeDuplicated_CSharp9() => - Builder.AddPaths("CastShouldNotBeDuplicated.CSharp9.cs") - .WithOptions(ParseOptionsHelper.FromCSharp9) - .Verify(); + [TestMethod] + public void CastShouldNotBeDuplicated_CSharp9() => + Builder.AddPaths("CastShouldNotBeDuplicated.CSharp9.cs") + .WithOptions(ParseOptionsHelper.FromCSharp9) + .Verify(); - [TestMethod] - public void CastShouldNotBeDuplicated_CSharp10() => - Builder.AddPaths("CastShouldNotBeDuplicated.CSharp10.cs") - .WithOptions(ParseOptionsHelper.FromCSharp10) - .Verify(); + [TestMethod] + public void CastShouldNotBeDuplicated_CSharp10() => + Builder.AddPaths("CastShouldNotBeDuplicated.CSharp10.cs") + .WithOptions(ParseOptionsHelper.FromCSharp10) + .Verify(); - [TestMethod] - public void CastShouldNotBeDuplicated_CSharp11() => - Builder.AddPaths("CastShouldNotBeDuplicated.CSharp11.cs") - .WithOptions(ParseOptionsHelper.FromCSharp11) - .Verify(); + [TestMethod] + public void CastShouldNotBeDuplicated_CSharp11() => + Builder.AddPaths("CastShouldNotBeDuplicated.CSharp11.cs") + .WithOptions(ParseOptionsHelper.FromCSharp11) + .Verify(); - [TestMethod] - public void CastShouldNotBeDuplicated_CSharp12() => - Builder.AddPaths("CastShouldNotBeDuplicated.CSharp12.cs") - .WithOptions(ParseOptionsHelper.FromCSharp12) - .Verify(); + [TestMethod] + public void CastShouldNotBeDuplicated_CSharp12() => + Builder.AddPaths("CastShouldNotBeDuplicated.CSharp12.cs") + .WithOptions(ParseOptionsHelper.FromCSharp12) + .Verify(); - [TestMethod] - public void CastShouldNotBeDuplicated_MvcView() => - Builder - .AddSnippet(""" - public class Base {} - public class Derived: Base - { - public int Prop { get; set; } - } - """) - .AddPaths("CastShouldNotBeDuplicated.cshtml") - .WithAdditionalFilePath(AnalysisScaffolding.CreateSonarProjectConfig(TestContext, ProjectType.Product)) - .VerifyNoIssues(); + [TestMethod] + public void CastShouldNotBeDuplicated_MvcView() => + Builder + .AddSnippet(""" + public class Base {} + public class Derived: Base + { + public int Prop { get; set; } + } + """) + .AddPaths("CastShouldNotBeDuplicated.cshtml") + .WithAdditionalFilePath(AnalysisScaffolding.CreateSonarProjectConfig(TestContext, ProjectType.Product)) + .VerifyNoIssues(); #endif - } }