Skip to content

Commit 0e00ae5

Browse files
authored
Produce warning when tech debt is fixed (#18)
Add `IsTechDebt` to the exception schema Produce `RP0006` when tech debt is fixed (i.e. exception to the forbidden rule no longer hits) Update samples, readme
1 parent 1e92f65 commit 0e00ae5

11 files changed

Lines changed: 470 additions & 4 deletions

File tree

Directory.Packages.props

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<PropertyGroup>
33
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
44
<CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled>
5-
<DisableTransitiveProjectReferences>true</DisableTransitiveProjectReferences>
5+
<DisableTransitiveProjectReferences Condition="'$(DisableTransitiveProjectReferences)' == ''">true</DisableTransitiveProjectReferences>
66
</PropertyGroup>
77
<ItemGroup>
88
<PackageVersion Include="Microsoft.Build.Framework" Version="17.14.28" />

README.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ The following warnings are generated by this package:
1515
| RP0003 | No dependency rules matched the current project |
1616
| RP0004 | Project reference 'x' ==> 'y' violates dependency rule or one of its exceptions |
1717
| RP0005 | Package reference 'x' ==> 'y' violates dependency rule or one of its exceptions |
18+
| RP0006 | Tech debt exception 'x' ==> 'y' no longer matches any declared reference and can be removed |
1819

1920
## How to use
2021
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).
@@ -72,6 +73,8 @@ Schema of the rules file is as follows:
7273
}
7374
```
7475

76+
Note: for stability - use the version in the schema url, instead of `main`. For example for version `v1.2.3` - use `"$schema": https://raw.githubusercontent.com/olstakh/ReferenceProtector/v1.2.3/src/Build/DependencyRules.schema.json`
77+
7578
Top `ProjectDependencies` object will contain a list of rules to validate against. Each rule has the following schema:
7679

7780
- `From` / `To` - Full path regex for source and target projects to be matched.
@@ -82,6 +85,8 @@ Top `ProjectDependencies` object will contain a list of rules to validate agains
8285

8386
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`)
8487

88+
Adding `IsTechDebt: true` to an exception (if the `Policy: "Forbidden"`) will mark the exception to the rule as undesireable and analyzer will produce a warning when such an exception no longer holds and can be removed from the rules.
89+
8590
## Matching logic
8691
Each reference between the projects / packages during the build is evaluated against provided list of policies. First each pair of dependencies is evaluated against `From` and `To` patterns, based on their full path. For project dependencies - 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. Package dependencies are only viewed as direct references. Then the exceptions are evaluated using the same pattern matching logic with `From` and `To` fields.
8792
The decision logic is as follows

samples/DependencyRules.json

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,19 @@
1212
"From": "*\\ClassA.csproj",
1313
"To": "*\\ClassB.csproj",
1414
"Justification": "This is an exception for testing purposes"
15+
},
16+
{
17+
"From": "*\\ClassA.csproj",
18+
"To": "*\\OldClass.csproj",
19+
"IsTechDebt": true,
20+
"Justification": "This is a tech debt exception for testing purposes. [Waning is expected]"
1521
}
1622
]
1723
},
1824
{
1925
"From": "*\\ClassA.csproj",
2026
"To": "*\\ClassC.csproj",
21-
"Description": "Can't reference this project transitively",
27+
"Description": "Can't reference this project transitively [Warning is expected]",
2228
"Policy": "Forbidden",
2329
"LinkType": "Transitive",
2430
"Exceptions": [

samples/Directory.Build.props

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
<GenerateDocumentationFile>true</GenerateDocumentationFile>
99
<DependencyRulesFile>$(MSBuildThisFileDirectory)DependencyRules.json</DependencyRulesFile>
1010
<ErrorLog>buildResult.sarif,version=2.1</ErrorLog>
11+
<DisableTransitiveProjectReferences>false</DisableTransitiveProjectReferences>
1112
</PropertyGroup>
1213

1314
</Project>

src/Analyzers/ReferenceProtector.Analyzers.Tests/ReferenceProtectorAnalyzerTests.cs

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,4 +231,229 @@ private AnalyzerTest<DefaultVerifier> GetAnalyzer() =>
231231
},
232232
ReferenceAssemblies = ReferenceAssemblies.Net.Net90
233233
};
234+
235+
/// <summary>
236+
/// Verifies that the analyzer reports RP0006 when a tech debt exception no longer matches any declared project reference.
237+
/// </summary>
238+
[Fact]
239+
public async Task TechDebtException_NoLongerNeeded_ShouldReportDiagnostic_Async()
240+
{
241+
var test = GetAnalyzer();
242+
test.TestState.AdditionalFiles.Add(
243+
("DependencyRules.json", """
244+
{
245+
"ProjectDependencies": [
246+
{
247+
"From": "*",
248+
"To": "*",
249+
"Description": "No direct project references allowed",
250+
"Policy": "Forbidden",
251+
"LinkType": "Direct",
252+
"Exceptions": [
253+
{
254+
"From": "TestProject.csproj",
255+
"To": "OldProject.csproj",
256+
"Justification": "Legacy dependency to be removed",
257+
"IsTechDebt": true
258+
}
259+
]
260+
}
261+
]
262+
}
263+
"""));
264+
265+
// OldProject.csproj is NOT in the declared references
266+
test.TestState.AdditionalFiles.Add(
267+
(ReferenceProtectorAnalyzer.DeclaredReferencesFile, """
268+
TestProject.csproj ProjectReferenceDirect SomeOtherProject.csproj
269+
"""));
270+
271+
test.ExpectedDiagnostics.Add(DiagnosticResult.CompilerWarning("RP0004")
272+
.WithNoLocation()
273+
.WithMessage("Project reference 'TestProject.csproj' ==> 'SomeOtherProject.csproj' violates dependency rule 'No direct project references allowed' or one of its exceptions. Please remove the dependency or update 'DependencyRules.json' file to allow it."));
274+
275+
test.ExpectedDiagnostics.Add(DiagnosticResult.CompilerWarning("RP0006")
276+
.WithNoLocation()
277+
.WithMessage("Tech debt exception 'TestProject.csproj' ==> 'OldProject.csproj' in rule 'No direct project references allowed' no longer matches any declared reference and can be removed from 'DependencyRules.json'"));
278+
279+
await test.RunAsync(TestContext.Current.CancellationToken);
280+
}
281+
282+
/// <summary>
283+
/// Verifies that the analyzer does NOT report RP0006 when a tech debt exception still matches a declared project reference.
284+
/// </summary>
285+
[Fact]
286+
public async Task TechDebtException_StillNeeded_ShouldNotReportDiagnostic_Async()
287+
{
288+
var test = GetAnalyzer();
289+
test.TestState.AdditionalFiles.Add(
290+
("DependencyRules.json", """
291+
{
292+
"ProjectDependencies": [
293+
{
294+
"From": "*",
295+
"To": "*",
296+
"Description": "No direct project references allowed",
297+
"Policy": "Forbidden",
298+
"LinkType": "Direct",
299+
"Exceptions": [
300+
{
301+
"From": "TestProject.csproj",
302+
"To": "ReferencedProject.csproj",
303+
"Justification": "Legacy dependency to be removed",
304+
"IsTechDebt": true
305+
}
306+
]
307+
}
308+
]
309+
}
310+
"""));
311+
312+
test.TestState.AdditionalFiles.Add(
313+
(ReferenceProtectorAnalyzer.DeclaredReferencesFile, """
314+
TestProject.csproj ProjectReferenceDirect ReferencedProject.csproj
315+
"""));
316+
317+
// No diagnostics expected - the tech debt exception is still needed
318+
await test.RunAsync(TestContext.Current.CancellationToken);
319+
}
320+
321+
/// <summary>
322+
/// Verifies that a non-tech-debt exception that no longer matches does NOT trigger RP0006.
323+
/// </summary>
324+
[Fact]
325+
public async Task NonTechDebtException_NoLongerNeeded_ShouldNotReportDiagnostic_Async()
326+
{
327+
var test = GetAnalyzer();
328+
test.TestState.AdditionalFiles.Add(
329+
("DependencyRules.json", """
330+
{
331+
"ProjectDependencies": [
332+
{
333+
"From": "*",
334+
"To": "*",
335+
"Description": "No direct project references allowed",
336+
"Policy": "Forbidden",
337+
"LinkType": "Direct",
338+
"Exceptions": [
339+
{
340+
"From": "TestProject.csproj",
341+
"To": "OldProject.csproj",
342+
"Justification": "Legitimate exception"
343+
}
344+
]
345+
}
346+
]
347+
}
348+
"""));
349+
350+
test.TestState.AdditionalFiles.Add(
351+
(ReferenceProtectorAnalyzer.DeclaredReferencesFile, """
352+
TestProject.csproj ProjectReferenceDirect SomeOtherProject.csproj
353+
"""));
354+
355+
// RP0004 for the violating reference, but NO RP0006 since the exception is not tech debt
356+
test.ExpectedDiagnostics.Add(DiagnosticResult.CompilerWarning("RP0004")
357+
.WithNoLocation()
358+
.WithMessage("Project reference 'TestProject.csproj' ==> 'SomeOtherProject.csproj' violates dependency rule 'No direct project references allowed' or one of its exceptions. Please remove the dependency or update 'DependencyRules.json' file to allow it."));
359+
360+
await test.RunAsync(TestContext.Current.CancellationToken);
361+
}
362+
363+
/// <summary>
364+
/// Verifies that the analyzer reports RP0006 when a tech debt exception for a package reference no longer matches.
365+
/// </summary>
366+
[Fact]
367+
public async Task TechDebtPackageException_NoLongerNeeded_ShouldReportDiagnostic_Async()
368+
{
369+
var test = GetAnalyzer();
370+
test.TestState.AdditionalFiles.Add(
371+
("DependencyRules.json", """
372+
{
373+
"PackageDependencies": [
374+
{
375+
"From": "TestProject.csproj",
376+
"To": "*",
377+
"Description": "No packages allowed",
378+
"Policy": "Forbidden",
379+
"Exceptions": [
380+
{
381+
"From": "TestProject.csproj",
382+
"To": "OldPackage",
383+
"Justification": "Legacy package to be removed",
384+
"IsTechDebt": true
385+
}
386+
]
387+
}
388+
],
389+
"ProjectDependencies": [
390+
{
391+
"From": "TestProject.csproj",
392+
"To": "*",
393+
"Description": "Allowed",
394+
"Policy": "Allowed",
395+
"LinkType": "DirectOrTransitive"
396+
}
397+
]
398+
}
399+
"""));
400+
401+
test.TestState.AdditionalFiles.Add(
402+
(ReferenceProtectorAnalyzer.DeclaredReferencesFile, """
403+
TestProject.csproj PackageReferenceDirect SomeOtherPackage
404+
"""));
405+
406+
test.ExpectedDiagnostics.Add(DiagnosticResult.CompilerWarning("RP0005")
407+
.WithNoLocation()
408+
.WithMessage("Package reference 'TestProject.csproj' ==> 'SomeOtherPackage' violates dependency rule 'No packages allowed' or one of its exceptions. Please remove the dependency or update 'DependencyRules.json' file to allow it."));
409+
410+
test.ExpectedDiagnostics.Add(DiagnosticResult.CompilerWarning("RP0006")
411+
.WithNoLocation()
412+
.WithMessage("Tech debt exception 'TestProject.csproj' ==> 'OldPackage' in rule 'No packages allowed' no longer matches any declared reference and can be removed from 'DependencyRules.json'"));
413+
414+
await test.RunAsync(TestContext.Current.CancellationToken);
415+
}
416+
417+
/// <summary>
418+
/// Verifies IsTechDebt defaults to false when not specified in JSON.
419+
/// </summary>
420+
[Fact]
421+
public async Task TechDebtException_DefaultFalse_NoFlagSpecified_ShouldNotReportDiagnostic_Async()
422+
{
423+
var test = GetAnalyzer();
424+
test.TestState.AdditionalFiles.Add(
425+
("DependencyRules.json", """
426+
{
427+
"ProjectDependencies": [
428+
{
429+
"From": "*",
430+
"To": "*",
431+
"Description": "No direct project references allowed",
432+
"Policy": "Forbidden",
433+
"LinkType": "Direct",
434+
"Exceptions": [
435+
{
436+
"From": "TestProject.csproj",
437+
"To": "OldProject.csproj",
438+
"Justification": "Legitimate exception",
439+
"IsTechDebt": false
440+
}
441+
]
442+
}
443+
]
444+
}
445+
"""));
446+
447+
test.TestState.AdditionalFiles.Add(
448+
(ReferenceProtectorAnalyzer.DeclaredReferencesFile, """
449+
TestProject.csproj ProjectReferenceDirect SomeOtherProject.csproj
450+
"""));
451+
452+
// Only RP0004 for SomeOtherProject, no RP0006 since IsTechDebt is explicitly false
453+
test.ExpectedDiagnostics.Add(DiagnosticResult.CompilerWarning("RP0004")
454+
.WithNoLocation()
455+
.WithMessage("Project reference 'TestProject.csproj' ==> 'SomeOtherProject.csproj' violates dependency rule 'No direct project references allowed' or one of its exceptions. Please remove the dependency or update 'DependencyRules.json' file to allow it."));
456+
457+
await test.RunAsync(TestContext.Current.CancellationToken);
458+
}
234459
}

src/Analyzers/ReferenceProtector.Analyzers/AnalyzerReleases.Shipped.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,10 @@ RP0002 | Usage | Warning | Make sure the dependency rules file '{0}' is in the c
1212
RP0003 | Usage | Info | No dependency rules matched the current project '{0}'
1313
RP0004 | Usage | Warning | Project reference '{0}' ==> '{1}' violates dependency rule '{2}' or one of its exceptions
1414
RP0005 | Usage | Warning | Package reference '{0}' ==> '{1}' violates dependency rule '{2}' or one of its exceptions
15+
16+
## Release 1.3.7
17+
18+
### New Rules
19+
Rule ID | Category | Severity | Notes
20+
--------|----------|----------|-------
21+
RP0006 | Usage | Warning | Technical debt dependency can be removed from the rules exception

src/Analyzers/ReferenceProtector.Analyzers/DiagnosticDescriptors.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,12 @@ internal static class Descriptors
4545
category: "Usage",
4646
defaultSeverity: DiagnosticSeverity.Warning,
4747
isEnabledByDefault: true);
48+
49+
public static readonly DiagnosticDescriptor TechDebtExceptionNoLongerNeeded = new(
50+
id: "RP0006",
51+
title: "Tech debt exception is no longer needed",
52+
messageFormat: "Tech debt exception '{0}' ==> '{1}' in rule '{2}' no longer matches any declared reference and can be removed from '{3}'",
53+
category: "Usage",
54+
defaultSeverity: DiagnosticSeverity.Warning,
55+
isEnabledByDefault: true);
4856
}

src/Analyzers/ReferenceProtector.Analyzers/Models/DependencyRules.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ internal record PackageDependency(
2525
internal record Exceptions(
2626
string From,
2727
string To,
28-
string Justification);
28+
string Justification,
29+
bool IsTechDebt = false);
2930

3031
internal enum Policy
3132
{

0 commit comments

Comments
 (0)