Skip to content

Dedup partial service-class declarations in the generator#4560

Closed
pepone wants to merge 1 commit intoicerpc:mainfrom
pepone:fix/service-generator-dedup-partial
Closed

Dedup partial service-class declarations in the generator#4560
pepone wants to merge 1 commit intoicerpc:mainfrom
pepone:fix/service-generator-dedup-partial

Conversation

@pepone
Copy link
Copy Markdown
Member

@pepone pepone commented Apr 27, 2026

Summary

  • ForAttributeWithMetadataName emits one ClassDeclarationSyntax per attribute application. If a user writes [Service] on more than one part of a partial class, the parser previously processed every syntax node and the generator called context.AddSource twice with the same hint name, which Roslyn rejects with an opaque generator-threw ArgumentException at build time.
  • Track classes by their resolved INamedTypeSymbol (using SymbolEqualityComparer.Default) and skip ones we've already processed. The parts together are one class — silent dedup is the natural behavior.

Fixes #4458

ForAttributeWithMetadataName emits one ClassDeclarationSyntax per
attribute application. If a user puts [Service] on more than one
partial part of the same class, the parser previously processed every
syntax node and the generator called AddSource twice with the same
hint name, which Roslyn rejects with an opaque generator-threw
ArgumentException at build time.

Track classes by their resolved INamedTypeSymbol and skip ones we've
already processed. The user's intent is unambiguous (the parts
together are one class), so silent dedup is the natural behavior; no
diagnostic is needed.
Copilot AI review requested due to automatic review settings April 27, 2026 13:58
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an edge case in IceRpc.ServiceGenerator where applying [Service] to multiple parts of the same partial class caused the generator to emit duplicate sources with the same hint name, triggering a Roslyn ArgumentException during build.

Changes:

  • Deduplicate service class processing by tracking seen INamedTypeSymbols with SymbolEqualityComparer.Default.
  • Skip subsequent partial declarations of the same class symbol to avoid duplicate AddSource hint names.
  • Add an explanatory comment documenting why the deduplication exists.

Comment on lines 49 to +60
@@ -53,7 +57,7 @@ internal IReadOnlyList<ServiceClass> GetServiceDefinitions(IEnumerable<ClassDecl

SemanticModel semanticModel = _compilation.GetSemanticModel(classDeclaration.SyntaxTree);
INamedTypeSymbol? classSymbol = semanticModel.GetDeclaredSymbol(classDeclaration, _cancellationToken);
if (classSymbol is null)
if (classSymbol is null || !seenClassSymbols.Add(classSymbol))
@pepone
Copy link
Copy Markdown
Member Author

pepone commented Apr 27, 2026

Closing as not applicable.

The audit's premise relies on the user being able to apply [Service] on multiple partial parts of the same class. ServiceAttribute is declared as [AttributeUsage(AttributeTargets.Class, Inherited = false)] — AllowMultiple defaults to false, so the C# compiler itself rejects this with CS0579 before the generator runs (verified empirically with a test that failed to compile). The duplicate-AddSource scenario is therefore unreachable, making the dedup defensive belt-and-braces at the wrong layer. See #4458.

@pepone pepone closed this Apr 27, 2026
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.

[Audit-Low] partial service classes with [Service] declared on multip…

2 participants