From 7b59557dcda15a352f9515faac32d9acb85f59bc Mon Sep 17 00:00:00 2001 From: Hidehito Yabuuchi Date: Mon, 5 Mar 2018 14:31:37 +0900 Subject: [PATCH 1/3] Don't lint range syntax with var name `start` and `end` --- clippy_lints/src/redundant_field_names.rs | 34 +++++++++++++++++++++-- tests/ui/redundant_field_names.rs | 18 ++++++++++++ tests/ui/redundant_field_names.stderr | 8 +++--- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/redundant_field_names.rs b/clippy_lints/src/redundant_field_names.rs index e4d113bd3dea..587454f64ebe 100644 --- a/clippy_lints/src/redundant_field_names.rs +++ b/clippy_lints/src/redundant_field_names.rs @@ -1,6 +1,8 @@ +use syntax::ast::Name; use rustc::lint::*; use rustc::hir::*; -use utils::{span_lint_and_sugg, match_var}; +use utils::{match_qpath, match_var, span_lint_and_sugg}; +use utils::paths; /// **What it does:** Checks for fields in struct literals where shorthands /// could be used. @@ -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 { for field in fields { let name = field.name.node; + if is_range_struct_field(path, &name) { + continue; + } + if match_var(&field.expr, name) && !field.is_shorthand { span_lint_and_sugg ( cx, @@ -54,3 +60,27 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantFieldNames { } } } + +/// ```rust +/// let start = 0; +/// let _ = start..; +/// +/// let end = 0; +/// let _ = ..end; +/// +/// let _ = start..end; +/// ``` +fn is_range_struct_field(path: &QPath, name: &Name) -> bool { + match name.as_str().as_ref() { + "start" => { + match_qpath(path, &paths::RANGE_STD) || match_qpath(path, &paths::RANGE_FROM_STD) + || match_qpath(path, &paths::RANGE_INCLUSIVE_STD) + }, + "end" => { + match_qpath(path, &paths::RANGE_STD) || match_qpath(path, &paths::RANGE_TO_STD) + || match_qpath(path, &paths::RANGE_INCLUSIVE_STD) + || match_qpath(path, &paths::RANGE_TO_INCLUSIVE_STD) + }, + _ => false, + } +} diff --git a/tests/ui/redundant_field_names.rs b/tests/ui/redundant_field_names.rs index 0eb9bef45b56..4ffd0e4cc629 100644 --- a/tests/ui/redundant_field_names.rs +++ b/tests/ui/redundant_field_names.rs @@ -1,5 +1,6 @@ #![warn(redundant_field_names)] #![allow(unused_variables)] +#![feature(inclusive_range,inclusive_range_syntax)] mod foo { pub const BAR: u8 = 0; @@ -27,4 +28,21 @@ fn main() { buzz: fizz, //should be ok foo: foo::BAR, //should be ok }; + + // Range syntax + let (start, end) = (0, 0); + + let _ = start..; + let _ = ..end; + let _ = start..end; + + let _ = ..=end; + let _ = start..=end; + + // TODO: the followings shoule be linted + let _ = ::std::ops::RangeFrom { start: start }; + let _ = ::std::ops::RangeTo { end: end }; + let _ = ::std::ops::Range { start: start, end: end }; + let _ = ::std::ops::RangeInclusive { start: start, end: end }; + let _ = ::std::ops::RangeToInclusive { end: end }; } diff --git a/tests/ui/redundant_field_names.stderr b/tests/ui/redundant_field_names.stderr index d6d752b93a35..443f30a9f508 100644 --- a/tests/ui/redundant_field_names.stderr +++ b/tests/ui/redundant_field_names.stderr @@ -1,15 +1,15 @@ error: redundant field names in struct initialization - --> $DIR/redundant_field_names.rs:23:9 + --> $DIR/redundant_field_names.rs:24:9 | -23 | gender: gender, +24 | 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:25:9 | -24 | age: age, +25 | age: age, | ^^^^^^^^ help: replace it with: `age` error: aborting due to 2 previous errors From 5b48b03375e11421490d23a7ea36ef05181e3842 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 5 Mar 2018 08:33:37 +0100 Subject: [PATCH 2/3] Typo --- tests/ui/redundant_field_names.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ui/redundant_field_names.rs b/tests/ui/redundant_field_names.rs index 4ffd0e4cc629..8a17ab0c9b67 100644 --- a/tests/ui/redundant_field_names.rs +++ b/tests/ui/redundant_field_names.rs @@ -39,7 +39,7 @@ fn main() { let _ = ..=end; let _ = start..=end; - // TODO: the followings shoule be linted + // TODO: the following should be linted let _ = ::std::ops::RangeFrom { start: start }; let _ = ::std::ops::RangeTo { end: end }; let _ = ::std::ops::Range { start: start, end: end }; From cdb60c6547fd83f5c11019dbc88346694f1bee17 Mon Sep 17 00:00:00 2001 From: Hidehito Yabuuchi Date: Mon, 5 Mar 2018 17:30:07 +0900 Subject: [PATCH 3/3] Make `redundant_field_name` not care range expressions Hand-written `Range` struct family are treated normally. --- clippy_lints/src/redundant_field_names.rs | 33 +++----------- clippy_lints/src/utils/mod.rs | 10 +++++ tests/ui/redundant_field_names.rs | 18 ++++---- tests/ui/redundant_field_names.stderr | 52 ++++++++++++++++++++--- 4 files changed, 72 insertions(+), 41 deletions(-) diff --git a/clippy_lints/src/redundant_field_names.rs b/clippy_lints/src/redundant_field_names.rs index 587454f64ebe..885e1aa9f8d1 100644 --- a/clippy_lints/src/redundant_field_names.rs +++ b/clippy_lints/src/redundant_field_names.rs @@ -1,8 +1,6 @@ -use syntax::ast::Name; use rustc::lint::*; use rustc::hir::*; -use utils::{match_qpath, match_var, span_lint_and_sugg}; -use utils::paths; +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. @@ -42,7 +40,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantFieldNames { for field in fields { let name = field.name.node; - if is_range_struct_field(path, &name) { + // 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; } @@ -60,27 +61,3 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantFieldNames { } } } - -/// ```rust -/// let start = 0; -/// let _ = start..; -/// -/// let end = 0; -/// let _ = ..end; -/// -/// let _ = start..end; -/// ``` -fn is_range_struct_field(path: &QPath, name: &Name) -> bool { - match name.as_str().as_ref() { - "start" => { - match_qpath(path, &paths::RANGE_STD) || match_qpath(path, &paths::RANGE_FROM_STD) - || match_qpath(path, &paths::RANGE_INCLUSIVE_STD) - }, - "end" => { - match_qpath(path, &paths::RANGE_STD) || match_qpath(path, &paths::RANGE_TO_STD) - || match_qpath(path, &paths::RANGE_INCLUSIVE_STD) - || match_qpath(path, &paths::RANGE_TO_INCLUSIVE_STD) - }, - _ => false, - } -} diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index c501dadeb793..2f2f0c04054f 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -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. diff --git a/tests/ui/redundant_field_names.rs b/tests/ui/redundant_field_names.rs index 8a17ab0c9b67..cb49283010ba 100644 --- a/tests/ui/redundant_field_names.rs +++ b/tests/ui/redundant_field_names.rs @@ -1,6 +1,8 @@ #![warn(redundant_field_names)] #![allow(unused_variables)] -#![feature(inclusive_range,inclusive_range_syntax)] +#![feature(inclusive_range, inclusive_range_syntax)] + +use std::ops::{Range, RangeFrom, RangeTo, RangeInclusive, RangeToInclusive}; mod foo { pub const BAR: u8 = 0; @@ -29,7 +31,7 @@ fn main() { foo: foo::BAR, //should be ok }; - // Range syntax + // Range expressions let (start, end) = (0, 0); let _ = start..; @@ -39,10 +41,10 @@ fn main() { let _ = ..=end; let _ = start..=end; - // TODO: the following should be linted - let _ = ::std::ops::RangeFrom { start: start }; - let _ = ::std::ops::RangeTo { end: end }; - let _ = ::std::ops::Range { start: start, end: end }; - let _ = ::std::ops::RangeInclusive { start: start, end: end }; - let _ = ::std::ops::RangeToInclusive { end: 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 }; } diff --git a/tests/ui/redundant_field_names.stderr b/tests/ui/redundant_field_names.stderr index 443f30a9f508..40315c6ffac2 100644 --- a/tests/ui/redundant_field_names.stderr +++ b/tests/ui/redundant_field_names.stderr @@ -1,16 +1,58 @@ error: redundant field names in struct initialization - --> $DIR/redundant_field_names.rs:24:9 + --> $DIR/redundant_field_names.rs:26:9 | -24 | 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:25:9 + --> $DIR/redundant_field_names.rs:27:9 | -25 | 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