Skip to content

Commit

Permalink
Use "NodeIdentifier" for PrimaryLocation
Browse files Browse the repository at this point in the history
  • Loading branch information
@martin-strecker-sonarsource
martin-strecker-sonarsource committed Oct 25, 2023
1 parent 9870e05 commit 0ebf903
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 18 deletions.
4 changes: 4 additions & 0 deletions analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ public static SyntaxNode WalkUpParentheses(this SyntaxNode node)
AttributeSyntax { Name: { } name } => GetIdentifier(name),

This comment has been minimized.

Sign in to view

Copy link
@cristian-ambrosini-sonarsource

cristian-ambrosini-sonarsource Oct 25, 2023 •
edited
Loading

Contributor

I'm not sure if we need to extend this list to cover all the syntaxes with identifiers, but I'll leave a list of some that are missing just to document them:

EnumDeclaration
ClassDeclaration
InterfaceDeclaration
RecordDeclaration
StructDeclaration
TupleElement
ForeachStatement
LabeledStatement
LocalFunctionStatement
SingleVariableDesignation
CatchDeclaration
(From, Join, JoinInto, Let Clause)
DefineDirectiveTrivia
UndefDirectiveTrivia

This comment has been minimized.

Sign in to view

Copy link
@martin-strecker-sonarsource

martin-strecker-sonarsource Oct 25, 2023 •
edited
Loading

Author Contributor

Yes. I know. The typedeclarations are covered by the base class TypeDeclarationSyntax but there are still a lot of cases missing. A lot of expressions are missing like e.g. the LiteralExpression kinds. I would add those as needed.
BaseTypeDeclarationSyntax { Identifier: var identifier } => identifier,
ConstructorDeclarationSyntax { Identifier: var identifier } => identifier,
ConstructorInitializerSyntax { ThisOrBaseKeyword: var keyword } => keyword,
ConversionOperatorDeclarationSyntax { Type: { } type } => GetIdentifier(type),
DelegateDeclarationSyntax { Identifier: var identifier } => identifier,
DestructorDeclarationSyntax { Identifier: var identifier } => identifier,
Expand Down Expand Up @@ -177,8 +178,11 @@ public static SyntaxNode WalkUpParentheses(this SyntaxNode node)
PostfixUnaryExpressionSyntax { Operand: { } operand } => GetIdentifier(operand),
UsingDirectiveSyntax { Alias.Name: { } name } => GetIdentifier(name),
VariableDeclaratorSyntax { Identifier: var identifier } => identifier,
{ } implicitNew when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(implicitNew) => ((ImplicitObjectCreationExpressionSyntaxWrapper)implicitNew).NewKeyword,
{ } fileScoped when FileScopedNamespaceDeclarationSyntaxWrapper.IsInstance(fileScoped)
&& ((FileScopedNamespaceDeclarationSyntaxWrapper)fileScoped).Name is { } name => GetIdentifier(name),
{ } primary when PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(primary)
&& ((PrimaryConstructorBaseTypeSyntaxWrapper)primary).Type is { } type => GetIdentifier(type),
{ } refType when RefTypeSyntaxWrapper.IsInstance(refType) => GetIdentifier(((RefTypeSyntaxWrapper)refType).Type),
_ => null
};
Expand Down
17 changes: 0 additions & 17 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,5 @@ public sealed class ParametersCorrectOrder : ParametersCorrectOrderBase<SyntaxKi
};

protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;

protected override Location PrimaryLocation(SyntaxNode node) =>
node switch
{
InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax { Name: { } name } } => name.GetLocation(), // A.B.C() -> C
InvocationExpressionSyntax { Expression: { } expression } => expression.GetLocation(), // A() -> A
ObjectCreationExpressionSyntax { Type: QualifiedNameSyntax { Right: { } right } } => right.GetLocation(), // new A.B.C() -> C
ObjectCreationExpressionSyntax { Type: { } type } => type.GetLocation(), // new A() -> A
ConstructorInitializerSyntax { ThisOrBaseKeyword: { } keyword } => keyword.GetLocation(), // this() -> this
_ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(node) =>
((ImplicitObjectCreationExpressionSyntaxWrapper)node).NewKeyword.GetLocation(), // new() -> new
_ when PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(node) && ((PrimaryConstructorBaseTypeSyntaxWrapper)node).Type is { } type =>
type is QualifiedNameSyntax { Right: { } right }
? right.GetLocation() // class A: B.C() -> C
: type.GetLocation(), // class A: B() -> B
_ => base.PrimaryLocation(node),
};
}
}
2 changes: 1 addition & 1 deletion analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ protected sealed override void Initialize(SonarAnalysisContext context) =>
}, InvocationKinds);

protected virtual Location PrimaryLocation(SyntaxNode node) =>
node.GetLocation();
Language.Syntax.NodeIdentifier(node)?.GetLocation() ?? node.GetLocation();

private bool MatchingNames(IParameterSymbol parameter, string argumentName) =>
Language.NameComparer.Equals(parameter.Name, argumentName);
Expand Down
28 changes: 28 additions & 0 deletions analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1056,6 +1056,8 @@ public class C
[DataRow("""record struct $$T$$ { }""", "T")] // BaseTypeDeclarationSyntax
[DataRow("""enum $$T$$ { }""", "T")] // BaseTypeDeclarationSyntax
[DataRow("""$$Test() { }$$""", "Test")] // ConstructorDeclarationSyntax
[DataRow("""Test() : $$this(1)$$ { }""", "this")] // ConstructorInitializerSyntax
[DataRow("""Test() : $$base()$$ { }""", "base")] // ConstructorInitializerSyntax
[DataRow("""$$public static implicit operator int(Test t) => 1;$$""", "int")] // ConversionOperatorDeclarationSyntax
[DataRow("""$$delegate void D();$$""", "D")] // DelegateDeclarationSyntax
[DataRow("""$$~Test() { }$$""", "Test")] // DestructorDeclarationSyntax
Expand Down Expand Up @@ -1086,6 +1088,7 @@ public class C
[DataRow("""void M<T>() where $$T : class$$ { }""", "T")] // TypeParameterConstraintClauseSyntax
[DataRow("""void M<$$T$$>() { }""", "T")] // TypeParameterSyntax
[DataRow("""int $$i$$;""", "i")] // VariableDeclaratorSyntax
[DataRow("""object o = $$new()$$;""", "new")] // ImplicitObjectCreationExpressionSyntax
[DataRow("""void M(int p) { $$ref int$$ i = ref p; }""", "int")] // RefTypeSyntax
public void GetIdentifier_Members(string member, string expected)
{
Expand All @@ -1096,6 +1099,7 @@ public void GetIdentifier_Members(string member, string expected)
unsafe class Test
{
static Func<int> Fun() => default;
public Test(int i) { }
{{member}}
}
""", LanguageNames.CSharp);
Expand Down Expand Up @@ -1133,6 +1137,30 @@ public void GetIdentifier_CompilationUnit(string member, string expected)
}
}

[DataTestMethod]
[DataRow(""" : $$Base(i)$$""", "Base")] // NamespaceDeclarationSyntax
[DataRow(""" : $$Test.Base(i)$$""", "Base")] // NamespaceDeclarationSyntax
public void GetIdentifier_PrimaryConstructor(string baseType, string expected)
{
var node = NodeBetweenMarkers($$"""
namespace Test;
public class Base(int i)
{
}
public class Derived(int i) {{baseType}} { }
""", LanguageNames.CSharp);
var actual = ExtensionsCS.GetIdentifier(node);
if (expected is null)
{
actual.Should().BeNull();
}
else
{
actual.Should().NotBeNull();
actual.Value.ValueText.Should().Be(expected);
}
}

private static SyntaxNode NodeBetweenMarkers(string code, string language, bool getInnermostNodeForTie = false)
{
var position = code.IndexOf("$$");
Expand Down

0 comments on commit 0ebf903

Please sign in to comment.