From 380f7218b3876225e726617531d9f02d42da0e38 Mon Sep 17 00:00:00 2001 From: ThibsG Date: Sat, 29 Feb 2020 18:41:18 +0100 Subject: [PATCH 01/13] Add lint on large const arrays --- CHANGELOG.md | 1 + clippy_lints/src/large_const_arrays.rs | 85 ++++++++++++++++++++++++++ clippy_lints/src/lib.rs | 4 ++ src/lintlist/mod.rs | 7 +++ tests/ui/large_const_arrays.fixed | 37 +++++++++++ tests/ui/large_const_arrays.rs | 37 +++++++++++ tests/ui/large_const_arrays.stderr | 76 +++++++++++++++++++++++ 7 files changed, 247 insertions(+) create mode 100644 clippy_lints/src/large_const_arrays.rs create mode 100644 tests/ui/large_const_arrays.fixed create mode 100644 tests/ui/large_const_arrays.rs create mode 100644 tests/ui/large_const_arrays.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index b7ac3cace2048..cbc7d98c8782f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1319,6 +1319,7 @@ Released 2018-09-13 [`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next [`iterator_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iterator_step_by_zero [`just_underscores_and_digits`]: https://rust-lang.github.io/rust-clippy/master/index.html#just_underscores_and_digits +[`large_const_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_const_arrays [`large_digit_groups`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_digit_groups [`large_enum_variant`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant [`large_stack_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_arrays diff --git a/clippy_lints/src/large_const_arrays.rs b/clippy_lints/src/large_const_arrays.rs new file mode 100644 index 0000000000000..2f62e6ad9dc5c --- /dev/null +++ b/clippy_lints/src/large_const_arrays.rs @@ -0,0 +1,85 @@ +use crate::rustc_target::abi::LayoutOf; +use crate::utils::span_lint_and_then; +use if_chain::if_chain; +use rustc::mir::interpret::ConstValue; +use rustc::ty::{self, ConstKind}; +use rustc_errors::Applicability; +use rustc_hir::{Item, ItemKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::{BytePos, Pos, Span}; +use rustc_typeck::hir_ty_to_ty; + +declare_clippy_lint! { + /// **What it does:** Checks for large `const` arrays that should + /// be defined as `static` instead. + /// + /// **Why is this bad?** Performance: const variables are inlined upon use. + /// Static items result in only one instance and has a fixed location in memory. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust,ignore + /// // Bad + /// pub const a = [0u32; 1_000_000]; + /// + /// // Good + /// pub static a = [0u32; 1_000_000]; + /// ``` + pub LARGE_CONST_ARRAYS, + perf, + "large non-scalar const array may cause performance overhead" +} + +pub struct LargeConstArrays { + maximum_allowed_size: u64, +} + +impl LargeConstArrays { + #[must_use] + pub fn new(maximum_allowed_size: u64) -> Self { + Self { maximum_allowed_size } + } +} + +impl_lint_pass!(LargeConstArrays => [LARGE_CONST_ARRAYS]); + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeConstArrays { + fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) { + if_chain! { + if !item.span.from_expansion(); + if let ItemKind::Const(hir_ty, _) = &item.kind; + let ty = hir_ty_to_ty(cx.tcx, hir_ty); + if let ty::Array(element_type, cst) = ty.kind; + if let ConstKind::Value(val) = cst.val; + if let ConstValue::Scalar(element_count) = val; + if let Ok(element_count) = element_count.to_machine_usize(&cx.tcx); + if let Ok(element_size) = cx.layout_of(element_type).map(|l| l.size.bytes()); + if self.maximum_allowed_size < element_count * element_size; + + then { + let hi_pos = item.ident.span.lo() - BytePos::from_usize(1); + let sugg_span = Span::new( + hi_pos - BytePos::from_usize("const".len()), + hi_pos, + item.span.ctxt(), + ); + span_lint_and_then( + cx, + LARGE_CONST_ARRAYS, + item.span, + "large array defined as const", + |db| { + db.span_suggestion( + sugg_span, + "make this a static item", + "static".to_string(), + Applicability::MachineApplicable, + ); + } + ); + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index b106113c2a983..909115f7a3a94 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -580,6 +580,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &int_plus_one::INT_PLUS_ONE, &integer_division::INTEGER_DIVISION, &items_after_statements::ITEMS_AFTER_STATEMENTS, + &large_const_arrays::LARGE_CONST_ARRAYS, &large_enum_variant::LARGE_ENUM_VARIANT, &large_stack_arrays::LARGE_STACK_ARRAYS, &len_zero::LEN_WITHOUT_IS_EMPTY, @@ -1024,6 +1025,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box to_digit_is_some::ToDigitIsSome); let array_size_threshold = conf.array_size_threshold; store.register_late_pass(move || box large_stack_arrays::LargeStackArrays::new(array_size_threshold)); + store.register_late_pass(move || box large_const_arrays::LargeConstArrays::new(array_size_threshold)); store.register_late_pass(move || box floating_point_arithmetic::FloatingPointArithmetic); store.register_early_pass(|| box as_conversions::AsConversions); store.register_early_pass(|| box utils::internal_lints::ProduceIce); @@ -1222,6 +1224,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY), LintId::of(&inline_fn_without_body::INLINE_FN_WITHOUT_BODY), LintId::of(&int_plus_one::INT_PLUS_ONE), + LintId::of(&large_const_arrays::LARGE_CONST_ARRAYS), LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT), LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY), LintId::of(&len_zero::LEN_ZERO), @@ -1651,6 +1654,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&bytecount::NAIVE_BYTECOUNT), LintId::of(&entry::MAP_ENTRY), LintId::of(&escape::BOXED_LOCAL), + LintId::of(&large_const_arrays::LARGE_CONST_ARRAYS), LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT), LintId::of(&loops::MANUAL_MEMCPY), LintId::of(&loops::NEEDLESS_COLLECT), diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index b3c77f3f48146..771b6d49634c8 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -941,6 +941,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "non_expressive_names", }, + Lint { + name: "large_const_arrays", + group: "perf", + desc: "large non-scalar const array may cause performance overhead", + deprecation: None, + module: "large_const_arrays", + }, Lint { name: "large_digit_groups", group: "pedantic", diff --git a/tests/ui/large_const_arrays.fixed b/tests/ui/large_const_arrays.fixed new file mode 100644 index 0000000000000..c5af07c8a1728 --- /dev/null +++ b/tests/ui/large_const_arrays.fixed @@ -0,0 +1,37 @@ +// run-rustfix + +#![warn(clippy::large_const_arrays)] +#![allow(dead_code)] + +#[derive(Clone, Copy)] +pub struct S { + pub data: [u64; 32], +} + +// Should lint +pub(crate) static FOO_PUB_CRATE: [u32; 1_000_000] = [0u32; 1_000_000]; +pub static FOO_PUB: [u32; 1_000_000] = [0u32; 1_000_000]; +static FOO: [u32; 1_000_000] = [0u32; 1_000_000]; + +// Good +pub(crate) const G_FOO_PUB_CRATE: [u32; 1_000] = [0u32; 1_000]; +pub const G_FOO_PUB: [u32; 1_000] = [0u32; 1_000]; +const G_FOO: [u32; 1_000] = [0u32; 1_000]; + +fn main() { + // Should lint + pub static BAR_PUB: [u32; 1_000_000] = [0u32; 1_000_000]; + static BAR: [u32; 1_000_000] = [0u32; 1_000_000]; + pub static BAR_STRUCT_PUB: [S; 5_000] = [S { data: [0; 32] }; 5_000]; + static BAR_STRUCT: [S; 5_000] = [S { data: [0; 32] }; 5_000]; + pub static BAR_S_PUB: [Option<&str>; 200_000] = [Some("str"); 200_000]; + static BAR_S: [Option<&str>; 200_000] = [Some("str"); 200_000]; + + // Good + pub const G_BAR_PUB: [u32; 1_000] = [0u32; 1_000]; + const G_BAR: [u32; 1_000] = [0u32; 1_000]; + pub const G_BAR_STRUCT_PUB: [S; 500] = [S { data: [0; 32] }; 500]; + const G_BAR_STRUCT: [S; 500] = [S { data: [0; 32] }; 500]; + pub const G_BAR_S_PUB: [Option<&str>; 200] = [Some("str"); 200]; + const G_BAR_S: [Option<&str>; 200] = [Some("str"); 200]; +} diff --git a/tests/ui/large_const_arrays.rs b/tests/ui/large_const_arrays.rs new file mode 100644 index 0000000000000..a160b9f8ad5b0 --- /dev/null +++ b/tests/ui/large_const_arrays.rs @@ -0,0 +1,37 @@ +// run-rustfix + +#![warn(clippy::large_const_arrays)] +#![allow(dead_code)] + +#[derive(Clone, Copy)] +pub struct S { + pub data: [u64; 32], +} + +// Should lint +pub(crate) const FOO_PUB_CRATE: [u32; 1_000_000] = [0u32; 1_000_000]; +pub const FOO_PUB: [u32; 1_000_000] = [0u32; 1_000_000]; +const FOO: [u32; 1_000_000] = [0u32; 1_000_000]; + +// Good +pub(crate) const G_FOO_PUB_CRATE: [u32; 1_000] = [0u32; 1_000]; +pub const G_FOO_PUB: [u32; 1_000] = [0u32; 1_000]; +const G_FOO: [u32; 1_000] = [0u32; 1_000]; + +fn main() { + // Should lint + pub const BAR_PUB: [u32; 1_000_000] = [0u32; 1_000_000]; + const BAR: [u32; 1_000_000] = [0u32; 1_000_000]; + pub const BAR_STRUCT_PUB: [S; 5_000] = [S { data: [0; 32] }; 5_000]; + const BAR_STRUCT: [S; 5_000] = [S { data: [0; 32] }; 5_000]; + pub const BAR_S_PUB: [Option<&str>; 200_000] = [Some("str"); 200_000]; + const BAR_S: [Option<&str>; 200_000] = [Some("str"); 200_000]; + + // Good + pub const G_BAR_PUB: [u32; 1_000] = [0u32; 1_000]; + const G_BAR: [u32; 1_000] = [0u32; 1_000]; + pub const G_BAR_STRUCT_PUB: [S; 500] = [S { data: [0; 32] }; 500]; + const G_BAR_STRUCT: [S; 500] = [S { data: [0; 32] }; 500]; + pub const G_BAR_S_PUB: [Option<&str>; 200] = [Some("str"); 200]; + const G_BAR_S: [Option<&str>; 200] = [Some("str"); 200]; +} diff --git a/tests/ui/large_const_arrays.stderr b/tests/ui/large_const_arrays.stderr new file mode 100644 index 0000000000000..3fb0acbca67de --- /dev/null +++ b/tests/ui/large_const_arrays.stderr @@ -0,0 +1,76 @@ +error: large array defined as const + --> $DIR/large_const_arrays.rs:12:1 + | +LL | pub(crate) const FOO_PUB_CRATE: [u32; 1_000_000] = [0u32; 1_000_000]; + | ^^^^^^^^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + | + = note: `-D clippy::large-const-arrays` implied by `-D warnings` + +error: large array defined as const + --> $DIR/large_const_arrays.rs:13:1 + | +LL | pub const FOO_PUB: [u32; 1_000_000] = [0u32; 1_000_000]; + | ^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + +error: large array defined as const + --> $DIR/large_const_arrays.rs:14:1 + | +LL | const FOO: [u32; 1_000_000] = [0u32; 1_000_000]; + | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + +error: large array defined as const + --> $DIR/large_const_arrays.rs:23:5 + | +LL | pub const BAR_PUB: [u32; 1_000_000] = [0u32; 1_000_000]; + | ^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + +error: large array defined as const + --> $DIR/large_const_arrays.rs:24:5 + | +LL | const BAR: [u32; 1_000_000] = [0u32; 1_000_000]; + | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + +error: large array defined as const + --> $DIR/large_const_arrays.rs:25:5 + | +LL | pub const BAR_STRUCT_PUB: [S; 5_000] = [S { data: [0; 32] }; 5_000]; + | ^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + +error: large array defined as const + --> $DIR/large_const_arrays.rs:26:5 + | +LL | const BAR_STRUCT: [S; 5_000] = [S { data: [0; 32] }; 5_000]; + | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + +error: large array defined as const + --> $DIR/large_const_arrays.rs:27:5 + | +LL | pub const BAR_S_PUB: [Option<&str>; 200_000] = [Some("str"); 200_000]; + | ^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + +error: large array defined as const + --> $DIR/large_const_arrays.rs:28:5 + | +LL | const BAR_S: [Option<&str>; 200_000] = [Some("str"); 200_000]; + | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + +error: aborting due to 9 previous errors + From 629cc4ada3666865eb90f8425d7ea657710cdc74 Mon Sep 17 00:00:00 2001 From: ThibsG Date: Mon, 23 Mar 2020 22:07:46 +0100 Subject: [PATCH 02/13] Update doc generation script --- clippy_lints/src/large_const_arrays.rs | 4 ++-- clippy_lints/src/lib.rs | 1 + clippy_lints/src/utils/conf.rs | 2 +- util/lintlib.py | 8 ++++---- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/large_const_arrays.rs b/clippy_lints/src/large_const_arrays.rs index 2f62e6ad9dc5c..4c3030cf2e7c4 100644 --- a/clippy_lints/src/large_const_arrays.rs +++ b/clippy_lints/src/large_const_arrays.rs @@ -1,11 +1,11 @@ use crate::rustc_target::abi::LayoutOf; use crate::utils::span_lint_and_then; use if_chain::if_chain; -use rustc::mir::interpret::ConstValue; -use rustc::ty::{self, ConstKind}; use rustc_errors::Applicability; use rustc_hir::{Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::mir::interpret::ConstValue; +use rustc_middle::ty::{self, ConstKind}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::{BytePos, Pos, Span}; use rustc_typeck::hir_ty_to_ty; diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 909115f7a3a94..f912c503c827f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -231,6 +231,7 @@ mod inline_fn_without_body; mod int_plus_one; mod integer_division; mod items_after_statements; +mod large_const_arrays; mod large_enum_variant; mod large_stack_arrays; mod len_zero; diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 722104e5b5249..afcfe34f5be8f 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -150,7 +150,7 @@ define_Conf! { (trivial_copy_size_limit, "trivial_copy_size_limit": Option, None), /// Lint: TOO_MANY_LINES. The maximum number of lines a function or method can have (too_many_lines_threshold, "too_many_lines_threshold": u64, 100), - /// Lint: LARGE_STACK_ARRAYS. The maximum allowed size for arrays on the stack + /// Lint: LARGE_STACK_ARRAYS, LARGE_CONST_ARRAYS. The maximum allowed size for arrays on the stack (array_size_threshold, "array_size_threshold": u64, 512_000), /// Lint: VEC_BOX. The size of the boxed type in bytes, where boxing in a `Vec` is allowed (vec_box_size_threshold, "vec_box_size_threshold": u64, 4096), diff --git a/util/lintlib.py b/util/lintlib.py index 16cc6ccfdae37..d0d9beb9b2d9f 100644 --- a/util/lintlib.py +++ b/util/lintlib.py @@ -14,7 +14,7 @@ group_re = re.compile(r'''\s*([a-z_][a-z_0-9]+)''') conf_re = re.compile(r'''define_Conf! {\n([^}]*)\n}''', re.MULTILINE) confvar_re = re.compile( - r'''/// Lint: (\w+)\. (.*)\n\s*\([^,]+,\s+"([^"]+)":\s+([^,]+),\s+([^\.\)]+).*\),''', re.MULTILINE) + r'''/// Lint: ([\w,\s]+)\. (.*)\n\s*\([^,]+,\s+"([^"]+)":\s+([^,]+),\s+([^\.\)]+).*\),''', re.MULTILINE) comment_re = re.compile(r'''\s*/// ?(.*)''') lint_levels = { @@ -93,9 +93,9 @@ def parse_configs(path): match = re.search(conf_re, contents) confvars = re.findall(confvar_re, match.group(1)) - for (lint, doc, name, ty, default) in confvars: - configs[lint.lower()] = Config(name.replace("_", "-"), ty, doc, default) - + for (lints, doc, name, ty, default) in confvars: + for lint in lints.split(','): + configs[lint.strip().lower()] = Config(name.replace("_", "-"), ty, doc, default) return configs From 23df4a0183e0d954d47db98824295411d50f742e Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 13 Apr 2020 13:11:19 +1000 Subject: [PATCH 03/13] Disallow bit-shifting in `integer_arithmetic` lint With this change, the lint checks all operations that are defined as being capable of overflow in the Rust Reference. --- clippy_lints/src/arithmetic.rs | 18 ++++++++------ src/lintlist/mod.rs | 2 +- tests/ui/integer_arithmetic.rs | 10 +++----- tests/ui/integer_arithmetic.stderr | 40 ++++++++++++++++++++++++------ 4 files changed, 48 insertions(+), 22 deletions(-) diff --git a/clippy_lints/src/arithmetic.rs b/clippy_lints/src/arithmetic.rs index a138c9d3545c8..6cbe10a5352d1 100644 --- a/clippy_lints/src/arithmetic.rs +++ b/clippy_lints/src/arithmetic.rs @@ -6,11 +6,17 @@ use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Span; declare_clippy_lint! { - /// **What it does:** Checks for plain integer arithmetic. + /// **What it does:** Checks for integer arithmetic operations which could overflow or panic. /// - /// **Why is this bad?** This is only checked against overflow in debug builds. - /// In some applications one wants explicitly checked, wrapping or saturating - /// arithmetic. + /// Specifically, checks for any operators (`+`, `-`, `*`, `<<`, etc) which are capable + /// of overflowing according to the [Rust + /// Reference](https://doc.rust-lang.org/reference/expressions/operator-expr.html#overflow), + /// or which can panic (`/`, `%`). No bounds analysis or sophisticated reasoning is + /// attempted. + /// + /// **Why is this bad?** Integer overflow will trigger a panic in debug builds or will wrap in + /// release mode. Division by zero will cause a panic in either mode. In some applications one + /// wants explicitly checked, wrapping or saturating arithmetic. /// /// **Known problems:** None. /// @@ -21,7 +27,7 @@ declare_clippy_lint! { /// ``` pub INTEGER_ARITHMETIC, restriction, - "any integer arithmetic statement" + "any integer arithmetic expression which could overflow or panic" } declare_clippy_lint! { @@ -71,8 +77,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Arithmetic { | hir::BinOpKind::BitAnd | hir::BinOpKind::BitOr | hir::BinOpKind::BitXor - | hir::BinOpKind::Shl - | hir::BinOpKind::Shr | hir::BinOpKind::Eq | hir::BinOpKind::Lt | hir::BinOpKind::Le diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index edebaff9f1422..8712f07e9e0c4 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -846,7 +846,7 @@ pub static ref ALL_LINTS: Vec = vec![ Lint { name: "integer_arithmetic", group: "restriction", - desc: "any integer arithmetic statement", + desc: "any integer arithmetic expression which could overflow or panic", deprecation: None, module: "arithmetic", }, diff --git a/tests/ui/integer_arithmetic.rs b/tests/ui/integer_arithmetic.rs index 2fe32c6ace875..7b1b64f390a5a 100644 --- a/tests/ui/integer_arithmetic.rs +++ b/tests/ui/integer_arithmetic.rs @@ -17,6 +17,8 @@ fn main() { i / 2; // no error, this is part of the expression in the preceding line i - 2 + 2 - i; -i; + i >> 1; + i << 1; // no error, overflows are checked by `overflowing_literals` -1; @@ -25,18 +27,16 @@ fn main() { i & 1; // no wrapping i | 1; i ^ 1; - i >> 1; - i << 1; i += 1; i -= 1; i *= 2; i /= 2; i %= 2; - - // no errors i <<= 3; i >>= 2; + + // no errors i |= 1; i &= 1; i ^= i; @@ -72,8 +72,6 @@ fn main() { 1 + 1 }; } - - } // warn on references as well! (#5328) diff --git a/tests/ui/integer_arithmetic.stderr b/tests/ui/integer_arithmetic.stderr index 64c44d7ecc7b0..83e8a9cde3ff1 100644 --- a/tests/ui/integer_arithmetic.stderr +++ b/tests/ui/integer_arithmetic.stderr @@ -31,6 +31,18 @@ error: integer arithmetic detected LL | -i; | ^^ +error: integer arithmetic detected + --> $DIR/integer_arithmetic.rs:20:5 + | +LL | i >> 1; + | ^^^^^^ + +error: integer arithmetic detected + --> $DIR/integer_arithmetic.rs:21:5 + | +LL | i << 1; + | ^^^^^^ + error: integer arithmetic detected --> $DIR/integer_arithmetic.rs:31:5 | @@ -62,46 +74,58 @@ LL | i %= 2; | ^^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:81:5 + --> $DIR/integer_arithmetic.rs:36:5 + | +LL | i <<= 3; + | ^^^^^^^ + +error: integer arithmetic detected + --> $DIR/integer_arithmetic.rs:37:5 + | +LL | i >>= 2; + | ^^^^^^^ + +error: integer arithmetic detected + --> $DIR/integer_arithmetic.rs:79:5 | LL | 3 + &1; | ^^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:82:5 + --> $DIR/integer_arithmetic.rs:80:5 | LL | &3 + 1; | ^^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:83:5 + --> $DIR/integer_arithmetic.rs:81:5 | LL | &3 + &1; | ^^^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:88:5 + --> $DIR/integer_arithmetic.rs:86:5 | LL | a + x | ^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:92:5 + --> $DIR/integer_arithmetic.rs:90:5 | LL | x + y | ^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:96:5 + --> $DIR/integer_arithmetic.rs:94:5 | LL | x + y | ^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:100:5 + --> $DIR/integer_arithmetic.rs:98:5 | LL | (&x + &y) | ^^^^^^^^^ -error: aborting due to 17 previous errors +error: aborting due to 21 previous errors From 69c3e9c90f9a7731dcfe129b10e55e440ee6f484 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Wed, 15 Apr 2020 09:55:02 +0200 Subject: [PATCH 04/13] large_enum_variant: Report sizes of variants This reports the sizes of the largest and second-largest variants. --- clippy_lints/src/large_enum_variant.rs | 25 +++++++++++++++++------ tests/ui/large_enum_variant.stderr | 28 ++++++++++++++++++++++---- 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/large_enum_variant.rs b/clippy_lints/src/large_enum_variant.rs index 00bbba64841a9..7ac83739be67b 100644 --- a/clippy_lints/src/large_enum_variant.rs +++ b/clippy_lints/src/large_enum_variant.rs @@ -21,11 +21,19 @@ declare_clippy_lint! { /// measure the change this lint suggests. /// /// **Example:** + /// /// ```rust + /// // Bad /// enum Test { /// A(i32), /// B([i32; 8000]), /// } + /// + /// // Possibly better + /// enum Test2 { + /// A(i32), + /// B(Box<[i32; 8000]>), + /// } /// ``` pub LARGE_ENUM_VARIANT, perf, @@ -84,12 +92,21 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeEnumVariant { if difference > self.maximum_size_difference_allowed { let (i, variant) = largest.1; + let help_text = "consider boxing the large fields to reduce the total size of the enum"; span_lint_and_then( cx, LARGE_ENUM_VARIANT, def.variants[i].span, "large size difference between variants", |db| { + db.span_label( + def.variants[(largest.1).0].span, + &format!("this variant is {} bytes", largest.0), + ); + db.span_note( + def.variants[(second.1).0].span, + &format!("and the second-largest variant is {} bytes:", second.0), + ); if variant.fields.len() == 1 { let span = match def.variants[i].data { VariantData::Struct(ref fields, ..) | VariantData::Tuple(ref fields, ..) => { @@ -100,18 +117,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeEnumVariant { if let Some(snip) = snippet_opt(cx, span) { db.span_suggestion( span, - "consider boxing the large fields to reduce the total size of the \ - enum", + help_text, format!("Box<{}>", snip), Applicability::MaybeIncorrect, ); return; } } - db.span_help( - def.variants[i].span, - "consider boxing the large fields to reduce the total size of the enum", - ); + db.span_help(def.variants[i].span, help_text); }, ); } diff --git a/tests/ui/large_enum_variant.stderr b/tests/ui/large_enum_variant.stderr index 5d659611533a5..8ce641a81f297 100644 --- a/tests/ui/large_enum_variant.stderr +++ b/tests/ui/large_enum_variant.stderr @@ -2,9 +2,14 @@ error: large size difference between variants --> $DIR/large_enum_variant.rs:7:5 | LL | B([i32; 8000]), - | ^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^ this variant is 32000 bytes | = note: `-D clippy::large-enum-variant` implied by `-D warnings` +note: and the second-largest variant is 4 bytes: + --> $DIR/large_enum_variant.rs:6:5 + | +LL | A(i32), + | ^^^^^^ help: consider boxing the large fields to reduce the total size of the enum | LL | B(Box<[i32; 8000]>), @@ -14,8 +19,13 @@ error: large size difference between variants --> $DIR/large_enum_variant.rs:31:5 | LL | ContainingLargeEnum(LargeEnum), - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this variant is 32004 bytes + | +note: and the second-largest variant is 8 bytes: + --> $DIR/large_enum_variant.rs:30:5 | +LL | VariantOk(i32, u32), + | ^^^^^^^^^^^^^^^^^^^ help: consider boxing the large fields to reduce the total size of the enum | LL | ContainingLargeEnum(Box), @@ -25,8 +35,13 @@ error: large size difference between variants --> $DIR/large_enum_variant.rs:41:5 | LL | StructLikeLarge { x: [i32; 8000], y: i32 }, - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this variant is 32004 bytes | +note: and the second-largest variant is 8 bytes: + --> $DIR/large_enum_variant.rs:40:5 + | +LL | VariantOk(i32, u32), + | ^^^^^^^^^^^^^^^^^^^ help: consider boxing the large fields to reduce the total size of the enum --> $DIR/large_enum_variant.rs:41:5 | @@ -37,8 +52,13 @@ error: large size difference between variants --> $DIR/large_enum_variant.rs:46:5 | LL | StructLikeLarge2 { x: [i32; 8000] }, - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this variant is 32000 bytes + | +note: and the second-largest variant is 8 bytes: + --> $DIR/large_enum_variant.rs:45:5 | +LL | VariantOk(i32, u32), + | ^^^^^^^^^^^^^^^^^^^ help: consider boxing the large fields to reduce the total size of the enum | LL | StructLikeLarge2 { x: Box<[i32; 8000]> }, From d3ebd06ec93cbc5a9cf47f7ced6e69704758db5e Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Wed, 15 Apr 2020 13:22:28 +0200 Subject: [PATCH 05/13] Make the single char threshold strict inequality --- clippy_lints/src/non_expressive_names.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/non_expressive_names.rs b/clippy_lints/src/non_expressive_names.rs index 98446eef9d732..45809b3598661 100644 --- a/clippy_lints/src/non_expressive_names.rs +++ b/clippy_lints/src/non_expressive_names.rs @@ -93,7 +93,7 @@ impl<'a, 'tcx> SimilarNamesLocalVisitor<'a, 'tcx> { fn check_single_char_names(&self) { let num_single_char_names = self.single_char_names.iter().flatten().count(); let threshold = self.lint.single_char_binding_names_threshold; - if num_single_char_names as u64 >= threshold { + if num_single_char_names as u64 > threshold { let span = self .single_char_names .iter() From 512f23fff1c8261ce537578b519f0c313f0ba174 Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Wed, 15 Apr 2020 13:22:50 +0200 Subject: [PATCH 06/13] Add test for zero single char names --- tests/ui-toml/zero_single_char_names/clippy.toml | 1 + tests/ui-toml/zero_single_char_names/zero_single_char_names.rs | 3 +++ 2 files changed, 4 insertions(+) create mode 100644 tests/ui-toml/zero_single_char_names/clippy.toml create mode 100644 tests/ui-toml/zero_single_char_names/zero_single_char_names.rs diff --git a/tests/ui-toml/zero_single_char_names/clippy.toml b/tests/ui-toml/zero_single_char_names/clippy.toml new file mode 100644 index 0000000000000..42a1067b95edd --- /dev/null +++ b/tests/ui-toml/zero_single_char_names/clippy.toml @@ -0,0 +1 @@ +single-char-binding-names-threshold = 0 diff --git a/tests/ui-toml/zero_single_char_names/zero_single_char_names.rs b/tests/ui-toml/zero_single_char_names/zero_single_char_names.rs new file mode 100644 index 0000000000000..22aaa242b9b9d --- /dev/null +++ b/tests/ui-toml/zero_single_char_names/zero_single_char_names.rs @@ -0,0 +1,3 @@ +#![warn(clippy::many_single_char_names)] + +fn main() {} From b2d986850d795f8aa3bdea1d2c840b080c9df81c Mon Sep 17 00:00:00 2001 From: Tom Milligan Date: Wed, 3 Oct 2018 17:53:39 +0100 Subject: [PATCH 07/13] Working basic dereference clip --- clippy_lints/src/dereference.rs | 74 +++++++++++++++++++++++++++++++++ clippy_lints/src/lib.rs | 6 ++- tests/ui/dereference.rs | 55 ++++++++++++++++++++++++ tests/ui/dereference.stderr | 52 +++++++++++++++++++++++ 4 files changed, 186 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/dereference.rs create mode 100644 tests/ui/dereference.rs create mode 100644 tests/ui/dereference.stderr diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs new file mode 100644 index 0000000000000..c29c0d466d10e --- /dev/null +++ b/clippy_lints/src/dereference.rs @@ -0,0 +1,74 @@ +use crate::rustc::hir::{Expr, ExprKind, QPath}; +use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; +use crate::rustc::{declare_tool_lint, lint_array}; +use crate::utils::{in_macro, span_lint_and_sugg}; +use if_chain::if_chain; + +/// **What it does:** Checks for explicit deref() or deref_mut() method calls. +/// +/// **Why is this bad?** Derefencing by &*x or &mut *x is clearer and more concise, +/// when not part of a method chain. +/// +/// **Example:** +/// ```rust +/// let b = a.deref(); +/// let c = a.deref_mut(); +/// +/// // excludes +/// let e = d.unwrap().deref(); +/// ``` +declare_clippy_lint! { + pub EXPLICIT_DEREF_METHOD, + pedantic, + "Explicit use of deref or deref_mut method while not in a method chain." +} + +pub struct Pass; + +impl LintPass for Pass { + fn get_lints(&self) -> LintArray { + lint_array!(EXPLICIT_DEREF_METHOD) + } +} + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { + fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &Expr) { + if in_macro(expr.span) { + return; + } + + if_chain! { + // if this is a method call + if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.node; + // on a Path (i.e. a variable/name, not another method) + if let ExprKind::Path(QPath::Resolved(None, path)) = &args[0].node; + then { + let name = method_name.ident.as_str(); + // alter help slightly to account for _mut + match &*name { + "deref" => { + span_lint_and_sugg( + cx, + EXPLICIT_DEREF_METHOD, + expr.span, + "explicit deref method call", + "try this", + format!("&*{}", path), + ); + }, + "deref_mut" => { + span_lint_and_sugg( + cx, + EXPLICIT_DEREF_METHOD, + expr.span, + "explicit deref_mut method call", + "try this", + format!("&mut *{}", path), + ); + }, + _ => () + }; + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index cb9fcfca8a1c3..a6ddd6573a81c 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -391,7 +391,7 @@ pub fn read_conf(args: &[rustc_ast::ast::NestedMetaItem], sess: &Session) -> Con } conf - }, + } Err((err, span)) => { sess.struct_span_err(span, err) .span_note(span, "Clippy will use default configuration") @@ -513,6 +513,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: ©_iterator::COPY_ITERATOR, &dbg_macro::DBG_MACRO, &default_trait_access::DEFAULT_TRAIT_ACCESS, + &dereference::DEREF_METHOD_EXPLICIT, &derive::DERIVE_HASH_XOR_EQ, &derive::EXPL_IMPL_CLONE_ON_COPY, &doc::DOC_MARKDOWN, @@ -1039,6 +1040,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box verbose_file_reads::VerboseFileReads); store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default()); store.register_late_pass(|| box unnamed_address::UnnamedAddress); + store.register_late_pass(|| box dereference::DerefMethodExplicit); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(&arithmetic::FLOAT_ARITHMETIC), @@ -1089,6 +1091,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&copies::SAME_FUNCTIONS_IN_IF_CONDITION), LintId::of(©_iterator::COPY_ITERATOR), LintId::of(&default_trait_access::DEFAULT_TRAIT_ACCESS), + LintId::of(&dereference::EXPLICIT_DEREF_METHOD), LintId::of(&derive::EXPL_IMPL_CLONE_ON_COPY), LintId::of(&doc::DOC_MARKDOWN), LintId::of(&doc::MISSING_ERRORS_DOC), @@ -1178,6 +1181,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&comparison_chain::COMPARISON_CHAIN), LintId::of(&copies::IFS_SAME_COND), LintId::of(&copies::IF_SAME_THEN_ELSE), + LintId::of(&dereference::EXPLICIT_DEREF_METHOD), LintId::of(&derive::DERIVE_HASH_XOR_EQ), LintId::of(&doc::MISSING_SAFETY_DOC), LintId::of(&doc::NEEDLESS_DOCTEST_MAIN), diff --git a/tests/ui/dereference.rs b/tests/ui/dereference.rs new file mode 100644 index 0000000000000..7800cd84c2426 --- /dev/null +++ b/tests/ui/dereference.rs @@ -0,0 +1,55 @@ +#![feature(tool_lints)] + +use std::ops::{Deref, DerefMut}; + +#[allow(clippy::many_single_char_names, clippy::clone_double_ref)] +#[allow(unused_variables)] +#[warn(clippy::explicit_deref_method)] +fn main() { + let a: &mut String = &mut String::from("foo"); + + // these should require linting + { + let b: &str = a.deref(); + } + + { + let b: &mut str = a.deref_mut(); + } + + { + let b: String = a.deref().clone(); + } + + { + let b: usize = a.deref_mut().len(); + } + + { + let b: &usize = &a.deref().len(); + } + + { + // only first deref should get linted here + let b: &str = a.deref().deref(); + } + + { + // both derefs should get linted here + let b: String = format!("{}, {}", a.deref(), a.deref()); + } + + // these should not require linting + { + let b: &str = &*a; + } + + { + let b: &mut str = &mut *a; + } + + { + macro_rules! expr_deref { ($body:expr) => { $body.deref() } } + let b: &str = expr_deref!(a); + } +} diff --git a/tests/ui/dereference.stderr b/tests/ui/dereference.stderr new file mode 100644 index 0000000000000..a4c2487d06b9f --- /dev/null +++ b/tests/ui/dereference.stderr @@ -0,0 +1,52 @@ +error: explicit deref method call + --> $DIR/dereference.rs:13:23 + | +13 | let b: &str = a.deref(); + | ^^^^^^^^^ help: try this: `&*a` + | + = note: `-D clippy::explicit-deref-method` implied by `-D warnings` + +error: explicit deref_mut method call + --> $DIR/dereference.rs:17:27 + | +17 | let b: &mut str = a.deref_mut(); + | ^^^^^^^^^^^^^ help: try this: `&mut *a` + +error: explicit deref method call + --> $DIR/dereference.rs:21:25 + | +21 | let b: String = a.deref().clone(); + | ^^^^^^^^^ help: try this: `&*a` + +error: explicit deref_mut method call + --> $DIR/dereference.rs:25:24 + | +25 | let b: usize = a.deref_mut().len(); + | ^^^^^^^^^^^^^ help: try this: `&mut *a` + +error: explicit deref method call + --> $DIR/dereference.rs:29:26 + | +29 | let b: &usize = &a.deref().len(); + | ^^^^^^^^^ help: try this: `&*a` + +error: explicit deref method call + --> $DIR/dereference.rs:34:23 + | +34 | let b: &str = a.deref().deref(); + | ^^^^^^^^^ help: try this: `&*a` + +error: explicit deref method call + --> $DIR/dereference.rs:39:43 + | +39 | let b: String = format!("{}, {}", a.deref(), a.deref()); + | ^^^^^^^^^ help: try this: `&*a` + +error: explicit deref method call + --> $DIR/dereference.rs:39:54 + | +39 | let b: String = format!("{}, {}", a.deref(), a.deref()); + | ^^^^^^^^^ help: try this: `&*a` + +error: aborting due to 8 previous errors + From 6b4ab827469529f4eda5f1e9492abcb9ad9d209a Mon Sep 17 00:00:00 2001 From: ThibsG Date: Thu, 23 Jan 2020 16:28:01 +0100 Subject: [PATCH 08/13] Global rework + fix imports --- CHANGELOG.md | 1 + clippy_lints/src/dereference.rs | 60 ++++++++++++++++++--------------- clippy_lints/src/lib.rs | 7 ++-- src/lintlist/mod.rs | 7 ++++ tests/ui/dereference.rs | 59 ++++++++++++-------------------- tests/ui/dereference.stderr | 48 +++++++++++++------------- 6 files changed, 89 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7ac3cace2048..eb5a6af97d361 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1256,6 +1256,7 @@ Released 2018-09-13 [`expect_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#expect_fun_call [`expl_impl_clone_on_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#expl_impl_clone_on_copy [`explicit_counter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_counter_loop +[`explicit_deref_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_deref_method [`explicit_into_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_into_iter_loop [`explicit_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_iter_loop [`explicit_write`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_write diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index c29c0d466d10e..dea00e5aa3b6c 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -1,47 +1,51 @@ -use crate::rustc::hir::{Expr, ExprKind, QPath}; -use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; -use crate::rustc::{declare_tool_lint, lint_array}; +use rustc_hir::{Expr, ExprKind, QPath}; +use rustc_errors::Applicability; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_tool_lint, declare_lint_pass}; use crate::utils::{in_macro, span_lint_and_sugg}; use if_chain::if_chain; -/// **What it does:** Checks for explicit deref() or deref_mut() method calls. -/// -/// **Why is this bad?** Derefencing by &*x or &mut *x is clearer and more concise, -/// when not part of a method chain. -/// -/// **Example:** -/// ```rust -/// let b = a.deref(); -/// let c = a.deref_mut(); -/// -/// // excludes -/// let e = d.unwrap().deref(); -/// ``` declare_clippy_lint! { + /// **What it does:** Checks for explicit `deref()` or `deref_mut()` method calls. + /// + /// **Why is this bad?** Derefencing by `&*x` or `&mut *x` is clearer and more concise, + /// when not part of a method chain. + /// + /// **Example:** + /// ```rust + /// let b = a.deref(); + /// let c = a.deref_mut(); + /// ``` + /// Could be written as: + /// ```rust + /// let b = &*a; + /// let c = &mut *a; + /// ``` + /// + /// This lint excludes + /// ```rust + /// let e = d.unwrap().deref(); + /// ``` pub EXPLICIT_DEREF_METHOD, pedantic, "Explicit use of deref or deref_mut method while not in a method chain." } -pub struct Pass; +declare_lint_pass!(Dereferencing => [ + EXPLICIT_DEREF_METHOD +]); -impl LintPass for Pass { - fn get_lints(&self) -> LintArray { - lint_array!(EXPLICIT_DEREF_METHOD) - } -} - -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { - fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &Expr) { +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { if in_macro(expr.span) { return; } if_chain! { // if this is a method call - if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.node; + if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.kind; // on a Path (i.e. a variable/name, not another method) - if let ExprKind::Path(QPath::Resolved(None, path)) = &args[0].node; + if let ExprKind::Path(QPath::Resolved(None, path)) = &args[0].kind; then { let name = method_name.ident.as_str(); // alter help slightly to account for _mut @@ -54,6 +58,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { "explicit deref method call", "try this", format!("&*{}", path), + Applicability::MachineApplicable ); }, "deref_mut" => { @@ -64,6 +69,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { "explicit deref_mut method call", "try this", format!("&mut *{}", path), + Applicability::MachineApplicable ); }, _ => () diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index a6ddd6573a81c..6443caa89d62c 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -391,7 +391,7 @@ pub fn read_conf(args: &[rustc_ast::ast::NestedMetaItem], sess: &Session) -> Con } conf - } + }, Err((err, span)) => { sess.struct_span_err(span, err) .span_note(span, "Clippy will use default configuration") @@ -513,7 +513,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: ©_iterator::COPY_ITERATOR, &dbg_macro::DBG_MACRO, &default_trait_access::DEFAULT_TRAIT_ACCESS, - &dereference::DEREF_METHOD_EXPLICIT, + &dereference::EXPLICIT_DEREF_METHOD, &derive::DERIVE_HASH_XOR_EQ, &derive::EXPL_IMPL_CLONE_ON_COPY, &doc::DOC_MARKDOWN, @@ -1040,7 +1040,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box verbose_file_reads::VerboseFileReads); store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default()); store.register_late_pass(|| box unnamed_address::UnnamedAddress); - store.register_late_pass(|| box dereference::DerefMethodExplicit); + store.register_late_pass(|| box dereference::Dereferencing); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(&arithmetic::FLOAT_ARITHMETIC), @@ -1181,7 +1181,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&comparison_chain::COMPARISON_CHAIN), LintId::of(&copies::IFS_SAME_COND), LintId::of(&copies::IF_SAME_THEN_ELSE), - LintId::of(&dereference::EXPLICIT_DEREF_METHOD), LintId::of(&derive::DERIVE_HASH_XOR_EQ), LintId::of(&doc::MISSING_SAFETY_DOC), LintId::of(&doc::NEEDLESS_DOCTEST_MAIN), diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 935ea180ebe21..3f1d31f0302c8 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -528,6 +528,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "loops", }, + Lint { + name: "explicit_deref_method", + group: "pedantic", + desc: "Explicit use of deref or deref_mut method while not in a method chain.", + deprecation: None, + module: "dereference", + }, Lint { name: "explicit_into_iter_loop", group: "pedantic", diff --git a/tests/ui/dereference.rs b/tests/ui/dereference.rs index 7800cd84c2426..07421eb715df0 100644 --- a/tests/ui/dereference.rs +++ b/tests/ui/dereference.rs @@ -1,55 +1,38 @@ -#![feature(tool_lints)] +#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)] +#![warn(clippy::explicit_deref_method)] use std::ops::{Deref, DerefMut}; -#[allow(clippy::many_single_char_names, clippy::clone_double_ref)] -#[allow(unused_variables)] -#[warn(clippy::explicit_deref_method)] fn main() { let a: &mut String = &mut String::from("foo"); // these should require linting - { - let b: &str = a.deref(); - } + let b: &str = a.deref(); - { - let b: &mut str = a.deref_mut(); - } + let b: &mut str = a.deref_mut(); - { - let b: String = a.deref().clone(); - } - - { - let b: usize = a.deref_mut().len(); - } - - { - let b: &usize = &a.deref().len(); - } + let b: String = a.deref().clone(); - { - // only first deref should get linted here - let b: &str = a.deref().deref(); - } + let b: usize = a.deref_mut().len(); - { - // both derefs should get linted here - let b: String = format!("{}, {}", a.deref(), a.deref()); - } + let b: &usize = &a.deref().len(); + + // only first deref should get linted here + let b: &str = a.deref().deref(); + + // both derefs should get linted here + let b: String = format!("{}, {}", a.deref(), a.deref()); // these should not require linting - { - let b: &str = &*a; - } - { - let b: &mut str = &mut *a; - } + let b: &str = &*a; + + let b: &mut str = &mut *a; - { - macro_rules! expr_deref { ($body:expr) => { $body.deref() } } - let b: &str = expr_deref!(a); + macro_rules! expr_deref { + ($body:expr) => { + $body.deref() + }; } + let b: &str = expr_deref!(a); } diff --git a/tests/ui/dereference.stderr b/tests/ui/dereference.stderr index a4c2487d06b9f..7169b689a860a 100644 --- a/tests/ui/dereference.stderr +++ b/tests/ui/dereference.stderr @@ -1,52 +1,52 @@ error: explicit deref method call - --> $DIR/dereference.rs:13:23 + --> $DIR/dereference.rs:10:19 | -13 | let b: &str = a.deref(); - | ^^^^^^^^^ help: try this: `&*a` +LL | let b: &str = a.deref(); + | ^^^^^^^^^ help: try this: `&*a` | = note: `-D clippy::explicit-deref-method` implied by `-D warnings` error: explicit deref_mut method call - --> $DIR/dereference.rs:17:27 + --> $DIR/dereference.rs:12:23 | -17 | let b: &mut str = a.deref_mut(); - | ^^^^^^^^^^^^^ help: try this: `&mut *a` +LL | let b: &mut str = a.deref_mut(); + | ^^^^^^^^^^^^^ help: try this: `&mut *a` error: explicit deref method call - --> $DIR/dereference.rs:21:25 + --> $DIR/dereference.rs:14:21 | -21 | let b: String = a.deref().clone(); - | ^^^^^^^^^ help: try this: `&*a` +LL | let b: String = a.deref().clone(); + | ^^^^^^^^^ help: try this: `&*a` error: explicit deref_mut method call - --> $DIR/dereference.rs:25:24 + --> $DIR/dereference.rs:16:20 | -25 | let b: usize = a.deref_mut().len(); - | ^^^^^^^^^^^^^ help: try this: `&mut *a` +LL | let b: usize = a.deref_mut().len(); + | ^^^^^^^^^^^^^ help: try this: `&mut *a` error: explicit deref method call - --> $DIR/dereference.rs:29:26 + --> $DIR/dereference.rs:18:22 | -29 | let b: &usize = &a.deref().len(); - | ^^^^^^^^^ help: try this: `&*a` +LL | let b: &usize = &a.deref().len(); + | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:34:23 + --> $DIR/dereference.rs:21:19 | -34 | let b: &str = a.deref().deref(); - | ^^^^^^^^^ help: try this: `&*a` +LL | let b: &str = a.deref().deref(); + | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:39:43 + --> $DIR/dereference.rs:24:39 | -39 | let b: String = format!("{}, {}", a.deref(), a.deref()); - | ^^^^^^^^^ help: try this: `&*a` +LL | let b: String = format!("{}, {}", a.deref(), a.deref()); + | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:39:54 + --> $DIR/dereference.rs:24:50 | -39 | let b: String = format!("{}, {}", a.deref(), a.deref()); - | ^^^^^^^^^ help: try this: `&*a` +LL | let b: String = format!("{}, {}", a.deref(), a.deref()); + | ^^^^^^^^^ help: try this: `&*a` error: aborting due to 8 previous errors From c1132434a7cf580a09d08a274bbfd2e776b324f8 Mon Sep 17 00:00:00 2001 From: ThibsG Date: Sun, 26 Jan 2020 19:48:30 +0100 Subject: [PATCH 09/13] Report using stmts and expr + tests --- clippy_lints/src/dereference.rs | 138 +++++++++++++++++++++++--------- tests/ui/dereference.rs | 36 +++++++-- tests/ui/dereference.stderr | 42 +++++----- 3 files changed, 148 insertions(+), 68 deletions(-) diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index dea00e5aa3b6c..9022617ebfc17 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -1,9 +1,11 @@ -use rustc_hir::{Expr, ExprKind, QPath}; +use crate::utils::{get_parent_expr, method_calls, snippet, span_lint_and_sugg}; +use if_chain::if_chain; use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_hir::{Expr, ExprKind, QPath, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_tool_lint, declare_lint_pass}; -use crate::utils::{in_macro, span_lint_and_sugg}; -use if_chain::if_chain; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::source_map::Span; declare_clippy_lint! { /// **What it does:** Checks for explicit `deref()` or `deref_mut()` method calls. @@ -21,7 +23,7 @@ declare_clippy_lint! { /// let b = &*a; /// let c = &mut *a; /// ``` - /// + /// /// This lint excludes /// ```rust /// let e = d.unwrap().deref(); @@ -36,45 +38,105 @@ declare_lint_pass!(Dereferencing => [ ]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { - fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { - if in_macro(expr.span) { - return; + fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx hir::Stmt<'_>) { + if_chain! { + if let StmtKind::Local(ref local) = stmt.kind; + if let Some(ref init) = local.init; + + then { + match init.kind { + ExprKind::Call(ref _method, args) => { + for arg in args { + if_chain! { + // Caller must call only one other function (deref or deref_mut) + // otherwise it can lead to error prone suggestions (ex: &*a.len()) + let (method_names, arg_list, _) = method_calls(arg, 2); + if method_names.len() == 1; + // Caller must be a variable + let variables = arg_list[0]; + if variables.len() == 1; + if let ExprKind::Path(QPath::Resolved(None, _)) = variables[0].kind; + + then { + let name = method_names[0].as_str(); + lint_deref(cx, &*name, variables[0].span, arg.span); + } + } + } + } + ExprKind::MethodCall(ref method_name, _, ref args) => { + if init.span.from_expansion() { + return; + } + if_chain! { + if args.len() == 1; + if let ExprKind::Path(QPath::Resolved(None, _)) = args[0].kind; + // Caller must call only one other function (deref or deref_mut) + // otherwise it can lead to error prone suggestions (ex: &*a.len()) + let (method_names, arg_list, _) = method_calls(init, 2); + if method_names.len() == 1; + // Caller must be a variable + let variables = arg_list[0]; + if variables.len() == 1; + if let ExprKind::Path(QPath::Resolved(None, _)) = variables[0].kind; + + then { + let name = method_name.ident.as_str(); + lint_deref(cx, &*name, args[0].span, init.span); + } + } + } + _ => () + } + } } + } + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { if_chain! { - // if this is a method call if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.kind; - // on a Path (i.e. a variable/name, not another method) - if let ExprKind::Path(QPath::Resolved(None, path)) = &args[0].kind; + if args.len() == 1; + if let Some(parent) = get_parent_expr(cx, &expr); + then { - let name = method_name.ident.as_str(); - // alter help slightly to account for _mut - match &*name { - "deref" => { - span_lint_and_sugg( - cx, - EXPLICIT_DEREF_METHOD, - expr.span, - "explicit deref method call", - "try this", - format!("&*{}", path), - Applicability::MachineApplicable - ); - }, - "deref_mut" => { - span_lint_and_sugg( - cx, - EXPLICIT_DEREF_METHOD, - expr.span, - "explicit deref_mut method call", - "try this", - format!("&mut *{}", path), - Applicability::MachineApplicable - ); - }, - _ => () - }; + // Call and MethodCall exprs are better reported using statements + match parent.kind { + ExprKind::Call(_, _) => return, + ExprKind::MethodCall(_, _, _) => return, + _ => { + let name = method_name.ident.as_str(); + lint_deref(cx, &*name, args[0].span, expr.span); + } + } } } } } + +fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, var_span: Span, expr_span: Span) { + match fn_name { + "deref" => { + span_lint_and_sugg( + cx, + EXPLICIT_DEREF_METHOD, + expr_span, + "explicit deref method call", + "try this", + format!("&*{}", &snippet(cx, var_span, "..")), + Applicability::MachineApplicable, + ); + }, + "deref_mut" => { + span_lint_and_sugg( + cx, + EXPLICIT_DEREF_METHOD, + expr_span, + "explicit deref_mut method call", + "try this", + format!("&mut *{}", &snippet(cx, var_span, "..")), + Applicability::MachineApplicable, + ); + }, + _ => (), + } +} diff --git a/tests/ui/dereference.rs b/tests/ui/dereference.rs index 07421eb715df0..5de201fb22f0b 100644 --- a/tests/ui/dereference.rs +++ b/tests/ui/dereference.rs @@ -3,28 +3,49 @@ use std::ops::{Deref, DerefMut}; +fn concat(deref_str: &str) -> String { + format!("{}bar", deref_str) +} + +fn just_return(deref_str: &str) -> &str { + deref_str +} + fn main() { let a: &mut String = &mut String::from("foo"); // these should require linting + let b: &str = a.deref(); let b: &mut str = a.deref_mut(); + // both derefs should get linted here + let b: String = format!("{}, {}", a.deref(), a.deref()); + + println!("{}", a.deref()); + + #[allow(clippy::match_single_binding)] + match a.deref() { + _ => (), + } + + let b: String = concat(a.deref()); + + // following should not require linting + + let b = just_return(a).deref(); + + let b: String = concat(just_return(a).deref()); + let b: String = a.deref().clone(); let b: usize = a.deref_mut().len(); let b: &usize = &a.deref().len(); - // only first deref should get linted here let b: &str = a.deref().deref(); - // both derefs should get linted here - let b: String = format!("{}, {}", a.deref(), a.deref()); - - // these should not require linting - let b: &str = &*a; let b: &mut str = &mut *a; @@ -35,4 +56,7 @@ fn main() { }; } let b: &str = expr_deref!(a); + + let opt_a = Some(a); + let b = opt_a.unwrap().deref(); } diff --git a/tests/ui/dereference.stderr b/tests/ui/dereference.stderr index 7169b689a860a..7e10add40b180 100644 --- a/tests/ui/dereference.stderr +++ b/tests/ui/dereference.stderr @@ -1,5 +1,5 @@ error: explicit deref method call - --> $DIR/dereference.rs:10:19 + --> $DIR/dereference.rs:19:19 | LL | let b: &str = a.deref(); | ^^^^^^^^^ help: try this: `&*a` @@ -7,46 +7,40 @@ LL | let b: &str = a.deref(); = note: `-D clippy::explicit-deref-method` implied by `-D warnings` error: explicit deref_mut method call - --> $DIR/dereference.rs:12:23 + --> $DIR/dereference.rs:21:23 | LL | let b: &mut str = a.deref_mut(); | ^^^^^^^^^^^^^ help: try this: `&mut *a` error: explicit deref method call - --> $DIR/dereference.rs:14:21 - | -LL | let b: String = a.deref().clone(); - | ^^^^^^^^^ help: try this: `&*a` - -error: explicit deref_mut method call - --> $DIR/dereference.rs:16:20 + --> $DIR/dereference.rs:24:39 | -LL | let b: usize = a.deref_mut().len(); - | ^^^^^^^^^^^^^ help: try this: `&mut *a` +LL | let b: String = format!("{}, {}", a.deref(), a.deref()); + | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:18:22 + --> $DIR/dereference.rs:24:50 | -LL | let b: &usize = &a.deref().len(); - | ^^^^^^^^^ help: try this: `&*a` +LL | let b: String = format!("{}, {}", a.deref(), a.deref()); + | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:21:19 + --> $DIR/dereference.rs:26:20 | -LL | let b: &str = a.deref().deref(); - | ^^^^^^^^^ help: try this: `&*a` +LL | println!("{}", a.deref()); + | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:24:39 + --> $DIR/dereference.rs:29:11 | -LL | let b: String = format!("{}, {}", a.deref(), a.deref()); - | ^^^^^^^^^ help: try this: `&*a` +LL | match a.deref() { + | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:24:50 + --> $DIR/dereference.rs:33:28 | -LL | let b: String = format!("{}, {}", a.deref(), a.deref()); - | ^^^^^^^^^ help: try this: `&*a` +LL | let b: String = concat(a.deref()); + | ^^^^^^^^^ help: try this: `&*a` -error: aborting due to 8 previous errors +error: aborting due to 7 previous errors From b6d43305502f15d3c3e1a49f488eedc5ce330b4f Mon Sep 17 00:00:00 2001 From: ThibsG Date: Tue, 25 Feb 2020 23:06:24 +0100 Subject: [PATCH 10/13] Check for Deref trait impl + add fixed version --- clippy_lints/src/dereference.rs | 77 ++++++++++++++++++-------------- clippy_lints/src/utils/paths.rs | 2 + tests/ui/dereference.fixed | 79 +++++++++++++++++++++++++++++++++ tests/ui/dereference.rs | 17 +++++++ tests/ui/dereference.stderr | 14 +++--- 5 files changed, 149 insertions(+), 40 deletions(-) create mode 100644 tests/ui/dereference.fixed diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index 9022617ebfc17..d11614d489d23 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -1,4 +1,6 @@ -use crate::utils::{get_parent_expr, method_calls, snippet, span_lint_and_sugg}; +use crate::utils::{ + get_parent_expr, get_trait_def_id, implements_trait, method_calls, paths, snippet, span_lint_and_sugg, +}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; @@ -15,18 +17,19 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust - /// let b = a.deref(); - /// let c = a.deref_mut(); + /// use std::ops::Deref; + /// let a: &mut String = &mut String::from("foo"); + /// let b: &str = a.deref(); /// ``` /// Could be written as: /// ```rust + /// let a: &mut String = &mut String::from("foo"); /// let b = &*a; - /// let c = &mut *a; /// ``` /// /// This lint excludes - /// ```rust - /// let e = d.unwrap().deref(); + /// ```rust,ignore + /// let _ = d.unwrap().deref(); /// ``` pub EXPLICIT_DEREF_METHOD, pedantic, @@ -49,7 +52,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { for arg in args { if_chain! { // Caller must call only one other function (deref or deref_mut) - // otherwise it can lead to error prone suggestions (ex: &*a.len()) + // otherwise it can lead to error prone suggestions (ie: &*a.len()) let (method_names, arg_list, _) = method_calls(arg, 2); if method_names.len() == 1; // Caller must be a variable @@ -59,7 +62,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { then { let name = method_names[0].as_str(); - lint_deref(cx, &*name, variables[0].span, arg.span); + lint_deref(cx, &*name, &variables[0], variables[0].span, arg.span); } } } @@ -72,7 +75,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { if args.len() == 1; if let ExprKind::Path(QPath::Resolved(None, _)) = args[0].kind; // Caller must call only one other function (deref or deref_mut) - // otherwise it can lead to error prone suggestions (ex: &*a.len()) + // otherwise it can lead to error prone suggestions (ie: &*a.len()) let (method_names, arg_list, _) = method_calls(init, 2); if method_names.len() == 1; // Caller must be a variable @@ -82,7 +85,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { then { let name = method_name.ident.as_str(); - lint_deref(cx, &*name, args[0].span, init.span); + lint_deref(cx, &*name, init, args[0].span, init.span); } } } @@ -96,16 +99,16 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { if_chain! { if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.kind; if args.len() == 1; + if let ExprKind::Path(QPath::Resolved(None, _)) = args[0].kind; if let Some(parent) = get_parent_expr(cx, &expr); then { - // Call and MethodCall exprs are better reported using statements match parent.kind { - ExprKind::Call(_, _) => return, - ExprKind::MethodCall(_, _, _) => return, + // Already linted using statements + ExprKind::Call(_, _) | ExprKind::MethodCall(_, _, _) => (), _ => { let name = method_name.ident.as_str(); - lint_deref(cx, &*name, args[0].span, expr.span); + lint_deref(cx, &*name, &args[0], args[0].span, expr.span); } } } @@ -113,29 +116,37 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { } } -fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, var_span: Span, expr_span: Span) { +fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) { match fn_name { "deref" => { - span_lint_and_sugg( - cx, - EXPLICIT_DEREF_METHOD, - expr_span, - "explicit deref method call", - "try this", - format!("&*{}", &snippet(cx, var_span, "..")), - Applicability::MachineApplicable, - ); + if get_trait_def_id(cx, &paths::DEREF_TRAIT) + .map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[])) + { + span_lint_and_sugg( + cx, + EXPLICIT_DEREF_METHOD, + expr_span, + "explicit deref method call", + "try this", + format!("&*{}", &snippet(cx, var_span, "..")), + Applicability::MachineApplicable, + ); + } }, "deref_mut" => { - span_lint_and_sugg( - cx, - EXPLICIT_DEREF_METHOD, - expr_span, - "explicit deref_mut method call", - "try this", - format!("&mut *{}", &snippet(cx, var_span, "..")), - Applicability::MachineApplicable, - ); + if get_trait_def_id(cx, &paths::DEREF_MUT_TRAIT) + .map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[])) + { + span_lint_and_sugg( + cx, + EXPLICIT_DEREF_METHOD, + expr_span, + "explicit deref_mut method call", + "try this", + format!("&mut *{}", &snippet(cx, var_span, "..")), + Applicability::MachineApplicable, + ); + } }, _ => (), } diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index d93f8a1e5609c..a49299e5f6fb3 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -25,7 +25,9 @@ pub const CSTRING: [&str; 4] = ["std", "ffi", "c_str", "CString"]; pub const CSTRING_AS_C_STR: [&str; 5] = ["std", "ffi", "c_str", "CString", "as_c_str"]; pub const DEFAULT_TRAIT: [&str; 3] = ["core", "default", "Default"]; pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"]; +pub const DEREF_MUT_TRAIT: [&str; 4] = ["core", "ops", "deref", "DerefMut"]; pub const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"]; +pub const DEREF_TRAIT: [&str; 4] = ["core", "ops", "deref", "Deref"]; pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"]; pub const DISPLAY_FMT_METHOD: [&str; 4] = ["core", "fmt", "Display", "fmt"]; pub const DISPLAY_TRAIT: [&str; 3] = ["core", "fmt", "Display"]; diff --git a/tests/ui/dereference.fixed b/tests/ui/dereference.fixed new file mode 100644 index 0000000000000..292324eeacbad --- /dev/null +++ b/tests/ui/dereference.fixed @@ -0,0 +1,79 @@ +// run-rustfix + +#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)] +#![warn(clippy::explicit_deref_method)] + +use std::ops::{Deref, DerefMut}; + +fn concat(deref_str: &str) -> String { + format!("{}bar", deref_str) +} + +fn just_return(deref_str: &str) -> &str { + deref_str +} + +fn main() { + let a: &mut String = &mut String::from("foo"); + + // these should require linting + + let b: &str = &*a; + + let b: &mut str = &mut *a; + + // both derefs should get linted here + let b: String = format!("{}, {}", &*a, &*a); + + println!("{}", &*a); + + #[allow(clippy::match_single_binding)] + match &*a { + _ => (), + } + + let b: String = concat(&*a); + + // following should not require linting + + let b = just_return(a).deref(); + + let b: String = concat(just_return(a).deref()); + + let b: String = a.deref().clone(); + + let b: usize = a.deref_mut().len(); + + let b: &usize = &a.deref().len(); + + let b: &str = a.deref().deref(); + + let b: &str = &*a; + + let b: &mut str = &mut *a; + + macro_rules! expr_deref { + ($body:expr) => { + $body.deref() + }; + } + let b: &str = expr_deref!(a); + + let opt_a = Some(a); + let b = opt_a.unwrap().deref(); + + // The struct does not implement Deref trait + #[derive(Copy, Clone)] + struct NoLint(u32); + impl NoLint { + pub fn deref(self) -> u32 { + self.0 + } + pub fn deref_mut(self) -> u32 { + self.0 + } + } + let no_lint = NoLint(42); + let b = no_lint.deref(); + let b = no_lint.deref_mut(); +} diff --git a/tests/ui/dereference.rs b/tests/ui/dereference.rs index 5de201fb22f0b..25e1c29e7fa5f 100644 --- a/tests/ui/dereference.rs +++ b/tests/ui/dereference.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)] #![warn(clippy::explicit_deref_method)] @@ -59,4 +61,19 @@ fn main() { let opt_a = Some(a); let b = opt_a.unwrap().deref(); + + // The struct does not implement Deref trait + #[derive(Copy, Clone)] + struct NoLint(u32); + impl NoLint { + pub fn deref(self) -> u32 { + self.0 + } + pub fn deref_mut(self) -> u32 { + self.0 + } + } + let no_lint = NoLint(42); + let b = no_lint.deref(); + let b = no_lint.deref_mut(); } diff --git a/tests/ui/dereference.stderr b/tests/ui/dereference.stderr index 7e10add40b180..97fab3a34e010 100644 --- a/tests/ui/dereference.stderr +++ b/tests/ui/dereference.stderr @@ -1,5 +1,5 @@ error: explicit deref method call - --> $DIR/dereference.rs:19:19 + --> $DIR/dereference.rs:21:19 | LL | let b: &str = a.deref(); | ^^^^^^^^^ help: try this: `&*a` @@ -7,37 +7,37 @@ LL | let b: &str = a.deref(); = note: `-D clippy::explicit-deref-method` implied by `-D warnings` error: explicit deref_mut method call - --> $DIR/dereference.rs:21:23 + --> $DIR/dereference.rs:23:23 | LL | let b: &mut str = a.deref_mut(); | ^^^^^^^^^^^^^ help: try this: `&mut *a` error: explicit deref method call - --> $DIR/dereference.rs:24:39 + --> $DIR/dereference.rs:26:39 | LL | let b: String = format!("{}, {}", a.deref(), a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:24:50 + --> $DIR/dereference.rs:26:50 | LL | let b: String = format!("{}, {}", a.deref(), a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:26:20 + --> $DIR/dereference.rs:28:20 | LL | println!("{}", a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:29:11 + --> $DIR/dereference.rs:31:11 | LL | match a.deref() { | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:33:28 + --> $DIR/dereference.rs:35:28 | LL | let b: String = concat(a.deref()); | ^^^^^^^^^ help: try this: `&*a` From 72b9ae2a10b13a209f09c5ace0a7b9763b740e62 Mon Sep 17 00:00:00 2001 From: ThibsG Date: Sat, 7 Mar 2020 15:33:27 +0100 Subject: [PATCH 11/13] Use only check_expr with parent expr and precedence --- CHANGELOG.md | 2 +- clippy_lints/src/dereference.rs | 103 +++++++++----------------------- clippy_lints/src/lib.rs | 4 +- clippy_lints/src/utils/paths.rs | 2 - src/lintlist/mod.rs | 2 +- tests/ui/dereference.fixed | 20 ++++--- tests/ui/dereference.rs | 18 +++--- tests/ui/dereference.stderr | 28 ++++++++- 8 files changed, 79 insertions(+), 100 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb5a6af97d361..2c4bb42566d06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1256,7 +1256,7 @@ Released 2018-09-13 [`expect_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#expect_fun_call [`expl_impl_clone_on_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#expl_impl_clone_on_copy [`explicit_counter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_counter_loop -[`explicit_deref_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_deref_method +[`explicit_deref_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_deref_methods [`explicit_into_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_into_iter_loop [`explicit_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_iter_loop [`explicit_write`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_write diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index d11614d489d23..f5d82c549163c 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -1,10 +1,8 @@ -use crate::utils::{ - get_parent_expr, get_trait_def_id, implements_trait, method_calls, paths, snippet, span_lint_and_sugg, -}; +use crate::utils::{get_parent_expr, implements_trait, snippet, span_lint_and_sugg}; use if_chain::if_chain; +use rustc_ast::util::parser::ExprPrecedence; use rustc_errors::Applicability; -use rustc_hir as hir; -use rustc_hir::{Expr, ExprKind, QPath, StmtKind}; +use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; @@ -31,100 +29,52 @@ declare_clippy_lint! { /// ```rust,ignore /// let _ = d.unwrap().deref(); /// ``` - pub EXPLICIT_DEREF_METHOD, + pub EXPLICIT_DEREF_METHODS, pedantic, "Explicit use of deref or deref_mut method while not in a method chain." } declare_lint_pass!(Dereferencing => [ - EXPLICIT_DEREF_METHOD + EXPLICIT_DEREF_METHODS ]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { - fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx hir::Stmt<'_>) { - if_chain! { - if let StmtKind::Local(ref local) = stmt.kind; - if let Some(ref init) = local.init; - - then { - match init.kind { - ExprKind::Call(ref _method, args) => { - for arg in args { - if_chain! { - // Caller must call only one other function (deref or deref_mut) - // otherwise it can lead to error prone suggestions (ie: &*a.len()) - let (method_names, arg_list, _) = method_calls(arg, 2); - if method_names.len() == 1; - // Caller must be a variable - let variables = arg_list[0]; - if variables.len() == 1; - if let ExprKind::Path(QPath::Resolved(None, _)) = variables[0].kind; - - then { - let name = method_names[0].as_str(); - lint_deref(cx, &*name, &variables[0], variables[0].span, arg.span); - } - } - } - } - ExprKind::MethodCall(ref method_name, _, ref args) => { - if init.span.from_expansion() { - return; - } - if_chain! { - if args.len() == 1; - if let ExprKind::Path(QPath::Resolved(None, _)) = args[0].kind; - // Caller must call only one other function (deref or deref_mut) - // otherwise it can lead to error prone suggestions (ie: &*a.len()) - let (method_names, arg_list, _) = method_calls(init, 2); - if method_names.len() == 1; - // Caller must be a variable - let variables = arg_list[0]; - if variables.len() == 1; - if let ExprKind::Path(QPath::Resolved(None, _)) = variables[0].kind; - - then { - let name = method_name.ident.as_str(); - lint_deref(cx, &*name, init, args[0].span, init.span); - } - } - } - _ => () - } - } - } - } - fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { if_chain! { + if !expr.span.from_expansion(); if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.kind; if args.len() == 1; - if let ExprKind::Path(QPath::Resolved(None, _)) = args[0].kind; - if let Some(parent) = get_parent_expr(cx, &expr); then { - match parent.kind { - // Already linted using statements - ExprKind::Call(_, _) | ExprKind::MethodCall(_, _, _) => (), - _ => { - let name = method_name.ident.as_str(); - lint_deref(cx, &*name, &args[0], args[0].span, expr.span); + if let Some(parent_expr) = get_parent_expr(cx, expr) { + // Check if we have the whole call chain here + if let ExprKind::MethodCall(..) = parent_expr.kind { + return; + } + // Check for unary precedence + if let ExprPrecedence::Unary = parent_expr.precedence() { + return; } } + let name = method_name.ident.as_str(); + lint_deref(cx, &*name, &args[0], args[0].span, expr.span); } } } } -fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) { - match fn_name { +fn lint_deref(cx: &LateContext<'_, '_>, method_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) { + match method_name { "deref" => { - if get_trait_def_id(cx, &paths::DEREF_TRAIT) + if cx + .tcx + .lang_items() + .deref_trait() .map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[])) { span_lint_and_sugg( cx, - EXPLICIT_DEREF_METHOD, + EXPLICIT_DEREF_METHODS, expr_span, "explicit deref method call", "try this", @@ -134,12 +84,15 @@ fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, call_expr: &Expr<'_>, var } }, "deref_mut" => { - if get_trait_def_id(cx, &paths::DEREF_MUT_TRAIT) + if cx + .tcx + .lang_items() + .deref_mut_trait() .map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[])) { span_lint_and_sugg( cx, - EXPLICIT_DEREF_METHOD, + EXPLICIT_DEREF_METHODS, expr_span, "explicit deref_mut method call", "try this", diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 6443caa89d62c..b6b6a0e7d21fb 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -513,7 +513,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: ©_iterator::COPY_ITERATOR, &dbg_macro::DBG_MACRO, &default_trait_access::DEFAULT_TRAIT_ACCESS, - &dereference::EXPLICIT_DEREF_METHOD, + &dereference::EXPLICIT_DEREF_METHODS, &derive::DERIVE_HASH_XOR_EQ, &derive::EXPL_IMPL_CLONE_ON_COPY, &doc::DOC_MARKDOWN, @@ -1091,7 +1091,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&copies::SAME_FUNCTIONS_IN_IF_CONDITION), LintId::of(©_iterator::COPY_ITERATOR), LintId::of(&default_trait_access::DEFAULT_TRAIT_ACCESS), - LintId::of(&dereference::EXPLICIT_DEREF_METHOD), + LintId::of(&dereference::EXPLICIT_DEREF_METHODS), LintId::of(&derive::EXPL_IMPL_CLONE_ON_COPY), LintId::of(&doc::DOC_MARKDOWN), LintId::of(&doc::MISSING_ERRORS_DOC), diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index a49299e5f6fb3..d93f8a1e5609c 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -25,9 +25,7 @@ pub const CSTRING: [&str; 4] = ["std", "ffi", "c_str", "CString"]; pub const CSTRING_AS_C_STR: [&str; 5] = ["std", "ffi", "c_str", "CString", "as_c_str"]; pub const DEFAULT_TRAIT: [&str; 3] = ["core", "default", "Default"]; pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"]; -pub const DEREF_MUT_TRAIT: [&str; 4] = ["core", "ops", "deref", "DerefMut"]; pub const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"]; -pub const DEREF_TRAIT: [&str; 4] = ["core", "ops", "deref", "Deref"]; pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"]; pub const DISPLAY_FMT_METHOD: [&str; 4] = ["core", "fmt", "Display", "fmt"]; pub const DISPLAY_TRAIT: [&str; 3] = ["core", "fmt", "Display"]; diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 3f1d31f0302c8..1d147e01066fc 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -529,7 +529,7 @@ pub static ref ALL_LINTS: Vec = vec![ module: "loops", }, Lint { - name: "explicit_deref_method", + name: "explicit_deref_methods", group: "pedantic", desc: "Explicit use of deref or deref_mut method while not in a method chain.", deprecation: None, diff --git a/tests/ui/dereference.fixed b/tests/ui/dereference.fixed index 292324eeacbad..51e3d512cdba9 100644 --- a/tests/ui/dereference.fixed +++ b/tests/ui/dereference.fixed @@ -1,7 +1,7 @@ // run-rustfix #![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)] -#![warn(clippy::explicit_deref_method)] +#![warn(clippy::explicit_deref_methods)] use std::ops::{Deref, DerefMut}; @@ -34,11 +34,18 @@ fn main() { let b: String = concat(&*a); - // following should not require linting + let b = &*just_return(a); + + let b: String = concat(&*just_return(a)); - let b = just_return(a).deref(); + let b: &str = &*a.deref(); - let b: String = concat(just_return(a).deref()); + let opt_a = Some(a.clone()); + let b = &*opt_a.unwrap(); + + // following should not require linting + + let b: &str = &*a.deref(); let b: String = a.deref().clone(); @@ -46,8 +53,6 @@ fn main() { let b: &usize = &a.deref().len(); - let b: &str = a.deref().deref(); - let b: &str = &*a; let b: &mut str = &mut *a; @@ -59,9 +64,6 @@ fn main() { } let b: &str = expr_deref!(a); - let opt_a = Some(a); - let b = opt_a.unwrap().deref(); - // The struct does not implement Deref trait #[derive(Copy, Clone)] struct NoLint(u32); diff --git a/tests/ui/dereference.rs b/tests/ui/dereference.rs index 25e1c29e7fa5f..3a59533741121 100644 --- a/tests/ui/dereference.rs +++ b/tests/ui/dereference.rs @@ -1,7 +1,7 @@ // run-rustfix #![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)] -#![warn(clippy::explicit_deref_method)] +#![warn(clippy::explicit_deref_methods)] use std::ops::{Deref, DerefMut}; @@ -34,20 +34,25 @@ fn main() { let b: String = concat(a.deref()); - // following should not require linting - let b = just_return(a).deref(); let b: String = concat(just_return(a).deref()); + let b: &str = a.deref().deref(); + + let opt_a = Some(a.clone()); + let b = opt_a.unwrap().deref(); + + // following should not require linting + + let b: &str = &*a.deref(); + let b: String = a.deref().clone(); let b: usize = a.deref_mut().len(); let b: &usize = &a.deref().len(); - let b: &str = a.deref().deref(); - let b: &str = &*a; let b: &mut str = &mut *a; @@ -59,9 +64,6 @@ fn main() { } let b: &str = expr_deref!(a); - let opt_a = Some(a); - let b = opt_a.unwrap().deref(); - // The struct does not implement Deref trait #[derive(Copy, Clone)] struct NoLint(u32); diff --git a/tests/ui/dereference.stderr b/tests/ui/dereference.stderr index 97fab3a34e010..d159214db2ffc 100644 --- a/tests/ui/dereference.stderr +++ b/tests/ui/dereference.stderr @@ -4,7 +4,7 @@ error: explicit deref method call LL | let b: &str = a.deref(); | ^^^^^^^^^ help: try this: `&*a` | - = note: `-D clippy::explicit-deref-method` implied by `-D warnings` + = note: `-D clippy::explicit-deref-methods` implied by `-D warnings` error: explicit deref_mut method call --> $DIR/dereference.rs:23:23 @@ -42,5 +42,29 @@ error: explicit deref method call LL | let b: String = concat(a.deref()); | ^^^^^^^^^ help: try this: `&*a` -error: aborting due to 7 previous errors +error: explicit deref method call + --> $DIR/dereference.rs:37:13 + | +LL | let b = just_return(a).deref(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*just_return(a)` + +error: explicit deref method call + --> $DIR/dereference.rs:39:28 + | +LL | let b: String = concat(just_return(a).deref()); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*just_return(a)` + +error: explicit deref method call + --> $DIR/dereference.rs:41:19 + | +LL | let b: &str = a.deref().deref(); + | ^^^^^^^^^^^^^^^^^ help: try this: `&*a.deref()` + +error: explicit deref method call + --> $DIR/dereference.rs:44:13 + | +LL | let b = opt_a.unwrap().deref(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*opt_a.unwrap()` + +error: aborting due to 11 previous errors From 3c2bbcf00e3b97ab1fb03665b4afc7b7df0cd25e Mon Sep 17 00:00:00 2001 From: ThibsG Date: Sat, 21 Mar 2020 19:34:56 +0100 Subject: [PATCH 12/13] Better precedence case management + more tests --- clippy_lints/src/dereference.rs | 15 +++++++++++---- clippy_lints/src/lib.rs | 1 + tests/ui/dereference.fixed | 12 ++++++++++++ tests/ui/dereference.rs | 12 ++++++++++++ tests/ui/dereference.stderr | 22 +++++++++++----------- 5 files changed, 47 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index f5d82c549163c..68ec07e2bcb0f 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -1,6 +1,6 @@ use crate::utils::{get_parent_expr, implements_trait, snippet, span_lint_and_sugg}; use if_chain::if_chain; -use rustc_ast::util::parser::ExprPrecedence; +use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX, PREC_PREFIX}; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; @@ -51,9 +51,16 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { if let ExprKind::MethodCall(..) = parent_expr.kind { return; } - // Check for unary precedence - if let ExprPrecedence::Unary = parent_expr.precedence() { - return; + // Check for Expr that we don't want to be linted + let precedence = parent_expr.precedence(); + match precedence { + // Lint a Call is ok though + ExprPrecedence::Call | ExprPrecedence::AddrOf => (), + _ => { + if precedence.order() >= PREC_PREFIX && precedence.order() <= PREC_POSTFIX { + return; + } + } } } let name = method_name.ident.as_str(); diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index b6b6a0e7d21fb..de1bab3a0b940 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -191,6 +191,7 @@ mod copies; mod copy_iterator; mod dbg_macro; mod default_trait_access; +mod dereference; mod derive; mod doc; mod double_comparison; diff --git a/tests/ui/dereference.fixed b/tests/ui/dereference.fixed index 51e3d512cdba9..459ca91b93b9e 100644 --- a/tests/ui/dereference.fixed +++ b/tests/ui/dereference.fixed @@ -13,6 +13,15 @@ fn just_return(deref_str: &str) -> &str { deref_str } +struct CustomVec(Vec); +impl Deref for CustomVec { + type Target = Vec; + + fn deref(&self) -> &Vec { + &self.0 + } +} + fn main() { let a: &mut String = &mut String::from("foo"); @@ -45,6 +54,9 @@ fn main() { // following should not require linting + let cv = CustomVec(vec![0, 42]); + let c = cv.deref()[0]; + let b: &str = &*a.deref(); let b: String = a.deref().clone(); diff --git a/tests/ui/dereference.rs b/tests/ui/dereference.rs index 3a59533741121..8dc5272e67fa5 100644 --- a/tests/ui/dereference.rs +++ b/tests/ui/dereference.rs @@ -13,6 +13,15 @@ fn just_return(deref_str: &str) -> &str { deref_str } +struct CustomVec(Vec); +impl Deref for CustomVec { + type Target = Vec; + + fn deref(&self) -> &Vec { + &self.0 + } +} + fn main() { let a: &mut String = &mut String::from("foo"); @@ -45,6 +54,9 @@ fn main() { // following should not require linting + let cv = CustomVec(vec![0, 42]); + let c = cv.deref()[0]; + let b: &str = &*a.deref(); let b: String = a.deref().clone(); diff --git a/tests/ui/dereference.stderr b/tests/ui/dereference.stderr index d159214db2ffc..d26b462a43362 100644 --- a/tests/ui/dereference.stderr +++ b/tests/ui/dereference.stderr @@ -1,5 +1,5 @@ error: explicit deref method call - --> $DIR/dereference.rs:21:19 + --> $DIR/dereference.rs:30:19 | LL | let b: &str = a.deref(); | ^^^^^^^^^ help: try this: `&*a` @@ -7,61 +7,61 @@ LL | let b: &str = a.deref(); = note: `-D clippy::explicit-deref-methods` implied by `-D warnings` error: explicit deref_mut method call - --> $DIR/dereference.rs:23:23 + --> $DIR/dereference.rs:32:23 | LL | let b: &mut str = a.deref_mut(); | ^^^^^^^^^^^^^ help: try this: `&mut *a` error: explicit deref method call - --> $DIR/dereference.rs:26:39 + --> $DIR/dereference.rs:35:39 | LL | let b: String = format!("{}, {}", a.deref(), a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:26:50 + --> $DIR/dereference.rs:35:50 | LL | let b: String = format!("{}, {}", a.deref(), a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:28:20 + --> $DIR/dereference.rs:37:20 | LL | println!("{}", a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:31:11 + --> $DIR/dereference.rs:40:11 | LL | match a.deref() { | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:35:28 + --> $DIR/dereference.rs:44:28 | LL | let b: String = concat(a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:37:13 + --> $DIR/dereference.rs:46:13 | LL | let b = just_return(a).deref(); | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*just_return(a)` error: explicit deref method call - --> $DIR/dereference.rs:39:28 + --> $DIR/dereference.rs:48:28 | LL | let b: String = concat(just_return(a).deref()); | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*just_return(a)` error: explicit deref method call - --> $DIR/dereference.rs:41:19 + --> $DIR/dereference.rs:50:19 | LL | let b: &str = a.deref().deref(); | ^^^^^^^^^^^^^^^^^ help: try this: `&*a.deref()` error: explicit deref method call - --> $DIR/dereference.rs:44:13 + --> $DIR/dereference.rs:53:13 | LL | let b = opt_a.unwrap().deref(); | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*opt_a.unwrap()` From ce372c17cd705c9537e07662d5b2ccdbc41741b3 Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Wed, 15 Apr 2020 17:58:26 +0200 Subject: [PATCH 13/13] Change default many single char names threshold --- clippy_lints/src/utils/conf.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 722104e5b5249..5ad5983872d7e 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -135,7 +135,7 @@ define_Conf! { /// Lint: TYPE_COMPLEXITY. The maximum complexity a type can have (type_complexity_threshold, "type_complexity_threshold": u64, 250), /// Lint: MANY_SINGLE_CHAR_NAMES. The maximum number of single char bindings a scope may have - (single_char_binding_names_threshold, "single_char_binding_names_threshold": u64, 5), + (single_char_binding_names_threshold, "single_char_binding_names_threshold": u64, 4), /// Lint: BOXED_LOCAL. The maximum size of objects (in bytes) that will be linted. Larger objects are ok on the heap (too_large_for_stack, "too_large_for_stack": u64, 200), /// Lint: ENUM_VARIANT_NAMES. The minimum number of enum variants for the lints about variant names to trigger