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

Feature request: Project-wide inner-method inspection #317

Open
WhitWaldo opened this issue Jun 24, 2024 · 2 comments
Open

Feature request: Project-wide inner-method inspection #317

WhitWaldo opened this issue Jun 24, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@WhitWaldo
Copy link

WhitWaldo commented Jun 24, 2024

Realizing that this is a big ask that might be outside of the immediate goals of Metalama and prompted by the response to #316 and #315 I'd nonetheless like to request direct support for doing compile-time analysis against a whole solution (or at the very least a project) on a per-method body basis to facilitate far more in-depth architecture and validation scenarios. In addition, where aspects today are implemented based on one type or method at a time, this would facilitate more of a whole-project analysis of how target types are utilized by their various references.

While in theory, I could implement either of the above two asks with a Roslyn code analysis project, I find it far easier to develop aspects in Metalama, the syntax is far more discoverable and understandable, test them and gauge reliability than I can the code analysis Roslyn projects, based on the few times I've attempted them. The Metalama documentation is far richer and at the end of the day, you have phenomenal support when I can't figure something out.

More concretely in regards to the ask, I'd like to be able to identify files, types and methods (including identify the entry method) across a project that match some given signature then be able to dive into the fields, properties and methods on those types (can already do this with a TypeAspect) but go still further and validate individual lines within the method (usually validating how something is invoked or used elsewhere in the project).

Here are some use cases I've had where such a capability would have been amazing for building aspects to identify runtime errors at compile time:

Use case 1 - Service Fabric XML configurations

I used to spend a lot of time developing applications that ran atop Azure Service Fabric. These deployed applications have a single XML file called ApplicationManifest.xml and a per-service XML file called ServiceManifest.xml through which the values for keys in the configuration can be inherited and utilized in the running services (e.g. exposed to an environment variable). Now, there's no strong-typing between the names of the keys in the XML documentation and the strings representing the environment variable names in the runtimes, so it wasn't that uncommon to experience a runtime error because an environment variable in the program was referencing an outdated name and apart from eyeballing the two files at build time, there was no way to confirm that this would work at runtime.

Being able to utilize Metalama's compile-time analysis (though at a solution level here), it would have been amazing to be able to do either of the following:

  • Read and parse the ApplicationManifest.xml and extract all the names of configuration values. Iterate through each ServiceManifest.xml and confirm that all configuration values named in the ServiceManifest.xml are present in the ApplicationManifest.xml. Throw an error at compile time if this isn't true.
  • On a per-project basis, read and parse each project's ServiceManifest.xml and warn if there is any string passed to an environment variable name that isn't contained in the ServiceManifest.xml.

Use case 2 - Invalid dependency injection utilization

Assuming DI as implemented via Microsoft's Microsoft.Extensions.DependencyInjection, types can be registered such that when an interface is injected into a type, it resolves as the concrete registered type. For example, at registration time, I might do the following:

builder.Services.AddScoped<IMyService, MyService>();

When looking to use this in a type, I need only provide for IMyService in the class constructor and DI will inject a scoped instance of MyService per the registration. However, if I were to instead make a mistake and provide for MyService in the class constructor instead of its interface, this will build without issue as the compiler doesn't know anything about dependency injection, but at runtime, it will fail because MyService isn't itself registered - only its interface is.

Having the ability to iterate though all classes in a project and validate that there exists a valid DI registration for each constructor argument (e.g. also have the ability to identify where DI registration is even happening at, or at a minimum, looking for an expression that resembles AddScoped or AddSingleton or AddTransient with the expected signatures) and throwing an error at build time if they're not found would save me from a very realizable runtime error for something that's not otherwise easily tested.

Use case 3 - Dapr workflow build-time validation

This is described in #315 and #316 but Dapr's workflow implements the durable task framework. Because of its cross-language support, Workflows invoke via a method that accepts the string-based name of the Workflow Activity and an argument that presents the input type. All Activities return something (even if it's just an object?) but the response is serialized on the Activity and deserialized on the Workflow to the type indicated on the latter.

All this means that there's fantastic opportunity for the wrong types to be provided as inputs and outputs and the wrong activities invoked, not to mention that because everything must be registered with the dependency injection provider, things may not be registered as expected.

Here's the list of aspects I'd love to be able to develop:

  • Validate that all Workflows that are invoked via any of the startup methods (e.g. ScheduleNewWorkflowAsync(nameof(MyWorkflow)) are registered with the DI provider
  • Validate that all Workflow Activities that are named in a call within a Workflow (via CallActivityAsync(nameof(MyActivity))) are registered with the DI provider
  • Validate that the first argument of all CallActivityAsync() method invocations utilize a nameof() approach instead of passing in a string value
  • Validate that the first generic argument of a type implementing WorkflowActivity matches the type provided by the second parameter passed to the CallActivityAsync() method from the Workflow (as that's where the input parameter is passed).
  • Validate that the second generic argument of a type implementing WorkflowActivity matches how the type is used by the invoking Workflow (e.g. if it's assigned to a specific type, validate that the types are compatible with one another and if it is assigned to a var, validate that the type inferred from var is compatible). For example, if the output type returns an int and the Workflow assigns the output to string?, this should throw an error at build time.

There are even more abstract capabilities I'd love to be able to write analysis validation for:

  • Validate that invocations within a Workflow are exclusively deterministic (e.g. all non-deterministic functionality must exist in a Workflow Activity
  • Validate that Workflow functions execute only on the workflow dispatch thread (as illustrated in more detail in this PR)

Especially as I continue to develop ever more elaborate and just more workflows, being able to confirm that these errors are caught at build time instead of runtime would be exceptionally valuable. Further, if I were able to get this particular use case built and working, I intended to contribute it to a lively open source project which could potentially be great marketing for Metalama and its capabilities. Either way, it would give the architectural validation concept far greater value and, given the immense potential to prevent runtime errors for loosely typed mismatches, would likely make Metalama a real must-have part of a developer's toolkit even more so than it is today.

Thank you for the consideration.

@svick
Copy link
Member

svick commented Jun 25, 2024

You're right that this would be a significant undertaking and we don't have any current plans for something like this. We'll keep this in mind, though, when considering the future of Metalama.

Thanks for the suggestion.

@gfraiteur gfraiteur added the enhancement New feature or request label Jul 19, 2024
@gfraiteur gfraiteur changed the title Feature: Project-wide inner-method inspection Feature request: Project-wide inner-method inspection Jul 19, 2024
@WhitWaldo
Copy link
Author

I'd like to add another use-case that could see an even broader use of this capability. We've got the [DependencyInjection] capability, but it rather relies on the referenced type being registered with the DI tool.

I'd like to be able to do provide a turn-key aspect that not only generates classes, but also is able to hunt down the DI registration method within the project (e.g. within Program.cs whether implemented as a top-level statement or not), look for the first variable of type WebApplicationBuilder and insert lines into the method allowing me to register my new type.

In my own specific use case here, I'm trying to generate a proxy class to interface with other remote services. This requires that I inject an IHttpClientFactory from which I'd like to create a named HttpClient, which named registration I'd like to register from the aspect itself so it's configured specifically as it should (instead of requiring the developer to independently register it properly). In the short term, I'm relying on a workaround in which I register the client myself and just pass in the name to the attribute, but it's not ideal, as I've explained.

Again, I appreciate the consideration!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants