fix(generator): qualify method receivers that collide with arm/storage import aliases#5504
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the ASO v2 code generator’s receiver-naming logic to prevent generated method receivers from shadowing conversion-package imports (notably arm and storage), which can otherwise cause compile failures in generated assignment/conversion code.
Changes:
- Extend
CreateReceiver()to qualify receiver names when the type-derived base name collides with known conversion-package names (arm,storage) in addition to the existing reserved-word and short-identifier guards. - Introduce a shared
forbiddenReceiverNamesset to centralize these disallowed receiver names. - Add a unit test case covering the
*Storagecollision scenario (including theStatus-suffixed variant).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| v2/tools/generator/internal/astmodel/identifier_factory.go | Adds a forbidden receiver-name set (arm/storage) and incorporates it into receiver qualification logic. |
| v2/tools/generator/internal/astmodel/identifier_factory_test.go | Adds regression tests ensuring *Storage types get a qualified receiver (e.g., deploymentStorage). |
|
@microsoft-github-policy-service agree |
|
Thanks for the contribution, much appreciated.
When did this error occur? I'd like to reproduce the problem in order to verify your fix works without breaking any of the existing edge cases - getting |
|
Our build checks have failed - some of the existing code is getting a new receiver name. This looks to be behaving as intended, and we most likely should merge this. What I need to understand is why you are seeing compilation errors in your situation, but we haven't seen similar errors previously, even when the receiver name selected has been |
…e import aliases A type whose name's last word is 'Storage' (e.g. FunctionsDeployment_Storage) yielded a method receiver 'storage', shadowing the storage conversion-package import alias in generated AssignProperties_To_* methods and breaking the build. CreateReceiver already qualifies reserved words and short (<=3 char) names; also qualify names that collide with the fixed conversion-package aliases (arm/storage). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The generator fix qualifies method receivers that collide with the arm/storage conversion-package import aliases. UserOwnedStorage's last word is "Storage", so its receiver changes from "storage" to "ownedStorage". Regenerated repo-wide; this is the only existing type affected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bbecbbe to
a7e76c3
Compare
|
Sure — I hit this adding the web 2025-05-01 version (#5503). The generated conversion for the new It's down to where the receiver shadows the
func (storage *FunctionsDeployment_Storage) AssignProperties_To_FunctionsDeployment_Storage(destination *storage.FunctionsDeployment_Storage) error {
// ...
if storage.Authentication != nil {
var authentication storage.FunctionsDeployment_Storage_Authentication // 'storage' is the receiver here, not the package
err := storage.Authentication.AssignProperties_To_FunctionsDeployment_Storage_Authentication(&authentication)
// ...
}
}In the body So the fix just stops |
I understand now, thanks for the explanation. The change looks solid to me, I've set the PR checks going. |
|
Checks passed, I've queued this to merge. Thanks once again! |
Problem
CreateReceiver()derives a method-receiver name from the last word of a type name. For a type whosename ends in
Storage(e.g.FunctionsDeployment_Storagein the web group), the receiver becomesstorage— which shadows thestorageconversion-package import alias used in generatedAssignProperties_To_*/AssignProperties_From_*methods. The generated code then fails to compile,e.g.
storage.FunctionsDeployment_Storage_Authentication is not a type.CreateReceiveralready qualifies Go reserved words and very short (≤3-char) identifiers — soarm(len 3) was already safe, but
storagewas not. This extends the same guard to the fixedconversion-package import aliases (
arm,storage) so a type-derived receiver can never shadow them.The receiver is renamed (e.g.
storage→deploymentStorage); type names and JSON wire tags areunchanged.
General fix: any current or future type whose name ends in
Storage(or otherwise collides with thesealiases) benefits.
Tests
go test ./internal/astmodel/...(incl. a new case covering the receiver/alias collision) → okgo test ./internal/codegen/...(golden tests) → ok, no regressions🤖 Generated with Claude Code