Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: Extract common method to check if any ancestor belongs to SyntaxKind #9202

Merged
merged 8 commits into from
May 2, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public override void VisitIdentifierName(IdentifierNameSyntax node)
bool IsUsedInLocalFunction(ISymbol symbol) =>
// We don't mute it if it's declared and used in local function
!(symbol.ContainingSymbol is IMethodSymbol containingSymbol && containingSymbol.MethodKind == MethodKindEx.LocalFunction)
&& HasAncestor(node, SyntaxKindEx.LocalFunctionStatement);
&& node.HasAncestor(SyntaxKindEx.LocalFunctionStatement);

bool IsInUnsupportedExpression() =>
node.FirstAncestorOrSelf<SyntaxNode>(x => x.IsAnyKind(SyntaxKindEx.IndexExpression, SyntaxKindEx.RangeExpression)) != null;
Expand All @@ -113,18 +113,15 @@ public override void VisitVariableDeclarator(VariableDeclaratorSyntax node)

private void InspectTryCatch(SyntaxNode node)
{
if (HasAncestor(node, SyntaxKind.TryStatement))
if (node.HasAncestor(SyntaxKind.TryStatement))
{
// We're only interested in "try" and "catch" blocks. Don't count "finally" block
isInTryOrCatch = isInTryOrCatch || !HasAncestor(node, SyntaxKind.FinallyClause);
isInTryOrCatch = isInTryOrCatch || !node.HasAncestor(SyntaxKind.FinallyClause);
}
else
{
isOutsideTryCatch = true;
}
}

private static bool HasAncestor(SyntaxNode node, SyntaxKind kind) =>
node.FirstAncestorOrSelf<SyntaxNode>(x => x.IsKind(kind)) != null;
}
}
115 changes: 41 additions & 74 deletions analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,24 +35,23 @@ public static class CSharpSyntaxHelper
SyntaxFacts.GetText(SyntaxKind.NameOfKeyword);

private static readonly SyntaxKind[] LiteralSyntaxKinds =
new[]
{
[
SyntaxKind.CharacterLiteralExpression,
SyntaxKind.FalseLiteralExpression,
SyntaxKind.NullLiteralExpression,
SyntaxKind.NumericLiteralExpression,
SyntaxKind.StringLiteralExpression,
SyntaxKind.TrueLiteralExpression,
};
SyntaxKind.TrueLiteralExpression
];

public static bool AnyOfKind(this IEnumerable<SyntaxNode> nodes, SyntaxKind kind) =>
nodes.Any(n => n.RawKind == (int)kind);
nodes.Any(x => x.RawKind == (int)kind);

public static bool AnyOfKind(this IEnumerable<SyntaxToken> tokens, SyntaxKind kind) =>
tokens.Any(n => n.RawKind == (int)kind);
tokens.Any(x => x.RawKind == (int)kind);

public static SyntaxNode GetTopMostContainingMethod(this SyntaxNode node) =>
node.AncestorsAndSelf().LastOrDefault(ancestor => ancestor is BaseMethodDeclarationSyntax || ancestor is PropertyDeclarationSyntax);
node.AncestorsAndSelf().LastOrDefault(x => x is BaseMethodDeclarationSyntax || x is PropertyDeclarationSyntax);

public static SyntaxNode GetSelfOrTopParenthesizedExpression(this SyntaxNode node)
{
Expand All @@ -70,46 +69,31 @@ public static SyntaxNode GetSelfOrTopParenthesizedExpression(this SyntaxNode nod
public static SyntaxNode GetFirstNonParenthesizedParent(this SyntaxNode node) =>
node.GetSelfOrTopParenthesizedExpression().Parent;

public static bool HasAncestor(this SyntaxNode syntaxNode, params SyntaxKind[] syntaxKinds) =>
syntaxNode.Ancestors().Any(x => x.IsAnyKind(syntaxKinds));

public static bool IsOnThis(this ExpressionSyntax expression) =>
IsOn(expression, SyntaxKind.ThisExpression);

public static bool IsOnBase(this ExpressionSyntax expression) =>
IsOn(expression, SyntaxKind.BaseExpression);
mary-georgiou-sonarsource marked this conversation as resolved.
Show resolved Hide resolved

private static bool IsOn(this ExpressionSyntax expression, SyntaxKind onKind)
{
switch (expression.Kind())
private static bool IsOn(this ExpressionSyntax expression, SyntaxKind onKind) =>
expression switch
{
case SyntaxKind.InvocationExpression:
return IsOn(((InvocationExpressionSyntax)expression).Expression, onKind);

case SyntaxKind.AliasQualifiedName:
case SyntaxKind.GenericName:
case SyntaxKind.IdentifierName:
case SyntaxKind.QualifiedName:
// This is a simplification as we don't check where the method is defined (so this could be this or base)
return true;

case SyntaxKind.PointerMemberAccessExpression:
case SyntaxKind.SimpleMemberAccessExpression:
return ((MemberAccessExpressionSyntax)expression).Expression.RemoveParentheses().IsKind(onKind);

case SyntaxKind.ConditionalAccessExpression:
return ((ConditionalAccessExpressionSyntax)expression).Expression.RemoveParentheses().IsKind(onKind);

default:
return false;
}
}
InvocationExpressionSyntax invocation => IsOn(invocation.Expression, onKind),
// Following statement is a simplification as we don't check where the method is defined (so this could be this or base)
AliasQualifiedNameSyntax or GenericNameSyntax or IdentifierNameSyntax or QualifiedNameSyntax => true,
MemberAccessExpressionSyntax memberAccess => memberAccess.Expression.RemoveParentheses().IsKind(onKind),
ConditionalAccessExpressionSyntax conditionalAccess => conditionalAccess.Expression.RemoveParentheses().IsKind(onKind),
_ => false,
};

public static bool IsInNameOfArgument(this ExpressionSyntax expression, SemanticModel semanticModel)
{
var parentInvocation = expression.FirstAncestorOrSelf<InvocationExpressionSyntax>();
return parentInvocation != null && parentInvocation.IsNameof(semanticModel);
return parentInvocation is not null && parentInvocation.IsNameof(semanticModel);
}

public static bool IsNameof(this InvocationExpressionSyntax expression, SemanticModel semanticModel) =>
expression != null &&
expression is not null &&
expression.Expression is IdentifierNameSyntax identifierNameSyntax &&
identifierNameSyntax.Identifier.ValueText == NameOfKeywordText &&
semanticModel.GetSymbolInfo(expression).Symbol?.Kind != SymbolKind.Method;
Expand All @@ -124,19 +108,19 @@ public static bool IsStringEmpty(this ExpressionSyntax expression, SemanticModel

var nameSymbolInfo = semanticModel.GetSymbolInfo(((MemberAccessExpressionSyntax)expression).Name);

return nameSymbolInfo.Symbol != null &&
return nameSymbolInfo.Symbol is not null &&
nameSymbolInfo.Symbol.IsInType(KnownType.System_String) &&
nameSymbolInfo.Symbol.Name == nameof(string.Empty);
}

public static bool IsNullLiteral(this SyntaxNode syntaxNode) =>
syntaxNode != null && syntaxNode.IsKind(SyntaxKind.NullLiteralExpression);
syntaxNode is not null && syntaxNode.IsKind(SyntaxKind.NullLiteralExpression);

public static bool IsAnyKind(this SyntaxNode syntaxNode, params SyntaxKind[] syntaxKinds) =>
syntaxNode != null && syntaxKinds.Contains((SyntaxKind)syntaxNode.RawKind);
syntaxNode is not null && syntaxKinds.Contains((SyntaxKind)syntaxNode.RawKind);

public static bool IsAnyKind(this SyntaxNode syntaxNode, ISet<SyntaxKind> syntaxKinds) =>
syntaxNode != null && syntaxKinds.Contains((SyntaxKind)syntaxNode.RawKind);
syntaxNode is not null && syntaxKinds.Contains((SyntaxKind)syntaxNode.RawKind);

public static bool IsAnyKind(this SyntaxToken syntaxToken, params SyntaxKind[] syntaxKinds) =>
syntaxKinds.Contains((SyntaxKind)syntaxToken.RawKind);
Expand All @@ -162,28 +146,19 @@ public static bool IsStringEmpty(this ExpressionSyntax expression, SemanticModel
return childNodes
.OfType<InvocationExpressionSyntax>()
.Where(syntaxPredicate)
.Select(e => e.Expression.SyntaxTree.GetSemanticModelOrDefault(semanticModel)?.GetSymbolInfo(e.Expression).Symbol)
.Select(x => x.Expression.SyntaxTree.GetSemanticModelOrDefault(semanticModel)?.GetSymbolInfo(x.Expression).Symbol)
.OfType<IMethodSymbol>()
.Any(symbolPredicate);
}

public static SyntaxToken? GetIdentifierOrDefault(this BaseMethodDeclarationSyntax methodDeclaration)
{
switch (methodDeclaration?.Kind())
public static SyntaxToken? GetIdentifierOrDefault(this BaseMethodDeclarationSyntax methodDeclaration) =>
methodDeclaration switch
{
case SyntaxKind.ConstructorDeclaration:
return ((ConstructorDeclarationSyntax)methodDeclaration).Identifier;

case SyntaxKind.DestructorDeclaration:
return ((DestructorDeclarationSyntax)methodDeclaration).Identifier;

case SyntaxKind.MethodDeclaration:
return ((MethodDeclarationSyntax)methodDeclaration).Identifier;

default:
return null;
}
}
ConstructorDeclarationSyntax constructor => (SyntaxToken?)constructor.Identifier,
DestructorDeclarationSyntax destructor => (SyntaxToken?)destructor.Identifier,
MethodDeclarationSyntax method => (SyntaxToken?)method.Identifier,
_ => null,
};

public static bool IsMethodInvocation(this InvocationExpressionSyntax invocation, KnownType type, string methodName, SemanticModel semanticModel) =>
invocation.Expression.NameIs(methodName) &&
Expand All @@ -210,7 +185,7 @@ public static bool IsStringEmpty(this ExpressionSyntax expression, SemanticModel
node.Sections.IndexOf(section => section.Labels.AnyOfKind(SyntaxKind.DefaultSwitchLabel));

public static bool HasBodyOrExpressionBody(this AccessorDeclarationSyntax node) =>
node.Body != null || node.ExpressionBody() != null;
node.Body is not null || node.ExpressionBody() is not null;

public static string GetName(this SyntaxNode node) =>
node.GetIdentifier()?.ValueText ?? string.Empty;
Expand All @@ -224,7 +199,7 @@ public static bool IsStringEmpty(this ExpressionSyntax expression, SemanticModel
|| Array.Exists(orNames, x => nodeName.Equals(x, StringComparison.Ordinal)));

public static bool HasConstantValue(this ExpressionSyntax expression, SemanticModel semanticModel) =>
expression.RemoveParentheses().IsAnyKind(LiteralSyntaxKinds) || expression.FindConstantValue(semanticModel) != null;
expression.RemoveParentheses().IsAnyKind(LiteralSyntaxKinds) || expression.FindConstantValue(semanticModel) is not null;

public static string StringValue(this SyntaxNode node, SemanticModel semanticModel) =>
node switch
Expand All @@ -242,20 +217,12 @@ public static bool IsLeftSideOfAssignment(this ExpressionSyntax expression)
&& assignment.Left == topParenthesizedExpression;
}

public static bool IsComment(this SyntaxTrivia trivia)
{
switch (trivia.Kind())
{
case SyntaxKind.SingleLineCommentTrivia:
case SyntaxKind.MultiLineCommentTrivia:
case SyntaxKind.SingleLineDocumentationCommentTrivia:
case SyntaxKind.MultiLineDocumentationCommentTrivia:
return true;

default:
return false;
}
}
public static bool IsComment(this SyntaxTrivia trivia) =>
trivia.IsAnyKind(
SyntaxKind.SingleLineCommentTrivia,
SyntaxKind.MultiLineCommentTrivia,
SyntaxKind.SingleLineDocumentationCommentTrivia,
SyntaxKind.MultiLineDocumentationCommentTrivia);

// creates a QualifiedNameSyntax "a.b"
public static QualifiedNameSyntax BuildQualifiedNameSyntax(string a, string b) =>
Expand All @@ -277,7 +244,7 @@ public static bool IsComment(this SyntaxTrivia trivia)
/// There can be zero, one or more results based on parameter type (Optional or ParamArray/params).
/// </summary>
public static ImmutableArray<SyntaxNode> ArgumentValuesForParameter(SemanticModel semanticModel, ArgumentListSyntax argumentList, string parameterName) =>
argumentList != null
argumentList is not null
&& new CSharpMethodParameterLookup(argumentList, semanticModel).TryGetSyntax(parameterName, out var expressions)
? expressions
: ImmutableArray<SyntaxNode>.Empty;
Expand Down
9 changes: 4 additions & 5 deletions analyzers/src/SonarAnalyzer.CSharp/ReuseClientBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,16 @@ protected bool IsReusableClient(SonarSyntaxNodeReportingContext context)
node.Parent is EqualsValueClauseSyntax { Parent: VariableDeclaratorSyntax { Parent: VariableDeclarationSyntax { Parent: LocalDeclarationStatementSyntax or UsingStatementSyntax } } };

