fix: resolve CS8032 assembly version mismatch (#850)#888
Conversation
…Immutable (#850) The analyzer DLL was compiled against System.Collections.Immutable 10.0.0.0 (via transitive pin) and AnalyzerUtilities 4.14.0 bundled a DLL referencing SCI 9.0.0.0. Neither version loads in .NET 8 SDK hosts which only provide SCI 8.0.0.0, breaking the analyzer for all users. Root cause fix: - Downgrade SCI transitive pin from 10.0.0 to 8.0.0 - Downgrade AnalyzerUtilities from 4.14.0 to 3.3.4 (bundled DLL referenced SCI 9.0.0.0; 3.3.4 references SCI 1.2.3.0) - Add SCI VersionOverride in Benchmarks project (not shipped) Defense in depth to prevent recurrence: - MSBuild target ValidateAnalyzerHostCompatibility fails the build if SCI exceeds the maximum allowed version for shipped projects - Unit test validates all three shipped DLLs' assembly references - Renovate rule blocks auto-merge for host-sensitive packages - Dependabot ignores SCI and System.Reflection.Metadata Closes #850 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds packaging-time MSBuild validation to cap System.Collections.Immutable and System.Reflection.Metadata major versions, pins corresponding packages, updates CI to validate shipped analyzers and packaging, adds local "gh act" guidance, and adds test/override entries to ensure non-shipped tooling can use newer SCI. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHubActions as "GitHub Actions (main)"
participant MSBuild as "MSBuild / dotnet"
participant PackagingTargets as "Packaging.targets"
participant ArtifactStore as "Artifacts / NuGet package"
Developer->>GitHubActions: push / PR triggers workflow
GitHubActions->>MSBuild: restore & ResolvePackageAssets
MSBuild->>PackagingTargets: After ResolvePackageAssets -> ValidateAnalyzerHostCompatibility
PackagingTargets->>MSBuild: inspect @(ResolvedCompileFileDefinitions)
PackagingTargets->>MSBuild: compare major versions (SCI / SRM) vs ceilings
alt version exceeds ceiling
PackagingTargets->>GitHubActions: fail build with descriptive error
else within limits
MSBuild->>ArtifactStore: produce package/artifacts
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @rjmurillo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses and resolves a critical Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
DeepSource reviewed changes in the commit range For detailed review results, please see the PR on DeepSource ↗ PR Report CardCode Review Summary
How are these analyzer statuses calculated?Administrators can configure which issue categories are reported and cause analysis to be marked as failed when detected. This helps prevent bad and insecure code from being introduced in the codebase. If you're an administrator, you can modify this in the repository's settings. |
There was a problem hiding this comment.
Code Review
This pull request provides a comprehensive fix for the CS8032 assembly version mismatch issue. The changes correctly downgrade System.Collections.Immutable and Microsoft.CodeAnalysis.AnalyzerUtilities to versions compatible with the minimum supported SDK host. The addition of multiple 'defense in depth' layers, including a new MSBuild validation target, unit tests, and dependency management configurations, is an excellent strategy to prevent this issue from recurring. The code is high quality and the approach is very thorough. I have one suggestion to further improve the new MSBuild validation target for even better consistency across the new preventative measures.
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Address PR review feedback: - Add System.Reflection.Metadata validation to ValidateAnalyzerHostCompatibility MSBuild target for consistency with unit tests and dependency management configs - Add SCI VersionOverride to PerfDiff.csproj (not shipped, needs higher version for TraceEvent dependency) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a PowerShell validation step that inspects all shipped analyzer DLLs' assembly references after build. Fails CI if any reference exceeds the .NET 8 SDK host ceiling. This runs on every build across the full OS matrix (Windows + Ubuntu). See: #850 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The validation step was using artifacts output paths (artifacts/bin/...) but this repository does not enable UseArtifactsOutput, so build outputs are in the standard project-relative bin/ directories. Fixes: #850
This comment has been minimized.
This comment has been minimized.
- Add System.Reflection.Metadata version check to build gate (Bug 1) - Verify bundled AnalyzerUtilities matches primary analyzer reference (Bug 2) - The test now validates the correct artifact that will be packaged (Bug 3) Addresses issues with compatibility validation not covering all host-provided assemblies and ensuring bundled third-party DLLs are properly validated.
This comment has been minimized.
This comment has been minimized.
Install the built nupkg into a minimal project and build it with .NET 8 and .NET 9 SDKs, failing on CS8032 or assembly binding errors. This catches host-incompatible analyzer DLLs before release. See: #850 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The analyzer-load-test script captured BUILD_EXIT from dotnet build but never validated it. If the build failed for reasons other than CS8032 or assembly binding failures, the step would still report success. This fix adds a check for non-zero BUILD_EXIT after the specific error pattern checks to ensure any build failure causes the CI step to fail.
This comment has been minimized.
This comment has been minimized.
Extend the integration test to cover .NET Framework TFMs reported in issue #850. Uses Microsoft.NETFramework.ReferenceAssemblies for cross-platform builds on Linux runners. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ARM runners don't support .NET Framework properly. Use GitHub-hosted ubuntu-24.04 (x64) for net472/net48/net481 entries while keeping ARM runners for modern .NET TFMs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reverts incorrect path change from 9ff4f72 (Cursor Agent). The build uses the artifacts layout (artifacts/bin/Moq.Analyzers/release/), not the legacy src/ layout. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The analyzer targets netstandard2.0 and loads in two different hosts: - dotnet CLI: csc runs on modern .NET (tested on Linux ARM) - msbuild.exe: csc runs on .NET Framework via VS (tested on Windows x64) The .NET Framework entries now use windows-latest (GitHub-hosted, has VS 2022) and build with msbuild.exe instead of dotnet build. This tests the actual .NET Framework compiler host path that VS users hit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add null check for nupkg in Create test project step - Add $LASTEXITCODE check after dotnet build and msbuild commands - Fix inaccurate comment: AnalyzerUtilities 3.3.4 references SCI 1.2.3.0 - Add AnalyzerUtilities to dependabot ignore list Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/main.yml:
- Around line 59-63: Replace the brittle hardcoded $shippedDlls array with
dynamic glob-based discovery: locate the $shippedDlls variable and change its
assignment to gather DLLs via a recursive glob pattern (e.g., matching
artifacts/bin/**/release/*.dll) and optional name filters to keep only the
intended assemblies; ensure you preserve any existing explicit filtering
semantics (for example by filtering on filename prefixes like "Moq.Analyzers" or
an allowlist) so the workflow validates the same set of shipped assemblies while
tolerating output-structure changes.
Fixes #850. The analyzer was broken for all users on v0.4.0 because the shipped DLLs referenced `System.Collections.Immutable` versions that don't exist in the user's compiler host: - `Moq.Analyzers.dll` referenced SCI **10.0.0.0** (via transitive pin from Renovate PR #822) - Bundled `AnalyzerUtilities.dll` (v4.14.0) referenced SCI **9.0.0.0** - .NET 8 SDK hosts only provide SCI **8.0.0.0**, causing CS8032 on every build - Downgrade SCI transitive pin: 10.0.0 -> 8.0.0 - Downgrade AnalyzerUtilities: 4.14.0 -> 3.3.4 (its bundled DLL also referenced SCI 9.0.0.0; v3.3.4 references SCI 1.2.3.0, compatible with all hosts) - Add SCI `VersionOverride` in Benchmarks project (not shipped, needs higher version for TraceEvent) | Layer | What | When | |-------|------|------| | MSBuild target | `ValidateAnalyzerHostCompatibility` in Packaging.targets | Every build (local + CI) | | Unit test | `AnalyzerAssemblyCompatibilityTests` checks all 3 shipped DLLs | Every test run | | Renovate rule | `automerge: false` + `analyzer-compat` label for SCI, SRM, AnalyzerUtilities | Every Renovate PR | | Dependabot ignore | SCI + System.Reflection.Metadata added to ignore list | Dependabot scheduling | | Comments | Warning in Directory.Packages.props | Human reads the file | ``` Moq.Analyzers.dll -> SCI 8.0.0.0 (was 10.0.0.0) Moq.CodeFixes.dll -> SCI 8.0.0.0 AnalyzerUtilities.dll -> SCI 1.2.3.0 (was 9.0.0.0) ``` - [x] Build succeeds with 0 errors, 0 warnings - [x] All 1928 existing tests pass (no regressions) - [x] 3 new `AnalyzerAssemblyCompatibilityTests` pass - [x] Assembly references verified: all shipped DLLs reference SCI <= 8.0.0.0 - [ ] CI passes - [ ] Manual: reference locally-built nupkg from a net8.0 project, confirm no CS8032 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **Tests** * Added analyzer assembly compatibility tests to verify shipped analyzers load correctly across supported hosts. * **Documentation** * Enhanced contribution guide with instructions for running CI workflows locally. * **Chores** * Added CI build/test/load-test steps and standardized artifact handling and matrices. * Enforced packaging checks and updated dependency pins; added tooling overrides and a Renovate rule requiring manual review for analyzer-related dependency updates. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Summary
Fixes #850. The analyzer was broken for all users on v0.4.0 because the shipped DLLs referenced
System.Collections.Immutableversions that don't exist in the user's compiler host:Moq.Analyzers.dllreferenced SCI 10.0.0.0 (via transitive pin from Renovate PR chore(deps): update dotnet monorepo to v10 (major) #822)AnalyzerUtilities.dll(v4.14.0) referenced SCI 9.0.0.0Root cause fix
VersionOverridein Benchmarks project (not shipped, needs higher version for TraceEvent)Defense in depth (prevent recurrence)
ValidateAnalyzerHostCompatibilityin Packaging.targetsAnalyzerAssemblyCompatibilityTestschecks all 3 shipped DLLsautomerge: false+analyzer-compatlabel for SCI, SRM, AnalyzerUtilitiesVerified assembly references after fix
Test plan
AnalyzerAssemblyCompatibilityTestspass🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Documentation
Chores