Skip to content

Commit

Permalink
Implement
Browse files Browse the repository at this point in the history
  • Loading branch information
@pavel-mikula-sonarsource
pavel-mikula-sonarsource committed Jun 27, 2024
1 parent e78ad28 commit a5286af
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 32 deletions.
69 changes: 54 additions & 15 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/CastShouldNotBeDuplicated.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using SonarAnalyzer.Common.Walkers;

namespace SonarAnalyzer.Rules.CSharp;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
Expand All @@ -28,6 +30,7 @@ public sealed class CastShouldNotBeDuplicated : SonarDiagnosticAnalyzer
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 ExtractRepetitiveCastMessage = "Extract this repetitive cast into a variable.";

private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);

Expand All @@ -39,51 +42,61 @@ protected override void Initialize(SonarAnalysisContext context)
context.RegisterNodeAction(IsPatternExpression, SyntaxKindEx.IsPatternExpression);
context.RegisterNodeAction(SwitchExpressionArm, SyntaxKindEx.SwitchExpressionArm);
context.RegisterNodeAction(CasePatternSwitchLabel, SyntaxKindEx.CasePatternSwitchLabel);
context.RegisterNodeAction(Block, SyntaxKind.Block);
}

private static void CasePatternSwitchLabel(SonarSyntaxNodeReportingContext analysisContext)
private static void CasePatternSwitchLabel(SonarSyntaxNodeReportingContext context)
{
var casePatternSwitch = (CasePatternSwitchLabelSyntaxWrapper)analysisContext.Node;
var casePatternSwitch = (CasePatternSwitchLabelSyntaxWrapper)context.Node;
if (casePatternSwitch.SyntaxNode.GetFirstNonParenthesizedParent().GetFirstNonParenthesizedParent() is SwitchStatementSyntax parentSwitchStatement)
{
ProcessPatternExpression(analysisContext, casePatternSwitch.Pattern, parentSwitchStatement.Expression, parentSwitchStatement);
ProcessPatternExpression(context, casePatternSwitch.Pattern, parentSwitchStatement.Expression, parentSwitchStatement);
}
}

private static void SwitchExpressionArm(SonarSyntaxNodeReportingContext analysisContext)
private static void SwitchExpressionArm(SonarSyntaxNodeReportingContext context)
{
var isSwitchExpression = (SwitchExpressionArmSyntaxWrapper)analysisContext.Node;
var isSwitchExpression = (SwitchExpressionArmSyntaxWrapper)context.Node;
var parent = isSwitchExpression.SyntaxNode.GetFirstNonParenthesizedParent();
if (parent.IsKind(SyntaxKindEx.SwitchExpression))
{
var switchExpression = (SwitchExpressionSyntaxWrapper)parent;
ProcessPatternExpression(analysisContext, isSwitchExpression.Pattern, switchExpression.GoverningExpression, isSwitchExpression);
ProcessPatternExpression(context, isSwitchExpression.Pattern, switchExpression.GoverningExpression, isSwitchExpression);
}
}

private static void IsPatternExpression(SonarSyntaxNodeReportingContext analysisContext)
private static void IsPatternExpression(SonarSyntaxNodeReportingContext context)
{
var isPatternExpression = (IsPatternExpressionSyntaxWrapper)analysisContext.Node;
var isPatternExpression = (IsPatternExpressionSyntaxWrapper)context.Node;
if (isPatternExpression.SyntaxNode.GetFirstNonParenthesizedParent() is IfStatementSyntax parentIfStatement)
{
ProcessPatternExpression(analysisContext, isPatternExpression.Pattern, isPatternExpression.Expression, parentIfStatement.Statement);
ProcessPatternExpression(context, isPatternExpression.Pattern, isPatternExpression.Expression, parentIfStatement.Statement);
}
}

private static void IsExpression(SonarSyntaxNodeReportingContext analysisContext)
private static void IsExpression(SonarSyntaxNodeReportingContext context)
{
var isExpression = (BinaryExpressionSyntax)analysisContext.Node;
var isExpression = (BinaryExpressionSyntax)context.Node;
if (isExpression.Right is TypeSyntax castType
&& isExpression.GetFirstNonParenthesizedParent() is IfStatementSyntax parentIfStatement)
{
ReportPatternAtMainVariable(analysisContext, isExpression.Left, isExpression.GetLocation(), parentIfStatement.Statement, castType, ReplaceWithAsAndNullCheckMessage);
ReportPatternAtMainVariable(context, isExpression.Left, isExpression.GetLocation(), parentIfStatement.Statement, castType, ReplaceWithAsAndNullCheckMessage);
}
}

private static void Block(SonarSyntaxNodeReportingContext context)
{
if (context.Node.Parent is MemberDeclarationSyntax || context.Node.Parent is AccessorDeclarationSyntax)
{
var walker = new RepetitiveCastWalker(context);
walker.SafeVisit(context.Node);
}
}

