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

Fix a log message for unable to convert string enum to enum shape #1372

Merged
merged 1 commit into from
Aug 30, 2022

Conversation

gosar
Copy link
Contributor

@gosar gosar commented Aug 27, 2022

For the case where the enum trait does not have names and synthesizeEnumNames
is false.

There is no functional difference with this change, however a more specific and appropriate log message will be used for case where names aren't defined in the enum trait (i.e., they would need to be synthesized) but the synthesizeEnumNames is set to false.

Testing

Without the EnumShape.java change in this PR, the newly added canConvertToEnumWithNamelessEnumTrait test case for assertFalse(EnumShape.canConvertToEnum(string, false)); would log

INFO: Unable to convert string shape `ns.foo#bar` to enum shape because it has at least one value which cannot be safely synthesized into a name: bar

With the change, the test case now logs the more appropriate log for this use case

INFO: Unable to convert string shape `ns.foo#bar` to enum shape because it doesn't define names. The `synthesizeNames` option may be able to synthesize the names for you.

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

For the case where the enum trait does not have names and synthesizeEnumNames
is false.
@gosar gosar requested a review from a team as a code owner August 27, 2022 08:28
@gosar gosar merged commit 9a1a815 into smithy-lang:main Aug 30, 2022
@gosar gosar deleted the unnamed-enum-log branch August 30, 2022 00:12
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