Make ATS analyzer packaged in Aspire.Hosting and opt-in#15083
Merged
eerhardt merged 14 commits intorelease/13.2from Mar 12, 2026
Merged
Make ATS analyzer packaged in Aspire.Hosting and opt-in#15083eerhardt merged 14 commits intorelease/13.2from
eerhardt merged 14 commits intorelease/13.2from
Conversation
Move the Aspire.Hosting analyzer DLL out of NuGet's auto-loaded analyzers path and gate activation behind an MSBuild property. Enable the property in-repo and add MSBuild tests covering default-off and opt-in behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15083Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15083" |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR changes how the Aspire.Hosting analyzers are distributed and activated for AppHost projects, so they are opt-in by MSBuild property instead of being auto-loaded by NuGet.
Changes:
- Move
Aspire.Hosting.Analyzers.dllout of the NuGetanalyzers/dotnet/csfolder intobuild/so it no longer auto-loads. - Add an
EnableAspireHostingAnalyzersMSBuild property (defaultfalsein the AppHost targets) and wire up analyzer loading when enabled. - Add MSBuild tests verifying analyzers are disabled by default and can be enabled via property.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Tests/MSBuildTests.cs | Adds tests and build scaffolding to validate default-off and property-enabled analyzer behavior. |
| src/Aspire.Hosting.AppHost/build/Aspire.Hosting.AppHost.in.targets | Introduces EnableAspireHostingAnalyzers default and conditionally adds the analyzer assembly. |
| src/Aspire.Hosting.AppHost/Aspire.Hosting.AppHost.csproj | Changes analyzer packing location from analyzers/dotnet/cs to build/. |
| playground/Directory.Build.targets | Gates in-repo analyzer project reference behind EnableAspireHostingAnalyzers. |
| Directory.Build.props | Enables EnableAspireHostingAnalyzers by default for this repo’s builds. |
src/Aspire.Hosting.AppHost/build/Aspire.Hosting.AppHost.in.targets
Outdated
Show resolved
Hide resolved
radical
reviewed
Mar 9, 2026
| } | ||
|
|
||
| [Fact] | ||
| public void AspireHostingAnalyzersAreDisabledByDefault() |
Member
There was a problem hiding this comment.
Is this enough or do we want a true nuget-based test?
Contributor
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #23013309605 |
radical
approved these changes
Mar 10, 2026
Use an analyzer assembly existence check instead of RepoRoot to allow opted-in Arcade-based consumers to load the analyzer from the package. Add a regression test covering RepoRoot being set. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
eerhardt
reviewed
Mar 10, 2026
src/Aspire.Hosting.AppHost/build/Aspire.Hosting.AppHost.in.targets
Outdated
Show resolved
Hide resolved
…ptin-ats-analyzer
… in Aspire.Hosting. Most integration packages don't reference the Aspire.Hosting.AppHost nuget package, only Aspire.Hosting. So we need to include the analyzer there.
Rename the MSBuild property that controls opt-in for the integration analyzer from EnableAspireExportAnalyzers to EnableAspireIntegrationAnalyzers to be consistent with the project name (Aspire.Hosting.Integration.Analyzers). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
eerhardt
changed the title
Contributor
|
The transient CI rerun workflow re-ran the following jobs from the failed CI run because they matched the retry-safe transient failure rules:
|
sebastienros
added
the
NO-MERGE
…ptin-ats-analyzer
Also move the build and analyzer logic into buildTransitive\net8.0 so it works transitively and this package only supports the current TFM, not all.
eerhardt
removed
the
NO-MERGE
Member
|
@sebastienros - I believe this PR is ready now. |
eerhardt
approved these changes
Mar 11, 2026
…in-ats-analyzer-r132
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
The transient CI rerun workflow requested reruns for the following jobs after analyzing the failed attempt.
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
|
@sebastienros should we merge? |
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Integration/Export analyzer (ASPIREEXPORT001-009) — split into own project, opt-in via Aspire.Hosting
code (WellKnownTypes, BoundedCacheWithFactory) is linked between both analyzer projects.
conditionally loads it.
true to enable the export analyzer.
Integration authors don't reference Aspire.Hosting.AppHost, just Aspire.Hosting. So we need to package these analyzers in Aspire.Hosting nuget package.
Also make them opt-in with an MSBuild property. Integration authors will need to reference Aspire.Hosting directly and set EnableAspireIntegrationAnalyzers=true to get the analyzers enabled.
Fixes #15081
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: