Skip to content

Add DP033 cross-assembly duplicate strategy key analyzer#113

Merged
Skymly merged 1 commit into
mainfrom
feature/analyzers/dp033-cross-assembly-duplicate-key
Jun 16, 2026
Merged

Add DP033 cross-assembly duplicate strategy key analyzer#113
Skymly merged 1 commit into
mainfrom
feature/analyzers/dp033-cross-assembly-duplicate-key

Conversation

@Skymly

@Skymly Skymly commented Jun 16, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add DP033 (Error): reports when the same strategy key for the same contract is registered in multiple referenced provider assemblies (plugin-host scenario).
  • Extend analyzer test harness with two-referenced-assembly + host compilation helper.
  • Fix generic [RegisterStrategy<T>] attribute recognition via MetadataName matching.

Closes #108.

Test plan

  • dotnet test tests/DesignPatterns.Analyzers.Tests (44 passed)
  • Full solution build
  • CI green on PR

Note

Medium Risk
New compile-time Error on multi-provider references can break existing hosts that relied on overlapping keys; scope is analyzer-only with no runtime behavior change.

Overview
Adds DP033 (Error) for plugin-host builds: when a compilation references multiple provider assemblies that both register the same strategy key for the same contract, the new CrossAssemblyRegistryKeyAnalyzer reports on each conflicting [RegisterStrategy] (single-assembly duplicates stay on DP003).

Supporting changes: TryGetContractTypeFromAttribute now reads the generic contract from type arguments first; IsRegisterStrategyAttribute matches via MetadataName so [RegisterStrategy<T>] is recognized reliably. Diagnostics/docs register DP033; plugin docs flip from “not implemented” to describing the analyzer; tests add a two-referenced-assembly + host harness and DP033 scenarios.

Reviewed by Cursor Bugbot for commit 0590bc7. Configure here.

Detect when multiple referenced provider assemblies register the same key for the same contract, which breaks plugin-host merges at runtime.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Attribute match ignores declaring namespace
    • Updated strategy attribute matching to compare fully qualified DesignPatterns.Behavioral metadata names instead of short attribute names.

Create PR

Or push these changes by commenting:

@cursor push 3d8fb5c934
Preview (3d8fb5c934)
diff --git a/DesignPatterns.Analyzers/StrategyAnalysisConstants.cs b/DesignPatterns.Analyzers/StrategyAnalysisConstants.cs
--- a/DesignPatterns.Analyzers/StrategyAnalysisConstants.cs
+++ b/DesignPatterns.Analyzers/StrategyAnalysisConstants.cs
@@ -14,9 +14,12 @@
             return false;
         }
 
-        return attributeClass.OriginalDefinition.MetadataName switch
+        var originalDefinition = attributeClass.OriginalDefinition;
+        var metadataName = originalDefinition.ContainingNamespace.ToDisplayString() + "." + originalDefinition.MetadataName;
+
+        return metadataName switch
         {
-            "RegisterStrategyAttribute" or "RegisterStrategyAttribute`1" => true,
+            RegisterStrategyMetadataName or RegisterStrategyGenericMetadataName => true,
             _ => false,
         };
     }

You can send follow-ups to the cloud agent here.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 0590bc7. Configure here.

{
"RegisterStrategyAttribute" or "RegisterStrategyAttribute`1" => true,
_ => false,
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attribute match ignores declaring namespace

Low Severity

IsRegisterStrategyAttribute treats any attribute whose OriginalDefinition.MetadataName is RegisterStrategyAttribute or RegisterStrategyAttribute`1 as a DesignPatterns strategy registration. Elsewhere (IsKeyedRegistrationAttribute, UnregisteredStrategyAnalyzer, generators) matching uses the fully qualified DesignPatterns.Behavioral metadata names.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0590bc7. Configure here.

@Skymly Skymly left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

DP033 closes the plugin-host gap left by DP003: duplicate strategy keys are caught per assembly by the generator, but multiple referenced provider assemblies could still collide at runtime until this analyzer runs at compilation.

What looks good

  • Compilation-scoped scan across referenced assemblies, grouped by contract FQN + key (not symbol identity), which matches how plugin providers are built.
  • Generic [RegisterStrategy<T>] recognition via MetadataName — the right fix for metadata round-trips.
  • Tests cover the happy conflict path, distinct keys, single-assembly duplicates (still DP003), and different contracts sharing a key.

Follow-ups (out of scope for #108)

  • DP034 info/doc hint deferred per issue.
  • User docs: DesignPatterns.Docs PR #8 adds the #dp033 anchor for IDE help links.

LGTM for merge.

@Skymly
Skymly merged commit 7f5c74b into main Jun 16, 2026
5 checks passed
@Skymly
Skymly deleted the feature/analyzers/dp033-cross-assembly-duplicate-key branch June 16, 2026 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Harden plugin registry diagnostics for multi-assembly merges (DP033+)

1 participant