diff --git a/CHANGELOG.md b/CHANGELOG.md index b7ac3cace2048..65eab69667f29 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_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 @@ -1319,6 +1320,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/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/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs new file mode 100644 index 0000000000000..68ec07e2bcb0f --- /dev/null +++ b/clippy_lints/src/dereference.rs @@ -0,0 +1,113 @@ +use crate::utils::{get_parent_expr, implements_trait, snippet, span_lint_and_sugg}; +use if_chain::if_chain; +use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX, PREC_PREFIX}; +use rustc_errors::Applicability; +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; + +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 + /// 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; + /// ``` + /// + /// This lint excludes + /// ```rust,ignore + /// let _ = d.unwrap().deref(); + /// ``` + 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_METHODS +]); + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { + 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; + + then { + 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 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(); + lint_deref(cx, &*name, &args[0], args[0].span, expr.span); + } + } + } +} + +fn lint_deref(cx: &LateContext<'_, '_>, method_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) { + match method_name { + "deref" => { + 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_METHODS, + expr_span, + "explicit deref method call", + "try this", + format!("&*{}", &snippet(cx, var_span, "..")), + Applicability::MachineApplicable, + ); + } + }, + "deref_mut" => { + 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_METHODS, + expr_span, + "explicit deref_mut method call", + "try this", + format!("&mut *{}", &snippet(cx, var_span, "..")), + Applicability::MachineApplicable, + ); + } + }, + _ => (), + } +} diff --git a/clippy_lints/src/large_const_arrays.rs b/clippy_lints/src/large_const_arrays.rs new file mode 100644 index 0000000000000..4c3030cf2e7c4 --- /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_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; + +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/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/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index cb9fcfca8a1c3..1158e70b7a3f6 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; @@ -231,6 +232,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; @@ -513,6 +515,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_METHODS, &derive::DERIVE_HASH_XOR_EQ, &derive::EXPL_IMPL_CLONE_ON_COPY, &doc::DOC_MARKDOWN, @@ -580,6 +583,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 +1028,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); @@ -1039,6 +1044,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::Dereferencing); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(&arithmetic::FLOAT_ARITHMETIC), @@ -1089,6 +1095,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_METHODS), LintId::of(&derive::EXPL_IMPL_CLONE_ON_COPY), LintId::of(&doc::DOC_MARKDOWN), LintId::of(&doc::MISSING_ERRORS_DOC), @@ -1221,6 +1228,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), @@ -1652,6 +1660,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/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() diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 722104e5b5249..4b81ff33495c7 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 @@ -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/src/lintlist/mod.rs b/src/lintlist/mod.rs index 935ea180ebe21..d4602a3ad8e6a 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_methods", + 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", @@ -846,7 +853,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", }, @@ -941,6 +948,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-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() {} diff --git a/tests/ui/dereference.fixed b/tests/ui/dereference.fixed new file mode 100644 index 0000000000000..459ca91b93b9e --- /dev/null +++ b/tests/ui/dereference.fixed @@ -0,0 +1,93 @@ +// run-rustfix + +#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)] +#![warn(clippy::explicit_deref_methods)] + +use std::ops::{Deref, DerefMut}; + +fn concat(deref_str: &str) -> String { + format!("{}bar", deref_str) +} + +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"); + + // 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); + + let b = &*just_return(a); + + let b: String = concat(&*just_return(a)); + + let b: &str = &*a.deref(); + + let opt_a = Some(a.clone()); + let b = &*opt_a.unwrap(); + + // 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(); + + let b: usize = a.deref_mut().len(); + + let b: &usize = &a.deref().len(); + + let b: &str = &*a; + + let b: &mut str = &mut *a; + + macro_rules! expr_deref { + ($body:expr) => { + $body.deref() + }; + } + let b: &str = expr_deref!(a); + + // 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 new file mode 100644 index 0000000000000..8dc5272e67fa5 --- /dev/null +++ b/tests/ui/dereference.rs @@ -0,0 +1,93 @@ +// run-rustfix + +#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)] +#![warn(clippy::explicit_deref_methods)] + +use std::ops::{Deref, DerefMut}; + +fn concat(deref_str: &str) -> String { + format!("{}bar", deref_str) +} + +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"); + + // 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()); + + 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 cv = CustomVec(vec![0, 42]); + let c = cv.deref()[0]; + + 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; + + let b: &mut str = &mut *a; + + macro_rules! expr_deref { + ($body:expr) => { + $body.deref() + }; + } + let b: &str = expr_deref!(a); + + // 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 new file mode 100644 index 0000000000000..d26b462a43362 --- /dev/null +++ b/tests/ui/dereference.stderr @@ -0,0 +1,70 @@ +error: explicit deref method call + --> $DIR/dereference.rs:30:19 + | +LL | let b: &str = a.deref(); + | ^^^^^^^^^ help: try this: `&*a` + | + = note: `-D clippy::explicit-deref-methods` implied by `-D warnings` + +error: explicit deref_mut method call + --> $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:35:39 + | +LL | let b: String = format!("{}, {}", a.deref(), a.deref()); + | ^^^^^^^^^ help: try this: `&*a` + +error: explicit deref method call + --> $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:37:20 + | +LL | println!("{}", a.deref()); + | ^^^^^^^^^ help: try this: `&*a` + +error: explicit deref method call + --> $DIR/dereference.rs:40:11 + | +LL | match a.deref() { + | ^^^^^^^^^ help: try this: `&*a` + +error: explicit deref method call + --> $DIR/dereference.rs:44:28 + | +LL | let b: String = concat(a.deref()); + | ^^^^^^^^^ help: try this: `&*a` + +error: explicit deref method call + --> $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: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:50:19 + | +LL | let b: &str = a.deref().deref(); + | ^^^^^^^^^^^^^^^^^ help: try this: `&*a.deref()` + +error: explicit deref method call + --> $DIR/dereference.rs:53:13 + | +LL | let b = opt_a.unwrap().deref(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*opt_a.unwrap()` + +error: aborting due to 11 previous errors + 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 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 + 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]> }, 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