Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Don't lint range syntax with var name start and/or end #2507

Merged
merged 3 commits into from
Mar 5, 2018

Conversation

ordovicia
Copy link
Contributor

Fixes #2491.

But this PR introduces new false-negative:
does not lint hand-written Range struct family with var name start and/or end,
i.e. std::ops::Range { start: start, end: end }
But I guess it's rare to write them explicitly.

Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

But I guess it's rare to write them explicitly.

I agree, still if we can detect this in a more general way it would be better

@@ -36,10 +38,14 @@ impl LintPass for RedundantFieldNames {

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantFieldNames {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if let ExprStruct(_, ref fields, _) = expr.node {
if let ExprStruct(ref path, ref fields, _) = expr.node {
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tried checking with the in_macro method? I think this should have expansion info, so we should be able to detect it that way

Copy link
Contributor Author

@ordovicia ordovicia Mar 5, 2018

Choose a reason for hiding this comment

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

I didn't know in_macro() until now.
But the method intends not to treat range expressions as "in_macro" (since #1373).

https://github.com/rust-lang-nursery/rust-clippy/blob/598acba7d5561b2096dfa72a39247983852b9bea/clippy_lints/src/utils/mod.rs#L56-L65

So I've added a method which determines whether an expression is a range expression.
https://github.com/rust-lang-nursery/rust-clippy/pull/2507/files#diff-5b47b8bbc8833f6268e936ea8682644dR68

Now range expressions are ignored, and hand-written Range family structs are treated normally by the lint.

Copy link
Contributor

Choose a reason for hiding this comment

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

nice! The diff lgtm. Let's wait for travis and then merge

Hand-written `Range` struct family are treated normally.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants