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

FromMeta parsing doesn't error on unrecognized literals in attribute list #192

Closed
leighmcculloch opened this issue Jul 20, 2022 · 5 comments · Fixed by #193
Closed

FromMeta parsing doesn't error on unrecognized literals in attribute list #192

leighmcculloch opened this issue Jul 20, 2022 · 5 comments · Fixed by #193

Comments

@leighmcculloch
Copy link
Contributor

leighmcculloch commented Jul 20, 2022

FromMeta doesn't error in all cases that unexpected tokens end up in the attribute list.

It doesn't error in the case that a string literal is in the attribute list. For example:

#[myattribute(myfield = "export", "randomliteral")]

Parsing the attribute list will error if it sees a field that is anticipated, but not for "randomliteral" which it will ignore.

cc @jonjove

@leighmcculloch
Copy link
Contributor Author

leighmcculloch commented Jul 20, 2022

I expect this is being caused because the NestedMeta::Lit case is not being treated as an error when it is encountered in the AttributeArgs.

@leighmcculloch

This comment was marked as outdated.

@leighmcculloch
Copy link
Contributor Author

leighmcculloch commented Jul 20, 2022

Maybe this block of generated code in the macro is the issue, since it ignores metas that are of the literal type:

for __item in __items {
if let ::syn::NestedMeta::Meta(ref __inner) = *__item {
let __name = ::darling::util::path_to_string(__inner.path());
match __name.as_str() {
#(#arms)*
__other => { #handle_unknown }
}
}
}

I suspect the fix might be the following, however it is unclear to me what the ramifications if this code path has broader use:

  for __item in __items { 
      if let ::syn::NestedMeta::Meta(ref __inner) = *__item { 
          let __name = ::darling::util::path_to_string(__inner.path()); 
          match __name.as_str() { 
              #(#arms)* 
              __other => { #handle_unknown } 
          } 
+     } else {
+         // TODO: Error?
      }
  } 

@leighmcculloch
Copy link
Contributor Author

@TedDriggs I've opened a fix for this issue in #193.

@casey
Copy link

casey commented Sep 5, 2022

Just wanted to mention that I ran into this. I was trying to parse something like this:

#[derive(MyTrait)]
#[my_trait("foo")]
struct Foo {
}
``

Which was not working, but which wasn't throwing an error, I think because the literal "foo" was being ignored.

TedDriggs pushed a commit that referenced this issue Sep 8, 2022
darling silently ignored literals in NestedMeta, which could indirectly cause confusing error messages if the user provided something that wasn't of the intended format.

This update makes that error more explicit, and includes tests to showcase the behavior.

Fixes #192
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 a pull request may close this issue.

2 participants