Skip to content

Fix CodeWriter field dedup to update references via MemberExpression#10134

Draft
Copilot wants to merge 3 commits intomainfrom
copilot/fix-deduplication-references
Draft

Fix CodeWriter field dedup to update references via MemberExpression#10134
Copilot wants to merge 3 commits intomainfrom
copilot/fix-deduplication-references

Conversation

Copy link
Contributor

Copilot AI commented Mar 25, 2026

CodeWriterDeclaration dedup (e.g. _scope_scope0) only applied to the declaration site. References through FieldProvider.AsValueExpression wrote the original name via AppendRaw(MemberName), bypassing the declaration's actual name entirely.

// Before: declaration deduped, but reference still uses original name
private readonly string _scope0; // ✅ deduped
_scope = value;                  // ❌ wrong name

// After: reference resolves through CodeWriterDeclaration
private readonly string _scope0; // ✅ deduped
_scope0 = value;                 // ✅ correct

Changes

  • MemberExpression: Added optional CodeWriterDeclaration? property. Write() delegates to writer.AppendDeclarationReference(Declaration) when present, which resolves the actual deduped name via scope lookup without accidentally creating new declarations.
  • CodeWriter.AppendDeclarationReference(): New method that resolves a declaration's deduped name when it exists in scope, but falls back to RequestedName otherwise. Unlike Append(CodeWriterDeclaration), it never calls WriteDeclaration, so it cannot accidentally register new deduped names in the wrong scope.
  • FieldProvider: Implicit operator to MemberExpression now sets Declaration = field.Declaration. Update() syncs _asMember.Declaration when name changes (since _declaration is reset).
  • Tests:
    • FieldDedupUpdatesReferencesViaMemberExpression — writes two fields with the same name, verifies the reference to the second field uses the deduped name.
    • FieldReferenceBeforeDeclarationUsesRequestedName — writes a field reference before its declaration, verifies it uses the original name without creating a spurious deduped name.

💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

When CodeWriter deduplicates field identifiers (e.g., appending "0" to
avoid name collisions), MemberExpression now uses the CodeWriterDeclaration
to resolve the actual name instead of the original string. This ensures
that references through FieldProvider.AsValueExpression use the deduped
name consistently with the declaration.

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/12da064c-553f-40a0-a78f-c68d46f80fba
@microsoft-github-policy-service microsoft-github-policy-service bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Mar 25, 2026
Copilot AI changed the title [WIP] Fix field deduplication references in CodeWriter Fix CodeWriter field dedup to update references via MemberExpression Mar 25, 2026
Copilot AI requested a review from jorgerangel-msft March 25, 2026 15:14
@jorgerangel-msft
Copy link
Contributor

@copilot something is not right here. The generated collectionresults definition is incorrectly referencing a field that doesn't exist Azure/azure-sdk-for-net#57468

…tions

When MemberExpression.Write() used writer.Append(Declaration), the
CodeWriter.Append method would call WriteDeclaration (registering a
new deduped name) if the field hadn't been declared in the current
scope yet. This caused field references like _token to become _token0.

Added CodeWriter.AppendDeclarationReference() which resolves the
deduped name when the declaration exists in scope, but falls back to
the requested name otherwise (never creates new declarations).

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/45a95dc7-febe-41a8-b56b-b0aad13b473f
Copy link
Contributor Author

Copilot AI commented Mar 25, 2026

@copilot something is not right here. The generated collectionresults definition is incorrectly referencing a field that doesn't exist Azure/azure-sdk-for-net#5...

Fixed in 2504336. The root cause was that MemberExpression.Write() called writer.Append(Declaration), and CodeWriter.Append(CodeWriterDeclaration) has a fallthrough path that calls WriteDeclaration when the declaration hasn't been registered in the current scope yet. This turned field references into accidental declarations, causing the deduplication suffix (_token_token0) on references while the actual field declarations kept their original names.

Added CodeWriter.AppendDeclarationReference() which resolves the deduped name when the declaration exists in scope, but falls back to RequestedName otherwise — it never creates new declarations. Also added a regression test FieldReferenceBeforeDeclarationUsesRequestedName.

}
}

internal void AppendDeclarationReference(CodeWriterDeclaration declaration)
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot this looks almost identical to the existing Append method. We should just update the existing method with an optional bool parameter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants