diff --git a/README.md b/README.md index a702cc9..d210c79 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ [![NuGet Version](https://img.shields.io/nuget/v/ReferenceProtector.svg)](https://www.nuget.org/packages/ReferenceProtector) [![GitHub license](https://img.shields.io/github/license/olstakh/ReferenceProtector.svg)](https://github.com/olstakh/ReferenceProtector/blob/main/LICENSE) -Protect from unwanted dependencies in your repository. As repo gets bigger - there's often a need to secure from bad dependencies between projects. +Protect from unwanted dependencies in your repository. As repo gets bigger - there's often a need to secure from bad dependencies between projects, or to unwanted packages. ## Rules The following warnings are generated by this package: @@ -13,7 +13,8 @@ The following warnings are generated by this package: | RP0001 | Provide DependencyRulesFile property to specify valid dependency rules file. | | RP0002 | Make sure the dependency rules file is in the correct json format | | RP0003 | No dependency rules matched the current project | -| RT0004 | Project reference 'x' ==> 'y' violates dependency rule or one of its exceptions | +| RP0004 | Project reference 'x' ==> 'y' violates dependency rule or one of its exceptions | +| RP0005 | Package reference 'x' ==> 'y' violates dependency rule or one of its exceptions | ## How to use Add a package reference to the [ReferenceProtector](https://www.nuget.org/packages/ReferenceProtector) package in your projects, or as a common package reference in the repo's [`Directory.Packages.props`](./Directory.Build.props). @@ -47,6 +48,22 @@ Schema of the rules file is as follows: // ... ] }, + "PackageDependencies": [ + { + "From": "", + "To": "", + "Policy": "", + // "LinkType": "", Only direct package references are analyzed, so LinkType is not needed in this section + "Description": "", + "Exceptions": [ + { + "From": "", + "To": "", + "Justification": "" + }, + // ... + ] + }, // ... ] } @@ -60,6 +77,8 @@ Top `ProjectDependencies` object will contain a list of rules to validate agains - `Description` - human readable explanation for this policy rule. Will be displayed in a build warning if policy is violated (rule `RP0004`). - `Exceptions` (Optional) - list of exceptions from this policy (can be due to tech debt, or for any other reason). +Top `PackageDependences` object will have the same format as `ProjectDependencies` with `LinkType` omitted, since only direct package references will be considered. Also, `Description` section will be part of `RP0005` warning (as opposed to `RP0004`) + ## Matching logic Each reference between the projects during the build is evaluated against provided list of policies. First each pair of dependent projects is evaluated against `From` and `To` patterns, based on their full path. If the match is successful - their link type is evaluated: if current pair has a direct dependency on each other and `LinkType` value is `Direct` or `DirectOrTransient` - the match is successful, otherwise (the dependency is transient) - `LinkType` should be `Transient` or `DirectOrTransient` for the match to be successful. Then we exceptions are evaluated using the same pattern matching logic with `From` and `To` fields. The decision logic is as follows @@ -117,6 +136,24 @@ Below are few examples of potential rules }, ``` +### Forbid referencing Newtonsoft.* packages + +``` +{ + "PackageDependencies": [ + { + "description": "Use System.Text.Json, instead of Newtonsoft.Json", + "Policy": "forbidden", + "from": "*", + "to": "Newtonsoft.json", + "exceptions": [ + // tech debt + ] + } + ] +} +``` + ## How it works First - MSBuild task with gather all direct / indirect project references and dump them into a file (typically in `obj/Debug/` folder), named `references.tsv` (inspired by [ReferenceTrimmer](https://github.com/dfederm/ReferenceTrimmer) implementation). During the second stage - Roslyn analyzer will read this file and match it against the dependency rules, defined in a file from `` property. Corresponding diagnostics will be produced if violations are found. diff --git a/samples/ClassB/ClassB.csproj b/samples/ClassB/ClassB.csproj index 1e3a1fa..8fcdcdb 100644 --- a/samples/ClassB/ClassB.csproj +++ b/samples/ClassB/ClassB.csproj @@ -3,4 +3,8 @@ + + + + diff --git a/src/Analyzers/ReferenceProtector.Analyzers.Tests/ReferenceProtectorAnalyzerTests.cs b/src/Analyzers/ReferenceProtector.Analyzers.Tests/ReferenceProtectorAnalyzerTests.cs index 8b3a6b1..0b0f207 100644 --- a/src/Analyzers/ReferenceProtector.Analyzers.Tests/ReferenceProtectorAnalyzerTests.cs +++ b/src/Analyzers/ReferenceProtector.Analyzers.Tests/ReferenceProtectorAnalyzerTests.cs @@ -58,6 +58,9 @@ public async Task DependencyRulesFileProvided_ShouldReportIfNoMatches_Async() .WithNoLocation() .WithMessage("No dependency rules matched the current project 'TestProject.csproj'")); + test.TestState.AdditionalFiles.Add( + (ReferenceProtectorAnalyzer.DeclaredReferencesFile, "")); + await test.RunAsync(TestContext.Current.CancellationToken); } @@ -107,6 +110,43 @@ TestProject.csproj ProjectReferenceTransitive TransitiveReferencedProject.csproj await test.RunAsync(TestContext.Current.CancellationToken); } + /// + /// Verifies that the analyzer reports a diagnostic when a package reference violates a defined dependency rule. + /// + [Fact] + public async Task ValidDependencyRulesFile_PackageDependencyViolated_ShouldReportDiagnostic_Async() + { + var test = GetAnalyzer(); + test.TestState.AdditionalFiles.Add( + ("DependencyRules.json", """ + { + "PackageDependencies": [ + { + "From": "TestProject.csproj", + "To": "Forbidden.Package", + "Description": "Can't reference this package", + "Policy": "Forbidden" + } + ] + } + """)); + + test.TestState.AdditionalFiles.Add( + (ReferenceProtectorAnalyzer.DeclaredReferencesFile, """ + TestProject.csproj PackageReferenceDirect Forbidden.Package + """)); + + test.ExpectedDiagnostics.Add(DiagnosticResult.CompilerWarning("RP0005") + .WithNoLocation() + .WithMessage("Package reference 'TestProject.csproj' ==> 'Forbidden.Package' violates dependency rule 'Can't reference this package' or one of its exceptions. Please remove the dependency or update 'DependencyRules.json' file to allow it.")); + + test.ExpectedDiagnostics.Add(new DiagnosticResult("RP0003", Microsoft.CodeAnalysis.DiagnosticSeverity.Info) + .WithNoLocation() + .WithMessage("No dependency rules matched the current project 'TestProject.csproj'")); + + await test.RunAsync(TestContext.Current.CancellationToken); + } + /// /// Verifies that the analyzer does not report a diagnostic when a project reference matches an exception in the dependency rules. /// diff --git a/src/Analyzers/ReferenceProtector.Analyzers/AnalyzerReleases.Shipped.md b/src/Analyzers/ReferenceProtector.Analyzers/AnalyzerReleases.Shipped.md index ee8569c..6c9b9c9 100644 --- a/src/Analyzers/ReferenceProtector.Analyzers/AnalyzerReleases.Shipped.md +++ b/src/Analyzers/ReferenceProtector.Analyzers/AnalyzerReleases.Shipped.md @@ -11,3 +11,4 @@ RP0001 | Usage | Warning | Provide DependencyRulesFile property to specify valid RP0002 | Usage | Warning | Make sure the dependency rules file '{0}' is in the correct json format RP0003 | Usage | Info | No dependency rules matched the current project '{0}' RP0004 | Usage | Warning | Project reference '{0}' ==> '{1}' violates dependency rule '{2}' or one of its exceptions +RP0005 | Usage | Warning | Package reference '{0}' ==> '{1}' violates dependency rule '{2}' or one of its exceptions diff --git a/src/Analyzers/ReferenceProtector.Analyzers/DiagnosticDescriptors.cs b/src/Analyzers/ReferenceProtector.Analyzers/DiagnosticDescriptors.cs index 5a1df79..619d960 100644 --- a/src/Analyzers/ReferenceProtector.Analyzers/DiagnosticDescriptors.cs +++ b/src/Analyzers/ReferenceProtector.Analyzers/DiagnosticDescriptors.cs @@ -37,4 +37,12 @@ internal static class Descriptors category: "Usage", defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true); + + public static readonly DiagnosticDescriptor PackageReferenceViolation = new( + id: "RP0005", + title: "Package reference violation", + messageFormat: "Package reference '{0}' ==> '{1}' violates dependency rule '{2}' or one of its exceptions. Please remove the dependency or update '{3}' file to allow it.", + category: "Usage", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true); } \ No newline at end of file diff --git a/src/Analyzers/ReferenceProtector.Analyzers/Models/DependencyRules.cs b/src/Analyzers/ReferenceProtector.Analyzers/Models/DependencyRules.cs index b1a07b4..2b3e38d 100644 --- a/src/Analyzers/ReferenceProtector.Analyzers/Models/DependencyRules.cs +++ b/src/Analyzers/ReferenceProtector.Analyzers/Models/DependencyRules.cs @@ -3,7 +3,8 @@ namespace ReferenceProtector.Analyzers.Models; internal record DependencyRules( - IReadOnlyCollection ProjectDependencies); + IReadOnlyCollection ProjectDependencies, + IReadOnlyCollection PackageDependencies); internal record ProjectDependency( string From, @@ -13,6 +14,14 @@ internal record ProjectDependency( LinkType LinkType, IReadOnlyCollection? Exceptions); +// Only direct package dependencies are considered for now, so LinkType is omitted +internal record PackageDependency( + string From, + string To, + string Description, + Policy Policy, + IReadOnlyCollection? Exceptions); + internal record Exceptions( string From, string To, diff --git a/src/Analyzers/ReferenceProtector.Analyzers/ReferenceProtector.Analyzers.cs b/src/Analyzers/ReferenceProtector.Analyzers/ReferenceProtector.Analyzers.cs index d6a960e..a052ba8 100644 --- a/src/Analyzers/ReferenceProtector.Analyzers/ReferenceProtector.Analyzers.cs +++ b/src/Analyzers/ReferenceProtector.Analyzers/ReferenceProtector.Analyzers.cs @@ -41,7 +41,9 @@ public class ReferenceProtectorAnalyzer : DiagnosticAnalyzer Descriptors.DependencyRulesNotProvided, Descriptors.InvalidDependencyRulesFormat, Descriptors.NoDependencyRulesMatchedCurrentProject, - Descriptors.ProjectReferenceViolation]; + Descriptors.ProjectReferenceViolation, + Descriptors.PackageReferenceViolation + ]; /// public override void Initialize(AnalysisContext context) @@ -97,7 +99,7 @@ private void AnalyzeDependencyRules(CompilationAnalysisContext context) { "Error", ex.Message } }.ToImmutableDictionary(), dependencyRulesFile.Path)); - return; + return; } if (rules == null) @@ -114,19 +116,6 @@ private void AnalyzeDependencyRules(CompilationAnalysisContext context) return; } - var thisProjectDependencyRules = (rules.ProjectDependencies ?? []) - .Where(pd => IsMatchByName(pd.From, projectPath)) - .ToList(); - - if (thisProjectDependencyRules.Count == 0) - { - context.ReportDiagnostic(Diagnostic.Create( - Descriptors.NoDependencyRulesMatchedCurrentProject, - Location.None, - projectPath)); - return; - } - var declaredReferencesFile = context.Options.AdditionalFiles .FirstOrDefault(file => file.Path.EndsWith(DeclaredReferencesFile, StringComparison.OrdinalIgnoreCase)); @@ -147,13 +136,41 @@ private void AnalyzeDependencyRules(CompilationAnalysisContext context) .Select(line => ReferenceItem.FromLine(line.ToString())) .Where(r => IsMatchByName(r.Source, projectPath)); - AnalyzeDeclaredReferences(context, declaredReferences.ToImmutableArray(), thisProjectDependencyRules.ToImmutableArray(), dependencyRulesFile.Path); + // Analyze project dependencies + { + var thisProjectDependencyRules = (rules.ProjectDependencies ?? []) + .Where(pd => IsMatchByName(pd.From, projectPath)) + .ToList(); + + if (thisProjectDependencyRules.Count == 0) + { + context.ReportDiagnostic(Diagnostic.Create( + Descriptors.NoDependencyRulesMatchedCurrentProject, + Location.None, + projectPath)); + } + + AnalyzeDeclaredProjectReferences(context, declaredReferences.ToImmutableArray(), thisProjectDependencyRules.ToImmutableArray(), Descriptors.ProjectReferenceViolation, dependencyRulesFile.Path); + } + + // Analyze package dependencies + { + var thisPackageDependencyRules = (rules.PackageDependencies ?? []) + .Where(pd => IsMatchByName(pd.From, projectPath)) + .ToList(); + + var packageReferences = declaredReferences + .Where(r => r.LinkType == ReferenceKind.PackageReferenceDirect); + + AnalyzeDeclaredPackageReferences(context, packageReferences.ToImmutableArray(), thisPackageDependencyRules.ToImmutableArray(), Descriptors.PackageReferenceViolation, dependencyRulesFile.Path); + } } - private void AnalyzeDeclaredReferences( + private void AnalyzeDeclaredProjectReferences( CompilationAnalysisContext context, ImmutableArray declaredReferences, ImmutableArray dependencyRules, + DiagnosticDescriptor descriptor, string dependencyRulesFile) { foreach (var reference in declaredReferences) @@ -182,7 +199,7 @@ private void AnalyzeDeclaredReferences( if (!referenceValid) { context.ReportDiagnostic(Diagnostic.Create( - Descriptors.ProjectReferenceViolation, + descriptor, Location.None, reference.Source, reference.Target, @@ -192,7 +209,49 @@ private void AnalyzeDeclaredReferences( } } } - + + private void AnalyzeDeclaredPackageReferences( + CompilationAnalysisContext context, + ImmutableArray declaredReferences, + ImmutableArray dependencyRules, + DiagnosticDescriptor descriptor, + string dependencyRulesFile) + { + foreach (var reference in declaredReferences) + { + var matchingRules = dependencyRules + .Where(rule => IsMatchByName(rule.From, reference.Source) && IsMatchByName(rule.To, reference.Target)) + .ToList(); + + // Process the matching rules + foreach (var rule in matchingRules) + { + // Apply the rule logic here + var anyExceptionsMatched = rule.Exceptions?.Any(exception => + IsMatchByName(exception.From, reference.Source) && + IsMatchByName(exception.To, reference.Target)) ?? false; + + var referenceValid = rule.Policy switch + { + Policy.Allowed when !anyExceptionsMatched => true, + Policy.Forbidden when anyExceptionsMatched => true, + _ => false + }; + + if (!referenceValid) + { + context.ReportDiagnostic(Diagnostic.Create( + descriptor, + Location.None, + reference.Source, + reference.Target, + rule.Description, + dependencyRulesFile)); + } + } + } + } + private static bool IsMatchByName(string pattern, string project) { var regex = Regex.Escape(pattern).Replace("\\*", ".*") + "$"; diff --git a/src/Build/ReferenceProtector.targets b/src/Build/ReferenceProtector.targets index 605fce7..a19d7c3 100644 --- a/src/Build/ReferenceProtector.targets +++ b/src/Build/ReferenceProtector.targets @@ -18,6 +18,7 @@ diff --git a/src/Shared/ReferenceModels.cs b/src/Shared/ReferenceModels.cs index 564938a..d1e9b84 100644 --- a/src/Shared/ReferenceModels.cs +++ b/src/Shared/ReferenceModels.cs @@ -22,4 +22,5 @@ internal enum ReferenceKind { ProjectReferenceDirect, ProjectReferenceTransitive, + PackageReferenceDirect, } diff --git a/src/Tasks/ReferenceProtector.Tasks.IntegrationTests/ReferenceProtector.Tasks.TestBase.cs b/src/Tasks/ReferenceProtector.Tasks.IntegrationTests/ReferenceProtector.Tasks.TestBase.cs index f75a0a9..3b4d2f4 100644 --- a/src/Tasks/ReferenceProtector.Tasks.IntegrationTests/ReferenceProtector.Tasks.TestBase.cs +++ b/src/Tasks/ReferenceProtector.Tasks.IntegrationTests/ReferenceProtector.Tasks.TestBase.cs @@ -132,6 +132,13 @@ internal async Task AddProjectReference(string projectName, string referenceProj await RunDotnetCommandAsync(TestDirectory, $"add {projectPath} reference {referencePath}", TestContext.Current.CancellationToken); } + internal async Task AddPackageReference(string projectName, string packageName) + { + var projectPath = Path.Combine(TestDirectory, projectName, $"{projectName}.csproj"); + + await RunDotnetCommandAsync(TestDirectory, $"add {projectPath} package {packageName}", TestContext.Current.CancellationToken); + } + internal async Task Build(string additionalArgs = "") { string buildArgs = diff --git a/src/Tasks/ReferenceProtector.Tasks.IntegrationTests/ReferenceProtector.Tasks.Tests.cs b/src/Tasks/ReferenceProtector.Tasks.IntegrationTests/ReferenceProtector.Tasks.Tests.cs index 924f953..4dbe857 100644 --- a/src/Tasks/ReferenceProtector.Tasks.IntegrationTests/ReferenceProtector.Tasks.Tests.cs +++ b/src/Tasks/ReferenceProtector.Tasks.IntegrationTests/ReferenceProtector.Tasks.Tests.cs @@ -89,4 +89,41 @@ public async Task CollectAllReferences_LinksAreCorrect_Async() Assert.Equal(expectedReferences, references); } + + /// + /// Verifies that the CollectAllReferences task correctly collects package references. + /// + [Fact] + public async Task CollectAllReferences_PackageReferences_AreIncluded() + { + CreateProject("A"); + CreateProject("B"); + await AddPackageReference("A", "System.Text.Json"); + await AddProjectReference("A", "B"); + await Build(); + + var generatedFiles = GetGeneratedReferencesFiles(); + Assert.Equal(2, generatedFiles.Count); + + var references = generatedFiles.SelectMany(file => ReferenceItemExtensions.LoadFromFile(file)) + .OrderBy(x => (x.Source, x.Target, x.LinkType)) + .ToList(); + + var expectedReferences = new List + { + new ReferenceItem("A", "B", ReferenceKind.ProjectReferenceDirect), + new ReferenceItem("A", "System.Text.Json", ReferenceKind.PackageReferenceDirect), + } + .Select(x => x with + { + Source = Path.Combine(TestDirectory, x.Source, $"{x.Source}.csproj"), + Target = x.LinkType == ReferenceKind.ProjectReferenceDirect + ? Path.Combine(TestDirectory, x.Target, $"{x.Target}.csproj") + : x.Target, // For package references, keep the package name as is + }) + .OrderBy(x => (x.Source, x.Target, x.LinkType)) + .ToList(); + + Assert.Equal(expectedReferences, references); + } } \ No newline at end of file diff --git a/src/Tasks/ReferenceProtector.Tasks/CollectAllReferences.cs b/src/Tasks/ReferenceProtector.Tasks/CollectAllReferences.cs index 2bb64f1..c48df6b 100644 --- a/src/Tasks/ReferenceProtector.Tasks/CollectAllReferences.cs +++ b/src/Tasks/ReferenceProtector.Tasks/CollectAllReferences.cs @@ -33,6 +33,11 @@ public class CollectAllReferences : Microsoft.Build.Utilities.Task /// public ITaskItem[] ProjectReferences { get; set; } = []; + /// + /// The package references to collect. + /// + public ITaskItem[] PackageReferences { get; set; } = []; + /// public override bool Execute() { @@ -66,6 +71,15 @@ public override bool Execute() LinkType: isTransitiveDependency ? ReferenceKind.ProjectReferenceTransitive : ReferenceKind.ProjectReferenceDirect)); } + foreach (var packageReference in PackageReferences) + { + var packageReferenceName = packageReference.ItemSpec; + references.Add(new ReferenceItem( + Source: MsBuildProjectFile, + Target: packageReferenceName, + LinkType: ReferenceKind.PackageReferenceDirect)); + } + if (OutputFile is not null) { references.SaveToFile(OutputFile); diff --git a/tests/ReferenceProtector.IntegrationTests/ReferenceProtector.IntegrationTests.TestBase.cs b/tests/ReferenceProtector.IntegrationTests/ReferenceProtector.IntegrationTests.TestBase.cs index 9295063..9b905f0 100644 --- a/tests/ReferenceProtector.IntegrationTests/ReferenceProtector.IntegrationTests.TestBase.cs +++ b/tests/ReferenceProtector.IntegrationTests/ReferenceProtector.IntegrationTests.TestBase.cs @@ -135,6 +135,13 @@ internal async Task AddProjectReference(string projectName, string referenceProj await RunDotnetCommandAsync(TestDirectory, $"add {projectPath} reference {referencePath}", TestContext.Current.CancellationToken); } + internal async Task AddPackageReference(string projectName, string packageName) + { + var projectPath = Path.Combine(TestDirectory, projectName, $"{projectName}.csproj"); + + await RunDotnetCommandAsync(TestDirectory, $"add {projectPath} package {packageName}", TestContext.Current.CancellationToken); + } + internal async Task> Build(string additionalArgs = "") { string logDirBase = Path.Combine(TestDirectory, "Logs"); diff --git a/tests/ReferenceProtector.IntegrationTests/ReferenceProtector.IntegrationTests.cs b/tests/ReferenceProtector.IntegrationTests/ReferenceProtector.IntegrationTests.cs index 09d68b7..c4dfeda 100644 --- a/tests/ReferenceProtector.IntegrationTests/ReferenceProtector.IntegrationTests.cs +++ b/tests/ReferenceProtector.IntegrationTests/ReferenceProtector.IntegrationTests.cs @@ -106,6 +106,40 @@ public async Task PackageReference_DependencyRuleViolated_ProducesWarnings_Async }, warning); } + /// + /// Validates that dependency policy violation for package references will produce a build warning. + /// + [Fact] + public async Task PackageReference_PackageDependencyRuleViolated_ProducesWarnings_Async() + { + var projectA = CreateProject("A"); + var packageX = "System.Text.Json"; + await AddPackageReference("A", packageX); + var testRulesPath = Path.Combine(TestDirectory, "testRules.json"); + File.WriteAllText(testRulesPath, $$""" + { + "PackageDependencies": [ + { + "From": "*", + "To": "System.Text.*", + "Policy": "Forbidden", + "Description": "test rule" + } + ] + } + """); + + var warnings = await Build(additionalArgs: + $"/p:DependencyRulesFile={testRulesPath}"); + + var warning = Assert.Single(warnings); + Assert.Equal(new Warning() + { + Message = $"RP0005: Package reference '{projectA}' ==> '{packageX}' violates dependency rule 'test rule' or one of its exceptions. Please remove the dependency or update '{testRulesPath}' file to allow it.", + Project = "A/A.csproj", + }, warning); + } + /// /// Validates that dependency policy violation will not produce a build warning when the feature is disabled. /// diff --git a/version.json b/version.json index 75f7541..1cc8d36 100644 --- a/version.json +++ b/version.json @@ -1,7 +1,7 @@ { "$schema": "https://raw.githubusercontent.com/AArnott/Nerdbank.GitVersioning/master/src/NerdBank.GitVersioning/version.schema.json", - "version": "1.2", - "assemblyVersion": "1.2", + "version": "1.3", + "assemblyVersion": "1.3", "buildNumberOffset": -1, "publicReleaseRefSpec": [ "^refs/tags/v\\d+\\.\\d+\\.\\d+"