private static Location[] DuplicatedCastLocations(SonarSyntaxNodeReportingContext context, SyntaxNode parentStatement, TypeSyntax castType, SyntaxNode typedVariable)
private static IEnumerable<Location> DuplicatedCastLocations(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().Where(IsDuplicatedCast).Select(x => x.GetLocation()).ToArray();
return typeExpressionSymbol is null ? [] : parentStatement.DescendantNodes().Where(IsDuplicatedCast).Select(x => x.GetLocation());

bool IsDuplicatedCast(SyntaxNode node)
{
Expand Down Expand Up @@ -208,7 +221,7 @@ private static void ReportPatternAtMainVariable(SonarSyntaxNodeReportingContext
TypeSyntax castType,
string message)
{
var duplicatedCastLocations = DuplicatedCastLocations(context, parentStatement, castType, variableExpression);
var duplicatedCastLocations = DuplicatedCastLocations(context, parentStatement, castType, variableExpression).Where(x => x != mainLocation).ToArray();
if (duplicatedCastLocations.Any())
{
context.ReportIssue(Rule, mainLocation, duplicatedCastLocations.ToSecondary(), message);
Expand All @@ -228,4 +241,30 @@ private static void ReportPatternAtCastLocation(SonarSyntaxNodeReportingContext
context.ReportIssue(Rule, castLocation, [patternLocation.ToSecondary()], message);
}
}

private sealed class RepetitiveCastWalker : SafeCSharpSyntaxWalker
{
private readonly SonarSyntaxNodeReportingContext context;
private readonly HashSet<CastExpressionSyntax> visited = new(new CSharpSyntaxNodeEqualityComparer<CastExpressionSyntax>());

public RepetitiveCastWalker(SonarSyntaxNodeReportingContext context) =>
this.context = context;

public override void Visit(SyntaxNode node)
{
if (!node.IsKind(SyntaxKind.Block) || node == context.Node) // We don't want to start the search in a nested node
{
base.Visit(node);
}
}

public override void VisitCastExpression(CastExpressionSyntax node)
{
if (visited.Add(node))
{
ReportPatternAtMainVariable(context, node.Expression, node.GetLocation(), node.FirstAncestorOrSelf<BlockSyntax>(), node.Type, ExtractRepetitiveCastMessage);
}
base.VisitCastExpression(node);
}
}
}
36 changes: 19 additions & 17 deletions analyzers/tests/SonarAnalyzer.Test/TestCases/CastShouldNotBeDuplicated.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ public void Foo(Object x)
if (x is Fruit) // Noncompliant
{
var f1 = (Fruit)x;
// ^^^^^^^^ Secondary
var f2 = (Fruit)x;
// ^^^^^^^^ Secondary
}

Expand Down Expand Up @@ -108,47 +106,49 @@ public void UnknownFoo(object x)

public void MultipleCasts_RootBlock(object arg)
{
_ = (Fruit)arg; // FN
_ = (Fruit)arg; // Sec-ondary
_ = (Fruit)arg; // Sec-ondary
_ = (Fruit)arg; // Noncompliant {{Extract this repetitive cast into a variable.}}
// ^^^^^^^^^^
_ = (Fruit)arg; // Secondary
_ = (Fruit/*Comment*/)arg; // Secondary
_ = ( Fruit )arg; // Secondary with extra whitespace
}

public void MultipleCasts_SameBlock(object arg)
{
if (true)
{
_ = (Fruit)arg; // FN
_ = (Fruit)arg; // Sec-ondary
_ = (Fruit)arg; // Sec-ondary
_ = (Fruit)arg; // Noncompliant {{Extract this repetitive cast into a variable.}}
_ = (Fruit)arg; // Secondary
_ = (Fruit)arg; // Secondary
}
}

public void MultipleCasts_NestedBlock(object arg)
{
_ = (Fruit)arg; // FN
_ = (Fruit)arg; // Noncompliant {{Extract this repetitive cast into a variable.}}
if (true)
{
_ = (Fruit)arg; // Sec-ondary
_ = (Fruit)arg; // Secondary
foreach(var ch in "Lorem ipsum")
{
_ = (Fruit)arg; // Sec-ondary
_ = (Fruit)arg; // Sec-ondary
_ = (Fruit)arg; // Secondary
_ = (Fruit)arg; // Secondary
}
_ = (Fruit)arg; // Sec-ondary
_ = (Fruit)arg; // Secondary
}
}

public void MultipleCasts_Lambda(object arg)
{
_ = (Fruit)arg; // FN
_ = (Fruit)arg; // Noncompliant {{Extract this repetitive cast into a variable.}}
Action a = () =>
{
_ = (Fruit)arg; // Sec-ondary
_ = (Fruit)arg; // Sec-ondary
_ = (Fruit)arg; // Secondary
_ = (Fruit)arg; // Secondary
};
Func<object, int> f = x =>
{
_ = (Fruit)arg; // Sec-ondary
_ = (Fruit)arg; // Secondary
_ = (Fruit)x;
return 0;
};
Expand All @@ -165,6 +165,8 @@ public void MultipleCastsDifferentBlocks(object arg)
{
_ = (Fruit)arg;
}

_ = (Fruit)arg;
}
}

Expand Down

0 comments on commit a5286af

Please sign in to comment.