Skip to content

used_underscore can't handle macros and derive properly at the same time #507

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

Closed
Manishearth opened this issue Dec 19, 2015 · 4 comments
Closed

Comments

@Manishearth
Copy link
Member

I removed the test in Manishearth@4a32445 because we need a stronger macro check to handle stuff like https://github.com/Manishearth/rust-clippy/issues/488#issuecomment-166032639

Might need to write a better macro check. Looks like hygiene works differently for MacroAttributes. Perhaps we should just check for those?

cc @devonhollowood

@devonhollowood
Copy link
Contributor

I can take a look at this tonight. While we are improving the macro checking, we might want to lint in cases like this:

macro_rules! increment {
    ($_x:expr) => {$_x + 1}
}

(currently we don't lint that)

@Manishearth
Copy link
Member Author

We don't want to lint in that case. The exact example is obvious, but it's possible that with a more complex macro that the unused variable was due to the exact set of arguments passed which may have deliberately made it unused.

devonhollowood added a commit to devonhollowood/rust-clippy that referenced this issue Dec 21, 2015
Make `used_underscore_binding` lint compatible with MacroAttributes
expansions. TODO: Add a good test for this.
@devonhollowood
Copy link
Contributor

I wrote something that I think would work, but I'm not sure how best to test this. I don't want to test using #[derive(RustcEncodable)] because it doesn't seem worth adding the rustc-serialize dependency for this. Just doing #[derive(Clone)] doesn't set off the lint, even if I remove the new check.

@Manishearth
Copy link
Member Author

(Ah, sorry, didn't see this message)

Eh, rustc-serialize is stable (since the derive functionality is part of the compiler in this case), so I'm okay with a dev-dependency on it. We already do this for regex for a different macro test.

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

No branches or pull requests

2 participants