Repository navigation
Support Central Package Management and .slnx solutions - #55
Merged
patriksvensson merged 27 commits intoSep 12, 2026
Merged
Conversation
Adds a Central Package Management fixture solution (including a GlobalPackageReference, which is what actually triggers the crash) and a .slnx copy of the existing fixture solution, plus the two tests that exercise them. Both currently fail with the reported errors: Version '' for package 'AsyncFixer' is not valid. Project or solution file do not exist.
A PackageReference managed centrally carries no Version of its own, and Buildalyzer reports its Version metadata as an empty string. The package was either dropped silently or, for a GlobalPackageReference, crashed the whole run with 'Version '' for package 'X' is not valid.' Packages without a version are now kept with no version and no range, and two such references to the same package compare as the same version, since the central version governs both. Closes spectresystems#39. Closes spectresystems#52.
IsSameVersion computed the range-to-range comparison and threw the result away, so two identical version ranges always reported as different.
Solution paths ending in .slnx are read as XML and every descendant Project element's Path is resolved, so projects nested in solution folders are picked up too. Directory probing now finds a .slnx as well. Paths in a .slnx use backslashes, so they are normalised for the current platform. Closes spectresystems#51.
Covers references without a version, exact version ranges, and how those compare against each other.
Extends the CPM fixture with a centrally pinned pre-release package and a
VersionOverride, and adds a --strict --no-prerelease test.
Both currently produce wrong results, because a centrally managed
reference carries no version:
- the overridden Autofac 4.9.3 is reported as removable instead of a
downgrade from 4.9.4
- the pre-release Newtonsoft.Json is not reported at all and the strict
run exits 0
A centrally managed PackageReference carries no version of its own, so every version dependent feature was dead under Central Package Management: differing versions were reported as removable rather than as an update or a downgrade, and --no-prerelease reported nothing at all. The version restore settled on is read from project.assets.json, which also accounts for VersionOverride, giving the same results as a project that pins its versions inline.
Projects nested in solution folders are found, and a directory holding a single .slnx is picked up without being named explicitly.
Building a multi targeting project also yields a result without a target framework. Passing that to NuGetFramework.Parse failed the whole run with 'Value cannot be null. (Parameter 'folderName')', so --tfm could not be used on any project with a TargetFrameworks property.
A project outside the Central Package Management cone pins its version inline while the project it references manages it centrally. Without the version being resolved, the identical versions compared as a mismatch.
…ment Adds a third level to the fixture so a package provided by a grandchild project is reported, and a project that pins a version inline despite central management, where the inline version has to win.
A PackageVersion can be conditioned on the target framework, so a project targeting for example net6.0 and net8.0 can manage a different version for each. Matching the lock file target only by framework identifier picked whichever came first, reporting the other framework's version.
Covers configurations, solution items and per project elements, so that only the actual project paths are picked up.
A directory holding both a .sln and a .slnx is rejected, matching what the dotnet CLI does for the same directory.
The dictionary of already built projects was keyed by file name, so a solution containing two different projects called Common.csproj only ever got the first one analyzed. The second reference resolved to the first project, and every package it alone provided went unreported. Key by the full path instead, which is what ProjectComparer already uses to identify a project. Project.File existed only to serve as that key and is now unused, so it goes with it.
PruneResults picked the first lock file target sharing the requested framework identifier, so for a multi targeting project net6.0 could be pruned against net8.0's dependencies and auto referenced or private packages would survive into the report. Prefer an exact framework match and keep the identifier match as a fallback, mirroring ProjectBuilder. Findings were also attributed to the root project by name, which is ambiguous now that two projects may share one.
A package a referenced project declares with PrivateAssets="all" does not flow to the projects referencing it, so the consumer has to declare it itself. The check for that only looked for the literal "compile", which "all" does not contain, so those packages were accumulated as provided by the child and the consumer's own reference was reported as removable. Removing it would have broken the build. Move the test onto Package.IsPrivate, which splits the metadata and accepts "all" as well as "compile", case insensitively.
Filter only rebuilt the list of packages to remove; the pre-release list was recomputed from the project, so an excluded package still showed up under --no-prerelease and still failed --strict with exit code 255. Pass the exclusions into the result so they apply to both.
Pointing snitch at a directory holding a lone .fsproj failed with "No project or solution file found." even though snitch analyzes F# projects happily when given the path explicitly. Probe for *.fsproj alongside *.csproj.
runs-on was "ubuntu-latest}", which is not a label any runner carries, so the build job could never be scheduled.
|
|
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved findings remain in package range and version handling, target-framework caching, and resolved-version tracking.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds Central Package Management and .slnx support while improving multi-targeting, project discovery, caching, package analysis, tests, documentation, and CI.
Changes:
- Resolves centrally managed package versions and private assets.
- Supports
.slnx, solution folders, F# discovery, and improved project caching. - Adds regression fixtures, verified outputs, documentation, and CI fixes.
File summaries
| File | Summary |
|---|---|
src/Snitch/Utilities/Extensions/AnalyzerResultExtensions.cs |
Handles aggregate multi-target results. |
src/Snitch/Snitch.csproj |
Enables test access to internals. |
src/Snitch/Analysis/Utilities/PathUtility.cs |
Adds .slnx and F# discovery. |
src/Snitch/Analysis/ProjectBuilder.cs |
Updates project building, version resolution, and caching. |
src/Snitch/Analysis/ProjectAnalyzerResult.cs |
Applies exclusions to prerelease reporting. |
src/Snitch/Analysis/ProjectAnalyzer.cs |
Handles project identity, private assets, and target matching. |
src/Snitch/Analysis/Models/Project.cs |
Removes filename-only project identity. |
src/Snitch/Analysis/Models/Package.cs |
Handles package versions, ranges, and private assets. |
src/Snitch.Tests/ProgramTests.cs |
Adds integration coverage. |
src/Snitch.Tests/PackageTests.cs |
Adds package parsing tests. |
src/Snitch.Tests/Expectations/Uiop.Default.verified.txt |
Adds verified expected output. |
src/Snitch.Tests/Expectations/Thuuud.Exclude_NoPreRelease.verified.txt |
Adds verified exclusion output. |
src/Snitch.Tests/Expectations/Solution.Ambiguous.verified.txt |
Adds verified solution output. |
src/Snitch.Tests/Expectations/Slnx.SolutionFolders.verified.txt |
Adds .slnx folder expectations. |
src/Snitch.Tests/Expectations/Slnx.Skip_Foo.verified.txt |
Adds .slnx skip expectations. |
src/Snitch.Tests/Expectations/Slnx.Discovered.verified.txt |
Adds .slnx discovery expectations. |
src/Snitch.Tests/Expectations/Slnx.Default.verified.txt |
Adds default .slnx expectations. |
src/Snitch.Tests/Expectations/PrivateAssets.All.verified.txt |
Adds private-assets expectations. |
src/Snitch.Tests/Expectations/Migrating.Default.verified.txt |
Adds migration scenario expectations. |
src/Snitch.Tests/Expectations/Grault.net8.0.verified.txt |
Adds target-framework expectations. |
src/Snitch.Tests/Expectations/FSharp.Discovered.verified.txt |
Adds F# discovery expectations. |
src/Snitch.Tests/Expectations/Duplicates.Default.verified.txt |
Adds duplicate-project expectations. |
src/Snitch.Tests/Expectations/Asdf.Strict_NoPreRelease.verified.txt |
Adds strict prerelease expectations. |
src/Snitch.Tests/Expectations/Asdf.Default.verified.txt |
Adds default package expectations. |
src/Snitch.Tests/Expectations/App.net6.0.verified.txt |
Adds .NET 6 expectations. |
src/Snitch.Tests.Fixtures/SolutionFolders/SolutionFolders.slnx |
Adds solution-folder fixture. |
src/Snitch.Tests.Fixtures/Snitch.Tests.Fixtures.slnx |
Adds .slnx test solution fixture. |
src/Snitch.Tests.Fixtures/PrivateAssets/Visible/Visible.csproj |
Adds visible private-assets fixture. |
src/Snitch.Tests.Fixtures/PrivateAssets/Hidden/Hidden.csproj |
Adds hidden private-assets fixture. |
src/Snitch.Tests.Fixtures/Migrating/Migrating.csproj |
Adds migrating project fixture. |
src/Snitch.Tests.Fixtures/Migrating/Class1.cs |
Adds migrating fixture source. |
src/Snitch.Tests.Fixtures/Grault/Grault.csproj |
Adds Grault project fixture. |
src/Snitch.Tests.Fixtures/Grault/Class1.cs |
Adds Grault fixture source. |
src/Snitch.Tests.Fixtures/Duplicates/Second/Common.csproj |
Adds duplicate-name project fixture. |
src/Snitch.Tests.Fixtures/Duplicates/First/Common.csproj |
Adds duplicate-name project fixture. |
src/Snitch.Tests.Fixtures/Duplicates/Consumer/Consumer.csproj |
Adds duplicate-project consumer fixture. |
src/Snitch.Tests.Fixtures/ConditionalVersions/Lib/Lib.csproj |
Adds conditional-version library fixture. |
src/Snitch.Tests.Fixtures/ConditionalVersions/Lib/Class1.cs |
Adds conditional-version library source. |
src/Snitch.Tests.Fixtures/ConditionalVersions/Directory.Packages.props |
Adds conditional central package versions. |
src/Snitch.Tests.Fixtures/ConditionalVersions/App/Class1.cs |
Adds conditional-version app source. |
src/Snitch.Tests.Fixtures/ConditionalVersions/App/App.csproj |
Adds conditional-version app fixture. |
src/Snitch.Tests.Fixtures/CentralizedPackageManagement/Zxcv/Zxcv.csproj |
Adds centralized package fixture. |
src/Snitch.Tests.Fixtures/CentralizedPackageManagement/Zxcv/Class1.cs |
Adds centralized package source. |
src/Snitch.Tests.Fixtures/CentralizedPackageManagement/Uiop/Uiop.csproj |
Adds centralized package fixture. |
src/Snitch.Tests.Fixtures/CentralizedPackageManagement/Uiop/Class1.cs |
Adds centralized package source. |
src/Snitch.Tests.Fixtures/CentralizedPackageManagement/Qwerty/Qwerty.csproj |
Adds centralized package fixture. |
src/Snitch.Tests.Fixtures/CentralizedPackageManagement/Qwerty/Class1.cs |
Adds centralized package source. |
src/Snitch.Tests.Fixtures/CentralizedPackageManagement/Directory.Packages.props |
Adds central package version definitions. |
src/Snitch.Tests.Fixtures/CentralizedPackageManagement/CentralizedPackageManagement.sln |
Adds centralized package solution fixture. |
src/Snitch.Tests.Fixtures/CentralizedPackageManagement/Asdf/Class1.cs |
Adds centralized package source. |
src/Snitch.Tests.Fixtures/CentralizedPackageManagement/Asdf/Asdf.csproj |
Adds centralized package fixture. |
README.md |
Documents .slnx and central package support. |
.github/workflows/ci.yaml |
Fixes the runner declaration. |
Review details
Suppressed comments (1)
src/Snitch/Analysis/ProjectBuilder.cs:183
MinVersionis the lower bound declared by the dependency, not the version restore selected. For a central version such as[4.9.0, 5.0.0)that restores to 4.9.4, this records 4.9.0 and can produce incorrect downgrade/equality findings despite the comment claiming to use the settled version. Read the matchingLockFileTargetlibrary's resolvedVersioninstead.
- Files reviewed: 53/53 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Package names, project names and versions were interpolated straight into Spectre.Console markup. A pinned version such as [3.5.4] is also a valid markup tag, so the run died with "Could not find color or style '3.5.4'".
A reference declaring '1.0.0' next to one declaring '[1.0.0, )' ended up in "might be removed", because the two parse into different fields and neither equality branch applied. Both sides are now compared as ranges. Also restores the error for a version that is neither, which the Central Package Management fix had turned into a silently version-less package.
The build cache was keyed by path alone, while a project carries the packages and lock file of the framework it was built for. Two root projects targeting different frameworks and referencing the same multi targeting project got whichever one was built first, so the second was analyzed against the wrong set of packages. The key is now the path plus the requested framework, and a project is also remembered under the framework it resolved to, so asking for that framework explicitly still hits the cache. A project asked for with no framework in particular can no longer be served from another framework's build, which costs one extra design time build where a root is also referenced by a root.
Keying the cache by the requested framework meant a project was built again whenever it was first built for a specific framework and later asked for without one. That only matters for a project that targets several frameworks; one that targets a single framework builds the same way either way, so it is now remembered for any request.
The escaping in the build progress output and in the analyzed file name had no test of its own, only the reporter did.
Asking for a framework nothing was compatible with indexed the mapping with the null the reducer had returned, so the run ended with "Value cannot be null. (Parameter 'key')" and no hint of what went wrong. It now names the framework that was asked for and the ones the project has. Also names the skipped project rather than the one referencing it in the "Skipping Non .NET Project" line, and covers the remaining untested paths: private assets that keep compile time assets, a reference that is not a .NET project, the old csproj format, and a target framework that is also markup.
patriksvensson
approved these changes
Sep 12, 2026
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.
Fixes several cases where snitch crashed or reported wrong results.
Crashes
Version, and anyGlobalPackageReference, died withVersion '' for package 'X' is not valid.NuGetFramework.Parseand threwValue cannot be null (Parameter 'folderName').--tfm— died withValue cannot be null (Parameter 'key'); it now names the frameworks the project actually targets.[3.5.4]was parsed as Spectre.Console markup and killed the run. User text is now escaped.Wrong results
project.assets.jsontarget could be picked.Utils.csprojinto one.PrivateAssets="all"— now honoured on a referenced project's packages, not just the root's.1.0.0and[1.0.0, )landed in "might be removed".--exclude— was ignored by the pre-release report.New support
.slnxsolutions, including solution folders,--skip, and directory probing.Issues
[3.5.4]#42