Repository navigation
Ship TaskAnalyzer as a separate package with safe defaults - #15045
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Critical package-consumption validation and related documentation and release-tracking updates remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR packages TaskAnalyzer with Microsoft.Build.Framework under analyzers/dotnet/cs and enables its shipping configuration.
Changes:
- Adds the analyzer to the Framework package.
- Enables shipping configuration.
- Updates TaskAnalyzer delivery documentation.
File summaries
| File | Review result |
|---|---|
src/TaskAnalyzer/TaskAnalyzer.csproj |
Enables shipping configuration. Nit (2 votes): release tracking still marks all rules unshipped. |
src/TaskAnalyzer/README.md |
Nit (3 votes): documentation conflicts with the bundled-package model and should distinguish direct references. |
src/Framework/Microsoft.Build.Framework.csproj |
Critical (3 votes): lacks package-consumption validation. Nit (1 vote): the Framework package README does not document the bundled analyzer. |
Review details
Suppressed comments (2)
src/Framework/Microsoft.Build.Framework.csproj:105
- The package README is
src/Framework/README.md(Directory.Build.targetssetsPackageReadmeFiletoREADME.md), but this change only updatesTaskAnalyzer/README.md. A consumer installingMicrosoft.Build.Frameworktherefore gets no documentation of the analyzer, its default MT-only scope, migration option, suppression, or warnings-as-errors behavior. Add this guidance to the Framework package README or link to packaged documentation.
<!-- Ship the TaskAnalyzer alongside the framework package so consumers receive it without a direct analyzer package reference. -->
<None Include="$(ArtifactsBinDir)TaskAnalyzer\$(Configuration)\netstandard2.0\Microsoft.Build.TaskAuthoring.Analyzer.dll"
Pack="true"
PackagePath="analyzers\dotnet\cs\Microsoft.Build.TaskAuthoring.Analyzer.dll" />
src/Framework/Microsoft.Build.Framework.csproj:105
- Placing this DLL under
analyzers/dotnet/csmakes every existingMicrosoft.Build.FrameworkPackageReference activate diagnostics automatically. The analyzer has warning rules that apply to regular tasks (MSBuildTask0004 and transitive MSBuildTask0005), while theWarningsNotAsErrorsexception exists only in the in-repo Tasks project; existing consumer builds usingTreatWarningsAsErrorscan therefore start failing on a Framework package update. This needs an explicit opt-out/ChangeWave or another opt-in activation path, plus an opt-out test, before shipping.
<None Include="$(ArtifactsBinDir)TaskAnalyzer\$(Configuration)\netstandard2.0\Microsoft.Build.TaskAuthoring.Analyzer.dll"
Pack="true"
PackagePath="analyzers\dotnet\cs\Microsoft.Build.TaskAuthoring.Analyzer.dll" />
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Lite
|
I do not think we want to ship the analyzers directly with Microsoft.Build.Framework - this prevents folks that are using older versions of the MSBuild APIs (i.e. for compatibility with older MSBuild hosts) from using newer versions of the analyzers, which still might find useful issues for them to solve. |
1606fe5 to
031fbc5
Compare
031fbc5 to
e4551cb
Compare
|
This looks pretty good to me. Specific requests:
|
|
Implemented the requested package changes in 73e86ae
Validated with the full repository build and all 369 TaskAnalyzer tests. |
73e86ae to
8d8639e
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Start the analyzer package at 0.1.0, add commit SHAs to local package versions, and validate the generated package contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
8d8639e to
e9d2a15
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This PR ships the TaskAnalyzer as a separate Microsoft.Build.TaskAuthoring.Analyzer package and declares the dependency from Microsoft.Build.Framework for build-time analyzer consumption. This keeps analyzer updates independent from MSBuild API versioning while preserving compatibility for older hosts.
Scope:
Fixes #15035