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

Prevent bad list, set, map recursion #204

Merged
merged 1 commit into from
Nov 14, 2019
Merged

Conversation

mtdowling
Copy link
Member

We currently don't allow a directly recursive list or set shape.
However, we still allow for list, set, and map shapes to be
transitively recursive unconditionally (e.g., listA -> listA is invalid,
but listA -> set -> listA is valid). This is problematic for code
generators in various languages that want to use parameteric types and
standard libraries to codegen lists, sets, and maps. For example, in
Java, a list in Smithy should be generated as a List<X> where X is
the code generated member of the list. However, if the list is recursive
onto itself, then it's impossible to generate valid code (e.g.,
List<List<List...).

This change requires that recursive list, set, and map shapes must have
one or more structure or union members in their recursive path in order to
define a valid shape. This will result in shapes that are far easier to
generate and provides constraints that are very helpful when converting
to other formats (for example, this might be useful in JSON schema
conversions if we want to always inline list and set shape definitions).
This change should not have a practical impact on models since no
current AWS service defines an unconditionally recursive list, set,
or map shape.

Issue #, if available:

Description of changes:

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

@mtdowling mtdowling requested a review from kstich November 14, 2019 03:55
@mtdowling mtdowling force-pushed the no-recursive-collections branch from 848a059 to 5a4c2d1 Compare November 14, 2019 03:56
docs/source/spec/core.rst Show resolved Hide resolved
docs/source/spec/core.rst Outdated Show resolved Hide resolved
@mtdowling mtdowling force-pushed the no-recursive-collections branch from 5a4c2d1 to 72fa877 Compare November 14, 2019 06:09
@mtdowling mtdowling requested a review from kstich November 14, 2019 06:10
We currently don't allow a directly recursive list or set shape.
However, we still allow for list, set, and map shapes to be
transitively recursive unconditionally (e.g., listA -> listA is invalid,
but listA -> set -> listA is valid). This is problematic for code
generators in various languages that want to use parameteric types and
standard libraries to codegen lists, sets, and maps. For example, in
Java, a list in Smithy should be generated as a `List<X>` where X is
the code generated member of the list. However, if the list is recursive
onto itself, then it's impossible to generate valid code (e.g.,
`List<List<List...`).

This change requires that recursive list, set, and map shapes must have
one or more structure or union members in their recursive path in order to
define a valid shape. This will result in shapes that are far easier to
generate and provides constraints that are very helpful when converting
to other formats (for example, this might be useful in JSON schema
conversions if we want to always inline list and set shape definitions).
This change should not have a practical impact on models since no
current AWS service defines an unconditionally recursive list, set,
or map shape.
@mtdowling mtdowling force-pushed the no-recursive-collections branch from 72fa877 to 7729b04 Compare November 14, 2019 17:32
@mtdowling mtdowling merged commit 0461aed into master Nov 14, 2019
@mtdowling mtdowling deleted the no-recursive-collections branch November 15, 2019 05:29
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