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

Error when parsing FromMeta containing NestedMeta::Lit #193

Merged
merged 8 commits into from Sep 8, 2022
Merged

Error when parsing FromMeta containing NestedMeta::Lit #193

merged 8 commits into from Sep 8, 2022

Conversation

leighmcculloch
Copy link
Contributor

@leighmcculloch leighmcculloch commented Jul 24, 2022

What

Generate an error when parsing derived FromMeta implementations that contain a NestedMeta::Lit.

Why

Literal values are unsupported in the code generated by FromMeta. Literal values, like strings, get ignored by the parse implementation that gets generated with the FromMeta generated code. This is confusing to a user since it appears like the value is being accepted and used in some way. We should error when the user provides a literal since the derived code will do nothing with it.

Fix #192

@leighmcculloch
Copy link
Contributor Author

👋🏻

@TedDriggs
Copy link
Owner

Thanks for the reminder! I’m away from a computer for a few days here, but will finish reviewing when I am back. Apologies for the slow turnaround.

Copy link
Owner

@TedDriggs TedDriggs left a comment

Choose a reason for hiding this comment

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

One small point of discussion, otherwise looks great.

core/src/codegen/variant_data.rs Show resolved Hide resolved
@leighmcculloch
Copy link
Contributor Author

I think leaving at as-shown in the PR is reasonable.

👍🏻

Anything I need to do for this PR for it to be mergeable?

@TedDriggs
Copy link
Owner

Nope - problems were all on my side.

@TedDriggs TedDriggs merged commit 161587a into TedDriggs:master Sep 8, 2022
TedDriggs added a commit that referenced this pull request Sep 8, 2022
@leighmcculloch leighmcculloch deleted the issue192 branch September 10, 2022 16:36
@leighmcculloch
Copy link
Contributor Author

Thanks for merging!

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.

FromMeta parsing doesn't error on unrecognized literals in attribute list
2 participants