Skip to content

fix(utilities): make GetHintName encoding injective (review findings from #152) - #153

Merged
github-actions[bot] merged 1 commit into
mainfrom
claude/hint-name-collisions
Jul 6, 2026
Merged

fix(utilities): make GetHintName encoding injective (review findings from #152)#153
github-actions[bot] merged 1 commit into
mainfrom
claude/hint-name-collisions

Conversation

@ANcpLua

@ANcpLua ANcpLua commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Summary

Both CodeRabbit findings on #152 survived adversarial verification — they are real, constructible collisions in an API documented as collision-free — so this fixes the encoding before anyone depends on the format (shipped in 2.2.32 minutes ago).

Collision class Before After
class Result_1 vs class Result<T> both Result_1.g.cs Result_1.g.cs vs Result(1).g.cs
namespace A { class B { class C } } vs namespace A.B { class C } both A.B.C.g.cs A.B-C.g.cs vs A.B.C.g.cs

The fix: both markers — (N) arity and - nesting separator — are characters no C# identifier or namespace segment can contain, so the mapping from declaration to hint name is injective. Example: Deep.Outer(1)-Middle-Inner(1).g.cs.

Also qualifies the ambiguous SourceProductionContext.AddSource cref that #152 introduced (its docs now reference the (string, string) overload explicitly) — zero ambiguous-cref warnings remain.

Verification

  • 215 passed, 0 failed (3 new):
    • regression tests for both collision classes above
    • a CSharpGeneratorDriver probe generator that calls AddSource(info.GetHintName(), …) for the gnarliest name — proving Roslyn's hint-name validation accepts the (N)/- charset empirically, not by assumption
  • Solution build: 0 errors, no new warnings

Status: complete-and-verified. Publishes as 2.2.33 via normal auto-bump.

🤖 Generated with Claude Code

…e cref

Review findings on #152 identified two real collision classes in the hint
name format:
- the _N arity suffix is forgeable by an ordinary identifier, so
  class Result_1 and class Result<T> both produced Result_1.g.cs
- namespace segments and nesting levels shared the '.' separator, so
  namespace A { class B { class C } } and namespace A.B { class C } both
  produced A.B.C.g.cs

Both markers are now characters no C# identifier can contain: arity is (N)
and nesting levels join with '-', making the encoding injective
(Deep.Outer(1)-Middle-Inner(1).g.cs). A CSharpGeneratorDriver probe test
proves Roslyn hint-name validation accepts the charset.

Also qualifies the SourceProductionContext.AddSource cref introduced in
#152 (ambiguous across its (string, string) / (string, SourceText)
overloads).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions
github-actions Bot merged commit d753453 into main Jul 6, 2026
3 of 4 checks passed
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7c38104d-2973-45aa-bf7c-28a3cf995dbb

📥 Commits

Reviewing files that changed from the base of the PR and between c249f33 and 2b21d0f.

⛔ Files ignored due to path filters (2)
  • .claude/TASK.md is excluded by none and included by none
  • README.md is excluded by none and included by none
📒 Files selected for processing (2)
  • src/ANcpLua.Roslyn.Utilities/Models/TypeDeclarationInfo.cs
  • tests/ANcpLua.Roslyn.Utilities.Testing.Tests/TypeDeclarationInfoTests.cs

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting


Summary by CodeRabbit

  • Bug Fixes
    • Updated generated source file names to use a clearer, more consistent format for nested and generic types.
    • Improved naming so type hierarchies are easier to distinguish and less likely to collide.
    • Ensured generated files now follow the expected .g.cs naming pattern.

Walkthrough

The hint-name encoding produced by TypeDeclarationInfo.GetHintName() is changed: containing-type nesting now uses - separators, generic arity markers use (N) instead of _N, and the suffix is corrected to .g.cs. Corresponding tests are updated and expanded, including a renamed test and new collision/nesting/Roslyn-validation coverage.

Changes

Hint Name Encoding Update

Layer / File(s) Summary
Hint name generation logic and docs
src/ANcpLua.Roslyn.Utilities/Models/TypeDeclarationInfo.cs
GetHintName and AppendHintNameLevel now join containing-type levels with -, encode generic arity as (N) instead of _N, and append a corrected .g.cs suffix; XML docs updated to match.
Test expectations and validation coverage
tests/ANcpLua.Roslyn.Utilities.Testing.Tests/TypeDeclarationInfoTests.cs
Expected hint names updated to the new format, overload-disambiguation test renamed, and new tests added for arity-marker collision resistance, namespace/containing-type nesting distinction, and Roslyn generator hint-name validation.

Estimated code review effort: 2 (Simple) | ~10 minutes


Comment @coderabbitai help to get the list of available commands.

github-actions Bot pushed a commit that referenced this pull request Jul 6, 2026
…, qyl bumped) (#154)

Full arc complete: GetHintName + cref fix merged via #152 and published as
2.2.32 (explicit version override, 2.2.31 skipped intentionally); review
findings on the hint-name encoding fixed via #153 and published as 2.2.33;
both versions tagged and indexed on nuget.org; qyl pins bumped in qyl#490
and qyl#492.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.

1 participant