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

Explicitly reject unsupported shapes/traits #185

Open
robin-aws opened this issue Mar 11, 2023 · 4 comments · May be fixed by #257
Open

Explicitly reject unsupported shapes/traits #185

robin-aws opened this issue Mar 11, 2023 · 4 comments · May be fixed by #257
Assignees
Labels
general-dafny-use New functionality or clean up for broader use of this repo going-public Should be done before launching 0.x publicly soundness Bugs that cause the generated code to compute the wrong value or crash

Comments

@robin-aws
Copy link
Contributor

robin-aws commented Mar 11, 2023

E.g. @streaming is currently silently ignored, leading to type errors (e.g. System.IO.Stream vs System.IO.MemoryStream).

Part of a more general class of "silently does the wrong thing on unrecognized shapes/traits" bugs - is there a more generic way code generation could fail closed on unsupported things?

I suspect we can write a selector expression for all shapes and traits this generator supports, and just do an initial application of this selector to every model and fail if anything is not covered. This would be similar to the selector the codegen README suggests using to filter out such unsupported features.

@robin-aws robin-aws added the general-dafny-use New functionality or clean up for broader use of this repo label Mar 11, 2023
@robin-aws robin-aws changed the title Explicitly reject @streaming trait Explicitly reject unsupported shapes/traits May 3, 2023
@robin-aws robin-aws added the going-public Should be done before launching 0.x publicly label May 3, 2023
@robin-aws
Copy link
Contributor Author

Note the set of supported features can be different between the Smithy plugin and the CLI: the former should reject resources as per #223, but the latter can allow them.

@robin-aws
Copy link
Contributor Author

We should also have different selector values per target language.

@robin-aws robin-aws added this to the Public 0.x launch milestone May 3, 2023
@robin-aws robin-aws self-assigned this May 3, 2023
robin-aws added a commit that referenced this issue Jun 7, 2023
Just the README updates from #185:

* Calling out runtime libraries limitations in the top level README
* Moving codegen/README.md to codegen/smithy-dafny-codegen/README.md
* Updating a few inaccuracies in the codegen README
@robin-aws robin-aws added the soundness Bugs that cause the generated code to compute the wrong value or crash label Jan 23, 2024
@robin-aws
Copy link
Contributor Author

Applied the soundness bug since there are lots of cases where silently ignoring shapes and traits we don't understand means we emit the wrong code, which easily becomes a soundness issue when targeting a language without enough static checking to save us.

@texastony
Copy link
Contributor

texastony commented Jan 23, 2024

@lucasmcdonald3 work currently uses the Java Docs trait to add documentation to generated Python.

It works pretty well.

As such, @robin-aws , what do you think about refactoring the JavaDoc trait to just be a poor man's documentation trait,
ideally with a warning that it only works in certain scenarios?

I'd rather not invent a PyDocs trait... but it would also to be weird to see the JavaDoc trait gain support in Python, and then Go... etc.

robin-aws pushed a commit that referenced this issue Apr 19, 2024
Add an extern to handle this extra case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general-dafny-use New functionality or clean up for broader use of this repo going-public Should be done before launching 0.x publicly soundness Bugs that cause the generated code to compute the wrong value or crash
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants