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 bug where mixin cycles were incorrectly detected #1628

Merged
merged 3 commits into from
Feb 23, 2023

Conversation

hpmellema
Copy link
Contributor

Description of bug
A bug was discovered where mixin cycles would be incorrectly detected if a non structure or union shape had a trait that was removed by a transform.

For example:
The following smithy model

// main.smithy
namespace example

@trait
@tags(["filtered"])
structure myTrait {}

@myTrait
operation OperationWithMixin with [ OperationMixin ] {
    input := {
        @required
        myInputField: String,
        other: String
    }
}

Using the following Transform:

{
  "version": "2.0",
  "projections": {
    "myProjection": {
      "transforms": [
       {
          "name": "excludeTraitsByTag",
          "args": {
            "tags": ["filtered"]
          }
        },
    ]}
  }
}

Would encounter a build exception like

Projection myProjection failed: software.amazon.smithy.model.loader.TopologicalShapeSort$CycleException: Mixin cycles detected among [example#OperationWithMixin]
software.amazon.smithy.model.loader.TopologicalShapeSort$CycleException: Mixin cycles detected among [example.#OperationWithMixin]

Description of changes:

  • Adds new public methods to the Model class to access shapes using mixins and mixin shapes
  • Uses the above methods to add all mixins and shapes with mixins to topological sorter in the ReplaceShapes class so that cycle detection can run correctly (previously only unions and structures were added)
  • Adds tests for the specific error encountered above
  • Adds simple unit test for methods added to Model class.

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

@hpmellema hpmellema requested a review from a team as a code owner February 22, 2023 21:01
@hpmellema hpmellema merged commit e05b0fd into smithy-lang:main Feb 23, 2023
@hpmellema hpmellema deleted the fix-mixin-cycle-error branch February 23, 2023 14:36
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.

5 participants