Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix replacing shapes when traits applied to mixed-in members #1509

Merged
merged 1 commit into from
Nov 22, 2022

Conversation

srchase
Copy link
Contributor

@srchase srchase commented Nov 22, 2022

This PR fixes how mixin shapes are updated in the ReplaceShapes transform, which is used by other transforms, such as the excludeShapesByTag or excludeTraitsByTag.

Take this model:

$version: "2.0"

namespace ns.example

@trait()
@tags(["internal"])
string mytrait

@mixin
structure MyMixin {
   foo: String
}

structure StructForMixin with [MyMixin] {
    bar: String
}

apply StructForMixin$foo mytrait("baz")

With this smithy-build.json:

{
  "version": "1.0",
  "projections": {
    "mytransform": {
      "transforms": [
        {
          "name": "excludeTraitsByTag",
          "args": {
            "tags": ["internal"]
          }
        }
      ]
    }
  }
}

Currently, when traits are applied to mixed-in members via an apply statement, a spurious CycleDetection error is thrown when attempting to update the mixed inns.example#StructForMixin$foo member to remove the mytrait trait that was added through an apply statement.

Projection mytransform failed: software.amazon.smithy.model.loader.TopologicalShapeSort$CycleException: Mixin cycles detected among [ns.example#StructForMixin$foo]
software.amazon.smithy.model.loader.TopologicalShapeSort.dequeueSortedShapes(TopologicalShapeSort.java:104)
software.amazon.smithy.model.transform.ReplaceShapes.updateMixins(ReplaceShapes.java:195)

This change prevents extra forward references from being enqueued by the Topological shape sorter that's used to update mixins.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@srchase srchase requested a review from a team as a code owner November 22, 2022 13:49
@srchase srchase merged commit f11c4cd into smithy-lang:main Nov 22, 2022
@srchase srchase deleted the fix-mixed-in-member-replacement branch November 22, 2022 16:10
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.

2 participants