private static bool IsInFieldOrPropertyInitializer(SyntaxNode node) =>
node.Ancestors().Any(x => x.IsAnyKind(SyntaxKind.FieldDeclaration, SyntaxKind.PropertyDeclaration))
&& !node.Ancestors().Any(x => x.IsAnyKind(SyntaxKind.GetAccessorDeclaration, SyntaxKind.SetAccessorDeclaration))
node.HasAncestor(SyntaxKind.FieldDeclaration, SyntaxKind.PropertyDeclaration)
&& !node.HasAncestor(SyntaxKind.GetAccessorDeclaration, SyntaxKind.SetAccessorDeclaration)
&& !node.Parent.IsKind(SyntaxKind.ArrowExpressionClause);

private static bool IsInConditionalCode(SyntaxNode node) =>
node.Ancestors().Any(x => x.IsAnyKind(
SyntaxKind.IfStatement,
node.HasAncestor(SyntaxKind.IfStatement,
SyntaxKind.SwitchStatement,
SyntaxKindEx.SwitchExpression,
SyntaxKind.ConditionalExpression,
SyntaxKindEx.CoalesceAssignmentExpression));
SyntaxKindEx.CoalesceAssignmentExpression);

private static bool IsAssignedToStaticFieldOrProperty(SonarSyntaxNodeReportingContext context) =>
context.Node.Parent.WalkUpParentheses() is AssignmentExpressionSyntax assignment
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,5 @@ public sealed class ControllersReuseClient : ReuseClientBase
&& method.Modifiers.Any(x => x.IsKind(SyntaxKind.PublicKeyword)));

private static bool IsInsideConstructor(SyntaxNode node) =>
node.Ancestors().Any(x => x.IsAnyKind(SyntaxKind.ConstructorDeclaration, SyntaxKindEx.PrimaryConstructorBaseType));
node.HasAncestor(SyntaxKind.ConstructorDeclaration, SyntaxKindEx.PrimaryConstructorBaseType);
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ private static void ReportOnViolation(SonarSyntaxNodeReportingContext context)
|| IsResultInContinueWithCall(memberAccessNameName, simpleMemberAccess)
|| IsChainedAfterThreadPoolCall(context.SemanticModel, simpleMemberAccess)
|| simpleMemberAccess.IsInNameOfArgument(context.SemanticModel)
|| simpleMemberAccess.Ancestors().Any(x => x is GlobalStatementSyntax))
|| simpleMemberAccess.HasAncestor(SyntaxKind.GlobalStatement))
{
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public sealed class HardcodedIpAddress : HardcodedIpAddressBase<SyntaxKind, Lite
public HardcodedIpAddress(IAnalyzerConfiguration analyzerConfiguration) : base(analyzerConfiguration) { }

protected override bool HasAttributes(SyntaxNode literalExpression) =>
literalExpression.Ancestors().AnyOfKind(SyntaxKind.Attribute);
literalExpression.HasAncestor(SyntaxKind.Attribute);

protected override string GetAssignedVariableName(SyntaxNode stringLiteral) =>
stringLiteral.FirstAncestorOrSelf<SyntaxNode>(IsVariableIdentifier)?.ToString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ public override void VisitCastExpression(CastExpressionSyntax node)
}

public static bool IsTopLevel(SyntaxNode node) =>
!node.Ancestors().Any(x => x is CheckedStatementSyntax || x is CheckedExpressionSyntax);
!node.HasAncestor(SyntaxKind.CheckedStatement, SyntaxKind.CheckedExpression);

private void VisitChecked<T>(T node, SyntaxKind checkedKind, SyntaxToken tokenToReport, Action<T> baseCall)
where T : SyntaxNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,10 @@ public override void Visit(SyntaxNode node)
public override void VisitIdentifierName(IdentifierNameSyntax node) =>
DereferencesMethodArguments |=
argumentNames.Contains(node.GetName())
&& node.Ancestors().Any(x => x.IsAnyKind(
AwaitExpression,
&& node.HasAncestor(AwaitExpression,
ElementAccessExpression,
ForEachStatement,
ThrowStatement,
SimpleMemberAccessExpression));
SimpleMemberAccessExpression);
}
}