postsharp / Metalama

Metalama is a Roslyn-based meta-programming framework. Use this repo to report bugs or ask questions.
164 stars 4 forks source link

Validation: Throw error if first argument in target method invocation doesn't use `nameof()` #315

Open WhitWaldo opened 1 week ago

WhitWaldo commented 1 week ago

I've got a collection of ideas that I'd like to write Metalama aspects for, but I'm struggling to figure out how to start. I'm looking to eventually contribute this work to an open source project, but as it'll probably be a larger discussion of taking on the Metalama dependency, I'm starting by implementing it on my own.

The project in question is the Dapr workflow and specifically as it's used in their .NET client. Without getting into a bunch of details, this is an implementation of the Durable Task framework and it's written to be language agnostic, so it invokes things via named references, one inputs objects and it outputs JSON values deserialized to the values you specify. All well and good in theory, but in practice, this means that there's a lot of room to mess things up (e.g. refactor an activity name and suddenly the workflow identifying the activity via a string doesn't work anymore), among other issues.

I've got a collection of things I'd like to address using Metalama (as it's simply easier to use and more flexible than the Roslyn code analysis tooling) to ideally throw a warning or an error during the build process based on the inspection, but I'm stumped about how to proceed. Here is the first one I'm trying to tackle (believe it or not, the easiest, I think):

Goal: Validate that whenever an activity is started, it passes the activity name via nameof() instead of a string value. An activity is typically invoked by something like the following within a type implementing a Workflow<InputType, OutputType>:

public override async Task<OutputType> RunAsync(WorkflowContext context, InputType input)
{
  await context.CallActivityAsync<bool>(nameof(MyActivity1), input.property);
}

Here, I imagine I'd use a fabric to apply the aspect to every class that implements Workflow<,> and has a RunAsync method. But where each of the Metalama examples either creates the implementation around the target method in some way or another, I don't see anything quite analogous to what I'm trying to do, namely: 1) Identify the name of the first argument (of type WorkflowContext on the RunAsync method itself 2) Identify each line within the method that starts with await <contextVariableName>.CallActivityAsync 3) Retrieve the first argument of the CallActivityAsync method invocation and validate that it is implemented as a nameof(). 4) If so, all is good, proceed without warning. If any call to CallActivityAsync doesn't use nameof() on the first argument, create and throw a build warning.

Is this possible? Thanks!

svick commented 1 week ago

Metalama is not well suited to inspecting the particular syntax that is used in a method body. But it does have escape hatches that let you access the Roslyn syntax of some piece of code. That could be done by accessing (MethodDeclarationSyntax)method.Sources.Single().NodeOrToken, or, probably better, by using a custom Metalama architecture validation predicate that accesses the syntax of the reference.

That works from the opposite end than your problem statement: instead of inspecting what RunAsync does, you're checking all calls to e.g. WorkflowContext.CallActivityAsync. It could look like this:

internal class FirstArgumentIstNameofPredicate : ReferencePredicate
{
    public FirstArgumentIsNameofPredicate(ReferencePredicateBuilder? builder) : base(builder) { }

    public override bool IsMatch(in ReferenceValidationContext context)
    {
        if (context.Source.NodeOrToken is not MemberAccessExpressionSyntax memberAccessExpression)
        {
            return true;
        }

        return memberAccessExpression is { Parent: InvocationExpressionSyntax { ArgumentList.Arguments: [{ Expression: InvocationExpressionSyntax { Expression: IdentifierNameSyntax { Identifier.ValueText: "nameof" } } }, ..] } };
    }
}

internal class Fabric : ProjectFabric
{
    public override void AmendProject(IProjectAmender amender)
    {
        amender.Verify()
            .SelectTypes(typeof(WorkflowContext))
            .SelectMany(type => type.Methods.OfName(nameof(WorkflowContext.CallActivityAsync)))
            .CanOnlyBeUsedFrom(r => new FirstArgumentIsNameofPredicate(r), "The first argument has to use nameof.", ReferenceKinds.Invocation);
    }
}

Or, if you want to simplify the syntax matching code, you can use Roslyn IOperation (this requires Metalama.Framework.Sdk):

var semanticModel = context.ReferencingDeclaration.Compilation.GetRoslynCompilation().GetSemanticModel(memberAccessExpression.SyntaxTree);

var operation = semanticModel.GetOperation(memberAccessExpression.Parent!);

return operation is IInvocationOperation { Arguments: [ { Value: INameOfOperation }, ..] };