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 support for unnamed enumerations #2

Merged
merged 3 commits into from
Oct 30, 2020
Merged

Add support for unnamed enumerations #2

merged 3 commits into from
Oct 30, 2020

Conversation

rcoh
Copy link
Collaborator

@rcoh rcoh commented Oct 28, 2020

Description of changes:
Unnamed enumerations are supported by creating a "newtype" that wraps the string. The newtype provides the valid values as a list for convenience.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Unnamed enumerations are supported by creating a "newtype" that wraps the string. The newtype provides the valid values as a list for convenience.
@rcoh rcoh requested review from kggilmer and a team October 28, 2020 19:18
@rcoh rcoh requested a review from justinboswell October 29, 2020 23:51
Copy link

@justinboswell justinboswell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question, LGTM otherwise

writer.write("pub struct $enumName(String);")
writer.rustBlock("impl $enumName") {
writer.rustBlock("pub fn as_str(&self) -> &str") {
write("&self.0")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this little bit of wizardry actually do? I don't know that I've seen that syntax before.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.0? it's just what the member is named in tuple structs (what we generate for unamed enums)

Copy link

@kggilmer kggilmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

EnumGenerator.deriveName(null, "Signal: creation is in progress") shouldBe("SignalCreationIsInProgress")
fun `it generates unamed enums`() {
val model = """
namespace test

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not particular to this CR or the Rust SDK, but it strikes me that any syntactical style of the emitted codegen is likely to age as languages and styles evolve. I think I recall that the Go SDK performs code formatting as a post processing step. The value to this becomes clear to me, in that the style formatting tool only needs to be updated to match evolving styles, rather that numerous finicky changes to codegen strings.

Copy link
Collaborator Author

@rcoh rcoh Oct 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah our codegenerator runs cargo format as part of codegen so our output code is consistently formatted.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

although the code is more designed to always compile vs. being legible. (as in, it doesn't actually use imports, it refers to everything with a fully qualified name, etc.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and in case it wasn't clear, the literal string here is smithy, not Rust code that we're validating against.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do that with clang-format on C++ code as well, and autopep8 for python. Highly recommend ignoring style during generation.

@rcoh rcoh merged commit 2d71e48 into main Oct 30, 2020
@rcoh rcoh deleted the russell/unnamed-enums branch October 30, 2020 18:06
Velfi added a commit that referenced this pull request Apr 5, 2024
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.

3 participants