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

Add 0.5.0 IDL and AST format #213

Merged
merged 3 commits into from
Dec 12, 2019
Merged

Add 0.5.0 IDL and AST format #213

merged 3 commits into from
Dec 12, 2019

Conversation

mtdowling
Copy link
Member

@mtdowling mtdowling commented Nov 25, 2019

Issue #, if available:

Closes #189

Description of changes:

This commit adds support for version 0.5.0 of the AST. This makes loading and parsing the AST model much simpler and more approachable for other languages and tools like grep.

A TON of stuff has been changed in this PR. Lots of the changes were done using ad-hoc scripts I wrote to migrate from the old JSON AST to the new one (including scripts used to migrate the RST example). I looked at each RST example carefully, but they should still be double-checked. Test files were updated until they worked, so I'm confident that their right, but could use a very cursory glance at best.

Things to really look for:

  • The JSON AST spec was updated significantly.
  • Loading JSON models was updated. A single model loader is used for all JSON formats, and it then delegates to specific versions.
  • We no longer have a bunch of logic for loading and parsing different versions of the IDL and comparing ranges of compatibility. We still support 0.4.0 and 0.5.0 and consider them compatible. Instead, we now just fail if we don't recognize the model version number. I think this is a much safer implementation choice.
  • Traits that target shapes must also use absolute shape IDs. When using the IDL, this is no big deal since it support forward references. However, this will be an issue for JSON models that are migrated from 0.4.0 to 0.5.0. A warning is emitted when such an ID is found in idRef or references traits.
  • smithyVersion was removed from Model. This really doesn't make sense because the version is relative to the version of smithy-model being used, not the version of the model files that were loaded. Put another way, you can't change the semantic model in memory based on the version loaded from a model file, so smithyVersion doesn't make sense.

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

@mtdowling mtdowling changed the base branch from deprecate-shape-index to master November 26, 2019 17:02
@mtdowling mtdowling changed the base branch from master to deprecate-shape-index November 26, 2019 17:02
@mtdowling mtdowling changed the base branch from deprecate-shape-index to master November 26, 2019 17:09
@mtdowling mtdowling changed the title Add 0.5.0 format Add 0.5.0 AST format Dec 4, 2019
@mtdowling mtdowling force-pushed the add-0_5_0-format branch 2 times, most recently from db77f29 to 12785e3 Compare December 10, 2019 18:32
- Remove onVersion since LoaderVisitor is version independent
- Simplify ModelLoader and use InputStream. This commit simplifies
  ModelLoader to be a single class with static methods rather than a
  series of internal-only ModelLoader interfaces. Further, it converts
  model loading to using InputStreams rather than Strings so that files
  can be parsed and streamed directly from their source rather than first
  loaded into a string and then into a Model. We should follow up on this
  by making the SmithyModelLexer streaming too.
- Update specification for 0.5.0 IDL
@mtdowling mtdowling changed the title Add 0.5.0 AST format 0.5.0 AST format Dec 12, 2019
@mtdowling mtdowling changed the title 0.5.0 AST format Add 0.5.0 IDL and AST format Dec 12, 2019
@mtdowling
Copy link
Member Author

This has been updated to include the AST, IDL, and protocol compliance tests. It's a rebase of those features and squashed into single commits for each. Nothing new here.

@mtdowling mtdowling merged commit 996f05b into master Dec 12, 2019
@mtdowling mtdowling deleted the add-0_5_0-format branch December 13, 2019 17:05
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.

Improve JSON AST model format
2 participants