[API Proposal]: Find redundant package references and version conflicts #281

Background and motivation

PackageGuard already reads every project's restore graph to decide what a solution depends on. The same graph answers two questions it does not ask yet:

1. Is this PackageReference needed? A direct reference is redundant when a referenced project, or another direct package, already brings the same package in. Teams accumulate these over years and have no way to find them.
2. Does one package resolve to different versions across projects? This is a common cause of "works in one project, fails in another", and it is invisible until something breaks at runtime.

Both are dependency hygiene rather than policy or risk, so they sit next to the existing analysis rather than inside it. The data needed is already loaded by DotNetLockFileLoader, so this adds analysis, not a new input path.

I wrote a working version of this for a different project before realising PackageGuard is the better home for it. Dennis and I discussed moving it here. It found a real redundancy on its first run against a production solution, and also a false positive that shaped the design (see Risks).

API Proposal

New analysis in PackageGuard.Core, consuming the NuGet.ProjectModel.LockFile that DotNetLockFileLoader already returns.

namespace PackageGuard.Core.CSharp;

/// <summary>The category of a <see cref="DependencyFinding"/>.</summary>
public enum DependencyFindingKind
{
    RedundantViaProject,
    RedundantViaPackage,
    VersionConflict
}

/// <summary>A single dependency hygiene problem found in a restored project.</summary>
public sealed class DependencyFinding
{
    public DependencyFindingKind Kind { get; init; }

    /// <summary>The project, or empty on a version conflict, which spans projects.</summary>
    public string Project { get; init; } = "";
    public string TargetFramework { get; init; } = "";
    public string PackageId { get; init; } = "";

    /// <summary>The resolved version. On a conflict, the highest of <see cref="ConflictingVersions"/>.</summary>
    public string ResolvedVersion { get; init; } = "";
    public string DeclaredVersion { get; init; } = "";

    /// <summary>Whether removing the reference keeps the same resolved version.</summary>
    public bool SafeToRemove { get; init; }

    /// <summary>The version it would drop to, or empty when removing it changes nothing.</summary>
    public string WouldResolveTo { get; init; } = "";

    /// <summary>The projects or packages already providing this package. Redundancy findings only.</summary>
    public IReadOnlyCollection<string> Providers { get; init; } = [];

    /// <summary>The versions it resolves to, highest first. Version conflicts only.</summary>
    public IReadOnlyCollection<string> ConflictingVersions { get; init; } = [];

    public string Description { get; init; } = "";
}

/// <summary>Finds redundant package references and version conflicts in a restored dependency graph.</summary>
public sealed class DependencyHygieneAnalyzer
{
    public IReadOnlyCollection<string> ExcludedPackageIds { get; set; } = [];

    public IReadOnlyCollection<DependencyFinding> Analyze(IReadOnlyCollection<LockFile> lockFiles);
}

Nothing on PackageInfo, ProjectPolicy or the risk types changes.

Where it hooks in. CSharpProjectAnalysisStrategy already takes an Action<string, LockFile>? onProjectLockFileLoaded callback and invokes it for every project it loads. It was added with the explain command in #256 and nothing passes it today. The analyzer can subscribe to it, so no second restore happens and no new input path is needed. If you would rather keep that callback for explain, a second one or a small collector interface works just as well.

API Usage

var analyzer = new DependencyHygieneAnalyzer
{
    ExcludedPackageIds = ["Microsoft.SourceLink.GitHub"]
};

IReadOnlyCollection<DependencyFinding> findings = analyzer.Analyze(lockFiles);

foreach (DependencyFinding finding in findings.Where(f => f.Kind == DependencyFindingKind.RedundantViaProject))
{
    Console.WriteLine(
        $"{finding.Project}: {finding.PackageId} is already provided by {string.Join(", ", finding.Providers)}");
}

On the command line, as a sibling of analyze and explain:

packageguard dependencies . --conflicts --severity error

--severity error exits with code 1 when a reported finding exists. Conflicts only count towards that when --conflicts is passed, so the exit code never depends on a finding the output hides.

Alternative Designs

Two things I would like your call on before I build anything.

1. A new packageguard dependencies command, or folded into analyze? I propose a new command. analyze answers "may I use this package", which is a policy question with a policy audience. Redundancy and version drift are hygiene questions, and a team usually wants them at a different moment, often not on every CI run. A separate command also keeps analyze's existing output and exit codes untouched. Folding it in means fewer concepts to learn, so if you prefer that I will do it that way instead.

2. One finding type, or two? DependencyFinding carries fields that only apply to one of its kinds: Providers and SafeToRemove are meaningless on a version conflict, and ConflictingVersions is meaningless on a redundancy. Reviewing my own branch, an earlier version had Providers holding the conflicting versions on a conflict, which was plainly wrong and is fixed. Splitting into two types would remove the empty fields at the cost of more public API, which your -100 points rule argues against. I left it as one type. Tell me if you want it split.

3. Naming. PackageGuard.Core already has NuGetPackageAnalyzer, which fetches per-package metadata from feeds over the network, and ProjectAnalyzer, which orchestrates the strategies. Mine is a third thing: it reads the already-loaded graph offline and never touches the network. I called it DependencyHygieneAnalyzer to keep those apart, but "hygiene" is a new word in this codebase. RedundantReferenceAnalyzer or PackageReferenceAnalyzer would also work. Your repo, your vocabulary — tell me which you want.

Report findings as PolicyViolation. Reuses the existing output and SARIF path. It is a poor fit because these are not policy decisions, there is no rule to point at, and the SafeToRemove and Providers detail has nowhere to live.

Read obj/project.assets.json directly. This is what my original version did, with its own model and file reader. It is the wrong choice here: DotNetLockFileLoader already handles restore-if-stale and returns NuGet.ProjectModel.LockFile, and LibraryDependency already exposes AutoReferenced and SuppressParent, which is exactly what the analysis needs. Reusing it removes about 220 lines of reader and most of a parallel model.

Risks

False positives are the main one, and one is already handled. A package marked PrivateAssets="all" or autoReferenced must never count as the thing that provides another reference. It is not a dependency of what the project ships, so removing the other reference would silently drop a shipped dependency. My first run hit exactly this. The fix is in, with a test that fails without it.

The graph is not the source code. A package can be redundant in the restore graph and still be referenced directly in code. Removing it would then break the build rather than cause a silent failure, so the cost is low and visible, but the docs must say it plainly. This is why redundancies are reported, never auto-fixed.

Projects must be restored. The analysis reads restore output, so it inherits whatever DotNetLockFileLoader already does about stale or missing project.assets.json.

Project references are matched by file name. A referenced project is matched to its library in the lock file by the .csproj file name. A project whose AssemblyName differs from its file name will not be matched, so a redundancy through it is missed. That is a false negative, never a false positive, so the failure mode is a finding you do not get rather than bad advice.

Scope. NuGet only. The npm strategy produces a different graph shape and is out of scope for this proposal.

Are you willing to help with a proof-of-concept (as PR in that or a separate repo) first and as pull-request later on?

Yes. A working implementation already exists, including tests, so the PR is mostly a port onto LockFile plus a rewrite of the tests to MSTest and FakeItEasy. I would rather agree the shape here first than send a large PR unannounced.

https://claude.ai/code/session_01TK6dHqkZwhjWUrZNnnb8MG