Use a qualified metadata key for removeTraits - #33
Merged
Merged
Conversation
Metadata keys are arbitrary strings, so `smithytransformations#removeTraits` is legal Smithy — the `#` carries no namespace meaning to the loader, it is just part of the key. Qualify it to avoid collisions: metadata is merged across every model loaded together, so a bare `removeTraits` would clash with any other library that happened to pick the same word. The prelude's unqualified `shapeClosures` is not a counter-example — the prelude owns the global namespace and has no collision to avoid. The transformer's name, referenced from `smithy4sModelTransformers` and `smithy-build.json`, stays `removeTraits`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #32, restoring the qualified metadata key.
Was it legal Smithy?
Yes. Metadata keys are arbitrary strings — the
keymember of the@metadatatrait is constrained only by@length(min: 1), and the spec's own examples use plain strings like"foo". A#in the key is just a character; the loader attaches no namespace meaning to it.smithytransformations#removeTraitsloaded and validated fine all along.I'd changed it to a bare
removeTraitson the basis that the prelude usesshapeClosures. That was a weak inference: one data point, and the prelude is a special case — it owns the global namespace, so it has no collision problem to solve. A third-party library does.Why qualify it
Metadata merges across every model loaded together, so a bare
removeTraitswould collide with any other library that picked the same word. The spec is silent on the question, which makes collision-avoidance our call rather than a convention to follow.The transformer's name — what
smithy4sModelTransformersandsmithy-build.jsonreference — staysremoveTraits. Only the metadata key is qualified.sbt clean testpasses (30 tests), scalafmt clean.🤖 Generated with Claude Code