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

Disable UseWithArgumentNamesAnalyzer in Expression Trees #751

Merged
merged 6 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

138 changes: 125 additions & 13 deletions Funcky.Analyzers/Funcky.Analyzers.Test/UseWithArgumentNamesTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,147 @@ namespace Funcky.Analyzers.Test;

public sealed class UseWithArgumentNamesTest
{
private const string AttributeSource =
"""
namespace Funcky.CodeAnalysis
{
[System.AttributeUsage(System.AttributeTargets.Method)]
internal sealed class UseWithArgumentNamesAttribute : System.Attribute { }
}
""";

[Fact]
public async Task ArgumentsThatAlreadyUseArgumentNamesGetNoDiagnostic()
{
var inputCode = await File.ReadAllTextAsync("TestCode/ValidUseWithArgumentNames.input");
await VerifyCS.VerifyAnalyzerAsync(inputCode);
var inputCode =
"""
using Funcky.CodeAnalysis;

class Test
{
private void Syntax()
{
Method(x: 10, y: 20);
}

[UseWithArgumentNames]
private void Method(int x, int y) { }
}
""";
await VerifyCS.VerifyAnalyzerAsync(inputCode + AttributeSource);
}

[Fact]
public async Task ArgumentsForCallsToMethodsWithoutAttributeGetNoDiagnostic()
{
var inputCode = await File.ReadAllTextAsync("TestCode/ValidUseWithArgumentNamesNoAttribute.input");
await VerifyCS.VerifyAnalyzerAsync(inputCode);
var inputCode =
"""
using Funcky.CodeAnalysis;

class Test
{
private void Syntax()
{
Method(10, 20);
}

private void Method(int x, int y) { }
}
""";
await VerifyCS.VerifyAnalyzerAsync(inputCode + AttributeSource);
}

[Fact]
public async Task UsagesOfMethodsAnnotatedWithShouldUseNamedArgumentsAttributeGetWarningAndAreFixed()
{
const string inputCode =
"""
using Funcky.CodeAnalysis;

class Test
{
private void Syntax()
{
Method(x: 10, 20);
Method(10, 20);
Method(
10, 20);
Method(
10,
20);
MethodWithKeywordAsArgument(10);
}

[UseWithArgumentNames]
private void Method(int x, int y) { }

[UseWithArgumentNames]
private void MethodWithKeywordAsArgument(int @int) { }
}
""";

const string fixedCode =
"""
using Funcky.CodeAnalysis;

class Test
{
private void Syntax()
{
Method(x: 10, y: 20);
Method(x: 10, y: 20);
Method(
x: 10, y: 20);
Method(
x: 10,
y: 20);
MethodWithKeywordAsArgument(@int: 10);
}

[UseWithArgumentNames]
private void Method(int x, int y) { }

[UseWithArgumentNames]
private void MethodWithKeywordAsArgument(int @int) { }
}
""";

var expectedDiagnostics = new[]
{
VerifyCS.Diagnostic().WithSpan(11, 27, 11, 29).WithArguments("y"),
VerifyCS.Diagnostic().WithSpan(12, 20, 12, 22).WithArguments("x"),
VerifyCS.Diagnostic().WithSpan(12, 24, 12, 26).WithArguments("y"),
VerifyCS.Diagnostic().WithSpan(14, 17, 14, 19).WithArguments("x"),
VerifyCS.Diagnostic().WithSpan(14, 21, 14, 23).WithArguments("y"),
VerifyCS.Diagnostic().WithSpan(16, 17, 16, 19).WithArguments("x"),
VerifyCS.Diagnostic().WithSpan(17, 17, 17, 19).WithArguments("y"),
VerifyCS.Diagnostic().WithSpan(18, 41, 18, 43).WithArguments("int"),
VerifyCS.Diagnostic().WithSpan(7, 23, 7, 25).WithArguments("y"),
VerifyCS.Diagnostic().WithSpan(8, 16, 8, 18).WithArguments("x"),
VerifyCS.Diagnostic().WithSpan(8, 20, 8, 22).WithArguments("y"),
VerifyCS.Diagnostic().WithSpan(10, 13, 10, 15).WithArguments("x"),
VerifyCS.Diagnostic().WithSpan(10, 17, 10, 19).WithArguments("y"),
VerifyCS.Diagnostic().WithSpan(12, 13, 12, 15).WithArguments("x"),
VerifyCS.Diagnostic().WithSpan(13, 13, 13, 15).WithArguments("y"),
VerifyCS.Diagnostic().WithSpan(14, 37, 14, 39).WithArguments("int"),
};

await VerifyWithSourceExample.VerifyDiagnosticAndCodeFix<UseWithArgumentNamesAnalyzer, AddArgumentNameCodeFix>(expectedDiagnostics, "UseWithArgumentNames");
await VerifyCS.VerifyAnalyzerAsync(inputCode + AttributeSource, expectedDiagnostics);
await VerifyCS.VerifyCodeFixAsync(inputCode + AttributeSource, expectedDiagnostics, fixedCode + AttributeSource);
}

[Fact]
public async Task IgnoresCallsToMethodsInsideExpressionTrees()
{
var inputCode =
"""
using System;
using System.Linq.Expressions;
using Funcky.CodeAnalysis;

class Test
{
private void Syntax()
{
Expression<Action> expr = () => Method(10, 20);
}

[UseWithArgumentNames]
private void Method(int x, int y) { }
}
""";
await VerifyCS.VerifyAnalyzerAsync(inputCode + AttributeSource);
}
}
2 changes: 2 additions & 0 deletions Funcky.Analyzers/Funcky.Analyzers/FunckyWellKnownTypeNames.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,6 @@ public static class FunckyWellKnownTypeNames
public static INamedTypeSymbol? GetSequenceType(this Compilation compilation) => compilation.GetTypeByMetadataName("Funcky.Sequence");

public static INamedTypeSymbol? GetFunctionalType(this Compilation compilation) => compilation.GetTypeByMetadataName("Funcky.Functional");

public static INamedTypeSymbol? GetExpressionOfTType(this Compilation compilation) => compilation.GetTypeByMetadataName("System.Linq.Expressions.Expression`1");
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace Funcky.Analyzers;

internal static partial class SyntaxNodeExtensions
{
// Adapted from Roslyn's source code as this API is not public:
// https://github.com/dotnet/roslyn/blob/232f7afa4966411958759c880de3a1765bdb28a0/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/SyntaxNodeExtensions.cs#L925
public static bool IsInExpressionTree(
[NotNullWhen(returnValue: true)] this SyntaxNode? node,
SemanticModel semanticModel,
[NotNullWhen(returnValue: true)] INamedTypeSymbol? expressionType,
CancellationToken cancellationToken)
=> expressionType is not null
&& node is not null
&& node
.AncestorsAndSelf()
.Any(current => IsExpressionTree(new(current, expressionType, semanticModel, cancellationToken)));

private static bool IsExpressionTree(IsExpressionTreeContext context)
=> context.Syntax switch
{
var node when node.IsAnyLambda() => LambdaIsExpressionTree(context),
SelectOrGroupClauseSyntax or OrderingSyntax => QueryExpressionIsExpressionTree(context),
QueryClauseSyntax queryClause => QueryClauseIsExpressionTree(context, queryClause),
_ => false,
};

private static bool LambdaIsExpressionTree(IsExpressionTreeContext context)
{
var typeInfo = context.SemanticModel.GetTypeInfo(context.Syntax, context.CancellationToken);
return SymbolEqualityComparer.Default.Equals(context.ExpressionType, typeInfo.ConvertedType?.OriginalDefinition);
}

private static bool QueryExpressionIsExpressionTree(IsExpressionTreeContext context)
{
var info = context.SemanticModel.GetSymbolInfo(context.Syntax, context.CancellationToken);
return TakesExpressionTree(info, context.ExpressionType);
}

private static bool QueryClauseIsExpressionTree(IsExpressionTreeContext context, QueryClauseSyntax queryClause)
{
var info = context.SemanticModel.GetQueryClauseInfo(queryClause, context.CancellationToken);
return TakesExpressionTree(info.CastInfo, context.ExpressionType)
|| TakesExpressionTree(info.OperationInfo, context.ExpressionType);
}

private static bool TakesExpressionTree(SymbolInfo info, INamedTypeSymbol expressionType)
=> GetAllSymbols(info).Any(symbol => TakesExpressionTreeAsFirstArgument(symbol, expressionType));

private static bool TakesExpressionTreeAsFirstArgument(ISymbol symbol, INamedTypeSymbol expressionType)
=> symbol is IMethodSymbol method
&& method.Parameters.Length > 0
&& SymbolEqualityComparer.Default.Equals(expressionType, method.Parameters[0].Type?.OriginalDefinition);

private sealed record IsExpressionTreeContext(
SyntaxNode Syntax,
INamedTypeSymbol ExpressionType,
SemanticModel SemanticModel,
CancellationToken CancellationToken);
}
19 changes: 19 additions & 0 deletions Funcky.Analyzers/Funcky.Analyzers/SyntaxNodeExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;

namespace Funcky.Analyzers;

internal static partial class SyntaxNodeExtensions
{
internal static ImmutableArray<ISymbol> GetAllSymbols(SymbolInfo info)
=> info.Symbol == null
? info.CandidateSymbols
: ImmutableArray.Create(info.Symbol);

// Copied from Roslyn's source code as this API is not public:
// https://github.com/dotnet/roslyn/blob/232f7afa4966411958759c880de3a1765bdb28a0/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/SyntaxNodeExtensions.cs#L925
internal static bool IsAnyLambda([NotNullWhen(returnValue: true)] this SyntaxNode? node)
=> node?.Kind() is SyntaxKind.ParenthesizedLambdaExpression or SyntaxKind.SimpleLambdaExpression;
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,20 @@ public override void Initialize(AnalysisContext context)
{
if (context.Compilation.GetTypeByMetadataName(AttributeFullName) is { } attributeSymbol)
{
context.RegisterOperationAction(AnalyzeInvocation(attributeSymbol), OperationKind.Invocation);
var expressionOfTType = context.Compilation.GetExpressionOfTType();
context.RegisterOperationAction(AnalyzeInvocation(attributeSymbol, expressionOfTType), OperationKind.Invocation);
}
});
}

private static Action<OperationAnalysisContext> AnalyzeInvocation(INamedTypeSymbol attributeSymbol)
private static Action<OperationAnalysisContext> AnalyzeInvocation(INamedTypeSymbol attributeSymbol, INamedTypeSymbol? expressionOfTType)
=> context =>
{
var invocation = (IInvocationOperation)context.Operation;
var semanticModel = invocation.SemanticModel ?? throw new InvalidOperationException("Semantic model is never be null for operations passed to an analyzer (according to docs)");

if (invocation.TargetMethod.GetAttributes().Any(attribute => SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, attributeSymbol)))
if (invocation.TargetMethod.GetAttributes().Any(attribute => SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, attributeSymbol))
&& !invocation.Syntax.IsInExpressionTree(semanticModel, expressionOfTType, context.CancellationToken))
{
foreach (var argument in invocation.Arguments)
{
Expand Down