Fix source generator type ambiguity and generic type test discovery#182
Conversation
Skip Spectre.Console.Ansi in source generator to prevent CS0433 when Spectre.Console 0.55+ exposes duplicate StringExtensions across assemblies. Also deduplicate types by fully-qualified name as a general safety net. Filter open generic type definitions from TestCommandsAreSupported since CommandInvoker cannot resolve unbound type parameters.
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on 04fd2ef..9fb585d
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (2)
• Rdmp.Core.Generators/TypeRegistryGenerator.cs
• Rdmp.Core.Tests/CommandExecution/TestCommandsAreSupported.cs
There was a problem hiding this comment.
Pull request overview
This PR updates the compile-time type registry source generator and command test discovery to avoid failures caused by cross-assembly type name collisions (notably with Spectre.Console 0.55+) and by open generic command types being discovered in tests.
Changes:
- Exclude
Spectre.Console.Ansifrom type collection to prevent CS0433 ambiguity when duplicate types exist across Spectre assemblies. - Deduplicate collected generator types by fully-qualified name before emitting the registry.
- Filter open generic type definitions out of
TestCommandsAreSupportedtest case discovery.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Rdmp.Core.Tests/CommandExecution/TestCommandsAreSupported.cs | Excludes open generic command types from discovered test cases so CommandInvoker isn’t asked to validate unbound generic definitions. |
| Rdmp.Core.Generators/TypeRegistryGenerator.cs | Avoids known problematic assemblies and deduplicates types to reduce risk of collisions when generating the compiled type registry. |
Comments suppressed due to low confidence (1)
Rdmp.Core.Generators/TypeRegistryGenerator.cs:188
group.Count()is computed for everyIGroupingand then the group is enumerated again viaFirst()/foreach, which results in repeated enumeration of each group. Since this generator can process many types, consider materializing each group once (or use a lookup that exposesCountcheaply) to keep generation time predictable.
// Group types by short name to handle duplicates
var grouped = deduped.GroupBy(t => t.ShortName);
foreach (var group in grouped)
{
if (group.Count() == 1)
{
var type = group.First();
sb.AppendLine($" // {type.FullName}");
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Strip global:: from dictionary keys so Type.FullName lookups hit the fast path. Materialize GroupBy results to avoid re-enumerating each group. Normalize dedup key to match the dictionary key space.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #182 +/- ##
=====================================
Coverage 0.00% 0.00%
=====================================
Files 1118 1118
Lines 66302 66302
Branches 8767 8767
=====================================
Misses 66302 66302 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
Spectre.Console.Ansiassembly in the source generator to prevent CS0433 ambiguity when Spectre.Console 0.55+ exposes duplicateStringExtensionsacross assembliesTestCommandsAreSupportedsinceCommandInvokercannot resolve unbound type parameters likeT,T1Context
Dependabot PRs #180 (NPOI 2.8.0) and #181 (Spectre.Console 0.55.0) are both failing CI. Once this merges, rebasing those PRs on main should fix both.
Test plan
High-level PR Summary
This PR fixes two build and test failures introduced by dependency updates: it prevents compiler ambiguity errors (CS0433) by excluding the
Spectre.Console.Ansiassembly from source generation and deduplicating types by fully-qualified name, and it filters out open generic type definitions from test discovery to preventTestCommandsAreSupportedfailures when encountering unbound type parameters.⏱️ Estimated Review Time: 5-15 minutes
💡 Review Order Suggestion
Rdmp.Core.Generators/TypeRegistryGenerator.csRdmp.Core.Tests/CommandExecution/TestCommandsAreSupported.csSummary by cubic
Fixes CS0433 type ambiguity in the source generator and stabilizes generic command test discovery with
Spectre.Console0.55+. Normalizes registry keys to improve lookups, so Dependabot PRs #181 and #180 should pass after a rebase.Spectre.Console.Ansiduring type collection to avoid duplicateStringExtensionsacrossSpectre.Consoleassemblies.global::), and materialize grouped results to prevent collisions and repeated enumeration while speeding up dictionary lookups.CommandInvoker.Written for commit 3c34d89. Summary will update on new commits.