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

Selectors: recursive mixin check? #2228

Closed
kubukoz opened this issue Apr 4, 2024 · 3 comments · Fixed by #2353
Closed

Selectors: recursive mixin check? #2228

kubukoz opened this issue Apr 4, 2024 · 3 comments · Fixed by #2353
Labels
feature-request A feature should be added or improved.

Comments

@kubukoz
Copy link
Contributor

kubukoz commented Apr 4, 2024

Hi! I was wondering if there's a way to write a selector that checks if the given shape has a mixin... but checking recursively.

For example, consider this model:

$version: "2"

namespace demo

@mixin
structure MyMixin {}

@hasMyMixin
@mixin
structure Direct with [MyMixin] {}

// structure Indirect with [Direct] {}

@trait(selector: ":test(-[mixin]-> structure [id = 'demo#MyMixin'])")
structure hasMyMixin {}

In its current state, it's valid. However, if you uncomment Indirect it no longer matches (it inherits the hasMyMixin trait, but that fails to validate on Indirect due to the mixin being transitive).

Would it make sense to expand the mixin relationship, or provide a recursive variant of -[...]->? Perhaps a "forward directed recursive neighbor" syntax, like -[...]~>?

Is there a better, existing way to do what I'm looking for? (without resorting to Java)

@kubukoz kubukoz changed the title Recursive mixin check? Selectors: recursive mixin check? Apr 4, 2024
@mtdowling
Copy link
Member

Maybe we could add a synthetic “transitiveMixin” relationship available to selectors which includes direct and transitive mixins. Or maybe a :hasMixin(a, c, c) function that returns true if all mixins are present on a shape. I like the relationship but we’d need to see how disruptive it is vs a function.

@JordonPhillips JordonPhillips added the feature-request A feature should be added or improved. label Apr 23, 2024
@mtdowling
Copy link
Member

mtdowling commented Jul 18, 2024

I pushed up a branch that has a proof of concept for :recursive(expr) function in it. Any result that hasn't been seen yet is recursively selected by the given expression, so you can do :recursive(-[mixin]->) to get all mixins of a shape. It has no tests or docs, and I still need to deeply think through how good of an idea this specific approach is.

If you wanna try it out, you could pull this down and then build the Smithy CLI using ./gradlew :smithy-cli:runtime. I want to make sure there are more use cases than just finding transitive mixins. Edit: I'll try to list some ideas on what this could do:

  1. Find direct and transitive mixins: :recursive(-[mixin]->).
  2. Find out if a shape has a mixin directly or transitively: :test(:recursive(-[mixin]->) [id=test#MyMixin])
  3. Find the enclosing resource hierarchy of a shape: :recursive(<-[resource, service]-)
  4. Find out if a shape is in the closure of a specific resource: :test(:recursive(<-[resource]-) [id=test#A])

0dfdc54

mtdowling added a commit that referenced this issue Jul 19, 2024
mtdowling added a commit that referenced this issue Jul 19, 2024
mtdowling added a commit that referenced this issue Aug 28, 2024
kstich pushed a commit that referenced this issue Aug 28, 2024
@kubukoz
Copy link
Contributor Author

kubukoz commented Aug 28, 2024

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants