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

unnecessary parentheses around type in attribute macro #106939

Open
lesurp opened this issue Jan 16, 2023 · 5 comments
Open

unnecessary parentheses around type in attribute macro #106939

lesurp opened this issue Jan 16, 2023 · 5 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-proc-macros Area: Procedural macros D-papercut Diagnostics: An error or lint that needs small tweaks. L-unused_parens Lint: unused_parens T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@lesurp
Copy link

lesurp commented Jan 16, 2023

Code

Since this is related to proc_macro reproducing is slightly troublesome. The original problem originated here: https://github.com/lesurp/OptionalStruct/blob/master/tests/nested.rs

Simplified call site:

struct Foo;
struct Baz;

#[optional_struct]
struct Bar {
    #[optional_rename(Foo)]
    log_config: Baz,
}

Note that the macro is not a derive macro (and the attribute optional_rename not a derive macro helper attributes).

Current output

warning: unnecessary parentheses around type
 --> tests/reverse_wrapping.rs:7:22
  |
7 |     #[optional_rename(Foo)]
  |                      ^                 ^
  |
  = note: `#[warn(unused_parens)]` on by default
help: remove these parentheses
  |
7 -     #[optional_rename(Foo)]
7 +     #[optional_renameFoo]
  |

Desired output

No warning.

Rationale and extra context

The parentheses are read as part of the type, but they are part of the attribute definition.

Other cases

No response

Anything else?

No response

@lesurp lesurp added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 16, 2023
@clubby789
Copy link
Contributor

The problem is that the tokens include the parentheses. You can use parse_args to parse the argument as a proc_macro2::Ident which does not include the parentheses.

@lesurp
Copy link
Author

lesurp commented Jan 16, 2023

Hi, thank you very much this indeed solved the problem. I did not even realize the warning came from the parentheses actually being unused, I thought that warning was emitted even before my macro was expanded.

I'm wondering if there would be any way to improve the diagnostic here?

lesurp added a commit to lesurp/OptionalStruct that referenced this issue Jan 16, 2023
lesurp added a commit to lesurp/OptionalStruct that referenced this issue Jan 17, 2023
@estebank estebank added the A-proc-macros Area: Procedural macros label Jan 17, 2023
@estebank
Copy link
Contributor

Sadly we somewhat rely on proc macros being well-formed, but we should make more of an effort helping proc-macro authors debug their own code.

@Antikyth
Copy link

Sadly we somewhat rely on proc macros being well-formed, but we should make more of an effort helping proc-macro authors debug their own code.

I've encountered similar when writing my (perhaps overly complex and overly ambition) proc macro - at a certain level of complexity, a lot of these errors for me originate either from:
(a) confusion as to what spans should be applied to what areas of the expanded code; and
(b) a general inability to annotate certain areas of expanded code as ignoring particular lints, while still allowing others to be linted as norm.

By (b), I mean that there are certain things, like applying a 'template' of expanded code to the user-provided syntax, that are likely to violate a number of lints (e.g. like in this issue, or more commonly things like unnecessary borrows which are sometimes necessary, depending on the types used in the syntax by the user), where you want to allow those lints on that expanded code, but then within those there might be syntax which you do want to be linted, because it was defined by the user (or mostly defined).

I realize this is not the place to discuss these errors, it's just what immediately came to mind when seeing the issue. I'm not sure where the correct place to discuss them would be if I were to do that.

@estebank
Copy link
Contributor

I'm not sure where the correct place to discuss them would be if I were to do that.

https://internals.rust-lang.org/ and https://rust-lang.zulipchat.com/#narrow/stream/147480-t-compiler.2Fwg-diagnostics

@estebank estebank added the D-papercut Diagnostics: An error or lint that needs small tweaks. label Mar 15, 2023
@jieyouxu jieyouxu added the L-unused_parens Lint: unused_parens label May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-proc-macros Area: Procedural macros D-papercut Diagnostics: An error or lint that needs small tweaks. L-unused_parens Lint: unused_parens T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants