Add initial app host analyzers to the Aspire.Hosting.AppHost package

[DamianEdwards]

Description

This adds initial set of analyzers to Aspire.Hosting.AppHost and infrastructure to build more out in the future. Initial analyzers included are:

• ASPIRE006: Application model items must have valid names

Diagnostics are reported as errors as they'll result in runtime exceptions.

Contributes to #1209

The analyzer is based on hosting method parameters being decorated with a new ResourceNameAttribute or EndpointNameAttribute, which both implement the new interface IModelNameParameter. All relevant hosting methods (including in hosting integration packages) have been updated with these attributes as appropriate to enable the analyzer.

If we think requiring the attribute to enable the analyzers for a method is too burdensome, we could update the analyzer to also look for method operations invoked on IDistributedApplicationBuilder for methods named like AddXXX that pass a parameter string name. We could keep the attributes to enable other methods to participate where that pattern doesn't suffice, e.g. IResourceBuilder<PostgresResource>.AddDatabase(string name), as it's likely not correct to always assume methods like that result in resources being added to the application model with the specified name.

The analyzer infrastructure is copied from dotnet/aspnetcore.

Checklist

• Is this feature complete?
  • Yes. Ready to ship.
• Are you including unit tests for the changes and scenario tests if relevant?
  • Yes
• Did you add public API?
  • Yes
    • If yes, did you have an API Review for it?
      • No
    • Did you add <remarks /> and <code /> elements on your triple slash comments?
      • Yes
• Does the change make any security assumptions or guarantees?
  • No
• Does the change require an update in our Aspire docs?
  • Yes
    • Link to aspire-docs issue: [New article]: Docs for Aspire hosting analyzers docs-aspire#1658

Microsoft Reviewers: Open in CodeFlow

[mitchdenny]

The unique name detection is going to need to handle branches:

    [Fact]
    public async Task ResourceNameUniqueCanHandleTernaryBranches()
    {
        var test = AnalyzerTest.Create<AppHostAnalyzer>("""
            using Aspire.Hosting;

            var builder = DistributedApplication.CreateBuilder(args);

            var parameter = builder.ExecutionContext.IsPublishMode
                ? builder.AddParameter("x")
                : builder.AddParameter("x");
            """, []);

        await test.RunAsync();
    }

    [Fact]
    public async Task ResourceNameUniqueCanHandleIfBranches()
    {
        var test = AnalyzerTest.Create<AppHostAnalyzer>("""
            using Aspire.Hosting;

            var builder = DistributedApplication.CreateBuilder(args);

            IResourceBuilder<ParameterResource>? parameter;

            if (builder.ExecutionContext.IsPublishMode)
            {
                parameter = builder.AddParameter("x");
            }
            else
            {
                parameter = builder.AddParameter("x");
            }
            """, []);

        await test.RunAsync();
    }

    [Fact]
    public async Task ResourceNameUniqueCanHandleSwitchBranches()
    {
        var test = AnalyzerTest.Create<AppHostAnalyzer>("""
            using Aspire.Hosting;

            var builder = DistributedApplication.CreateBuilder(args);

            var parameter = builder.ExecutionContext.Operation switch {
                DistributedApplicationOperation.Run => builder.AddParameter("x"),
                _ => builder.AddParameter("x")
            };
            """, []);

        await test.RunAsync();
    }

[JamesNK]

ASPIRE0001: Resource names must be unique

I wrote an analyzer like this for ASP.NET Core routing and reporting duplicate routes. It took forever to get right.

With this kind of analyzer you have to be really conservative about finding duplicates. You can only report a duplicate if they are both in the same context: different methods, if branches, switch statements, ternary expressions, null coalesing, etc, etc.

[JamesNK]

My analyzer: https://github.com/dotnet/aspnetcore/blob/d05f3586adc0439595436ff6b8677548302fd64d/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/DetectAmbiguousRoutes.cs

My analyzer is very complex because it's possible to add extension methods to MapGet and friends, and as soon as you do that, you don't know if it is truely a duplicate or not. But you should be able to copy logic about only analyzing in the same context.

[mitchdenny]

Funny you say that @JamesNK as I was looking at this I was remembering that analyzer thinking what a good job you did with it around handling these corner cases (although some of these will be pretty common patterns in Aspire).

[JamesNK]

I didn't do that good a job. There were a bunch of backports during .NET 8 (9?) to fix issues.

[mitchdenny]

... eventually. :)

[DamianEdwards]

With this kind of analyzer you have to be really conservative about finding duplicates. You can only report a duplicate if they are both in the same context: different methods, if branches, switch statements, ternary expressions, null coalesing, etc, etc.

Hmm OK. I think I'll just remove it for now and we can revisit it later. This is mostly about getting the infra in and the first analyzer to verify the resource name is valid.

[DamianEdwards]

I think all feedback has been addressed. Just looking for an approval now 😃
@eerhardt @JamesNK @mitchdenny @captainsafia

[eerhardt]

LGTM on the infra side. Just some random questions.

[radical]

For analyzers in general, we should install these for the workload/template tests too, have them turned on, and use warnAsError so we can catch issues that users might run into.

[radical]

Created #5876 .