From 9a002e52e55211f2753ef5e217d1be042dd4fdf9 Mon Sep 17 00:00:00 2001 From: bootandy Date: Wed, 28 Feb 2018 10:24:10 -0500 Subject: [PATCH 1/3] Lint passing Cow by reference Add lint for reference to Cow to the same place in the code where lint for reference to String lives. https://github.com/rust-lang-nursery/rust-clippy/issues/2405 --- clippy_lints/src/ptr.rs | 15 +++++++++++++++ tests/ui/needless_borrow.rs | 11 ++++++++++- tests/ui/needless_borrow.stderr | 12 +++++++++++- 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index a6a3690202fa..44ea35a9fb80 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -213,6 +213,21 @@ fn check_fn(cx: &LateContext, decl: &FnDecl, fn_id: NodeId, opt_body_id: Option< }, ); } + } else if match_type(cx, ty, &paths::COW) { + let as_str = format!("{}", snippet_opt(cx, arg.span).unwrap()); + let mut cc = as_str.chars(); + cc.next(); + let replacement: String = cc.collect(); + + span_lint_and_then( + cx, + PTR_ARG, + arg.span, + "using a reference to `Cow` is not recommended.", + |db| { + db.span_suggestion(arg.span, "change this to", replacement); + }, + ); } } } diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs index 78c1a125c94f..088d33b875f9 100644 --- a/tests/ui/needless_borrow.rs +++ b/tests/ui/needless_borrow.rs @@ -1,4 +1,4 @@ - +use std::borrow::Cow; fn x(y: &i32) -> i32 { @@ -51,3 +51,12 @@ fn issue_1432() { let _ = v.iter().filter(|&a| a.is_empty()); } + +#[allow(dead_code)] +fn test_cow_with_ref(c: &Cow<[i32]>) { +} + +#[allow(dead_code)] +fn test_cow(c: Cow<[i32]>) { + let _c = c; +} diff --git a/tests/ui/needless_borrow.stderr b/tests/ui/needless_borrow.stderr index fde38508b323..6cdcbb275cd2 100644 --- a/tests/ui/needless_borrow.stderr +++ b/tests/ui/needless_borrow.stderr @@ -38,5 +38,15 @@ error: this pattern creates a reference to a reference 50 | let _ = v.iter().filter(|&ref a| a.is_empty()); | ^^^^^ help: change this to: `a` -error: aborting due to 6 previous errors +a> $DIR/needless_borrow.rs:56:25: 56:36 +b> $DIR/needless_borrow.rs:56:25: 56:36 +error: using a reference to `Cow` is not recommended. + --> $DIR/needless_borrow.rs:56:25 + | +56 | fn test_cow_with_ref(c: &Cow<[i32]>) { + | ^^^^^^^^^^^ help: change this to: `Cow<[i32]>` + | + = note: `-D ptr-arg` implied by `-D warnings` + +error: aborting due to 7 previous errors From fc5b377cec9e9444a227063b032d1525d2b40c5e Mon Sep 17 00:00:00 2001 From: Alex Butler Date: Thu, 1 Mar 2018 15:15:41 +0000 Subject: [PATCH 2/3] Fix #2494 add suggestion for unreadable_literal Add `rustc --explain E0308` line to relevant tests --- clippy_lints/src/literal_representation.rs | 22 ++++++++++-------- tests/ui/builtin-type-shadow.stderr | 1 + tests/ui/conf_bad_arg.stderr | 1 + tests/ui/conf_bad_toml.stderr | 1 + tests/ui/conf_bad_type.stderr | 1 + tests/ui/conf_french_blacklisted_name.stderr | 1 + tests/ui/conf_path_non_string.stderr | 1 + tests/ui/conf_unknown_key.stderr | 1 + .../ui/decimal_literal_representation.stderr | 19 ++++----------- tests/ui/inconsistent_digit_grouping.stderr | 19 ++++----------- tests/ui/large_digit_groups.stderr | 23 +++++-------------- tests/ui/unreadable_literal.stderr | 15 ++++-------- 12 files changed, 40 insertions(+), 65 deletions(-) diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs index 9633ac00b154..1b93f6bf3f49 100644 --- a/clippy_lints/src/literal_representation.rs +++ b/clippy_lints/src/literal_representation.rs @@ -4,7 +4,7 @@ use rustc::lint::*; use syntax::ast::*; use syntax_pos; -use utils::{in_external_macro, snippet_opt, span_help_and_lint}; +use utils::{in_external_macro, snippet_opt, span_lint_and_sugg}; /// **What it does:** Warns if a long integral or floating-point constant does /// not contain underscores. @@ -222,33 +222,37 @@ enum WarningType { impl WarningType { pub fn display(&self, grouping_hint: &str, cx: &EarlyContext, span: &syntax_pos::Span) { match *self { - WarningType::UnreadableLiteral => span_help_and_lint( + WarningType::UnreadableLiteral => span_lint_and_sugg( cx, UNREADABLE_LITERAL, *span, "long literal lacking separators", - &format!("consider: {}", grouping_hint), + "consider", + grouping_hint.to_owned(), ), - WarningType::LargeDigitGroups => span_help_and_lint( + WarningType::LargeDigitGroups => span_lint_and_sugg( cx, LARGE_DIGIT_GROUPS, *span, "digit groups should be smaller", - &format!("consider: {}", grouping_hint), + "consider", + grouping_hint.to_owned(), ), - WarningType::InconsistentDigitGrouping => span_help_and_lint( + WarningType::InconsistentDigitGrouping => span_lint_and_sugg( cx, INCONSISTENT_DIGIT_GROUPING, *span, "digits grouped inconsistently by underscores", - &format!("consider: {}", grouping_hint), + "consider", + grouping_hint.to_owned(), ), - WarningType::DecimalRepresentation => span_help_and_lint( + WarningType::DecimalRepresentation => span_lint_and_sugg( cx, DECIMAL_LITERAL_REPRESENTATION, *span, "integer literal has a better hexadecimal representation", - &format!("consider: {}", grouping_hint), + "consider", + grouping_hint.to_owned(), ), }; } diff --git a/tests/ui/builtin-type-shadow.stderr b/tests/ui/builtin-type-shadow.stderr index eb4c73b65c69..85595fb0233a 100644 --- a/tests/ui/builtin-type-shadow.stderr +++ b/tests/ui/builtin-type-shadow.stderr @@ -19,3 +19,4 @@ error[E0308]: mismatched types error: aborting due to 2 previous errors +If you want more information on this error, try using "rustc --explain E0308" diff --git a/tests/ui/conf_bad_arg.stderr b/tests/ui/conf_bad_arg.stderr index bc44cebdbbbf..30a87e232750 100644 --- a/tests/ui/conf_bad_arg.stderr +++ b/tests/ui/conf_bad_arg.stderr @@ -8,3 +8,4 @@ error[E0658]: compiler plugins are experimental and possibly buggy (see issue #2 error: aborting due to previous error +If you want more information on this error, try using "rustc --explain E0658" diff --git a/tests/ui/conf_bad_toml.stderr b/tests/ui/conf_bad_toml.stderr index d42369265229..f01b5605a519 100644 --- a/tests/ui/conf_bad_toml.stderr +++ b/tests/ui/conf_bad_toml.stderr @@ -8,3 +8,4 @@ error[E0658]: compiler plugins are experimental and possibly buggy (see issue #2 error: aborting due to previous error +If you want more information on this error, try using "rustc --explain E0658" diff --git a/tests/ui/conf_bad_type.stderr b/tests/ui/conf_bad_type.stderr index 440437d140e2..ea9cf0acdd8d 100644 --- a/tests/ui/conf_bad_type.stderr +++ b/tests/ui/conf_bad_type.stderr @@ -8,3 +8,4 @@ error[E0658]: compiler plugins are experimental and possibly buggy (see issue #2 error: aborting due to previous error +If you want more information on this error, try using "rustc --explain E0658" diff --git a/tests/ui/conf_french_blacklisted_name.stderr b/tests/ui/conf_french_blacklisted_name.stderr index 19c8e5c97778..d09ae43301c8 100644 --- a/tests/ui/conf_french_blacklisted_name.stderr +++ b/tests/ui/conf_french_blacklisted_name.stderr @@ -8,3 +8,4 @@ error[E0658]: compiler plugins are experimental and possibly buggy (see issue #2 error: aborting due to previous error +If you want more information on this error, try using "rustc --explain E0658" diff --git a/tests/ui/conf_path_non_string.stderr b/tests/ui/conf_path_non_string.stderr index 7a0aebb572eb..6af3b595921f 100644 --- a/tests/ui/conf_path_non_string.stderr +++ b/tests/ui/conf_path_non_string.stderr @@ -8,3 +8,4 @@ error[E0658]: compiler plugins are experimental and possibly buggy (see issue #2 error: aborting due to previous error +If you want more information on this error, try using "rustc --explain E0658" diff --git a/tests/ui/conf_unknown_key.stderr b/tests/ui/conf_unknown_key.stderr index d1957c311ada..80a60bd8f2e6 100644 --- a/tests/ui/conf_unknown_key.stderr +++ b/tests/ui/conf_unknown_key.stderr @@ -8,3 +8,4 @@ error[E0658]: compiler plugins are experimental and possibly buggy (see issue #2 error: aborting due to previous error +If you want more information on this error, try using "rustc --explain E0658" diff --git a/tests/ui/decimal_literal_representation.stderr b/tests/ui/decimal_literal_representation.stderr index e3fbeba81483..baed3c41180c 100644 --- a/tests/ui/decimal_literal_representation.stderr +++ b/tests/ui/decimal_literal_representation.stderr @@ -2,42 +2,33 @@ error: integer literal has a better hexadecimal representation --> $DIR/decimal_literal_representation.rs:18:9 | 18 | 32_773, // 0x8005 - | ^^^^^^ + | ^^^^^^ help: consider: `0x8005` | = note: `-D decimal-literal-representation` implied by `-D warnings` - = help: consider: 0x8005 error: integer literal has a better hexadecimal representation --> $DIR/decimal_literal_representation.rs:19:9 | 19 | 65_280, // 0xFF00 - | ^^^^^^ - | - = help: consider: 0xFF00 + | ^^^^^^ help: consider: `0xFF00` error: integer literal has a better hexadecimal representation --> $DIR/decimal_literal_representation.rs:20:9 | 20 | 2_131_750_927, // 0x7F0F_F00F - | ^^^^^^^^^^^^^ - | - = help: consider: 0x7F0F_F00F + | ^^^^^^^^^^^^^ help: consider: `0x7F0F_F00F` error: integer literal has a better hexadecimal representation --> $DIR/decimal_literal_representation.rs:21:9 | 21 | 2_147_483_647, // 0x7FFF_FFFF - | ^^^^^^^^^^^^^ - | - = help: consider: 0x7FFF_FFFF + | ^^^^^^^^^^^^^ help: consider: `0x7FFF_FFFF` error: integer literal has a better hexadecimal representation --> $DIR/decimal_literal_representation.rs:22:9 | 22 | 4_042_322_160, // 0xF0F0_F0F0 - | ^^^^^^^^^^^^^ - | - = help: consider: 0xF0F0_F0F0 + | ^^^^^^^^^^^^^ help: consider: `0xF0F0_F0F0` error: aborting due to 5 previous errors diff --git a/tests/ui/inconsistent_digit_grouping.stderr b/tests/ui/inconsistent_digit_grouping.stderr index 12d9e3cf0fd7..4d30529d820a 100644 --- a/tests/ui/inconsistent_digit_grouping.stderr +++ b/tests/ui/inconsistent_digit_grouping.stderr @@ -2,42 +2,33 @@ error: digits grouped inconsistently by underscores --> $DIR/inconsistent_digit_grouping.rs:7:16 | 7 | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32); - | ^^^^^^^^ + | ^^^^^^^^ help: consider: `123_456` | = note: `-D inconsistent-digit-grouping` implied by `-D warnings` - = help: consider: 123_456 error: digits grouped inconsistently by underscores --> $DIR/inconsistent_digit_grouping.rs:7:26 | 7 | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32); - | ^^^^^^^^^^ - | - = help: consider: 12_345_678 + | ^^^^^^^^^^ help: consider: `12_345_678` error: digits grouped inconsistently by underscores --> $DIR/inconsistent_digit_grouping.rs:7:38 | 7 | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32); - | ^^^^^^^^ - | - = help: consider: 1_234_567 + | ^^^^^^^^ help: consider: `1_234_567` error: digits grouped inconsistently by underscores --> $DIR/inconsistent_digit_grouping.rs:7:48 | 7 | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32); - | ^^^^^^^^^^^^^^ - | - = help: consider: 1_234.567_8_f32 + | ^^^^^^^^^^^^^^ help: consider: `1_234.567_8_f32` error: digits grouped inconsistently by underscores --> $DIR/inconsistent_digit_grouping.rs:7:64 | 7 | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32); - | ^^^^^^^^^^^^^^ - | - = help: consider: 1.234_567_8_f32 + | ^^^^^^^^^^^^^^ help: consider: `1.234_567_8_f32` error: aborting due to 5 previous errors diff --git a/tests/ui/large_digit_groups.stderr b/tests/ui/large_digit_groups.stderr index 6fc285274a0b..284c5ecf3395 100644 --- a/tests/ui/large_digit_groups.stderr +++ b/tests/ui/large_digit_groups.stderr @@ -2,50 +2,39 @@ error: digit groups should be smaller --> $DIR/large_digit_groups.rs:7:16 | 7 | let bad = (0b1_10110_i64, 0x1_23456_78901_usize, 1_23456_f32, 1_23456.12_f32, 1_23456.12345_f32, 1_23456.12345_6_f32); - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^ help: consider: `0b11_0110_i64` | = note: `-D large-digit-groups` implied by `-D warnings` - = help: consider: 0b11_0110_i64 error: digit groups should be smaller --> $DIR/large_digit_groups.rs:7:31 | 7 | let bad = (0b1_10110_i64, 0x1_23456_78901_usize, 1_23456_f32, 1_23456.12_f32, 1_23456.12345_f32, 1_23456.12345_6_f32); - | ^^^^^^^^^^^^^^^^^^^^^ - | - = help: consider: 0x123_4567_8901_usize + | ^^^^^^^^^^^^^^^^^^^^^ help: consider: `0x123_4567_8901_usize` error: digit groups should be smaller --> $DIR/large_digit_groups.rs:7:54 | 7 | let bad = (0b1_10110_i64, 0x1_23456_78901_usize, 1_23456_f32, 1_23456.12_f32, 1_23456.12345_f32, 1_23456.12345_6_f32); - | ^^^^^^^^^^^ - | - = help: consider: 123_456_f32 + | ^^^^^^^^^^^ help: consider: `123_456_f32` error: digit groups should be smaller --> $DIR/large_digit_groups.rs:7:67 | 7 | let bad = (0b1_10110_i64, 0x1_23456_78901_usize, 1_23456_f32, 1_23456.12_f32, 1_23456.12345_f32, 1_23456.12345_6_f32); - | ^^^^^^^^^^^^^^ - | - = help: consider: 123_456.12_f32 + | ^^^^^^^^^^^^^^ help: consider: `123_456.12_f32` error: digit groups should be smaller --> $DIR/large_digit_groups.rs:7:83 | 7 | let bad = (0b1_10110_i64, 0x1_23456_78901_usize, 1_23456_f32, 1_23456.12_f32, 1_23456.12345_f32, 1_23456.12345_6_f32); - | ^^^^^^^^^^^^^^^^^ - | - = help: consider: 123_456.123_45_f32 + | ^^^^^^^^^^^^^^^^^ help: consider: `123_456.123_45_f32` error: digit groups should be smaller --> $DIR/large_digit_groups.rs:7:102 | 7 | let bad = (0b1_10110_i64, 0x1_23456_78901_usize, 1_23456_f32, 1_23456.12_f32, 1_23456.12345_f32, 1_23456.12345_6_f32); - | ^^^^^^^^^^^^^^^^^^^ - | - = help: consider: 123_456.123_456_f32 + | ^^^^^^^^^^^^^^^^^^^ help: consider: `123_456.123_456_f32` error: aborting due to 6 previous errors diff --git a/tests/ui/unreadable_literal.stderr b/tests/ui/unreadable_literal.stderr index 72cb160fafcb..4fcae9bf725b 100644 --- a/tests/ui/unreadable_literal.stderr +++ b/tests/ui/unreadable_literal.stderr @@ -2,34 +2,27 @@ error: long literal lacking separators --> $DIR/unreadable_literal.rs:7:16 | 7 | let bad = (0b10110_i64, 0x12345678901_usize, 12345_f32, 1.23456_f32); - | ^^^^^^^^^^^ + | ^^^^^^^^^^^ help: consider: `0b1_0110_i64` | = note: `-D unreadable-literal` implied by `-D warnings` - = help: consider: 0b1_0110_i64 error: long literal lacking separators --> $DIR/unreadable_literal.rs:7:29 | 7 | let bad = (0b10110_i64, 0x12345678901_usize, 12345_f32, 1.23456_f32); - | ^^^^^^^^^^^^^^^^^^^ - | - = help: consider: 0x123_4567_8901_usize + | ^^^^^^^^^^^^^^^^^^^ help: consider: `0x123_4567_8901_usize` error: long literal lacking separators --> $DIR/unreadable_literal.rs:7:50 | 7 | let bad = (0b10110_i64, 0x12345678901_usize, 12345_f32, 1.23456_f32); - | ^^^^^^^^^ - | - = help: consider: 12_345_f32 + | ^^^^^^^^^ help: consider: `12_345_f32` error: long literal lacking separators --> $DIR/unreadable_literal.rs:7:61 | 7 | let bad = (0b10110_i64, 0x12345678901_usize, 12345_f32, 1.23456_f32); - | ^^^^^^^^^^^ - | - = help: consider: 1.234_56_f32 + | ^^^^^^^^^^^ help: consider: `1.234_56_f32` error: aborting due to 4 previous errors From e3c13da830cf4a6a1b6df8ec0683e3e7b44aeb41 Mon Sep 17 00:00:00 2001 From: bootandy Date: Fri, 2 Mar 2018 19:13:54 -0500 Subject: [PATCH 3/3] Change recomendation to: &[type] from Cow --- clippy_lints/src/ptr.rs | 41 ++++++++++++++++++++++----------- tests/ui/needless_borrow.stderr | 4 +--- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index 44ea35a9fb80..139b5883fb0d 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -3,6 +3,7 @@ use std::borrow::Cow; use rustc::hir::*; use rustc::hir::map::NodeItem; +use rustc::hir::QPath; use rustc::lint::*; use rustc::ty; use syntax::ast::NodeId; @@ -214,20 +215,32 @@ fn check_fn(cx: &LateContext, decl: &FnDecl, fn_id: NodeId, opt_body_id: Option< ); } } else if match_type(cx, ty, &paths::COW) { - let as_str = format!("{}", snippet_opt(cx, arg.span).unwrap()); - let mut cc = as_str.chars(); - cc.next(); - let replacement: String = cc.collect(); - - span_lint_and_then( - cx, - PTR_ARG, - arg.span, - "using a reference to `Cow` is not recommended.", - |db| { - db.span_suggestion(arg.span, "change this to", replacement); - }, - ); + if_chain! { + if let TyRptr(_, MutTy { ref ty, ..} ) = arg.node; + if let TyPath(ref path) = ty.node; + if let QPath::Resolved(None, ref pp) = *path; + if let [ref bx] = *pp.segments; + if let Some(ref params) = bx.parameters; + if !params.parenthesized; + if let [ref inner] = *params.types; + then { + let replacement = snippet_opt(cx, inner.span); + match replacement { + Some(r) => { + span_lint_and_then( + cx, + PTR_ARG, + arg.span, + "using a reference to `Cow` is not recommended.", + |db| { + db.span_suggestion(arg.span, "change this to", "&".to_owned() + &r); + }, + ); + }, + None => (), + } + } + } } } } diff --git a/tests/ui/needless_borrow.stderr b/tests/ui/needless_borrow.stderr index 6cdcbb275cd2..e319efa939cc 100644 --- a/tests/ui/needless_borrow.stderr +++ b/tests/ui/needless_borrow.stderr @@ -38,13 +38,11 @@ error: this pattern creates a reference to a reference 50 | let _ = v.iter().filter(|&ref a| a.is_empty()); | ^^^^^ help: change this to: `a` -a> $DIR/needless_borrow.rs:56:25: 56:36 -b> $DIR/needless_borrow.rs:56:25: 56:36 error: using a reference to `Cow` is not recommended. --> $DIR/needless_borrow.rs:56:25 | 56 | fn test_cow_with_ref(c: &Cow<[i32]>) { - | ^^^^^^^^^^^ help: change this to: `Cow<[i32]>` + | ^^^^^^^^^^^ help: change this to: `&[i32]` | = note: `-D ptr-arg` implied by `-D warnings`