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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions clippy_lints/src/redundant_field_names.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use rustc::lint::*;
use rustc::hir::*;
use utils::{span_lint_and_sugg, match_var};
use utils::{is_range_expression, match_var, span_lint_and_sugg};

/// **What it does:** Checks for fields in struct literals where shorthands
/// could be used.
Expand Down Expand Up @@ -36,10 +36,17 @@ 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

for field in fields {
let name = field.name.node;

// Do not care about range expressions.
// They could have redundant field name when desugared to structs.
// e.g. `start..end` is desugared to `Range { start: start, end: end }`
if is_range_expression(expr.span) {
continue;
}

if match_var(&field.expr, name) && !field.is_shorthand {
span_lint_and_sugg (
cx,
Expand Down
10 changes: 10 additions & 0 deletions clippy_lints/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,16 @@ pub fn in_macro(span: Span) -> bool {
})
}

/// Returns true if `expn_info` was expanded by range expressions.
pub fn is_range_expression(span: Span) -> bool {
span.ctxt().outer().expn_info().map_or(false, |info| {
match info.callee.format {
ExpnFormat::CompilerDesugaring(CompilerDesugaringKind::DotFill) => true,
_ => false,
}
})
}

/// Returns true if the macro that expanded the crate was outside of the
/// current crate or was a
/// compiler plugin.
Expand Down
20 changes: 20 additions & 0 deletions tests/ui/redundant_field_names.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#![warn(redundant_field_names)]
#![allow(unused_variables)]
#![feature(inclusive_range, inclusive_range_syntax)]

use std::ops::{Range, RangeFrom, RangeTo, RangeInclusive, RangeToInclusive};

mod foo {
pub const BAR: u8 = 0;
Expand Down Expand Up @@ -27,4 +30,21 @@ fn main() {
buzz: fizz, //should be ok
foo: foo::BAR, //should be ok
};

// Range expressions
let (start, end) = (0, 0);

let _ = start..;
let _ = ..end;
let _ = start..end;

let _ = ..=end;
let _ = start..=end;

// hand-written Range family structs are linted
let _ = RangeFrom { start: start };
let _ = RangeTo { end: end };
let _ = Range { start: start, end: end };
let _ = RangeInclusive { start: start, end: end };
let _ = RangeToInclusive { end: end };
}
52 changes: 47 additions & 5 deletions tests/ui/redundant_field_names.stderr
Original file line number Diff line number Diff line change
@@ -1,16 +1,58 @@
error: redundant field names in struct initialization
--> $DIR/redundant_field_names.rs:23:9
--> $DIR/redundant_field_names.rs:26:9
|
23 | gender: gender,
26 | gender: gender,
| ^^^^^^^^^^^^^^ help: replace it with: `gender`
|
= note: `-D redundant-field-names` implied by `-D warnings`

error: redundant field names in struct initialization
--> $DIR/redundant_field_names.rs:24:9
--> $DIR/redundant_field_names.rs:27:9
|
24 | age: age,
27 | age: age,
| ^^^^^^^^ help: replace it with: `age`

error: aborting due to 2 previous errors
error: redundant field names in struct initialization
--> $DIR/redundant_field_names.rs:45:25
|
45 | let _ = RangeFrom { start: start };
| ^^^^^^^^^^^^ help: replace it with: `start`

error: redundant field names in struct initialization
--> $DIR/redundant_field_names.rs:46:23
|
46 | let _ = RangeTo { end: end };
| ^^^^^^^^ help: replace it with: `end`

error: redundant field names in struct initialization
--> $DIR/redundant_field_names.rs:47:21
|
47 | let _ = Range { start: start, end: end };
| ^^^^^^^^^^^^ help: replace it with: `start`

error: redundant field names in struct initialization
--> $DIR/redundant_field_names.rs:47:35
|
47 | let _ = Range { start: start, end: end };
| ^^^^^^^^ help: replace it with: `end`

error: redundant field names in struct initialization
--> $DIR/redundant_field_names.rs:48:30
|
48 | let _ = RangeInclusive { start: start, end: end };
| ^^^^^^^^^^^^ help: replace it with: `start`

error: redundant field names in struct initialization
--> $DIR/redundant_field_names.rs:48:44
|
48 | let _ = RangeInclusive { start: start, end: end };
| ^^^^^^^^ help: replace it with: `end`

error: redundant field names in struct initialization
--> $DIR/redundant_field_names.rs:49:32
|
49 | let _ = RangeToInclusive { end: end };
| ^^^^^^^^ help: replace it with: `end`

error: aborting due to 9 previous errors