diff options
| author | flip1995 <hello@philkrones.com> | 2022-06-16 17:39:06 +0200 |
|---|---|---|
| committer | flip1995 <hello@philkrones.com> | 2022-06-16 17:39:06 +0200 |
| commit | f8f9d01c2ad0dff565bdd60feeb4cbd09dada8cd (patch) | |
| tree | c87b416454f6d0cbc909fd94d8af6d4a951abfb3 /clippy_lints/src | |
| parent | bd071bf5b2395edced30dfc5197eafb355c49b4d (diff) | |
| download | rust-f8f9d01c2ad0dff565bdd60feeb4cbd09dada8cd.tar.gz rust-f8f9d01c2ad0dff565bdd60feeb4cbd09dada8cd.zip | |
Merge commit 'd7b5cbf065b88830ca519adcb73fad4c0d24b1c7' into clippyup
Diffstat (limited to 'clippy_lints/src')
93 files changed, 1653 insertions, 1088 deletions
diff --git a/clippy_lints/src/assertions_on_constants.rs b/clippy_lints/src/assertions_on_constants.rs index c82837746bd..2705ffffdcb 100644 --- a/clippy_lints/src/assertions_on_constants.rs +++ b/clippy_lints/src/assertions_on_constants.rs @@ -14,9 +14,6 @@ declare_clippy_lint! { /// Will be optimized out by the compiler or should probably be replaced by a /// `panic!()` or `unreachable!()` /// - /// ### Known problems - /// None - /// /// ### Example /// ```rust,ignore /// assert!(false) diff --git a/clippy_lints/src/async_yields_async.rs b/clippy_lints/src/async_yields_async.rs index 0619490e73c..27c2896e1e5 100644 --- a/clippy_lints/src/async_yields_async.rs +++ b/clippy_lints/src/async_yields_async.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::diagnostics::span_lint_hir_and_then; use clippy_utils::source::snippet; use clippy_utils::ty::implements_trait; use rustc_errors::Applicability; @@ -24,6 +24,7 @@ declare_clippy_lint! { /// }; /// } /// ``` + /// /// Use instead: /// ```rust /// async fn foo() {} @@ -63,9 +64,10 @@ impl<'tcx> LateLintPass<'tcx> for AsyncYieldsAsync { _ => None, }; if let Some(return_expr_span) = return_expr_span { - span_lint_and_then( + span_lint_hir_and_then( cx, ASYNC_YIELDS_ASYNC, + body.value.hir_id, return_expr_span, "an async construct yields a type which is itself awaitable", |db| { diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 770cb6a3d7b..ed12ad9c367 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -340,7 +340,7 @@ impl<'tcx> LateLintPass<'tcx> for Attributes { for lint in lint_list { match item.kind { ItemKind::Use(..) => { - if is_word(lint, sym!(unused_imports)) + if is_word(lint, sym::unused_imports) || is_word(lint, sym::deprecated) || is_word(lint, sym!(unreachable_pub)) || is_word(lint, sym!(unused)) @@ -355,7 +355,7 @@ impl<'tcx> LateLintPass<'tcx> for Attributes { } }, ItemKind::ExternCrate(..) => { - if is_word(lint, sym!(unused_imports)) && skip_unused_imports { + if is_word(lint, sym::unused_imports) && skip_unused_imports { return; } if is_word(lint, sym!(unused_extern_crates)) { diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs index 5b7c4591504..eee5f90d178 100644 --- a/clippy_lints/src/await_holding_invalid.rs +++ b/clippy_lints/src/await_holding_invalid.rs @@ -140,8 +140,6 @@ declare_clippy_lint! { /// from a memory access perspective but will cause bugs at runtime if they /// are held in such a way. /// - /// ### Known problems - /// /// ### Example /// /// ```toml diff --git a/clippy_lints/src/bool_assert_comparison.rs b/clippy_lints/src/bool_assert_comparison.rs index c50e214be28..95abe8aa59f 100644 --- a/clippy_lints/src/bool_assert_comparison.rs +++ b/clippy_lints/src/bool_assert_comparison.rs @@ -17,11 +17,12 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// assert_eq!("a".is_empty(), false); /// assert_ne!("a".is_empty(), true); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// assert!(!"a".is_empty()); /// ``` #[clippy::version = "1.53.0"] diff --git a/clippy_lints/src/borrow_deref_ref.rs b/clippy_lints/src/borrow_deref_ref.rs index ec2f31cf673..1582ec9ee5c 100644 --- a/clippy_lints/src/borrow_deref_ref.rs +++ b/clippy_lints/src/borrow_deref_ref.rs @@ -18,7 +18,7 @@ declare_clippy_lint! { /// Dereferencing and then borrowing a reference value has no effect in most cases. /// /// ### Known problems - /// false negative on such code: + /// False negative on such code: /// ``` /// let x = &12; /// let addr_x = &x as *const _ as usize; @@ -29,17 +29,20 @@ declare_clippy_lint! { /// /// ### Example /// ```rust + /// fn foo(_x: &str) {} + /// /// let s = &String::new(); /// - /// // Bad /// let a: &String = &* s; /// foo(&*s); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # fn foo(_x: &str) {} + /// # let s = &String::new(); /// let a: &String = s; /// foo(&**s); - /// - /// fn foo(_: &str){ } /// ``` #[clippy::version = "1.59.0"] pub BORROW_DEREF_REF, diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index daf3b7b4ce4..02c2f30a4dd 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -219,13 +219,14 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// fn fun() -> i32 { 1 } - /// let a = fun as i64; + /// let _ = fun as i64; + /// ``` /// - /// // Good - /// fn fun2() -> i32 { 1 } - /// let a = fun2 as usize; + /// Use instead: + /// ```rust + /// # fn fun() -> i32 { 1 } + /// let _ = fun as usize; /// ``` #[clippy::version = "pre 1.29.0"] pub FN_TO_NUMERIC_CAST, @@ -245,17 +246,19 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// fn fn1() -> i16 { /// 1 /// }; /// let _ = fn1 as i32; + /// ``` /// - /// // Better: Cast to usize first, then comment with the reason for the truncation - /// fn fn2() -> i16 { + /// Use instead: + /// ```rust + /// // Cast to usize first, then comment with the reason for the truncation + /// fn fn1() -> i16 { /// 1 /// }; - /// let fn_ptr = fn2 as usize; + /// let fn_ptr = fn1 as usize; /// let fn_ptr_truncated = fn_ptr as i32; /// ``` #[clippy::version = "pre 1.29.0"] @@ -277,19 +280,24 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad: fn1 is cast as `usize` + /// // fn1 is cast as `usize` /// fn fn1() -> u16 { /// 1 /// }; /// let _ = fn1 as usize; + /// ``` /// - /// // Good: maybe you intended to call the function? + /// Use instead: + /// ```rust + /// // maybe you intended to call the function? /// fn fn2() -> u16 { /// 1 /// }; /// let _ = fn2() as usize; /// - /// // Good: maybe you intended to cast it to a function type? + /// // or + /// + /// // maybe you intended to cast it to a function type? /// fn fn3() -> u16 { /// 1 /// } @@ -406,7 +414,7 @@ declare_clippy_lint! { /// enum E { X = 256 }; /// let _ = E::X as u8; /// ``` - #[clippy::version = "1.60.0"] + #[clippy::version = "1.61.0"] pub CAST_ENUM_TRUNCATION, suspicious, "casts from an enum type to an integral type which will truncate the value" @@ -451,7 +459,7 @@ declare_clippy_lint! { /// println!("{:?}", &*new_ptr); /// } /// ``` - #[clippy::version = "1.60.0"] + #[clippy::version = "1.61.0"] pub CAST_SLICE_DIFFERENT_SIZES, correctness, "casting using `as` between raw pointers to slices of types with different sizes" diff --git a/clippy_lints/src/checked_conversions.rs b/clippy_lints/src/checked_conversions.rs index 1010340c712..17fc81951f9 100644 --- a/clippy_lints/src/checked_conversions.rs +++ b/clippy_lints/src/checked_conversions.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::{meets_msrv, msrvs, SpanlessEq}; +use clippy_utils::{in_constant, meets_msrv, msrvs, SpanlessEq}; use if_chain::if_chain; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; @@ -22,18 +22,14 @@ declare_clippy_lint! { /// ### Example /// ```rust /// # let foo: u32 = 5; - /// # let _ = - /// foo <= i32::MAX as u32 - /// # ; + /// foo <= i32::MAX as u32; /// ``` /// - /// Could be written: - /// + /// Use instead: /// ```rust /// # let foo = 1; - /// # let _ = - /// i32::try_from(foo).is_ok() - /// # ; + /// # #[allow(unused)] + /// i32::try_from(foo).is_ok(); /// ``` #[clippy::version = "1.37.0"] pub CHECKED_CONVERSIONS, @@ -61,6 +57,7 @@ impl<'tcx> LateLintPass<'tcx> for CheckedConversions { } let result = if_chain! { + if !in_constant(cx, item.hir_id); if !in_external_macro(cx.sess(), item.span); if let ExprKind::Binary(op, left, right) = &item.kind; diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs index 3eceb848822..90430b71a0e 100644 --- a/clippy_lints/src/collapsible_if.rs +++ b/clippy_lints/src/collapsible_if.rs @@ -32,20 +32,20 @@ declare_clippy_lint! { /// makes code look more complex than it really is. /// /// ### Example - /// ```rust,ignore + /// ```rust + /// # let (x, y) = (true, true); /// if x { /// if y { - /// … + /// // … /// } /// } - /// /// ``` /// /// Use instead: - /// - /// ```rust,ignore + /// ```rust + /// # let (x, y) = (true, true); /// if x && y { - /// … + /// // … /// } /// ``` #[clippy::version = "pre 1.29.0"] diff --git a/clippy_lints/src/comparison_chain.rs b/clippy_lints/src/comparison_chain.rs index 913e081af3b..a05b41eb3ab 100644 --- a/clippy_lints/src/comparison_chain.rs +++ b/clippy_lints/src/comparison_chain.rs @@ -35,7 +35,6 @@ declare_clippy_lint! { /// ``` /// /// Use instead: - /// /// ```rust,ignore /// use std::cmp::Ordering; /// # fn a() {} diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index 1e9a1153011..1deff9684a1 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -1,18 +1,18 @@ use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_then}; use clippy_utils::source::{first_line_of_span, indent_of, reindent_multiline, snippet, snippet_opt}; use clippy_utils::{ - both, count_eq, eq_expr_value, get_enclosing_block, get_parent_expr, if_sequence, is_else_clause, is_lint_allowed, - search_same, ContainsName, SpanlessEq, SpanlessHash, + eq_expr_value, get_enclosing_block, hash_expr, hash_stmt, if_sequence, is_else_clause, is_lint_allowed, + search_same, ContainsName, HirEqInterExpr, SpanlessEq, }; -use if_chain::if_chain; -use rustc_data_structures::fx::FxHashSet; -use rustc_errors::{Applicability, Diagnostic}; -use rustc_hir::intravisit::{self, Visitor}; -use rustc_hir::{Block, Expr, ExprKind, HirId}; -use rustc_lint::{LateContext, LateLintPass, LintContext}; -use rustc_middle::hir::nested_filter; +use core::iter; +use rustc_errors::Applicability; +use rustc_hir::intravisit; +use rustc_hir::{BinOpKind, Block, Expr, ExprKind, HirId, Stmt, StmtKind}; +use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::{source_map::Span, symbol::Symbol, BytePos}; +use rustc_span::hygiene::walk_chain; +use rustc_span::source_map::SourceMap; +use rustc_span::{BytePos, Span, Symbol}; use std::borrow::Cow; declare_clippy_lint! { @@ -165,243 +165,315 @@ declare_lint_pass!(CopyAndPaste => [ impl<'tcx> LateLintPass<'tcx> for CopyAndPaste { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if !expr.span.from_expansion() { - if let ExprKind::If(_, _, _) = expr.kind { - // skip ifs directly in else, it will be checked in the parent if - if let Some(&Expr { - kind: ExprKind::If(_, _, Some(else_expr)), - .. - }) = get_parent_expr(cx, expr) - { - if else_expr.hir_id == expr.hir_id { - return; - } - } - - let (conds, blocks) = if_sequence(expr); - // Conditions - lint_same_cond(cx, &conds); - lint_same_fns_in_if_cond(cx, &conds); - // Block duplication - lint_same_then_else(cx, &conds, &blocks, conds.len() == blocks.len(), expr); + if !expr.span.from_expansion() && matches!(expr.kind, ExprKind::If(..)) && !is_else_clause(cx.tcx, expr) { + let (conds, blocks) = if_sequence(expr); + lint_same_cond(cx, &conds); + lint_same_fns_in_if_cond(cx, &conds); + let all_same = + !is_lint_allowed(cx, IF_SAME_THEN_ELSE, expr.hir_id) && lint_if_same_then_else(cx, &conds, &blocks); + if !all_same && conds.len() != blocks.len() { + lint_branches_sharing_code(cx, &conds, &blocks, expr); } } } } -/// Implementation of `BRANCHES_SHARING_CODE` and `IF_SAME_THEN_ELSE` if the blocks are equal. -fn lint_same_then_else<'tcx>( +/// Checks if the given expression is a let chain. +fn contains_let(e: &Expr<'_>) -> bool { + match e.kind { + ExprKind::Let(..) => true, + ExprKind::Binary(op, lhs, rhs) if op.node == BinOpKind::And => { + matches!(lhs.kind, ExprKind::Let(..)) || contains_let(rhs) + }, + _ => false, + } +} + +fn lint_if_same_then_else(cx: &LateContext<'_>, conds: &[&Expr<'_>], blocks: &[&Block<'_>]) -> bool { + let mut eq = SpanlessEq::new(cx); + blocks + .array_windows::<2>() + .enumerate() + .fold(true, |all_eq, (i, &[lhs, rhs])| { + if eq.eq_block(lhs, rhs) && !contains_let(conds[i]) && conds.get(i + 1).map_or(true, |e| !contains_let(e)) { + span_lint_and_note( + cx, + IF_SAME_THEN_ELSE, + lhs.span, + "this `if` has identical blocks", + Some(rhs.span), + "same as this", + ); + all_eq + } else { + false + } + }) +} + +fn lint_branches_sharing_code<'tcx>( cx: &LateContext<'tcx>, conds: &[&'tcx Expr<'_>], blocks: &[&Block<'tcx>], - has_conditional_else: bool, expr: &'tcx Expr<'_>, ) { // We only lint ifs with multiple blocks - if blocks.len() < 2 || is_else_clause(cx.tcx, expr) { - return; - } - - // Check if each block has shared code - let has_expr = blocks[0].expr.is_some(); - - let (start_eq, mut end_eq, expr_eq) = if let Some(block_eq) = scan_block_for_eq(cx, conds, blocks) { - (block_eq.start_eq, block_eq.end_eq, block_eq.expr_eq) - } else { + let &[first_block, ref blocks @ ..] = blocks else { return; }; - - // BRANCHES_SHARING_CODE prerequisites - if has_conditional_else || (start_eq == 0 && end_eq == 0 && (has_expr && !expr_eq)) { + let &[.., last_block] = blocks else { return; - } - - // Only the start is the same - if start_eq != 0 && end_eq == 0 && (!has_expr || !expr_eq) { - let block = blocks[0]; - let start_stmts = block.stmts.split_at(start_eq).0; - - let mut start_walker = UsedValueFinderVisitor::new(cx); - for stmt in start_stmts { - intravisit::walk_stmt(&mut start_walker, stmt); - } + }; - emit_branches_sharing_code_lint( - cx, - start_eq, - 0, - false, - check_for_warn_of_moved_symbol(cx, &start_walker.def_symbols, expr), - blocks, - expr, - ); - } else if end_eq != 0 || (has_expr && expr_eq) { - let block = blocks[blocks.len() - 1]; - let (start_stmts, block_stmts) = block.stmts.split_at(start_eq); - let (block_stmts, end_stmts) = block_stmts.split_at(block_stmts.len() - end_eq); + let res = scan_block_for_eq(cx, conds, first_block, blocks); + let sm = cx.tcx.sess.source_map(); + let start_suggestion = res.start_span(first_block, sm).map(|span| { + let first_line_span = first_line_of_span(cx, expr.span); + let replace_span = first_line_span.with_hi(span.hi()); + let cond_span = first_line_span.until(first_block.span); + let cond_snippet = reindent_multiline(snippet(cx, cond_span, "_"), false, None); + let cond_indent = indent_of(cx, cond_span); + let moved_snippet = reindent_multiline(snippet(cx, span, "_"), true, None); + let suggestion = moved_snippet.to_string() + "\n" + &cond_snippet + "{"; + let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, cond_indent); + (replace_span, suggestion.to_string()) + }); + let end_suggestion = res.end_span(last_block, sm).map(|span| { + let moved_snipped = reindent_multiline(snippet(cx, span, "_"), true, None); + let indent = indent_of(cx, expr.span.shrink_to_hi()); + let suggestion = "}\n".to_string() + &moved_snipped; + let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, indent); - // Scan start - let mut start_walker = UsedValueFinderVisitor::new(cx); - for stmt in start_stmts { - intravisit::walk_stmt(&mut start_walker, stmt); + let span = span.with_hi(last_block.span.hi()); + // Improve formatting if the inner block has indention (i.e. normal Rust formatting) + let test_span = Span::new(span.lo() - BytePos(4), span.lo(), span.ctxt(), span.parent()); + let span = if snippet_opt(cx, test_span).map_or(false, |snip| snip == " ") { + span.with_lo(test_span.lo()) + } else { + span + }; + (span, suggestion.to_string()) + }); + + let (span, msg, end_span) = match (&start_suggestion, &end_suggestion) { + (&Some((span, _)), &Some((end_span, _))) => ( + span, + "all if blocks contain the same code at both the start and the end", + Some(end_span), + ), + (&Some((span, _)), None) => (span, "all if blocks contain the same code at the start", None), + (None, &Some((span, _))) => (span, "all if blocks contain the same code at the end", None), + (None, None) => return, + }; + span_lint_and_then(cx, BRANCHES_SHARING_CODE, span, msg, |diag| { + if let Some(span) = end_span { + diag.span_note(span, "this code is shared at the end"); } - let mut moved_syms = start_walker.def_symbols; - - // Scan block - let mut block_walker = UsedValueFinderVisitor::new(cx); - for stmt in block_stmts { - intravisit::walk_stmt(&mut block_walker, stmt); + if let Some((span, sugg)) = start_suggestion { + diag.span_suggestion( + span, + "consider moving these statements before the if", + sugg, + Applicability::Unspecified, + ); } - let mut block_defs = block_walker.defs; - - // Scan moved stmts - let mut moved_start: Option<usize> = None; - let mut end_walker = UsedValueFinderVisitor::new(cx); - for (index, stmt) in end_stmts.iter().enumerate() { - intravisit::walk_stmt(&mut end_walker, stmt); - - for value in &end_walker.uses { - // Well we can't move this and all prev statements. So reset - if block_defs.contains(value) { - moved_start = Some(index + 1); - end_walker.defs.drain().for_each(|x| { - block_defs.insert(x); - }); - - end_walker.def_symbols.clear(); - } + if let Some((span, sugg)) = end_suggestion { + diag.span_suggestion( + span, + "consider moving these statements after the if", + sugg, + Applicability::Unspecified, + ); + if !cx.typeck_results().expr_ty(expr).is_unit() { + diag.note("the end suggestion probably needs some adjustments to use the expression result correctly"); } - - end_walker.uses.clear(); } - - if let Some(moved_start) = moved_start { - end_eq -= moved_start; + if check_for_warn_of_moved_symbol(cx, &res.moved_locals, expr) { + diag.warn("some moved values might need to be renamed to avoid wrong references"); } + }); +} - let end_linable = block.expr.map_or_else( - || end_eq != 0, - |expr| { - intravisit::walk_expr(&mut end_walker, expr); - end_walker.uses.iter().any(|x| !block_defs.contains(x)) - }, - ); - - if end_linable { - end_walker.def_symbols.drain().for_each(|x| { - moved_syms.insert(x); - }); +struct BlockEq { + /// The end of the range of equal stmts at the start. + start_end_eq: usize, + /// The start of the range of equal stmts at the end. + end_begin_eq: Option<usize>, + /// The name and id of every local which can be moved at the beginning and the end. + moved_locals: Vec<(HirId, Symbol)>, +} +impl BlockEq { + fn start_span(&self, b: &Block<'_>, sm: &SourceMap) -> Option<Span> { + match &b.stmts[..self.start_end_eq] { + [first, .., last] => Some(sm.stmt_span(first.span, b.span).to(sm.stmt_span(last.span, b.span))), + [s] => Some(sm.stmt_span(s.span, b.span)), + [] => None, } + } - emit_branches_sharing_code_lint( - cx, - start_eq, - end_eq, - end_linable, - check_for_warn_of_moved_symbol(cx, &moved_syms, expr), - blocks, - expr, - ); + fn end_span(&self, b: &Block<'_>, sm: &SourceMap) -> Option<Span> { + match (&b.stmts[b.stmts.len() - self.end_begin_eq?..], b.expr) { + ([first, .., last], None) => Some(sm.stmt_span(first.span, b.span).to(sm.stmt_span(last.span, b.span))), + ([first, ..], Some(last)) => Some(sm.stmt_span(first.span, b.span).to(sm.stmt_span(last.span, b.span))), + ([s], None) => Some(sm.stmt_span(s.span, b.span)), + ([], Some(e)) => Some(walk_chain(e.span, b.span.ctxt())), + ([], None) => None, + } } } -struct BlockEqual { - /// The amount statements that are equal from the start - start_eq: usize, - /// The amount statements that are equal from the end - end_eq: usize, - /// An indication if the block expressions are the same. This will also be true if both are - /// `None` - expr_eq: bool, +/// If the statement is a local, checks if the bound names match the expected list of names. +fn eq_binding_names(s: &Stmt<'_>, names: &[(HirId, Symbol)]) -> bool { + if let StmtKind::Local(l) = s.kind { + let mut i = 0usize; + let mut res = true; + l.pat.each_binding_or_first(&mut |_, _, _, name| { + if names.get(i).map_or(false, |&(_, n)| n == name.name) { + i += 1; + } else { + res = false; + } + }); + res && i == names.len() + } else { + false + } } -/// This function can also trigger the `IF_SAME_THEN_ELSE` in which case it'll return `None` to -/// abort any further processing and avoid duplicate lint triggers. -fn scan_block_for_eq(cx: &LateContext<'_>, conds: &[&Expr<'_>], blocks: &[&Block<'_>]) -> Option<BlockEqual> { - let mut start_eq = usize::MAX; - let mut end_eq = usize::MAX; - let mut expr_eq = true; - let mut iter = blocks.windows(2).enumerate(); - while let Some((i, &[block0, block1])) = iter.next() { - let l_stmts = block0.stmts; - let r_stmts = block1.stmts; - - // `SpanlessEq` now keeps track of the locals and is therefore context sensitive clippy#6752. - // The comparison therefore needs to be done in a way that builds the correct context. - let mut evaluator = SpanlessEq::new(cx); - let mut evaluator = evaluator.inter_expr(); - - let current_start_eq = count_eq(&mut l_stmts.iter(), &mut r_stmts.iter(), |l, r| evaluator.eq_stmt(l, r)); +/// Checks if the given statement should be considered equal to the statement in the same position +/// for each block. +fn eq_stmts( + stmt: &Stmt<'_>, + blocks: &[&Block<'_>], + get_stmt: impl for<'a> Fn(&'a Block<'a>) -> Option<&'a Stmt<'a>>, + eq: &mut HirEqInterExpr<'_, '_, '_>, + moved_bindings: &mut Vec<(HirId, Symbol)>, +) -> bool { + (if let StmtKind::Local(l) = stmt.kind { + let old_count = moved_bindings.len(); + l.pat.each_binding_or_first(&mut |_, id, _, name| { + moved_bindings.push((id, name.name)); + }); + let new_bindings = &moved_bindings[old_count..]; + blocks + .iter() + .all(|b| get_stmt(b).map_or(false, |s| eq_binding_names(s, new_bindings))) + } else { + true + }) && blocks + .iter() + .all(|b| get_stmt(b).map_or(false, |s| eq.eq_stmt(s, stmt))) +} - let current_end_eq = { - // We skip the middle statements which can't be equal - let end_comparison_count = l_stmts.len().min(r_stmts.len()) - current_start_eq; - let it1 = l_stmts.iter().skip(l_stmts.len() - end_comparison_count); - let it2 = r_stmts.iter().skip(r_stmts.len() - end_comparison_count); - it1.zip(it2) - .fold(0, |acc, (l, r)| if evaluator.eq_stmt(l, r) { acc + 1 } else { 0 }) +fn scan_block_for_eq(cx: &LateContext<'_>, _conds: &[&Expr<'_>], block: &Block<'_>, blocks: &[&Block<'_>]) -> BlockEq { + let mut eq = SpanlessEq::new(cx); + let mut eq = eq.inter_expr(); + let mut moved_locals = Vec::new(); + + let start_end_eq = block + .stmts + .iter() + .enumerate() + .find(|&(i, stmt)| !eq_stmts(stmt, blocks, |b| b.stmts.get(i), &mut eq, &mut moved_locals)) + .map_or(block.stmts.len(), |(i, _)| i); + + // Walk backwards through the final expression/statements so long as their hashes are equal. Note + // `SpanlessHash` treats all local references as equal allowing locals declared earlier in the block + // to match those in other blocks. e.g. If each block ends with the following the hash value will be + // the same even though each `x` binding will have a different `HirId`: + // let x = foo(); + // x + 50 + let expr_hash_eq = if let Some(e) = block.expr { + let hash = hash_expr(cx, e); + blocks + .iter() + .all(|b| b.expr.map_or(false, |e| hash_expr(cx, e) == hash)) + } else { + blocks.iter().all(|b| b.expr.is_none()) + }; + if !expr_hash_eq { + return BlockEq { + start_end_eq, + end_begin_eq: None, + moved_locals, }; - let block_expr_eq = both(&block0.expr, &block1.expr, |l, r| evaluator.eq_expr(l, r)); - - // IF_SAME_THEN_ELSE - if_chain! { - if block_expr_eq; - if l_stmts.len() == r_stmts.len(); - if l_stmts.len() == current_start_eq; - // `conds` may have one last item than `blocks`. - // Any `i` from `blocks.windows(2)` will exist in `conds`, but `i+1` may not exist on the last iteration. - if !matches!(conds[i].kind, ExprKind::Let(..)); - if !matches!(conds.get(i + 1).map(|e| &e.kind), Some(ExprKind::Let(..))); - if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, block0.hir_id); - if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, block1.hir_id); - then { - span_lint_and_note( - cx, - IF_SAME_THEN_ELSE, - block0.span, - "this `if` has identical blocks", - Some(block1.span), - "same as this", - ); - - return None; + } + let end_search_start = block.stmts[start_end_eq..] + .iter() + .rev() + .enumerate() + .find(|&(offset, stmt)| { + let hash = hash_stmt(cx, stmt); + blocks.iter().any(|b| { + b.stmts + // the bounds check will catch the underflow + .get(b.stmts.len().wrapping_sub(offset + 1)) + .map_or(true, |s| hash != hash_stmt(cx, s)) + }) + }) + .map_or(block.stmts.len() - start_end_eq, |(i, _)| i); + + let moved_locals_at_start = moved_locals.len(); + let mut i = end_search_start; + let end_begin_eq = block.stmts[block.stmts.len() - end_search_start..] + .iter() + .zip(iter::repeat_with(move || { + let x = i; + i -= 1; + x + })) + .fold(end_search_start, |init, (stmt, offset)| { + if eq_stmts( + stmt, + blocks, + |b| b.stmts.get(b.stmts.len() - offset), + &mut eq, + &mut moved_locals, + ) { + init + } else { + // Clear out all locals seen at the end so far. None of them can be moved. + let stmts = &blocks[0].stmts; + for stmt in &stmts[stmts.len() - init..=stmts.len() - offset] { + if let StmtKind::Local(l) = stmt.kind { + l.pat.each_binding_or_first(&mut |_, id, _, _| { + eq.locals.remove(&id); + }); + } + } + moved_locals.truncate(moved_locals_at_start); + offset - 1 + } + }); + if let Some(e) = block.expr { + for block in blocks { + if block.expr.map_or(false, |expr| !eq.eq_expr(expr, e)) { + moved_locals.truncate(moved_locals_at_start); + return BlockEq { + start_end_eq, + end_begin_eq: None, + moved_locals, + }; } } - - start_eq = start_eq.min(current_start_eq); - end_eq = end_eq.min(current_end_eq); - expr_eq &= block_expr_eq; - } - - if !expr_eq { - end_eq = 0; } - // Check if the regions are overlapping. Set `end_eq` to prevent the overlap - let min_block_size = blocks.iter().map(|x| x.stmts.len()).min().unwrap(); - if (start_eq + end_eq) > min_block_size { - end_eq = min_block_size - start_eq; + BlockEq { + start_end_eq, + end_begin_eq: Some(end_begin_eq), + moved_locals, } - - Some(BlockEqual { - start_eq, - end_eq, - expr_eq, - }) } -fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &FxHashSet<Symbol>, if_expr: &Expr<'_>) -> bool { +fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &[(HirId, Symbol)], if_expr: &Expr<'_>) -> bool { get_enclosing_block(cx, if_expr.hir_id).map_or(false, |block| { let ignore_span = block.span.shrink_to_lo().to(if_expr.span); symbols .iter() - .filter(|sym| !sym.as_str().starts_with('_')) - .any(move |sym| { - let mut walker = ContainsName { - name: *sym, - result: false, - }; + .filter(|&&(_, name)| !name.as_str().starts_with('_')) + .any(|&(_, name)| { + let mut walker = ContainsName { name, result: false }; // Scan block block @@ -419,194 +491,9 @@ fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &FxHashSet<Symb }) } -fn emit_branches_sharing_code_lint( - cx: &LateContext<'_>, - start_stmts: usize, - end_stmts: usize, - lint_end: bool, - warn_about_moved_symbol: bool, - blocks: &[&Block<'_>], - if_expr: &Expr<'_>, -) { - if start_stmts == 0 && !lint_end { - return; - } - - // (help, span, suggestion) - let mut suggestions: Vec<(&str, Span, String)> = vec![]; - let mut add_expr_note = false; - - // Construct suggestions - let sm = cx.sess().source_map(); - if start_stmts > 0 { - let block = blocks[0]; - let span_start = first_line_of_span(cx, if_expr.span).shrink_to_lo(); - let span_end = sm.stmt_span(block.stmts[start_stmts - 1].span, block.span); - - let cond_span = first_line_of_span(cx, if_expr.span).until(block.span); - let cond_snippet = reindent_multiline(snippet(cx, cond_span, "_"), false, None); - let cond_indent = indent_of(cx, cond_span); - let moved_span = block.stmts[0].span.source_callsite().to(span_end); - let moved_snippet = reindent_multiline(snippet(cx, moved_span, "_"), true, None); - let suggestion = moved_snippet.to_string() + "\n" + &cond_snippet + "{"; - let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, cond_indent); - - let span = span_start.to(span_end); - suggestions.push(("start", span, suggestion.to_string())); - } - - if lint_end { - let block = blocks[blocks.len() - 1]; - let span_end = block.span.shrink_to_hi(); - - let moved_start = if end_stmts == 0 && block.expr.is_some() { - block.expr.unwrap().span.source_callsite() - } else { - sm.stmt_span(block.stmts[block.stmts.len() - end_stmts].span, block.span) - }; - let moved_end = block.expr.map_or_else( - || sm.stmt_span(block.stmts[block.stmts.len() - 1].span, block.span), - |expr| expr.span.source_callsite(), - ); - - let moved_span = moved_start.to(moved_end); - let moved_snipped = reindent_multiline(snippet(cx, moved_span, "_"), true, None); - let indent = indent_of(cx, if_expr.span.shrink_to_hi()); - let suggestion = "}\n".to_string() + &moved_snipped; - let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, indent); - - let mut span = moved_start.to(span_end); - // Improve formatting if the inner block has indention (i.e. normal Rust formatting) - let test_span = Span::new(span.lo() - BytePos(4), span.lo(), span.ctxt(), span.parent()); - if snippet_opt(cx, test_span) - .map(|snip| snip == " ") - .unwrap_or_default() - { - span = span.with_lo(test_span.lo()); - } - - suggestions.push(("end", span, suggestion.to_string())); - add_expr_note = !cx.typeck_results().expr_ty(if_expr).is_unit(); - } - - let add_optional_msgs = |diag: &mut Diagnostic| { - if add_expr_note { - diag.note("The end suggestion probably needs some adjustments to use the expression result correctly"); - } - - if warn_about_moved_symbol { - diag.warn("Some moved values might need to be renamed to avoid wrong references"); - } - }; - - // Emit lint - if suggestions.len() == 1 { - let (place_str, span, sugg) = suggestions.pop().unwrap(); - let msg = format!("all if blocks contain the same code at the {}", place_str); - let help = format!("consider moving the {} statements out like this", place_str); - span_lint_and_then(cx, BRANCHES_SHARING_CODE, span, msg.as_str(), |diag| { - diag.span_suggestion(span, help.as_str(), sugg, Applicability::Unspecified); - - add_optional_msgs(diag); - }); - } else if suggestions.len() == 2 { - let (_, end_span, end_sugg) = suggestions.pop().unwrap(); - let (_, start_span, start_sugg) = suggestions.pop().unwrap(); - span_lint_and_then( - cx, - BRANCHES_SHARING_CODE, - start_span, - "all if blocks contain the same code at the start and the end. Here at the start", - move |diag| { - diag.span_note(end_span, "and here at the end"); - - diag.span_suggestion( - start_span, - "consider moving the start statements out like this", - start_sugg, - Applicability::Unspecified, - ); - - diag.span_suggestion( - end_span, - "and consider moving the end statements out like this", - end_sugg, - Applicability::Unspecified, - ); - - add_optional_msgs(diag); - }, - ); - } -} - -/// This visitor collects `HirId`s and Symbols of defined symbols and `HirId`s of used values. -struct UsedValueFinderVisitor<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - - /// The `HirId`s of defined values in the scanned statements - defs: FxHashSet<HirId>, - - /// The Symbols of the defined symbols in the scanned statements - def_symbols: FxHashSet<Symbol>, - - /// The `HirId`s of the used values - uses: FxHashSet<HirId>, -} - -impl<'a, 'tcx> UsedValueFinderVisitor<'a, 'tcx> { - fn new(cx: &'a LateContext<'tcx>) -> Self { - UsedValueFinderVisitor { - cx, - defs: FxHashSet::default(), - def_symbols: FxHashSet::default(), - uses: FxHashSet::default(), - } - } -} - -impl<'a, 'tcx> Visitor<'tcx> for UsedValueFinderVisitor<'a, 'tcx> { - type NestedFilter = nested_filter::All; - - fn nested_visit_map(&mut self) -> Self::Map { - self.cx.tcx.hir() - } - - fn visit_local(&mut self, l: &'tcx rustc_hir::Local<'tcx>) { - let local_id = l.pat.hir_id; - self.defs.insert(local_id); - - if let Some(sym) = l.pat.simple_ident() { - self.def_symbols.insert(sym.name); - } - - if let Some(expr) = l.init { - intravisit::walk_expr(self, expr); - } - } - - fn visit_qpath(&mut self, qpath: &'tcx rustc_hir::QPath<'tcx>, id: HirId, _span: rustc_span::Span) { - if let rustc_hir::QPath::Resolved(_, path) = *qpath { - if path.segments.len() == 1 { - if let rustc_hir::def::Res::Local(var) = self.cx.qpath_res(qpath, id) { - self.uses.insert(var); - } - } - } - } -} - /// Implementation of `IFS_SAME_COND`. fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) { - let hash: &dyn Fn(&&Expr<'_>) -> u64 = &|expr| -> u64 { - let mut h = SpanlessHash::new(cx); - h.hash_expr(expr); - h.finish() - }; - - let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool { eq_expr_value(cx, lhs, rhs) }; - - for (i, j) in search_same(conds, hash, eq) { + for (i, j) in search_same(conds, |e| hash_expr(cx, e), |lhs, rhs| eq_expr_value(cx, lhs, rhs)) { span_lint_and_note( cx, IFS_SAME_COND, @@ -620,12 +507,6 @@ fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) { /// Implementation of `SAME_FUNCTIONS_IN_IF_CONDITION`. fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) { - let hash: &dyn Fn(&&Expr<'_>) -> u64 = &|expr| -> u64 { - let mut h = SpanlessHash::new(cx); - h.hash_expr(expr); - h.finish() - }; - let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool { // Do not lint if any expr originates from a macro if lhs.span.from_expansion() || rhs.span.from_expansion() { @@ -638,7 +519,7 @@ fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) { SpanlessEq::new(cx).eq_expr(lhs, rhs) }; - for (i, j) in search_same(conds, hash, eq) { + for (i, j) in search_same(conds, |e| hash_expr(cx, e), eq) { span_lint_and_note( cx, SAME_FUNCTIONS_IN_IF_CONDITION, diff --git a/clippy_lints/src/create_dir.rs b/clippy_lints/src/create_dir.rs index 6bc4054a5ab..18d34370a7b 100644 --- a/clippy_lints/src/create_dir.rs +++ b/clippy_lints/src/create_dir.rs @@ -15,12 +15,12 @@ declare_clippy_lint! { /// Sometimes `std::fs::create_dir` is mistakenly chosen over `std::fs::create_dir_all`. /// /// ### Example - /// - /// ```rust + /// ```rust,ignore /// std::fs::create_dir("foo"); /// ``` + /// /// Use instead: - /// ```rust + /// ```rust,ignore /// std::fs::create_dir_all("foo"); /// ``` #[clippy::version = "1.48.0"] diff --git a/clippy_lints/src/dbg_macro.rs b/clippy_lints/src/dbg_macro.rs index 17deccf8c39..fe9f4f9ae3c 100644 --- a/clippy_lints/src/dbg_macro.rs +++ b/clippy_lints/src/dbg_macro.rs @@ -18,10 +18,11 @@ declare_clippy_lint! { /// /// ### Example /// ```rust,ignore - /// // Bad /// dbg!(true) + /// ``` /// - /// // Good + /// Use instead: + /// ```rust,ignore /// true /// ``` #[clippy::version = "1.34.0"] diff --git a/clippy_lints/src/default.rs b/clippy_lints/src/default.rs index 243dfd3a461..d99a1aa2969 100644 --- a/clippy_lints/src/default.rs +++ b/clippy_lints/src/default.rs @@ -18,15 +18,16 @@ declare_clippy_lint! { /// Checks for literal calls to `Default::default()`. /// /// ### Why is this bad? - /// It's more clear to the reader to use the name of the type whose default is - /// being gotten than the generic `Default`. + /// It's easier for the reader if the name of the type is used, rather than the + /// generic `Default`. /// /// ### Example /// ```rust - /// // Bad /// let s: String = Default::default(); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// let s = String::default(); /// ``` #[clippy::version = "pre 1.29.0"] @@ -47,13 +48,13 @@ declare_clippy_lint! { /// Assignments to patterns that are of tuple type are not linted. /// /// ### Example - /// Bad: /// ``` /// # #[derive(Default)] /// # struct A { i: i32 } /// let mut a: A = Default::default(); /// a.i = 42; /// ``` + /// /// Use instead: /// ``` /// # #[derive(Default)] diff --git a/clippy_lints/src/default_numeric_fallback.rs b/clippy_lints/src/default_numeric_fallback.rs index 3d9f9ed41ce..fb418a3251f 100644 --- a/clippy_lints/src/default_numeric_fallback.rs +++ b/clippy_lints/src/default_numeric_fallback.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::diagnostics::span_lint_hir_and_then; use clippy_utils::numeric_literal; use clippy_utils::source::snippet_opt; use if_chain::if_chain; @@ -76,7 +76,7 @@ impl<'a, 'tcx> NumericFallbackVisitor<'a, 'tcx> { } /// Check whether a passed literal has potential to cause fallback or not. - fn check_lit(&self, lit: &Lit, lit_ty: Ty<'tcx>) { + fn check_lit(&self, lit: &Lit, lit_ty: Ty<'tcx>, emit_hir_id: HirId) { if_chain! { if !in_external_macro(self.cx.sess(), lit.span); if let Some(ty_bound) = self.ty_bounds.last(); @@ -101,14 +101,15 @@ impl<'a, 'tcx> NumericFallbackVisitor<'a, 'tcx> { } }; let sugg = numeric_literal::format(&src, Some(suffix), is_float); - span_lint_and_sugg( + span_lint_hir_and_then( self.cx, DEFAULT_NUMERIC_FALLBACK, + emit_hir_id, lit.span, "default numeric fallback might occur", - "consider adding suffix", - sugg, - Applicability::MaybeIncorrect, + |diag| { + diag.span_suggestion(lit.span, "consider adding suffix", sugg, Applicability::MaybeIncorrect); + } ); } } @@ -179,7 +180,7 @@ impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> { ExprKind::Lit(lit) => { let ty = self.cx.typeck_results().expr_ty(expr); - self.check_lit(lit, ty); + self.check_lit(lit, ty, expr.hir_id); return; }, diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index 527529965a9..b47441eff37 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; +use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then}; use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; use clippy_utils::sugg::has_enclosing_paren; use clippy_utils::ty::peel_mid_ty_refs; @@ -30,13 +30,14 @@ declare_clippy_lint! { /// let a: &mut String = &mut String::from("foo"); /// let b: &str = a.deref(); /// ``` - /// Could be written as: + /// + /// Use instead: /// ```rust /// let a: &mut String = &mut String::from("foo"); /// let b = &*a; /// ``` /// - /// This lint excludes + /// This lint excludes: /// ```rust,ignore /// let _ = d.unwrap().deref(); /// ``` @@ -59,11 +60,13 @@ declare_clippy_lint! { /// ```rust /// fn fun(_a: &i32) {} /// - /// // Bad /// let x: &i32 = &&&&&&5; /// fun(&x); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # fn fun(_a: &i32) {} /// let x: &i32 = &5; /// fun(x); /// ``` @@ -82,13 +85,14 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// let x = Some(""); /// if let Some(ref x) = x { /// // use `x` here /// } + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// let x = Some(""); /// if let Some(x) = x { /// // use `&x` here @@ -131,6 +135,7 @@ pub struct Dereferencing { struct StateData { /// Span of the top level expression span: Span, + hir_id: HirId, } enum State { @@ -165,6 +170,8 @@ struct RefPat { app: Applicability, /// All the replacements which need to be made. replacements: Vec<(Span, String)>, + /// The [`HirId`] that the lint should be emitted at. + hir_id: HirId, } impl<'tcx> LateLintPass<'tcx> for Dereferencing { @@ -218,7 +225,10 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)), target_mut, }, - StateData { span: expr.span }, + StateData { + span: expr.span, + hir_id: expr.hir_id, + }, )); }, RefOp::AddrOf => { @@ -290,7 +300,10 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { required_precedence, msg, }, - StateData { span: expr.span }, + StateData { + span: expr.span, + hir_id: expr.hir_id, + }, )); } }, @@ -383,6 +396,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { spans: vec![pat.span], app, replacements: vec![(pat.span, snip.into())], + hir_id: pat.hir_id }), ); } @@ -395,13 +409,15 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { for pat in self.ref_locals.drain(..).filter_map(|(_, x)| x) { let replacements = pat.replacements; let app = pat.app; - span_lint_and_then( + let lint = if pat.always_deref { + NEEDLESS_BORROW + } else { + REF_BINDING_TO_REFERENCE + }; + span_lint_hir_and_then( cx, - if pat.always_deref { - NEEDLESS_BORROW - } else { - REF_BINDING_TO_REFERENCE - }, + lint, + pat.hir_id, pat.spans, "this pattern creates a reference to a reference", |diag| { @@ -638,19 +654,14 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, state: State, data: S } => { let mut app = Applicability::MachineApplicable; let snip = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app).0; - span_lint_and_sugg( - cx, - NEEDLESS_BORROW, - data.span, - msg, - "change this to", - if required_precedence > expr.precedence().order() && !has_enclosing_paren(&snip) { + span_lint_hir_and_then(cx, NEEDLESS_BORROW, data.hir_id, data.span, msg, |diag| { + let sugg = if required_precedence > expr.precedence().order() && !has_enclosing_paren(&snip) { format!("({})", snip) } else { snip.into() - }, - app, - ); + }; + diag.span_suggestion(data.span, "change this to", sugg, app); + }); }, } } diff --git a/clippy_lints/src/derivable_impls.rs b/clippy_lints/src/derivable_impls.rs index e98691fd5bb..4d7f4076d7b 100644 --- a/clippy_lints/src/derivable_impls.rs +++ b/clippy_lints/src/derivable_impls.rs @@ -30,8 +30,7 @@ declare_clippy_lint! { /// } /// ``` /// - /// Could be written as: - /// + /// Use instead: /// ```rust /// #[derive(Default)] /// struct Foo { @@ -45,7 +44,6 @@ declare_clippy_lint! { /// specialized than what derive will produce. This lint can't detect the manual `impl` /// has exactly equal bounds, and therefore this lint is disabled for types with /// generic parameters. - /// #[clippy::version = "1.57.0"] pub DERIVABLE_IMPLS, complexity, diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index 99347ebadc6..28f218a8e34 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -4,14 +4,19 @@ use clippy_utils::ty::{implements_trait, implements_trait_with_env, is_copy}; use clippy_utils::{is_lint_allowed, match_def_path}; use if_chain::if_chain; use rustc_errors::Applicability; +use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{walk_expr, walk_fn, walk_item, FnKind, Visitor}; use rustc_hir::{ - self as hir, BlockCheckMode, BodyId, Expr, ExprKind, FnDecl, HirId, Impl, Item, ItemKind, UnsafeSource, Unsafety, + self as hir, BlockCheckMode, BodyId, Constness, Expr, ExprKind, FnDecl, HirId, Impl, Item, ItemKind, UnsafeSource, + Unsafety, }; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::nested_filter; -use rustc_middle::ty::subst::GenericArg; -use rustc_middle::ty::{self, BoundConstness, ImplPolarity, ParamEnv, PredicateKind, TraitPredicate, TraitRef, Ty}; +use rustc_middle::traits::Reveal; +use rustc_middle::ty::{ + self, Binder, BoundConstness, GenericParamDefKind, ImplPolarity, ParamEnv, PredicateKind, TraitPredicate, TraitRef, + Ty, TyCtxt, Visibility, +}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; use rustc_span::sym; @@ -459,50 +464,18 @@ impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> { fn check_partial_eq_without_eq<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_ref: &hir::TraitRef<'_>, ty: Ty<'tcx>) { if_chain! { if let ty::Adt(adt, substs) = ty.kind(); + if cx.tcx.visibility(adt.did()) == Visibility::Public; if let Some(eq_trait_def_id) = cx.tcx.get_diagnostic_item(sym::Eq); - if let Some(peq_trait_def_id) = cx.tcx.get_diagnostic_item(sym::PartialEq); if let Some(def_id) = trait_ref.trait_def_id(); if cx.tcx.is_diagnostic_item(sym::PartialEq, def_id); - // New `ParamEnv` replacing `T: PartialEq` with `T: Eq` - let param_env = ParamEnv::new( - cx.tcx.mk_predicates(cx.param_env.caller_bounds().iter().map(|p| { - let kind = p.kind(); - match kind.skip_binder() { - PredicateKind::Trait(p) - if p.trait_ref.def_id == peq_trait_def_id - && p.trait_ref.substs.get(0) == p.trait_ref.substs.get(1) - && matches!(p.trait_ref.self_ty().kind(), ty::Param(_)) - && p.constness == BoundConstness::NotConst - && p.polarity == ImplPolarity::Positive => - { - cx.tcx.mk_predicate(kind.rebind(PredicateKind::Trait(TraitPredicate { - trait_ref: TraitRef::new( - eq_trait_def_id, - cx.tcx.mk_substs([GenericArg::from(p.trait_ref.self_ty())].into_iter()), - ), - constness: BoundConstness::NotConst, - polarity: ImplPolarity::Positive, - }))) - }, - _ => p, - } - })), - cx.param_env.reveal(), - cx.param_env.constness(), - ); - if !implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, substs); + let param_env = param_env_for_derived_eq(cx.tcx, adt.did(), eq_trait_def_id); + if !implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, &[]); + // If all of our fields implement `Eq`, we can implement `Eq` too + if adt + .all_fields() + .map(|f| f.ty(cx.tcx, substs)) + .all(|ty| implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, &[])); then { - // If all of our fields implement `Eq`, we can implement `Eq` too - for variant in adt.variants() { - for field in &variant.fields { - let ty = field.ty(cx.tcx, substs); - - if !implements_trait(cx, ty, eq_trait_def_id, substs) { - return; - } - } - } - span_lint_and_sugg( cx, DERIVE_PARTIAL_EQ_WITHOUT_EQ, @@ -515,3 +488,41 @@ fn check_partial_eq_without_eq<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_r } } } + +/// Creates the `ParamEnv` used for the give type's derived `Eq` impl. +fn param_env_for_derived_eq(tcx: TyCtxt<'_>, did: DefId, eq_trait_id: DefId) -> ParamEnv<'_> { + // Initial map from generic index to param def. + // Vec<(param_def, needs_eq)> + let mut params = tcx + .generics_of(did) + .params + .iter() + .map(|p| (p, matches!(p.kind, GenericParamDefKind::Type { .. }))) + .collect::<Vec<_>>(); + + let ty_predicates = tcx.predicates_of(did).predicates; + for (p, _) in ty_predicates { + if let PredicateKind::Trait(p) = p.kind().skip_binder() + && p.trait_ref.def_id == eq_trait_id + && let ty::Param(self_ty) = p.trait_ref.self_ty().kind() + && p.constness == BoundConstness::NotConst + { + // Flag types which already have an `Eq` bound. + params[self_ty.index as usize].1 = false; + } + } + + ParamEnv::new( + tcx.mk_predicates(ty_predicates.iter().map(|&(p, _)| p).chain( + params.iter().filter(|&&(_, needs_eq)| needs_eq).map(|&(param, _)| { + tcx.mk_predicate(Binder::dummy(PredicateKind::Trait(TraitPredicate { + trait_ref: TraitRef::new(eq_trait_id, tcx.mk_substs([tcx.mk_param_from_def(param)].into_iter())), + constness: BoundConstness::NotConst, + polarity: ImplPolarity::Positive, + }))) + }), + )), + Reveal::UserFacing, + Constness::NotConst, + ) +} diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index aaec88f50c7..da111e7378e 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -163,7 +163,7 @@ declare_clippy_lint! { /// } /// } /// ``` - #[clippy::version = "1.52.0"] + #[clippy::version = "1.51.0"] pub MISSING_PANICS_DOC, pedantic, "`pub fn` may panic without `# Panics` in doc comment" @@ -178,7 +178,7 @@ declare_clippy_lint! { /// if the `fn main()` is left implicit. /// /// ### Examples - /// ``````rust + /// ```rust /// /// An example of a doctest with a `main()` function /// /// /// /// # Examples @@ -191,7 +191,7 @@ declare_clippy_lint! { /// fn needless_main() { /// unimplemented!(); /// } - /// `````` + /// ``` #[clippy::version = "1.40.0"] pub NEEDLESS_DOCTEST_MAIN, style, diff --git a/clippy_lints/src/enum_variants.rs b/clippy_lints/src/enum_variants.rs index 263a5b573c9..23b75104570 100644 --- a/clippy_lints/src/enum_variants.rs +++ b/clippy_lints/src/enum_variants.rs @@ -60,7 +60,8 @@ declare_clippy_lint! { /// struct BlackForestCake; /// } /// ``` - /// Could be written as: + /// + /// Use instead: /// ```rust /// mod cake { /// struct BlackForest; diff --git a/clippy_lints/src/eq_op.rs b/clippy_lints/src/eq_op.rs index c3176d987c6..2f4c90d07cf 100644 --- a/clippy_lints/src/eq_op.rs +++ b/clippy_lints/src/eq_op.rs @@ -52,15 +52,13 @@ declare_clippy_lint! { /// ### Why is this bad? /// It is more idiomatic to dereference the other argument. /// - /// ### Known problems - /// None - /// /// ### Example - /// ```ignore - /// // Bad + /// ```rust,ignore /// &x == y + /// ``` /// - /// // Good + /// Use instead: + /// ```rust,ignore /// x == *y /// ``` #[clippy::version = "pre 1.29.0"] diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs index 197cac86a57..a5a763c37d1 100644 --- a/clippy_lints/src/eta_reduction.rs +++ b/clippy_lints/src/eta_reduction.rs @@ -34,14 +34,14 @@ declare_clippy_lint! { /// /// ### Example /// ```rust,ignore - /// // Bad /// xs.map(|x| foo(x)) + /// ``` /// - /// // Good + /// Use instead: + /// ```rust,ignore + /// // where `foo(_)` is a plain function that takes the exact argument type of `x`. /// xs.map(foo) /// ``` - /// where `foo(_)` is a plain function that takes the exact argument type of - /// `x`. #[clippy::version = "pre 1.29.0"] pub REDUNDANT_CLOSURE, style, diff --git a/clippy_lints/src/excessive_bools.rs b/clippy_lints/src/excessive_bools.rs index a2af10e2ba5..f7a92bc0795 100644 --- a/clippy_lints/src/excessive_bools.rs +++ b/clippy_lints/src/excessive_bools.rs @@ -54,12 +54,11 @@ declare_clippy_lint! { /// API easier to use. /// /// ### Example - /// Bad: /// ```rust,ignore /// fn f(is_round: bool, is_hot: bool) { ... } /// ``` /// - /// Good: + /// Use instead: /// ```rust,ignore /// enum Shape { /// Round, diff --git a/clippy_lints/src/float_literal.rs b/clippy_lints/src/float_literal.rs index f850ea31f4d..f2e07980963 100644 --- a/clippy_lints/src/float_literal.rs +++ b/clippy_lints/src/float_literal.rs @@ -45,10 +45,11 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// let _: f32 = 16_777_217.0; // 16_777_216.0 + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// let _: f32 = 16_777_216.0; /// let _: f64 = 16_777_217.0; /// ``` diff --git a/clippy_lints/src/functions/mod.rs b/clippy_lints/src/functions/mod.rs index ad031cbc09d..73261fb8a44 100644 --- a/clippy_lints/src/functions/mod.rs +++ b/clippy_lints/src/functions/mod.rs @@ -76,12 +76,13 @@ declare_clippy_lint! { /// /// ### Example /// ```rust,ignore - /// // Bad /// pub fn foo(x: *const u8) { /// println!("{}", unsafe { *x }); /// } + /// ``` /// - /// // Good + /// Use instead: + /// ```rust,ignore /// pub unsafe fn foo(x: *const u8) { /// println!("{}", unsafe { *x }); /// } diff --git a/clippy_lints/src/get_first.rs b/clippy_lints/src/get_first.rs index 0748ab45252..529f7babaa5 100644 --- a/clippy_lints/src/get_first.rs +++ b/clippy_lints/src/get_first.rs @@ -20,13 +20,12 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// let x = vec![2, 3, 5]; /// let first_element = x.get(0); /// ``` + /// /// Use instead: /// ```rust - /// // Good /// let x = vec![2, 3, 5]; /// let first_element = x.first(); /// ``` diff --git a/clippy_lints/src/implicit_saturating_sub.rs b/clippy_lints/src/implicit_saturating_sub.rs index ae4158662d4..46654bc61e0 100644 --- a/clippy_lints/src/implicit_saturating_sub.rs +++ b/clippy_lints/src/implicit_saturating_sub.rs @@ -16,17 +16,21 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// let end: u32 = 10; - /// let start: u32 = 5; - /// + /// # let end: u32 = 10; + /// # let start: u32 = 5; /// let mut i: u32 = end - start; /// - /// // Bad /// if i != 0 { /// i -= 1; /// } + /// ``` + /// + /// Use instead: + /// ```rust + /// # let end: u32 = 10; + /// # let start: u32 = 5; + /// let mut i: u32 = end - start; /// - /// // Good /// i = i.saturating_sub(1); /// ``` #[clippy::version = "1.44.0"] @@ -48,7 +52,7 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub { // Check if the conditional expression is a binary operation if let ExprKind::Binary(ref cond_op, cond_left, cond_right) = cond.kind; - // Ensure that the binary operator is >, != and < + // Ensure that the binary operator is >, !=, or < if BinOpKind::Ne == cond_op.node || BinOpKind::Gt == cond_op.node || BinOpKind::Lt == cond_op.node; // Check if assign operation is done diff --git a/clippy_lints/src/index_refutable_slice.rs b/clippy_lints/src/index_refutable_slice.rs index 9ce5b8e17a9..d0c6495e35a 100644 --- a/clippy_lints/src/index_refutable_slice.rs +++ b/clippy_lints/src/index_refutable_slice.rs @@ -45,7 +45,7 @@ declare_clippy_lint! { /// println!("{}", first); /// } /// ``` - #[clippy::version = "1.58.0"] + #[clippy::version = "1.59.0"] pub INDEX_REFUTABLE_SLICE, nursery, "avoid indexing on slices which could be destructed" diff --git a/clippy_lints/src/indexing_slicing.rs b/clippy_lints/src/indexing_slicing.rs index 4ba7477add8..4a375752e1d 100644 --- a/clippy_lints/src/indexing_slicing.rs +++ b/clippy_lints/src/indexing_slicing.rs @@ -17,19 +17,20 @@ declare_clippy_lint! { /// ### Why is this bad? /// This will always panic at runtime. /// - /// ### Known problems - /// Hopefully none. - /// /// ### Example - /// ```no_run + /// ```rust,no_run /// # #![allow(const_err)] /// let x = [1, 2, 3, 4]; /// - /// // Bad /// x[9]; /// &x[2..9]; + /// ``` + /// + /// Use instead: + /// ```rust + /// # let x = [1, 2, 3, 4]; + /// // Index within bounds /// - /// // Good /// x[0]; /// x[3]; /// ``` @@ -49,42 +50,32 @@ declare_clippy_lint! { /// Indexing and slicing can panic at runtime and there are /// safe alternatives. /// - /// ### Known problems - /// Hopefully none. - /// /// ### Example /// ```rust,no_run /// // Vector /// let x = vec![0; 5]; /// - /// // Bad /// x[2]; /// &x[2..100]; - /// &x[2..]; - /// &x[..100]; - /// - /// // Good - /// x.get(2); - /// x.get(2..100); - /// x.get(2..); - /// x.get(..100); /// /// // Array /// let y = [0, 1, 2, 3]; /// - /// // Bad /// &y[10..100]; /// &y[10..]; - /// &y[..100]; + /// ``` + /// + /// Use instead: + /// ```rust + /// # #![allow(unused)] + /// + /// # let x = vec![0; 5]; + /// # let y = [0, 1, 2, 3]; + /// x.get(2); + /// x.get(2..100); /// - /// // Good - /// &y[2..]; - /// &y[..2]; - /// &y[0..3]; /// y.get(10); /// y.get(10..100); - /// y.get(10..); - /// y.get(..100); /// ``` #[clippy::version = "pre 1.29.0"] pub INDEXING_SLICING, diff --git a/clippy_lints/src/infinite_iter.rs b/clippy_lints/src/infinite_iter.rs index 41e1fc4e3c2..78b5ec8ec1e 100644 --- a/clippy_lints/src/infinite_iter.rs +++ b/clippy_lints/src/infinite_iter.rs @@ -41,6 +41,7 @@ declare_clippy_lint! { /// ### Example /// ```rust /// let infinite_iter = 0..; + /// # #[allow(unused)] /// [0..].iter().zip(infinite_iter.take_while(|x| *x > 5)); /// ``` #[clippy::version = "pre 1.29.0"] diff --git a/clippy_lints/src/inherent_to_string.rs b/clippy_lints/src/inherent_to_string.rs index 55c04a1186f..39f68a8a1b4 100644 --- a/clippy_lints/src/inherent_to_string.rs +++ b/clippy_lints/src/inherent_to_string.rs @@ -14,12 +14,8 @@ declare_clippy_lint! { /// ### Why is this bad? /// This method is also implicitly defined if a type implements the `Display` trait. As the functionality of `Display` is much more versatile, it should be preferred. /// - /// ### Known problems - /// None - /// /// ### Example /// ```rust - /// // Bad /// pub struct A; /// /// impl A { @@ -29,8 +25,8 @@ declare_clippy_lint! { /// } /// ``` /// + /// Use instead: /// ```rust - /// // Good /// use std::fmt; /// /// pub struct A; @@ -54,12 +50,8 @@ declare_clippy_lint! { /// ### Why is this bad? /// This method is also implicitly defined if a type implements the `Display` trait. The less versatile inherent method will then shadow the implementation introduced by `Display`. /// - /// ### Known problems - /// None - /// /// ### Example /// ```rust - /// // Bad /// use std::fmt; /// /// pub struct A; @@ -77,8 +69,8 @@ declare_clippy_lint! { /// } /// ``` /// + /// Use instead: /// ```rust - /// // Good /// use std::fmt; /// /// pub struct A; diff --git a/clippy_lints/src/int_plus_one.rs b/clippy_lints/src/int_plus_one.rs index 8db7b307ddb..9a944def3eb 100644 --- a/clippy_lints/src/int_plus_one.rs +++ b/clippy_lints/src/int_plus_one.rs @@ -21,8 +21,7 @@ declare_clippy_lint! { /// if x >= y + 1 {} /// ``` /// - /// Could be written as: - /// + /// Use instead: /// ```rust /// # let x = 1; /// # let y = 1; diff --git a/clippy_lints/src/integer_division.rs b/clippy_lints/src/integer_division.rs index fa786205678..3effba56826 100644 --- a/clippy_lints/src/integer_division.rs +++ b/clippy_lints/src/integer_division.rs @@ -15,11 +15,12 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// let x = 3 / 2; /// println!("{}", x); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// let x = 3f32 / 2f32; /// println!("{}", x); /// ``` diff --git a/clippy_lints/src/items_after_statements.rs b/clippy_lints/src/items_after_statements.rs index cdefe627efd..46d439b4497 100644 --- a/clippy_lints/src/items_after_statements.rs +++ b/clippy_lints/src/items_after_statements.rs @@ -17,7 +17,6 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// fn foo() { /// println!("cake"); /// } @@ -31,8 +30,8 @@ declare_clippy_lint! { /// } /// ``` /// + /// Use instead: /// ```rust - /// // Good /// fn foo() { /// println!("cake"); /// } diff --git a/clippy_lints/src/large_const_arrays.rs b/clippy_lints/src/large_const_arrays.rs index e10993ba7dd..289755bfec6 100644 --- a/clippy_lints/src/large_const_arrays.rs +++ b/clippy_lints/src/large_const_arrays.rs @@ -20,10 +20,11 @@ declare_clippy_lint! { /// /// ### Example /// ```rust,ignore - /// // Bad /// pub const a = [0u32; 1_000_000]; + /// ``` /// - /// // Good + /// Use instead: + /// ```rust.ignore /// pub static a = [0u32; 1_000_000]; /// ``` #[clippy::version = "1.44.0"] diff --git a/clippy_lints/src/large_enum_variant.rs b/clippy_lints/src/large_enum_variant.rs index 63ac092dfaf..9be057bcf90 100644 --- a/clippy_lints/src/large_enum_variant.rs +++ b/clippy_lints/src/large_enum_variant.rs @@ -38,12 +38,14 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// enum Test { /// A(i32), /// B([i32; 8000]), /// } + /// ``` /// + /// Use instead: + /// ```rust /// // Possibly better /// enum Test2 { /// A(i32), diff --git a/clippy_lints/src/large_include_file.rs b/clippy_lints/src/large_include_file.rs index 8bef13c682d..84dd61a1e4b 100644 --- a/clippy_lints/src/large_include_file.rs +++ b/clippy_lints/src/large_include_file.rs @@ -22,10 +22,11 @@ declare_clippy_lint! { /// let included_bytes = include_bytes!("very_large_file.txt"); /// ``` /// - /// Instead, you can load the file at runtime: + /// Use instead: /// ```rust,ignore /// use std::fs; /// + /// // You can load the file at runtime /// let string = fs::read_to_string("very_large_file.txt")?; /// let bytes = fs::read("very_large_file.txt")?; /// ``` diff --git a/clippy_lints/src/let_underscore.rs b/clippy_lints/src/let_underscore.rs index cb1ef01f5ba..26c540e2223 100644 --- a/clippy_lints/src/let_underscore.rs +++ b/clippy_lints/src/let_underscore.rs @@ -45,13 +45,11 @@ declare_clippy_lint! { /// `std::mem::drop` conveys your intention better and is less error-prone. /// /// ### Example - /// - /// Bad: /// ```rust,ignore /// let _ = mutex.lock(); /// ``` /// - /// Good: + /// Use instead: /// ```rust,ignore /// let _lock = mutex.lock(); /// ``` @@ -75,24 +73,20 @@ declare_clippy_lint! { /// better and is less error-prone. /// /// ### Example - /// - /// Bad: - /// ```rust,ignore - /// struct Droppable; - /// impl Drop for Droppable { - /// fn drop(&mut self) {} - /// } + /// ```rust + /// # struct DroppableItem; /// { - /// let _ = Droppable; - /// // ^ dropped here + /// let _ = DroppableItem; + /// // ^ dropped here /// /* more code */ /// } /// ``` /// - /// Good: - /// ```rust,ignore + /// Use instead: + /// ```rust + /// # struct DroppableItem; /// { - /// let _droppable = Droppable; + /// let _droppable = DroppableItem; /// /* more code */ /// // dropped at end of scope /// } diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index d4ec046d0bb..8a2cfbff953 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -242,6 +242,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(needless_bool::NEEDLESS_BOOL), LintId::of(needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE), LintId::of(needless_late_init::NEEDLESS_LATE_INIT), + LintId::of(needless_parens_on_range_literals::NEEDLESS_PARENS_ON_RANGE_LITERALS), LintId::of(needless_question_mark::NEEDLESS_QUESTION_MARK), LintId::of(needless_update::NEEDLESS_UPDATE), LintId::of(neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD), @@ -270,6 +271,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(ranges::RANGE_ZIP_WITH_LEN), LintId::of(ranges::REVERSED_EMPTY_RANGES), LintId::of(rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT), + LintId::of(read_zero_byte_vec::READ_ZERO_BYTE_VEC), LintId::of(redundant_clone::REDUNDANT_CLONE), LintId::of(redundant_closure_call::REDUNDANT_CLOSURE_CALL), LintId::of(redundant_field_names::REDUNDANT_FIELD_NAMES), diff --git a/clippy_lints/src/lib.register_correctness.rs b/clippy_lints/src/lib.register_correctness.rs index 50cdd0af923..92a3a0aabf1 100644 --- a/clippy_lints/src/lib.register_correctness.rs +++ b/clippy_lints/src/lib.register_correctness.rs @@ -55,6 +55,7 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve LintId::of(ptr::INVALID_NULL_PTR_USAGE), LintId::of(ptr::MUT_FROM_REF), LintId::of(ranges::REVERSED_EMPTY_RANGES), + LintId::of(read_zero_byte_vec::READ_ZERO_BYTE_VEC), LintId::of(regex::INVALID_REGEX), LintId::of(self_assignment::SELF_ASSIGNMENT), LintId::of(serde_api::SERDE_API_MISUSE), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index b927ba3b17c..8ad984c68b8 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -408,6 +408,7 @@ store.register_lints(&[ needless_continue::NEEDLESS_CONTINUE, needless_for_each::NEEDLESS_FOR_EACH, needless_late_init::NEEDLESS_LATE_INIT, + needless_parens_on_range_literals::NEEDLESS_PARENS_ON_RANGE_LITERALS, needless_pass_by_value::NEEDLESS_PASS_BY_VALUE, needless_question_mark::NEEDLESS_QUESTION_MARK, needless_update::NEEDLESS_UPDATE, @@ -458,6 +459,7 @@ store.register_lints(&[ ranges::RANGE_ZIP_WITH_LEN, ranges::REVERSED_EMPTY_RANGES, rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT, + read_zero_byte_vec::READ_ZERO_BYTE_VEC, redundant_clone::REDUNDANT_CLONE, redundant_closure_call::REDUNDANT_CLOSURE_CALL, redundant_else::REDUNDANT_ELSE, diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs index 35575351784..b6992ae0ad2 100644 --- a/clippy_lints/src/lib.register_style.rs +++ b/clippy_lints/src/lib.register_style.rs @@ -91,6 +91,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![ LintId::of(mut_mutex_lock::MUT_MUTEX_LOCK), LintId::of(mut_reference::UNNECESSARY_MUT_PASSED), LintId::of(needless_late_init::NEEDLESS_LATE_INIT), + LintId::of(needless_parens_on_range_literals::NEEDLESS_PARENS_ON_RANGE_LITERALS), LintId::of(neg_multiply::NEG_MULTIPLY), LintId::of(new_without_default::NEW_WITHOUT_DEFAULT), LintId::of(non_copy_const::BORROW_INTERIOR_MUTABLE_CONST), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index ee0416fc0ff..84898eae05a 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1,5 +1,3 @@ -// error-pattern:cargo-clippy - #![feature(array_windows)] #![feature(binary_heap_into_iter_sorted)] #![feature(box_patterns)] @@ -88,10 +86,11 @@ use rustc_session::Session; /// /// /// /// ### Example /// /// ```rust -/// /// // Bad /// /// Insert a short example of code that triggers the lint +/// /// ``` /// /// -/// /// // Good +/// /// Use instead: +/// /// ```rust /// /// Insert a short example of improved code that doesn't trigger the lint /// /// ``` /// pub LINT_NAME, @@ -315,6 +314,7 @@ mod needless_borrowed_ref; mod needless_continue; mod needless_for_each; mod needless_late_init; +mod needless_parens_on_range_literals; mod needless_pass_by_value; mod needless_question_mark; mod needless_update; @@ -348,6 +348,7 @@ mod pub_use; mod question_mark; mod ranges; mod rc_clone_in_vec_init; +mod read_zero_byte_vec; mod redundant_clone; mod redundant_closure_call; mod redundant_else; @@ -746,6 +747,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(|| Box::new(collapsible_if::CollapsibleIf)); store.register_early_pass(|| Box::new(items_after_statements::ItemsAfterStatements)); store.register_early_pass(|| Box::new(precedence::Precedence)); + store.register_late_pass(|| Box::new(needless_parens_on_range_literals::NeedlessParensOnRangeLiterals)); store.register_early_pass(|| Box::new(needless_continue::NeedlessContinue)); store.register_early_pass(|| Box::new(redundant_else::RedundantElse)); store.register_late_pass(|| Box::new(create_dir::CreateDir)); @@ -907,6 +909,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(swap_ptr_to_ref::SwapPtrToRef)); store.register_late_pass(|| Box::new(mismatching_type_param_order::TypeParamMismatch)); store.register_late_pass(|| Box::new(as_underscore::AsUnderscore)); + store.register_late_pass(|| Box::new(read_zero_byte_vec::ReadZeroByteVec)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index 070c7e59142..93f5663312f 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/clippy_lints/src/lifetimes.rs @@ -36,12 +36,14 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad: unnecessary lifetime annotations + /// // Unnecessary lifetime annotations /// fn in_and_out<'a>(x: &'a u8, y: u8) -> &'a u8 { /// x /// } + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// fn elided(x: &u8, y: u8) -> &u8 { /// x /// } @@ -65,12 +67,14 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad: unnecessary lifetimes + /// // unnecessary lifetimes /// fn unused_lifetime<'a>(x: u8) { /// // .. /// } + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// fn no_lifetime(x: u8) { /// // ... /// } diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs index 9998712b852..fb2104861c8 100644 --- a/clippy_lints/src/literal_representation.rs +++ b/clippy_lints/src/literal_representation.rs @@ -22,11 +22,16 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad - /// let x: u64 = 61864918973511; + /// # let _: u64 = + /// 61864918973511 + /// # ; + /// ``` /// - /// // Good - /// let x: u64 = 61_864_918_973_511; + /// Use instead: + /// ```rust + /// # let _: u64 = + /// 61_864_918_973_511 + /// # ; /// ``` #[clippy::version = "pre 1.29.0"] pub UNREADABLE_LITERAL, @@ -46,6 +51,7 @@ declare_clippy_lint! { /// - Does not match on `_127` since that is a valid grouping for decimal and octal numbers /// /// ### Example + /// ```ignore /// `2_32` => `2_i32` /// `250_8 => `250_u8` /// ``` @@ -66,11 +72,16 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad - /// let x: u64 = 618_64_9189_73_511; + /// # let _: u64 = + /// 618_64_9189_73_511 + /// # ; + /// ``` /// - /// // Good - /// let x: u64 = 61_864_918_973_511; + /// Use instead: + /// ```rust + /// # let _: u64 = + /// 61_864_918_973_511 + /// # ; /// ``` #[clippy::version = "pre 1.29.0"] pub INCONSISTENT_DIGIT_GROUPING, @@ -125,9 +136,11 @@ declare_clippy_lint! { /// readable than a decimal representation. /// /// ### Example + /// ```text /// `255` => `0xFF` /// `65_535` => `0xFFFF` /// `4_042_322_160` => `0xF0F0_F0F0` + /// ``` #[clippy::version = "pre 1.29.0"] pub DECIMAL_LITERAL_REPRESENTATION, restriction, diff --git a/clippy_lints/src/loops/for_loops_over_fallibles.rs b/clippy_lints/src/loops/for_loops_over_fallibles.rs index 90530ebf003..77de90fd7b9 100644 --- a/clippy_lints/src/loops/for_loops_over_fallibles.rs +++ b/clippy_lints/src/loops/for_loops_over_fallibles.rs @@ -7,9 +7,22 @@ use rustc_lint::LateContext; use rustc_span::symbol::sym; /// Checks for `for` loops over `Option`s and `Result`s. -pub(super) fn check(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) { +pub(super) fn check(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>, method_name: Option<&str>) { let ty = cx.typeck_results().expr_ty(arg); if is_type_diagnostic_item(cx, ty, sym::Option) { + let help_string = if let Some(method_name) = method_name { + format!( + "consider replacing `for {0} in {1}.{method_name}()` with `if let Some({0}) = {1}`", + snippet(cx, pat.span, "_"), + snippet(cx, arg.span, "_") + ) + } else { + format!( + "consider replacing `for {0} in {1}` with `if let Some({0}) = {1}`", + snippet(cx, pat.span, "_"), + snippet(cx, arg.span, "_") + ) + }; span_lint_and_help( cx, FOR_LOOPS_OVER_FALLIBLES, @@ -20,13 +33,22 @@ pub(super) fn check(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) { snippet(cx, arg.span, "_") ), None, - &format!( - "consider replacing `for {0} in {1}` with `if let Some({0}) = {1}`", - snippet(cx, pat.span, "_"), - snippet(cx, arg.span, "_") - ), + &help_string, ); } else if is_type_diagnostic_item(cx, ty, sym::Result) { + let help_string = if let Some(method_name) = method_name { + format!( + "consider replacing `for {0} in {1}.{method_name}()` with `if let Ok({0}) = {1}`", + snippet(cx, pat.span, "_"), + snippet(cx, arg.span, "_") + ) + } else { + format!( + "consider replacing `for {0} in {1}` with `if let Ok({0}) = {1}`", + snippet(cx, pat.span, "_"), + snippet(cx, arg.span, "_") + ) + }; span_lint_and_help( cx, FOR_LOOPS_OVER_FALLIBLES, @@ -37,11 +59,7 @@ pub(super) fn check(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) { snippet(cx, arg.span, "_") ), None, - &format!( - "consider replacing `for {0} in {1}` with `if let Ok({0}) = {1}`", - snippet(cx, pat.span, "_"), - snippet(cx, arg.span, "_") - ), + &help_string, ); } } diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs index d61be78895f..391de922e1e 100644 --- a/clippy_lints/src/loops/mod.rs +++ b/clippy_lints/src/loops/mod.rs @@ -42,7 +42,8 @@ declare_clippy_lint! { /// dst[i + 64] = src[i]; /// } /// ``` - /// Could be written as: + /// + /// Use instead: /// ```rust /// # let src = vec![1]; /// # let mut dst = vec![0; 65]; @@ -70,7 +71,8 @@ declare_clippy_lint! { /// println!("{}", vec[i]); /// } /// ``` - /// Could be written as: + /// + /// Use instead: /// ```rust /// let vec = vec!['a', 'b', 'c']; /// for i in vec { @@ -103,7 +105,8 @@ declare_clippy_lint! { /// // .. /// } /// ``` - /// can be rewritten to + /// + /// Use instead: /// ```rust /// # let y = vec![1]; /// for x in &y { @@ -188,6 +191,10 @@ declare_clippy_lint! { /// for x in &res { /// // .. /// } + /// + /// for x in res.iter() { + /// // .. + /// } /// ``` /// /// Use instead: @@ -282,7 +289,8 @@ declare_clippy_lint! { /// i += 1; /// } /// ``` - /// Could be written as + /// + /// Use instead: /// ```rust /// # let v = vec![1]; /// # fn bar(bar: usize, baz: usize) {} @@ -469,7 +477,7 @@ declare_clippy_lint! { /// /// ### Why is this bad? /// This kind of operation can be expressed more succinctly with - /// `vec![item;SIZE]` or `vec.resize(NEW_SIZE, item)` and using these alternatives may also + /// `vec![item; SIZE]` or `vec.resize(NEW_SIZE, item)` and using these alternatives may also /// have better performance. /// /// ### Example @@ -484,7 +492,8 @@ declare_clippy_lint! { /// vec.push(item2); /// } /// ``` - /// could be written as + /// + /// Use instead: /// ```rust /// let item1 = 2; /// let item2 = 3; @@ -512,7 +521,8 @@ declare_clippy_lint! { /// println!("{}", item); /// } /// ``` - /// could be written as + /// + /// Use instead: /// ```rust /// let item1 = 2; /// let item = &item1; @@ -586,7 +596,7 @@ declare_clippy_lint! { /// std::hint::spin_loop() /// } /// ``` - #[clippy::version = "1.59.0"] + #[clippy::version = "1.61.0"] pub MISSING_SPIN_LOOP, perf, "An empty busy waiting loop" @@ -695,10 +705,14 @@ fn check_for_loop_arg(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) { let method_name = method.ident.as_str(); // check for looping over x.iter() or x.iter_mut(), could use &x or &mut x match method_name { - "iter" | "iter_mut" => explicit_iter_loop::check(cx, self_arg, arg, method_name), + "iter" | "iter_mut" => { + explicit_iter_loop::check(cx, self_arg, arg, method_name); + for_loops_over_fallibles::check(cx, pat, self_arg, Some(method_name)); + }, "into_iter" => { explicit_iter_loop::check(cx, self_arg, arg, method_name); explicit_into_iter_loop::check(cx, self_arg, arg); + for_loops_over_fallibles::check(cx, pat, self_arg, Some(method_name)); }, "next" => { next_loop_linted = iter_next_loop::check(cx, arg); @@ -708,6 +722,6 @@ fn check_for_loop_arg(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) { } if !next_loop_linted { - for_loops_over_fallibles::check(cx, pat, arg); + for_loops_over_fallibles::check(cx, pat, arg, None); } } diff --git a/clippy_lints/src/loops/never_loop.rs b/clippy_lints/src/loops/never_loop.rs index 99d21466935..32de20f6531 100644 --- a/clippy_lints/src/loops/never_loop.rs +++ b/clippy_lints/src/loops/never_loop.rs @@ -117,13 +117,20 @@ fn never_loop_expr(expr: &Expr<'_>, main_loop_id: HirId) -> NeverLoopResult { | ExprKind::Type(e, _) | ExprKind::Field(e, _) | ExprKind::AddrOf(_, _, e) - | ExprKind::Struct(_, _, Some(e)) | ExprKind::Repeat(e, _) | ExprKind::DropTemps(e) => never_loop_expr(e, main_loop_id), ExprKind::Let(let_expr) => never_loop_expr(let_expr.init, main_loop_id), ExprKind::Array(es) | ExprKind::MethodCall(_, es, _) | ExprKind::Tup(es) => { never_loop_expr_all(&mut es.iter(), main_loop_id) }, + ExprKind::Struct(_, fields, base) => { + let fields = never_loop_expr_all(&mut fields.iter().map(|f| f.expr), main_loop_id); + if let Some(base) = base { + combine_both(fields, never_loop_expr(base, main_loop_id)) + } else { + fields + } + }, ExprKind::Call(e, es) => never_loop_expr_all(&mut once(e).chain(es.iter()), main_loop_id), ExprKind::Binary(_, e1, e2) | ExprKind::Assign(e1, e2, _) @@ -180,8 +187,7 @@ fn never_loop_expr(expr: &Expr<'_>, main_loop_id: HirId) -> NeverLoopResult { | InlineAsmOperand::SymStatic { .. } => NeverLoopResult::Otherwise, }) .fold(NeverLoopResult::Otherwise, combine_both), - ExprKind::Struct(_, _, None) - | ExprKind::Yield(_, _) + ExprKind::Yield(_, _) | ExprKind::Closure { .. } | ExprKind::Path(_) | ExprKind::ConstBlock(_) diff --git a/clippy_lints/src/matches/match_same_arms.rs b/clippy_lints/src/matches/match_same_arms.rs index 4f8baf7efb0..16fefea5520 100644 --- a/clippy_lints/src/matches/match_same_arms.rs +++ b/clippy_lints/src/matches/match_same_arms.rs @@ -110,14 +110,9 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) { arm1.span, "this match arm has an identical body to the `_` wildcard arm", |diag| { - diag.span_suggestion( - arm1.span, - "try removing the arm", - "", - Applicability::MaybeIncorrect, - ) - .help("or try changing either arm body") - .span_note(arm2.span, "`_` wildcard arm here"); + diag.span_suggestion(arm1.span, "try removing the arm", "", Applicability::MaybeIncorrect) + .help("or try changing either arm body") + .span_note(arm2.span, "`_` wildcard arm here"); }, ); } else { diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index d1e42f39e47..3e765173fb9 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -43,13 +43,16 @@ declare_clippy_lint! { /// ```rust /// # fn bar(stool: &str) {} /// # let x = Some("abc"); - /// // Bad /// match x { /// Some(ref foo) => bar(foo), /// _ => (), /// } + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # fn bar(stool: &str) {} + /// # let x = Some("abc"); /// if let Some(ref foo) = x { /// bar(foo); /// } @@ -114,14 +117,15 @@ declare_clippy_lint! { /// /// ### Example /// ```rust,ignore - /// // Bad /// match x { /// &A(ref y) => foo(y), /// &B => bar(), /// _ => frob(&x), /// } + /// ``` /// - /// // Good + /// Use instead: + /// ```rust,ignore /// match *x { /// A(ref y) => foo(y), /// B => bar(), @@ -227,13 +231,16 @@ declare_clippy_lint! { /// ```rust /// let x: Option<()> = None; /// - /// // Bad /// let r: Option<&()> = match x { /// None => None, /// Some(ref v) => Some(v), /// }; + /// ``` + /// + /// Use instead: + /// ```rust + /// let x: Option<()> = None; /// - /// // Good /// let r: Option<&()> = x.as_ref(); /// ``` #[clippy::version = "pre 1.29.0"] @@ -257,13 +264,16 @@ declare_clippy_lint! { /// ```rust /// # enum Foo { A(usize), B(usize) } /// # let x = Foo::B(1); - /// // Bad /// match x { /// Foo::A(_) => {}, /// _ => {}, /// } + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # enum Foo { A(usize), B(usize) } + /// # let x = Foo::B(1); /// match x { /// Foo::A(_) => {}, /// Foo::B(_) => {}, @@ -290,14 +300,17 @@ declare_clippy_lint! { /// ```rust /// # enum Foo { A, B, C } /// # let x = Foo::B; - /// // Bad /// match x { /// Foo::A => {}, /// Foo::B => {}, /// _ => {}, /// } + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # enum Foo { A, B, C } + /// # let x = Foo::B; /// match x { /// Foo::A => {}, /// Foo::B => {}, @@ -320,14 +333,17 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad - /// match "foo" { + /// # let s = "foo"; + /// match s { /// "a" => {}, /// "bar" | _ => {}, /// } + /// ``` /// - /// // Good - /// match "foo" { + /// Use instead: + /// ```rust + /// # let s = "foo"; + /// match s { /// "a" => {}, /// _ => {}, /// } @@ -389,15 +405,17 @@ declare_clippy_lint! { /// ```rust /// # let a = 1; /// # let b = 2; - /// - /// // Bad /// match (a, b) { /// (c, d) => { /// // useless match /// } /// } + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # let a = 1; + /// # let b = 2; /// let (c, d) = (a, b); /// ``` #[clippy::version = "1.43.0"] @@ -419,13 +437,16 @@ declare_clippy_lint! { /// # struct A { a: i32 } /// let a = A { a: 5 }; /// - /// // Bad /// match a { /// A { a: 5, .. } => {}, /// _ => {}, /// } + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # struct A { a: i32 } + /// # let a = A { a: 5 }; /// match a { /// A { a: 5 } => {}, /// _ => {}, @@ -509,7 +530,6 @@ declare_clippy_lint! { /// ```rust /// let x = Some(5); /// - /// // Bad /// let a = match x { /// Some(0) => true, /// _ => false, @@ -520,8 +540,11 @@ declare_clippy_lint! { /// } else { /// false /// }; + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// let x = Some(5); /// let a = matches!(x, Some(0)); /// ``` #[clippy::version = "1.47.0"] @@ -695,19 +718,18 @@ declare_clippy_lint! { /// let arr = vec![0, 1, 2, 3]; /// let idx = 1; /// - /// // Bad /// match arr[idx] { /// 0 => println!("{}", 0), /// 1 => println!("{}", 3), /// _ => {}, /// } /// ``` + /// /// Use instead: /// ```rust, no_run /// let arr = vec![0, 1, 2, 3]; /// let idx = 1; /// - /// // Good /// match arr.get(idx) { /// Some(0) => println!("{}", 0), /// Some(1) => println!("{}", 3), diff --git a/clippy_lints/src/methods/filter_map.rs b/clippy_lints/src/methods/filter_map.rs index 3efccd703a6..58c3e52e138 100644 --- a/clippy_lints/src/methods/filter_map.rs +++ b/clippy_lints/src/methods/filter_map.rs @@ -6,7 +6,7 @@ use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::def::Res; -use rustc_hir::{Expr, ExprKind, PatKind, QPath, UnOp}; +use rustc_hir::{Expr, ExprKind, PatKind, PathSegment, QPath, UnOp}; use rustc_lint::LateContext; use rustc_span::source_map::Span; use rustc_span::symbol::{sym, Symbol}; @@ -155,7 +155,15 @@ pub(super) fn check<'tcx>( } false }; - if SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg); + + if match map_arg.kind { + ExprKind::MethodCall(method, [original_arg], _) => { + acceptable_methods(method) + && SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, original_arg) + }, + _ => SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg) + }; + then { let span = filter_span.with_hi(expr.span.hi()); let (filter_name, lint) = if is_find { @@ -171,3 +179,18 @@ pub(super) fn check<'tcx>( } } } + +fn acceptable_methods(method: &PathSegment<'_>) -> bool { + let methods: [Symbol; 8] = [ + sym::clone, + sym::as_ref, + sym!(copied), + sym!(cloned), + sym!(as_deref), + sym!(as_mut), + sym!(as_deref_mut), + sym!(to_owned), + ]; + + methods.contains(&method.ident.name) +} diff --git a/clippy_lints/src/methods/iter_overeager_cloned.rs b/clippy_lints/src/methods/iter_overeager_cloned.rs index 54c9ca435a4..06a39c5997e 100644 --- a/clippy_lints/src/methods/iter_overeager_cloned.rs +++ b/clippy_lints/src/methods/iter_overeager_cloned.rs @@ -1,70 +1,59 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::snippet; -use clippy_utils::ty::{get_iterator_item_ty, implements_trait, is_copy}; -use itertools::Itertools; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::snippet_opt; +use clippy_utils::ty::{get_associated_type, implements_trait, is_copy}; use rustc_errors::Applicability; -use rustc_hir as hir; +use rustc_hir::Expr; use rustc_lint::LateContext; use rustc_middle::ty; use rustc_span::sym; -use std::ops::Not; use super::ITER_OVEREAGER_CLONED; use crate::redundant_clone::REDUNDANT_CLONE; -/// lint overeager use of `cloned()` for `Iterator`s pub(super) fn check<'tcx>( cx: &LateContext<'tcx>, - expr: &'tcx hir::Expr<'_>, - recv: &'tcx hir::Expr<'_>, - name: &str, - map_arg: &[hir::Expr<'_>], + expr: &'tcx Expr<'_>, + cloned_call: &'tcx Expr<'_>, + cloned_recv: &'tcx Expr<'_>, + is_count: bool, + needs_into_iter: bool, ) { - // Check if it's iterator and get type associated with `Item`. - let inner_ty = if_chain! { - if let Some(iterator_trait_id) = cx.tcx.get_diagnostic_item(sym::Iterator); - let recv_ty = cx.typeck_results().expr_ty(recv); - if implements_trait(cx, recv_ty, iterator_trait_id, &[]); - if let Some(inner_ty) = get_iterator_item_ty(cx, cx.typeck_results().expr_ty_adjusted(recv)); - then { - inner_ty - } else { + let typeck = cx.typeck_results(); + if let Some(iter_id) = cx.tcx.get_diagnostic_item(sym::Iterator) + && let Some(method_id) = typeck.type_dependent_def_id(expr.hir_id) + && cx.tcx.trait_of_item(method_id) == Some(iter_id) + && let Some(method_id) = typeck.type_dependent_def_id(cloned_call.hir_id) + && cx.tcx.trait_of_item(method_id) == Some(iter_id) + && let cloned_recv_ty = typeck.expr_ty_adjusted(cloned_recv) + && let Some(iter_assoc_ty) = get_associated_type(cx, cloned_recv_ty, iter_id, "Item") + && matches!(*iter_assoc_ty.kind(), ty::Ref(_, ty, _) if !is_copy(cx, ty)) + { + if needs_into_iter + && let Some(into_iter_id) = cx.tcx.get_diagnostic_item(sym::IntoIterator) + && !implements_trait(cx, iter_assoc_ty, into_iter_id, &[]) + { return; } - }; - - match inner_ty.kind() { - ty::Ref(_, ty, _) if !is_copy(cx, *ty) => {}, - _ => return, - }; - let (lint, preserve_cloned) = match name { - "count" => (REDUNDANT_CLONE, false), - _ => (ITER_OVEREAGER_CLONED, true), - }; - let wildcard_params = map_arg.is_empty().not().then(|| "...").unwrap_or_default(); - let msg = format!( - "called `cloned().{}({})` on an `Iterator`. It may be more efficient to call `{}({}){}` instead", - name, - wildcard_params, - name, - wildcard_params, - preserve_cloned.then(|| ".cloned()").unwrap_or_default(), - ); + let (lint, msg, trailing_clone) = if is_count { + (REDUNDANT_CLONE, "unneeded cloning of iterator items", "") + } else { + (ITER_OVEREAGER_CLONED, "unnecessarily eager cloning of iterator items", ".cloned()") + }; - span_lint_and_sugg( - cx, - lint, - expr.span, - &msg, - "try this", - format!( - "{}.{}({}){}", - snippet(cx, recv.span, ".."), - name, - map_arg.iter().map(|a| snippet(cx, a.span, "..")).join(", "), - preserve_cloned.then(|| ".cloned()").unwrap_or_default(), - ), - Applicability::MachineApplicable, - ); + span_lint_and_then( + cx, + lint, + expr.span, + msg, + |diag| { + let method_span = expr.span.with_lo(cloned_call.span.hi()); + if let Some(mut snip) = snippet_opt(cx, method_span) { + snip.push_str(trailing_clone); + let replace_span = expr.span.with_lo(cloned_recv.span.hi()); + diag.span_suggestion(replace_span, "try this", snip, Applicability::MachineApplicable); + } + } + ); + } } diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 7308e74c323..9bb7bb7a7ab 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -124,28 +124,24 @@ declare_clippy_lint! { /// It's often inefficient to clone all elements of an iterator, when eventually, only some /// of them will be consumed. /// + /// ### Known Problems + /// This `lint` removes the side of effect of cloning items in the iterator. + /// A code that relies on that side-effect could fail. + /// /// ### Examples /// ```rust /// # let vec = vec!["string".to_string()]; - /// - /// // Bad /// vec.iter().cloned().take(10); - /// - /// // Good - /// vec.iter().take(10).cloned(); - /// - /// // Bad /// vec.iter().cloned().last(); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # let vec = vec!["string".to_string()]; + /// vec.iter().take(10).cloned(); /// vec.iter().last().cloned(); - /// /// ``` - /// ### Known Problems - /// This `lint` removes the side of effect of cloning items in the iterator. - /// A code that relies on that side-effect could fail. - /// - #[clippy::version = "1.59.0"] + #[clippy::version = "1.60.0"] pub ITER_OVEREAGER_CLONED, perf, "using `cloned()` early with `Iterator::iter()` can lead to some performance inefficiencies" @@ -342,11 +338,12 @@ declare_clippy_lint! { /// ### Example /// ```rust /// # let x = Ok::<_, ()>(()); - /// - /// // Bad /// x.ok().expect("why did I do this again?"); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # let x = Ok::<_, ()>(()); /// x.expect("why did I do this again?"); /// ``` #[clippy::version = "pre 1.29.0"] @@ -390,12 +387,13 @@ declare_clippy_lint! { /// ### Examples /// ```rust /// # let x = Some(1); - /// - /// // Bad /// x.unwrap_or_else(Default::default); /// x.unwrap_or_else(u32::default); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # let x = Some(1); /// x.unwrap_or_default(); /// ``` #[clippy::version = "1.56.0"] @@ -453,11 +451,12 @@ declare_clippy_lint! { /// ### Example /// ```rust /// # let opt = Some(1); - /// - /// // Bad /// opt.map_or(None, |a| Some(a + 1)); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # let opt = Some(1); /// opt.and_then(|a| Some(a + 1)); /// ``` #[clippy::version = "pre 1.29.0"] @@ -475,13 +474,12 @@ declare_clippy_lint! { /// `_.ok()`. /// /// ### Example - /// Bad: /// ```rust /// # let r: Result<u32, &str> = Ok(1); /// assert_eq!(Some(1), r.map_or(None, Some)); /// ``` /// - /// Good: + /// Use instead: /// ```rust /// # let r: Result<u32, &str> = Ok(1); /// assert_eq!(Some(1), r.ok()); @@ -538,7 +536,8 @@ declare_clippy_lint! { /// # let vec = vec![1]; /// vec.iter().filter(|x| **x == 0).next(); /// ``` - /// Could be written as + /// + /// Use instead: /// ```rust /// # let vec = vec![1]; /// vec.iter().find(|x| **x == 0); @@ -562,7 +561,8 @@ declare_clippy_lint! { /// # let vec = vec![1]; /// vec.iter().skip_while(|x| **x == 0).next(); /// ``` - /// Could be written as + /// + /// Use instead: /// ```rust /// # let vec = vec![1]; /// vec.iter().find(|x| **x != 0); @@ -586,11 +586,14 @@ declare_clippy_lint! { /// let vec = vec![vec![1]]; /// let opt = Some(5); /// - /// // Bad /// vec.iter().map(|x| x.iter()).flatten(); /// opt.map(|x| Some(x * 2)).flatten(); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # let vec = vec![vec![1]]; + /// # let opt = Some(5); /// vec.iter().flat_map(|x| x.iter()); /// opt.and_then(|x| Some(x * 2)); /// ``` @@ -610,15 +613,16 @@ declare_clippy_lint! { /// less performant. /// /// ### Example - /// Bad: /// ```rust + /// # #![allow(unused)] /// (0_i32..10) /// .filter(|n| n.checked_add(1).is_some()) /// .map(|n| n.checked_add(1).unwrap()); /// ``` /// - /// Good: + /// Use instead: /// ```rust + /// # #[allow(unused)] /// (0_i32..10).filter_map(|n| n.checked_add(1)); /// ``` #[clippy::version = "1.51.0"] @@ -637,14 +641,13 @@ declare_clippy_lint! { /// less performant. /// /// ### Example - /// Bad: /// ```rust /// (0_i32..10) /// .find(|n| n.checked_add(1).is_some()) /// .map(|n| n.checked_add(1).unwrap()); /// ``` /// - /// Good: + /// Use instead: /// ```rust /// (0_i32..10).find_map(|n| n.checked_add(1)); /// ``` @@ -712,17 +715,20 @@ declare_clippy_lint! { /// /// ### Example /// ```rust + /// # #![allow(unused)] /// let vec = vec![1]; /// vec.iter().find(|x| **x == 0).is_some(); /// - /// let _ = "hello world".find("world").is_none(); + /// "hello world".find("world").is_none(); /// ``` - /// Could be written as + /// + /// Use instead: /// ```rust /// let vec = vec![1]; /// vec.iter().any(|x| *x == 0); /// - /// let _ = !"hello world".contains("world"); + /// # #[allow(unused)] + /// !"hello world".contains("world"); /// ``` #[clippy::version = "pre 1.29.0"] pub SEARCH_IS_SOME, @@ -744,7 +750,8 @@ declare_clippy_lint! { /// let name = "foo"; /// if name.chars().next() == Some('_') {}; /// ``` - /// Could be written as + /// + /// Use instead: /// ```rust /// let name = "foo"; /// if name.starts_with('_') {}; @@ -899,10 +906,13 @@ declare_clippy_lint! { /// # use std::rc::Rc; /// let x = Rc::new(1); /// - /// // Bad /// x.clone(); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # use std::rc::Rc; + /// # let x = Rc::new(1); /// Rc::clone(&x); /// ``` #[clippy::version = "pre 1.29.0"] @@ -1034,11 +1044,13 @@ declare_clippy_lint! { /// /// ### Example /// ```rust,ignore - /// // Bad /// _.split("x"); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust,ignore /// _.split('x'); + /// ``` #[clippy::version = "pre 1.29.0"] pub SINGLE_CHAR_PATTERN, perf, @@ -1099,12 +1111,14 @@ declare_clippy_lint! { /// ### Example /// ```rust /// # use std::collections::HashSet; - /// // Bad /// # let mut s = HashSet::new(); /// # s.insert(1); /// let x = s.iter().nth(0); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # use std::collections::HashSet; /// # let mut s = HashSet::new(); /// # s.insert(1); /// let x = s.iter().next(); @@ -1210,11 +1224,12 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// let x = vec![2, 3, 5]; /// let last_element = x.get(x.len() - 1); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// let x = vec![2, 3, 5]; /// let last_element = x.last(); /// ``` @@ -1273,10 +1288,14 @@ declare_clippy_lint! { /// let mut a = vec![1, 2, 3]; /// let mut b = vec![4, 5, 6]; /// - /// // Bad /// a.extend(b.drain(..)); + /// ``` + /// + /// Use instead: + /// ```rust + /// let mut a = vec![1, 2, 3]; + /// let mut b = vec![4, 5, 6]; /// - /// // Good /// a.append(&mut b); /// ``` #[clippy::version = "1.55.0"] @@ -1351,11 +1370,12 @@ declare_clippy_lint! { /// ### Example /// ```rust /// # let name = "_"; - /// - /// // Bad /// name.chars().last() == Some('_') || name.chars().next_back() == Some('-'); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # let name = "_"; /// name.ends_with('_') || name.ends_with('-'); /// ``` #[clippy::version = "pre 1.29.0"] @@ -1401,11 +1421,13 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// let _ = (0..3).fold(false, |acc, x| acc || x > 2); + /// # #[allow(unused)] + /// (0..3).fold(false, |acc, x| acc || x > 2); /// ``` - /// This could be written as: + /// + /// Use instead: /// ```rust - /// let _ = (0..3).any(|x| x > 2); + /// (0..3).any(|x| x > 2); /// ``` #[clippy::version = "pre 1.29.0"] pub UNNECESSARY_FOLD, @@ -1485,11 +1507,14 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad - /// let _ = (&vec![3, 4, 5]).into_iter(); + /// # let vec = vec![3, 4, 5]; + /// (&vec).into_iter(); + /// ``` /// - /// // Good - /// let _ = (&vec![3, 4, 5]).iter(); + /// Use instead: + /// ```rust + /// # let vec = vec![3, 4, 5]; + /// (&vec).iter(); /// ``` #[clippy::version = "1.32.0"] pub INTO_ITER_ON_REF, @@ -1704,13 +1729,14 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// let mut string = String::new(); + /// # let mut string = String::new(); /// string.insert_str(0, "R"); /// string.push_str("R"); /// ``` - /// Could be written as + /// + /// Use instead: /// ```rust - /// let mut string = String::new(); + /// # let mut string = String::new(); /// string.insert(0, 'R'); /// string.push('R'); /// ``` @@ -1881,7 +1907,7 @@ declare_clippy_lint! { /// let x = [1, 2, 3]; /// let y: Vec<_> = x.iter().map(|x| 2*x).collect(); /// ``` - #[clippy::version = "1.52.0"] + #[clippy::version = "1.47.0"] pub MAP_IDENTITY, complexity, "using iterator.map(|x| x)" @@ -1897,11 +1923,14 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad - /// let _ = "Hello".bytes().nth(3); + /// # #[allow(unused)] + /// "Hello".bytes().nth(3); + /// ``` /// - /// // Good - /// let _ = "Hello".as_bytes().get(3); + /// Use instead: + /// ```rust + /// # #[allow(unused)] + /// "Hello".as_bytes().get(3); /// ``` #[clippy::version = "1.52.0"] pub BYTES_NTH, @@ -1945,15 +1974,19 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad + /// # #![allow(unused)] /// let some_vec = vec![0, 1, 2, 3]; - /// let _ = some_vec.iter().count(); - /// let _ = &some_vec[..].iter().count(); /// - /// // Good + /// some_vec.iter().count(); + /// &some_vec[..].iter().count(); + /// ``` + /// + /// Use instead: + /// ```rust /// let some_vec = vec![0, 1, 2, 3]; - /// let _ = some_vec.len(); - /// let _ = &some_vec[..].len(); + /// + /// some_vec.len(); + /// &some_vec[..].len(); /// ``` #[clippy::version = "1.52.0"] pub ITER_COUNT, @@ -1973,16 +2006,17 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad - /// let s = ""; + /// # let s = ""; /// for x in s.splitn(1, ":") { - /// // use x + /// // .. /// } + /// ``` /// - /// // Good - /// let s = ""; + /// Use instead: + /// ```rust + /// # let s = ""; /// for x in s.splitn(2, ":") { - /// // use x + /// // .. /// } /// ``` #[clippy::version = "1.54.0"] @@ -2000,10 +2034,11 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// let x: String = std::iter::repeat('x').take(10).collect(); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// let x: String = "x".repeat(10); /// ``` #[clippy::version = "1.54.0"] @@ -2021,7 +2056,6 @@ declare_clippy_lint! { /// /// ### Example /// ```rust,ignore - /// // Bad /// let s = "key=value=add"; /// let (key, value) = s.splitn(2, '=').next_tuple()?; /// let value = s.splitn(2, '=').nth(1)?; @@ -2030,9 +2064,9 @@ declare_clippy_lint! { /// let key = parts.next()?; /// let value = parts.next()?; /// ``` + /// /// Use instead: /// ```rust,ignore - /// // Good /// let s = "key=value=add"; /// let (key, value) = s.split_once('=')?; /// let value = s.split_once('=')?.1; @@ -2057,17 +2091,16 @@ declare_clippy_lint! { /// that both functions return a lazy iterator. /// ### Example /// ```rust - /// // Bad /// let str = "key=value=add"; /// let _ = str.splitn(3, '=').next().unwrap(); /// ``` + /// /// Use instead: /// ```rust - /// // Good /// let str = "key=value=add"; /// let _ = str.split('=').next().unwrap(); /// ``` - #[clippy::version = "1.58.0"] + #[clippy::version = "1.59.0"] pub NEEDLESS_SPLITN, complexity, "usages of `str::splitn` that can be replaced with `str::split`" @@ -2098,7 +2131,7 @@ declare_clippy_lint! { /// foo(&path.to_string_lossy()); /// fn foo(s: &str) {} /// ``` - #[clippy::version = "1.58.0"] + #[clippy::version = "1.59.0"] pub UNNECESSARY_TO_OWNED, perf, "unnecessary calls to `to_owned`-like functions" @@ -2149,7 +2182,8 @@ declare_clippy_lint! { /// let a = Some(&1); /// let b = a.as_deref(); // goes from Option<&i32> to Option<&i32> /// ``` - /// Could be written as: + /// + /// Use instead: /// ```rust /// let a = Some(&1); /// let b = a; @@ -2583,8 +2617,8 @@ impl Methods { }, _ => {}, }, - (name @ "count", args @ []) => match method_call(recv) { - Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv2, name, args), + ("count", []) => match method_call(recv) { + Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, true, false), Some((name2 @ ("into_iter" | "iter" | "iter_mut"), [recv2], _)) => { iter_count::check(cx, expr, recv2, name2); }, @@ -2614,9 +2648,9 @@ impl Methods { flat_map_identity::check(cx, expr, arg, span); flat_map_option::check(cx, expr, arg, span); }, - (name @ "flatten", args @ []) => match method_call(recv) { + ("flatten", []) => match method_call(recv) { Some(("map", [recv, map_arg], map_span)) => map_flatten::check(cx, expr, recv, map_arg, map_span), - Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv2, name, args), + Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, true), _ => {}, }, ("fold", [init, acc]) => unnecessary_fold::check(cx, expr, init, acc, span), @@ -2636,10 +2670,10 @@ impl Methods { unnecessary_join::check(cx, expr, recv, join_arg, span); } }, - ("last", args @ []) | ("skip", args @ [_]) => { + ("last", []) | ("skip", [_]) => { if let Some((name2, [recv2, args2 @ ..], _span2)) = method_call(recv) { if let ("cloned", []) = (name2, args2) { - iter_overeager_cloned::check(cx, expr, recv2, name, args); + iter_overeager_cloned::check(cx, expr, recv, recv2, false, false); } } }, @@ -2660,10 +2694,10 @@ impl Methods { map_identity::check(cx, expr, recv, m_arg, name, span); }, ("map_or", [def, map]) => option_map_or_none::check(cx, expr, recv, def, map), - (name @ "next", args @ []) => { + ("next", []) => { if let Some((name2, [recv2, args2 @ ..], _)) = method_call(recv) { match (name2, args2) { - ("cloned", []) => iter_overeager_cloned::check(cx, expr, recv2, name, args), + ("cloned", []) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false), ("filter", [arg]) => filter_next::check(cx, expr, recv2, arg), ("filter_map", [arg]) => filter_map_next::check(cx, expr, recv2, arg, self.msrv), ("iter", []) => iter_next_slice::check(cx, expr, recv2), @@ -2673,9 +2707,9 @@ impl Methods { } } }, - ("nth", args @ [n_arg]) => match method_call(recv) { + ("nth", [n_arg]) => match method_call(recv) { Some(("bytes", [recv2], _)) => bytes_nth::check(cx, expr, recv2, n_arg), - Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv2, name, args), + Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false), Some(("iter", [recv2], _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, false), Some(("iter_mut", [recv2], _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, true), _ => iter_nth_zero::check(cx, expr, recv, n_arg), @@ -2698,10 +2732,10 @@ impl Methods { } }, ("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg), - ("take", args @ [_arg]) => { + ("take", [_arg]) => { if let Some((name2, [recv2, args2 @ ..], _span2)) = method_call(recv) { if let ("cloned", []) = (name2, args2) { - iter_overeager_cloned::check(cx, expr, recv2, name, args); + iter_overeager_cloned::check(cx, expr, recv, recv2, false, false); } } }, diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 55665699453..01bf871198a 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -45,16 +45,13 @@ declare_clippy_lint! { /// dereferences, e.g., changing `*x` to `x` within the function. /// /// ### Example - /// ```rust,ignore - /// // Bad - /// fn foo(ref x: u8) -> bool { - /// true - /// } + /// ```rust + /// fn foo(ref _x: u8) {} + /// ``` /// - /// // Good - /// fn foo(x: &u8) -> bool { - /// true - /// } + /// Use instead: + /// ```rust + /// fn foo(_x: &u8) {} /// ``` #[clippy::version = "pre 1.29.0"] pub TOPLEVEL_REF_ARG, @@ -73,11 +70,12 @@ declare_clippy_lint! { /// ### Example /// ```rust /// # let x = 1.0; - /// - /// // Bad /// if x == f32::NAN { } + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # let x = 1.0f32; /// if x.is_nan() { } /// ``` #[clippy::version = "pre 1.29.0"] @@ -139,7 +137,8 @@ declare_clippy_lint! { /// # let y = String::from("foo"); /// if x.to_owned() == y {} /// ``` - /// Could be written as + /// + /// Use instead: /// ```rust /// # let x = "foo"; /// # let y = String::from("foo"); @@ -232,10 +231,11 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// let a = 0 as *const u32; + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// let a = std::ptr::null::<u32>(); /// ``` #[clippy::version = "pre 1.29.0"] diff --git a/clippy_lints/src/misc_early/mod.rs b/clippy_lints/src/misc_early/mod.rs index 6860b60acbd..704918c0b97 100644 --- a/clippy_lints/src/misc_early/mod.rs +++ b/clippy_lints/src/misc_early/mod.rs @@ -34,13 +34,21 @@ declare_clippy_lint! { /// # } /// let f = Foo { a: 0, b: 0, c: 0 }; /// - /// // Bad /// match f { /// Foo { a: _, b: 0, .. } => {}, /// Foo { a: _, b: _, c: _ } => {}, /// } + /// ``` + /// + /// Use instead: + /// ```rust + /// # struct Foo { + /// # a: i32, + /// # b: i32, + /// # c: i32, + /// # } + /// let f = Foo { a: 0, b: 0, c: 0 }; /// - /// // Good /// match f { /// Foo { b: 0, .. } => {}, /// Foo { .. } => {}, @@ -62,10 +70,11 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// fn foo(a: i32, _a: i32) {} + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// fn bar(a: i32, _b: i32) {} /// ``` #[clippy::version = "pre 1.29.0"] @@ -103,11 +112,16 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad - /// let y = 0x1a9BAcD; + /// # let _ = + /// 0x1a9BAcD + /// # ; + /// ``` /// - /// // Good - /// let y = 0x1A9BACD; + /// Use instead: + /// ```rust + /// # let _ = + /// 0x1A9BACD + /// # ; /// ``` #[clippy::version = "pre 1.29.0"] pub MIXED_CASE_HEX_LITERALS, @@ -127,11 +141,16 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad - /// let y = 123832i32; + /// # let _ = + /// 123832i32 + /// # ; + /// ``` /// - /// // Good - /// let y = 123832_i32; + /// Use instead: + /// ```rust + /// # let _ = + /// 123832_i32 + /// # ; /// ``` #[clippy::version = "pre 1.29.0"] pub UNSEPARATED_LITERAL_SUFFIX, @@ -150,11 +169,16 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad - /// let y = 123832_i32; + /// # let _ = + /// 123832_i32 + /// # ; + /// ``` /// - /// // Good - /// let y = 123832i32; + /// Use instead: + /// ```rust + /// # let _ = + /// 123832i32 + /// # ; /// ``` #[clippy::version = "1.58.0"] pub SEPARATED_LITERAL_SUFFIX, @@ -234,14 +258,15 @@ declare_clippy_lint! { /// ### Example /// ```rust /// # let v = Some("abc"); - /// - /// // Bad /// match v { /// Some(x) => (), /// y @ _ => (), /// } + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # let v = Some("abc"); /// match v { /// Some(x) => (), /// y => (), @@ -262,6 +287,7 @@ declare_clippy_lint! { /// means there are 0 or more elements left. This can make a difference /// when refactoring, but shouldn't result in errors in the refactored code, /// since the wildcard pattern isn't used anyway. + /// /// ### Why is this bad? /// The wildcard pattern is unneeded as the rest pattern /// can match that element as well. @@ -270,13 +296,16 @@ declare_clippy_lint! { /// ```rust /// # struct TupleStruct(u32, u32, u32); /// # let t = TupleStruct(1, 2, 3); - /// // Bad /// match t { /// TupleStruct(0, .., _) => (), /// _ => (), /// } + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # struct TupleStruct(u32, u32, u32); + /// # let t = TupleStruct(1, 2, 3); /// match t { /// TupleStruct(0, ..) => (), /// _ => (), diff --git a/clippy_lints/src/mixed_read_write_in_expression.rs b/clippy_lints/src/mixed_read_write_in_expression.rs index c3b850fbb9d..a2419c277e9 100644 --- a/clippy_lints/src/mixed_read_write_in_expression.rs +++ b/clippy_lints/src/mixed_read_write_in_expression.rs @@ -25,14 +25,16 @@ declare_clippy_lint! { /// ```rust /// let mut x = 0; /// - /// // Bad /// let a = { /// x = 1; /// 1 /// } + x; /// // Unclear whether a is 1 or 2. + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # let mut x = 0; /// let tmp = { /// x = 1; /// 1 diff --git a/clippy_lints/src/mut_reference.rs b/clippy_lints/src/mut_reference.rs index 9d8f8999ce4..f434a655f8a 100644 --- a/clippy_lints/src/mut_reference.rs +++ b/clippy_lints/src/mut_reference.rs @@ -16,12 +16,17 @@ declare_clippy_lint! { /// the value. Also the code misleads about the intent of the call site. /// /// ### Example - /// ```ignore - /// // Bad - /// my_vec.push(&mut value) + /// ```rust + /// # let mut vec = Vec::new(); + /// # let mut value = 5; + /// vec.push(&mut value); + /// ``` /// - /// // Good - /// my_vec.push(&value) + /// Use instead: + /// ```rust + /// # let mut vec = Vec::new(); + /// # let value = 5; + /// vec.push(&value); /// ``` #[clippy::version = "pre 1.29.0"] pub UNNECESSARY_MUT_PASSED, diff --git a/clippy_lints/src/mutex_atomic.rs b/clippy_lints/src/mutex_atomic.rs index 73823779e49..a98577093ed 100644 --- a/clippy_lints/src/mutex_atomic.rs +++ b/clippy_lints/src/mutex_atomic.rs @@ -27,12 +27,13 @@ declare_clippy_lint! { /// ### Example /// ```rust /// # let y = true; - /// - /// // Bad /// # use std::sync::Mutex; /// let x = Mutex::new(&y); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # let y = true; /// # use std::sync::atomic::AtomicBool; /// let x = AtomicBool::new(y); /// ``` @@ -60,8 +61,10 @@ declare_clippy_lint! { /// ```rust /// # use std::sync::Mutex; /// let x = Mutex::new(0usize); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// # use std::sync::atomic::AtomicUsize; /// let x = AtomicUsize::new(0usize); /// ``` diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs index 778d49cb4b6..a4eec95b371 100644 --- a/clippy_lints/src/needless_bool.rs +++ b/clippy_lints/src/needless_bool.rs @@ -30,16 +30,21 @@ declare_clippy_lint! { /// shorter code. /// /// ### Example - /// ```rust,ignore + /// ```rust + /// # let x = true; /// if x { /// false /// } else { /// true /// } + /// # ; /// ``` - /// Could be written as - /// ```rust,ignore + /// + /// Use instead: + /// ```rust + /// # let x = true; /// !x + /// # ; /// ``` #[clippy::version = "pre 1.29.0"] pub NEEDLESS_BOOL, diff --git a/clippy_lints/src/needless_borrowed_ref.rs b/clippy_lints/src/needless_borrowed_ref.rs index 0fcc419e722..05c012b92e8 100644 --- a/clippy_lints/src/needless_borrowed_ref.rs +++ b/clippy_lints/src/needless_borrowed_ref.rs @@ -27,16 +27,17 @@ declare_clippy_lint! { /// ``` /// /// ### Example - /// Bad: /// ```rust /// let mut v = Vec::<String>::new(); - /// let _ = v.iter_mut().filter(|&ref a| a.is_empty()); + /// # #[allow(unused)] + /// v.iter_mut().filter(|&ref a| a.is_empty()); /// ``` /// - /// Good: + /// Use instead: /// ```rust /// let mut v = Vec::<String>::new(); - /// let _ = v.iter_mut().filter(|a| a.is_empty()); + /// # #[allow(unused)] + /// v.iter_mut().filter(|a| a.is_empty()); /// ``` #[clippy::version = "pre 1.29.0"] pub NEEDLESS_BORROWED_REFERENCE, diff --git a/clippy_lints/src/needless_late_init.rs b/clippy_lints/src/needless_late_init.rs index 1f8c4c85cc2..ff2999b1f4a 100644 --- a/clippy_lints/src/needless_late_init.rs +++ b/clippy_lints/src/needless_late_init.rs @@ -56,7 +56,7 @@ declare_clippy_lint! { /// -1 /// }; /// ``` - #[clippy::version = "1.58.0"] + #[clippy::version = "1.59.0"] pub NEEDLESS_LATE_INIT, style, "late initializations that can be replaced by a `let` statement with an initializer" @@ -185,14 +185,14 @@ fn assignment_suggestions<'tcx>( let suggestions = assignments .iter() - .map(|assignment| Some((assignment.span.until(assignment.rhs_span), String::new()))) - .chain(assignments.iter().map(|assignment| { - Some(( + .flat_map(|assignment| { + [ + assignment.span.until(assignment.rhs_span), assignment.rhs_span.shrink_to_hi().with_hi(assignment.span.hi()), - String::new(), - )) - })) - .collect::<Option<Vec<(Span, String)>>>()?; + ] + }) + .map(|span| (span, String::new())) + .collect::<Vec<(Span, String)>>(); match suggestions.len() { // All of `exprs` are never types diff --git a/clippy_lints/src/needless_parens_on_range_literals.rs b/clippy_lints/src/needless_parens_on_range_literals.rs new file mode 100644 index 00000000000..6e54b243c03 --- /dev/null +++ b/clippy_lints/src/needless_parens_on_range_literals.rs @@ -0,0 +1,87 @@ +use clippy_utils::{ + diagnostics::span_lint_and_then, + higher, + source::{snippet, snippet_with_applicability}, +}; + +use rustc_ast::ast; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; + +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// The lint checks for parenthesis on literals in range statements that are + /// superfluous. + /// + /// ### Why is this bad? + /// Having superfluous parenthesis makes the code less readable + /// overhead when reading. + /// + /// ### Example + /// + /// ```rust + /// for i in (0)..10 { + /// println!("{i}"); + /// } + /// ``` + /// + /// Use instead: + /// + /// ```rust + /// for i in 0..10 { + /// println!("{i}"); + /// } + /// ``` + #[clippy::version = "1.63.0"] + pub NEEDLESS_PARENS_ON_RANGE_LITERALS, + style, + "needless parenthesis on range literals can be removed" +} + +declare_lint_pass!(NeedlessParensOnRangeLiterals => [NEEDLESS_PARENS_ON_RANGE_LITERALS]); + +fn snippet_enclosed_in_parenthesis(snippet: &str) -> bool { + snippet.starts_with('(') && snippet.ends_with(')') +} + +fn check_for_parens(cx: &LateContext<'_>, e: &Expr<'_>, is_start: bool) { + if is_start && + let ExprKind::Lit(ref literal) = e.kind && + let ast::LitKind::Float(_sym, ast::LitFloatType::Unsuffixed) = literal.node + { + // don't check floating point literals on the start expression of a range + return; + } + if_chain! { + if let ExprKind::Lit(ref literal) = e.kind; + // the indicator that parenthesis surround the literal is that the span of the expression and the literal differ + if (literal.span.data().hi - literal.span.data().lo) != (e.span.data().hi - e.span.data().lo); + // inspect the source code of the expression for parenthesis + if snippet_enclosed_in_parenthesis(&snippet(cx, e.span, "")); + then { + let mut applicability = Applicability::MachineApplicable; + span_lint_and_then(cx, NEEDLESS_PARENS_ON_RANGE_LITERALS, e.span, + "needless parenthesis on range literals can be removed", + |diag| { + let suggestion = snippet_with_applicability(cx, literal.span, "_", &mut applicability); + diag.span_suggestion(e.span, "try", suggestion, applicability); + }); + } + } +} + +impl<'tcx> LateLintPass<'tcx> for NeedlessParensOnRangeLiterals { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if let Some(higher::Range { start, end, .. }) = higher::Range::hir(expr) { + if let Some(start) = start { + check_for_parens(cx, start, true); + } + if let Some(end) = end { + check_for_parens(cx, end, false); + } + } + } +} diff --git a/clippy_lints/src/needless_update.rs b/clippy_lints/src/needless_update.rs index c87c174ef73..0bd29d1776b 100644 --- a/clippy_lints/src/needless_update.rs +++ b/clippy_lints/src/needless_update.rs @@ -24,16 +24,17 @@ declare_clippy_lint! { /// # z: i32, /// # } /// # let zero_point = Point { x: 0, y: 0, z: 0 }; - /// - /// // Bad /// Point { /// x: 1, /// y: 1, /// z: 1, /// ..zero_point /// }; + /// ``` /// - /// // Ok + /// Use instead: + /// ```rust,ignore + /// // Missing field `z` /// Point { /// x: 1, /// y: 1, diff --git a/clippy_lints/src/neg_cmp_op_on_partial_ord.rs b/clippy_lints/src/neg_cmp_op_on_partial_ord.rs index efe31a15441..a7e0e35787c 100644 --- a/clippy_lints/src/neg_cmp_op_on_partial_ord.rs +++ b/clippy_lints/src/neg_cmp_op_on_partial_ord.rs @@ -19,17 +19,17 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// use std::cmp::Ordering; - /// - /// // Bad /// let a = 1.0; /// let b = f64::NAN; /// - /// let _not_less_or_equal = !(a <= b); + /// let not_less_or_equal = !(a <= b); + /// ``` /// - /// // Good - /// let a = 1.0; - /// let b = f64::NAN; + /// Use instead: + /// ```rust + /// use std::cmp::Ordering; + /// # let a = 1.0; + /// # let b = f64::NAN; /// /// let _not_less_or_equal = match a.partial_cmp(&b) { /// None | Some(Ordering::Greater) => true, diff --git a/clippy_lints/src/neg_multiply.rs b/clippy_lints/src/neg_multiply.rs index 707f3b2181a..ce6bb38b7c0 100644 --- a/clippy_lints/src/neg_multiply.rs +++ b/clippy_lints/src/neg_multiply.rs @@ -19,12 +19,13 @@ declare_clippy_lint! { /// This only catches integers (for now). /// /// ### Example - /// ```ignore - /// // Bad + /// ```rust,ignore /// let a = x * -1; + /// ``` /// - /// // Good - /// let b = -x; + /// Use instead: + /// ```rust,ignore + /// let a = -x; /// ``` #[clippy::version = "pre 1.29.0"] pub NEG_MULTIPLY, diff --git a/clippy_lints/src/non_copy_const.rs b/clippy_lints/src/non_copy_const.rs index 7163cfe5e3a..1727275a4e0 100644 --- a/clippy_lints/src/non_copy_const.rs +++ b/clippy_lints/src/non_copy_const.rs @@ -59,12 +59,14 @@ declare_clippy_lint! { /// ```rust /// use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; /// - /// // Bad. /// const CONST_ATOM: AtomicUsize = AtomicUsize::new(12); /// CONST_ATOM.store(6, SeqCst); // the content of the atomic is unchanged /// assert_eq!(CONST_ATOM.load(SeqCst), 12); // because the CONST_ATOM in these lines are distinct + /// ``` /// - /// // Good. + /// Use instead: + /// ```rust + /// # use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; /// static STATIC_ATOM: AtomicUsize = AtomicUsize::new(15); /// STATIC_ATOM.store(9, SeqCst); /// assert_eq!(STATIC_ATOM.load(SeqCst), 9); // use a `static` item to refer to the same instance @@ -105,11 +107,15 @@ declare_clippy_lint! { /// use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; /// const CONST_ATOM: AtomicUsize = AtomicUsize::new(12); /// - /// // Bad. /// CONST_ATOM.store(6, SeqCst); // the content of the atomic is unchanged /// assert_eq!(CONST_ATOM.load(SeqCst), 12); // because the CONST_ATOM in these lines are distinct + /// ``` + /// + /// Use instead: + /// ```rust + /// use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; + /// const CONST_ATOM: AtomicUsize = AtomicUsize::new(12); /// - /// // Good. /// static STATIC_ATOM: AtomicUsize = CONST_ATOM; /// STATIC_ATOM.store(9, SeqCst); /// assert_eq!(STATIC_ATOM.load(SeqCst), 9); // use a `static` item to refer to the same instance diff --git a/clippy_lints/src/octal_escapes.rs b/clippy_lints/src/octal_escapes.rs index e8532db4f71..3ab4b6c4f6f 100644 --- a/clippy_lints/src/octal_escapes.rs +++ b/clippy_lints/src/octal_escapes.rs @@ -33,15 +33,16 @@ declare_clippy_lint! { /// /// # Example /// ```rust - /// // Bad /// let one = "\033[1m Bold? \033[0m"; // \033 intended as escape /// let two = "\033\0"; // \033 intended as null-3-3 + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// let one = "\x1b[1mWill this be bold?\x1b[0m"; /// let two = "\x0033\x00"; /// ``` - #[clippy::version = "1.58.0"] + #[clippy::version = "1.59.0"] pub OCTAL_ESCAPES, suspicious, "string escape sequences looking like octal characters" diff --git a/clippy_lints/src/only_used_in_recursion.rs b/clippy_lints/src/only_used_in_recursion.rs index de5f77f3ad9..677ac998b56 100644 --- a/clippy_lints/src/only_used_in_recursion.rs +++ b/clippy_lints/src/only_used_in_recursion.rs @@ -87,7 +87,7 @@ declare_clippy_lint! { /// # print!("{}", f(1)); /// # } /// ``` - #[clippy::version = "1.60.0"] + #[clippy::version = "1.61.0"] pub ONLY_USED_IN_RECURSION, nursery, "arguments that is only used in recursion can be removed" diff --git a/clippy_lints/src/pass_by_ref_or_value.rs b/clippy_lints/src/pass_by_ref_or_value.rs index 5a93431f25a..05ab62786f4 100644 --- a/clippy_lints/src/pass_by_ref_or_value.rs +++ b/clippy_lints/src/pass_by_ref_or_value.rs @@ -57,12 +57,11 @@ declare_clippy_lint! { /// ### Example /// /// ```rust - /// // Bad /// fn foo(v: &u32) {} /// ``` /// + /// Use instead: /// ```rust - /// // Better /// fn foo(v: u32) {} /// ``` #[clippy::version = "pre 1.29.0"] @@ -89,14 +88,13 @@ declare_clippy_lint! { /// #[derive(Clone, Copy)] /// struct TooLarge([u8; 2048]); /// - /// // Bad /// fn foo(v: TooLarge) {} /// ``` - /// ```rust - /// #[derive(Clone, Copy)] - /// struct TooLarge([u8; 2048]); /// - /// // Good + /// Use instead: + /// ```rust + /// # #[derive(Clone, Copy)] + /// # struct TooLarge([u8; 2048]); /// fn foo(v: &TooLarge) {} /// ``` #[clippy::version = "1.49.0"] diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index 0b96f6ff683..b06eba13d2f 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -48,10 +48,11 @@ declare_clippy_lint! { /// /// ### Example /// ```ignore - /// // Bad /// fn foo(&Vec<u32>) { .. } + /// ``` /// - /// // Good + /// Use instead: + /// ```ignore /// fn foo(&[u32]) { .. } /// ``` #[clippy::version = "pre 1.29.0"] @@ -70,15 +71,18 @@ declare_clippy_lint! { /// method instead /// /// ### Example - /// ```ignore - /// // Bad + /// ```rust,ignore + /// use std::ptr; + /// /// if x == ptr::null { - /// .. + /// // .. /// } + /// ``` /// - /// // Good + /// Use instead: + /// ```rust,ignore /// if x.is_null() { - /// .. + /// // .. /// } /// ``` #[clippy::version = "pre 1.29.0"] @@ -129,12 +133,12 @@ declare_clippy_lint! { /// /// ### Example /// ```ignore - /// // Bad. Undefined behavior + /// // Undefined behavior /// unsafe { std::slice::from_raw_parts(ptr::null(), 0); } /// ``` /// + /// Use instead: /// ```ignore - /// // Good /// unsafe { std::slice::from_raw_parts(NonNull::dangling().as_ptr(), 0); } /// ``` #[clippy::version = "1.53.0"] diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index eea036178b8..547d4da8187 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -27,12 +27,13 @@ declare_clippy_lint! { /// ### Example /// ```rust /// # let x = vec![1]; - /// x.iter().zip(0..x.len()); + /// let _ = x.iter().zip(0..x.len()); /// ``` - /// Could be written as + /// + /// Use instead: /// ```rust /// # let x = vec![1]; - /// x.iter().enumerate(); + /// let _ = x.iter().enumerate(); /// ``` #[clippy::version = "pre 1.29.0"] pub RANGE_ZIP_WITH_LEN, @@ -65,12 +66,21 @@ declare_clippy_lint! { /// ([#3307](https://github.com/rust-lang/rust-clippy/issues/3307)). /// /// ### Example - /// ```rust,ignore - /// for x..(y+1) { .. } + /// ```rust + /// # let x = 0; + /// # let y = 1; + /// for i in x..(y+1) { + /// // .. + /// } /// ``` - /// Could be written as - /// ```rust,ignore - /// for x..=y { .. } + /// + /// Use instead: + /// ```rust + /// # let x = 0; + /// # let y = 1; + /// for i in x..=y { + /// // .. + /// } /// ``` #[clippy::version = "pre 1.29.0"] pub RANGE_PLUS_ONE, @@ -94,12 +104,21 @@ declare_clippy_lint! { /// ([#3307](https://github.com/rust-lang/rust-clippy/issues/3307)). /// /// ### Example - /// ```rust,ignore - /// for x..=(y-1) { .. } + /// ```rust + /// # let x = 0; + /// # let y = 1; + /// for i in x..=(y-1) { + /// // .. + /// } /// ``` - /// Could be written as - /// ```rust,ignore - /// for x..y { .. } + /// + /// Use instead: + /// ```rust + /// # let x = 0; + /// # let y = 1; + /// for i in x..y { + /// // .. + /// } /// ``` #[clippy::version = "pre 1.29.0"] pub RANGE_MINUS_ONE, diff --git a/clippy_lints/src/read_zero_byte_vec.rs b/clippy_lints/src/read_zero_byte_vec.rs new file mode 100644 index 00000000000..9538a810473 --- /dev/null +++ b/clippy_lints/src/read_zero_byte_vec.rs @@ -0,0 +1,142 @@ +use clippy_utils::{ + diagnostics::{span_lint, span_lint_and_sugg}, + higher::{get_vec_init_kind, VecInitKind}, + source::snippet, + visitors::expr_visitor_no_bodies, +}; +use hir::{intravisit::Visitor, ExprKind, Local, PatKind, PathSegment, QPath, StmtKind}; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// This lint catches reads into a zero-length `Vec`. + /// Especially in the case of a call to `with_capacity`, this lint warns that read + /// gets the number of bytes from the `Vec`'s length, not its capacity. + /// + /// ### Why is this bad? + /// Reading zero bytes is almost certainly not the intended behavior. + /// + /// ### Known problems + /// In theory, a very unusual read implementation could assign some semantic meaning + /// to zero-byte reads. But it seems exceptionally unlikely that code intending to do + /// a zero-byte read would allocate a `Vec` for it. + /// + /// ### Example + /// ```rust + /// use std::io; + /// fn foo<F: io::Read>(mut f: F) { + /// let mut data = Vec::with_capacity(100); + /// f.read(&mut data).unwrap(); + /// } + /// ``` + /// Use instead: + /// ```rust + /// use std::io; + /// fn foo<F: io::Read>(mut f: F) { + /// let mut data = Vec::with_capacity(100); + /// data.resize(100, 0); + /// f.read(&mut data).unwrap(); + /// } + /// ``` + #[clippy::version = "1.63.0"] + pub READ_ZERO_BYTE_VEC, + correctness, + "checks for reads into a zero-length `Vec`" +} +declare_lint_pass!(ReadZeroByteVec => [READ_ZERO_BYTE_VEC]); + +impl<'tcx> LateLintPass<'tcx> for ReadZeroByteVec { + fn check_block(&mut self, cx: &LateContext<'tcx>, block: &hir::Block<'tcx>) { + for (idx, stmt) in block.stmts.iter().enumerate() { + if !stmt.span.from_expansion() + // matches `let v = Vec::new();` + && let StmtKind::Local(local) = stmt.kind + && let Local { pat, init: Some(init), .. } = local + && let PatKind::Binding(_, _, ident, _) = pat.kind + && let Some(vec_init_kind) = get_vec_init_kind(cx, init) + { + // finds use of `_.read(&mut v)` + let mut read_found = false; + let mut visitor = expr_visitor_no_bodies(|expr| { + if let ExprKind::MethodCall(path, [_self, arg], _) = expr.kind + && let PathSegment { ident: read_or_read_exact, .. } = *path + && matches!(read_or_read_exact.as_str(), "read" | "read_exact") + && let ExprKind::AddrOf(_, hir::Mutability::Mut, inner) = arg.kind + && let ExprKind::Path(QPath::Resolved(None, inner_path)) = inner.kind + && let [inner_seg] = inner_path.segments + && ident.name == inner_seg.ident.name + { + read_found = true; + } + !read_found + }); + + let next_stmt_span; + if idx == block.stmts.len() - 1 { + // case { .. stmt; expr } + if let Some(e) = block.expr { + visitor.visit_expr(e); + next_stmt_span = e.span; + } else { + return; + } + } else { + // case { .. stmt; stmt; .. } + let next_stmt = &block.stmts[idx + 1]; + visitor.visit_stmt(next_stmt); + next_stmt_span = next_stmt.span; + } + drop(visitor); + + if read_found && !next_stmt_span.from_expansion() { + let applicability = Applicability::MaybeIncorrect; + match vec_init_kind { + VecInitKind::WithConstCapacity(len) => { + span_lint_and_sugg( + cx, + READ_ZERO_BYTE_VEC, + next_stmt_span, + "reading zero byte data to `Vec`", + "try", + format!("{}.resize({}, 0); {}", + ident.as_str(), + len, + snippet(cx, next_stmt_span, "..") + ), + applicability, + ); + } + VecInitKind::WithExprCapacity(hir_id) => { + let e = cx.tcx.hir().expect_expr(hir_id); + span_lint_and_sugg( + cx, + READ_ZERO_BYTE_VEC, + next_stmt_span, + "reading zero byte data to `Vec`", + "try", + format!("{}.resize({}, 0); {}", + ident.as_str(), + snippet(cx, e.span, ".."), + snippet(cx, next_stmt_span, "..") + ), + applicability, + ); + } + _ => { + span_lint( + cx, + READ_ZERO_BYTE_VEC, + next_stmt_span, + "reading zero byte data to `Vec`", + ); + + } + } + } + } + } + } +} diff --git a/clippy_lints/src/redundant_closure_call.rs b/clippy_lints/src/redundant_closure_call.rs index 4c2016fe3f7..65ed798867d 100644 --- a/clippy_lints/src/redundant_closure_call.rs +++ b/clippy_lints/src/redundant_closure_call.rs @@ -23,12 +23,13 @@ declare_clippy_lint! { /// complexity. /// /// ### Example - /// ```rust,ignore - /// // Bad - /// let a = (|| 42)() + /// ```rust + /// let a = (|| 42)(); + /// ``` /// - /// // Good - /// let a = 42 + /// Use instead: + /// ```rust + /// let a = 42; /// ``` #[clippy::version = "pre 1.29.0"] pub REDUNDANT_CLOSURE_CALL, diff --git a/clippy_lints/src/redundant_slicing.rs b/clippy_lints/src/redundant_slicing.rs index 25a9072ef6e..db6c97f3739 100644 --- a/clippy_lints/src/redundant_slicing.rs +++ b/clippy_lints/src/redundant_slicing.rs @@ -60,7 +60,7 @@ declare_clippy_lint! { /// let vec = vec![1, 2, 3]; /// let slice = &*vec; /// ``` - #[clippy::version = "1.60.0"] + #[clippy::version = "1.61.0"] pub DEREF_BY_SLICING, restriction, "slicing instead of dereferencing" diff --git a/clippy_lints/src/reference.rs b/clippy_lints/src/reference.rs index f789cec6d6a..a642e2da3ba 100644 --- a/clippy_lints/src/reference.rs +++ b/clippy_lints/src/reference.rs @@ -21,11 +21,12 @@ declare_clippy_lint! { /// /// ### Example /// ```rust,ignore - /// // Bad /// let a = f(*&mut b); /// let c = *&d; + /// ``` /// - /// // Good + /// Use instead: + /// ```rust,ignore /// let a = f(b); /// let c = d; /// ``` diff --git a/clippy_lints/src/return_self_not_must_use.rs b/clippy_lints/src/return_self_not_must_use.rs index 91e5e1e8b28..60be6bd335f 100644 --- a/clippy_lints/src/return_self_not_must_use.rs +++ b/clippy_lints/src/return_self_not_must_use.rs @@ -26,19 +26,20 @@ declare_clippy_lint! { /// if it was added on constructors for example. /// /// ### Example - /// Missing attribute /// ```rust /// pub struct Bar; /// impl Bar { - /// // Bad + /// // Missing attribute /// pub fn bar(&self) -> Self { /// Self /// } /// } /// ``` /// - /// It's better to have the `#[must_use]` attribute on the method like this: + /// Use instead: /// ```rust + /// # { + /// // It's better to have the `#[must_use]` attribute on the method like this: /// pub struct Bar; /// impl Bar { /// #[must_use] @@ -46,10 +47,10 @@ declare_clippy_lint! { /// Self /// } /// } - /// ``` + /// # } /// - /// Or on the type definition like this: - /// ```rust + /// # { + /// // Or on the type definition like this: /// #[must_use] /// pub struct Bar; /// impl Bar { @@ -57,6 +58,7 @@ declare_clippy_lint! { /// Self /// } /// } + /// # } /// ``` #[clippy::version = "1.59.0"] pub RETURN_SELF_NOT_MUST_USE, diff --git a/clippy_lints/src/same_name_method.rs b/clippy_lints/src/same_name_method.rs index c5c174cc8f6..20184d54b76 100644 --- a/clippy_lints/src/same_name_method.rs +++ b/clippy_lints/src/same_name_method.rs @@ -1,7 +1,7 @@ -use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::diagnostics::span_lint_hir_and_then; use rustc_data_structures::fx::FxHashMap; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::{Impl, ItemKind, Node, Path, QPath, TraitRef, TyKind}; +use rustc_hir::{HirId, Impl, ItemKind, Node, Path, QPath, TraitRef, TyKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::AssocKind; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -42,11 +42,12 @@ declare_clippy_lint! { declare_lint_pass!(SameNameMethod => [SAME_NAME_METHOD]); struct ExistingName { - impl_methods: BTreeMap<Symbol, Span>, + impl_methods: BTreeMap<Symbol, (Span, HirId)>, trait_methods: BTreeMap<Symbol, Vec<Span>>, } impl<'tcx> LateLintPass<'tcx> for SameNameMethod { + #[expect(clippy::too_many_lines)] fn check_crate_post(&mut self, cx: &LateContext<'tcx>) { let mut map = FxHashMap::<Res, ExistingName>::default(); @@ -97,10 +98,11 @@ impl<'tcx> LateLintPass<'tcx> for SameNameMethod { }; let mut check_trait_method = |method_name: Symbol, trait_method_span: Span| { - if let Some(impl_span) = existing_name.impl_methods.get(&method_name) { - span_lint_and_then( + if let Some((impl_span, hir_id)) = existing_name.impl_methods.get(&method_name) { + span_lint_hir_and_then( cx, SAME_NAME_METHOD, + *hir_id, *impl_span, "method's name is the same as an existing method in a trait", |diag| { @@ -136,10 +138,12 @@ impl<'tcx> LateLintPass<'tcx> for SameNameMethod { }) { let method_name = impl_item_ref.ident.name; let impl_span = impl_item_ref.span; + let hir_id = impl_item_ref.id.hir_id(); if let Some(trait_spans) = existing_name.trait_methods.get(&method_name) { - span_lint_and_then( + span_lint_hir_and_then( cx, SAME_NAME_METHOD, + hir_id, impl_span, "method's name is the same as an existing method in a trait", |diag| { @@ -152,7 +156,7 @@ impl<'tcx> LateLintPass<'tcx> for SameNameMethod { }, ); } - existing_name.impl_methods.insert(method_name, impl_span); + existing_name.impl_methods.insert(method_name, (impl_span, hir_id)); } }, } diff --git a/clippy_lints/src/shadow.rs b/clippy_lints/src/shadow.rs index 4f74c1e44c2..bf318c055da 100644 --- a/clippy_lints/src/shadow.rs +++ b/clippy_lints/src/shadow.rs @@ -23,10 +23,12 @@ declare_clippy_lint! { /// ### Example /// ```rust /// # let x = 1; - /// // Bad /// let x = &x; + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # let x = 1; /// let y = &x; // use different variable name /// ``` #[clippy::version = "pre 1.29.0"] @@ -79,11 +81,14 @@ declare_clippy_lint! { /// # let y = 1; /// # let z = 2; /// let x = y; - /// - /// // Bad /// let x = z; // shadows the earlier binding + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # let y = 1; + /// # let z = 2; + /// let x = y; /// let w = z; // use different variable name /// ``` #[clippy::version = "pre 1.29.0"] diff --git a/clippy_lints/src/single_char_lifetime_names.rs b/clippy_lints/src/single_char_lifetime_names.rs index aa306a630c4..3dc995e2fa5 100644 --- a/clippy_lints/src/single_char_lifetime_names.rs +++ b/clippy_lints/src/single_char_lifetime_names.rs @@ -33,7 +33,7 @@ declare_clippy_lint! { /// source: &'src str, /// } /// ``` - #[clippy::version = "1.59.0"] + #[clippy::version = "1.60.0"] pub SINGLE_CHAR_LIFETIME_NAMES, restriction, "warns against single-character lifetime names" diff --git a/clippy_lints/src/slow_vector_initialization.rs b/clippy_lints/src/slow_vector_initialization.rs index b4ad5dcbe3e..975a0a06e38 100644 --- a/clippy_lints/src/slow_vector_initialization.rs +++ b/clippy_lints/src/slow_vector_initialization.rs @@ -23,15 +23,16 @@ declare_clippy_lint! { /// ```rust /// # use core::iter::repeat; /// # let len = 4; - /// - /// // Bad /// let mut vec1 = Vec::with_capacity(len); /// vec1.resize(len, 0); /// /// let mut vec2 = Vec::with_capacity(len); /// vec2.extend(repeat(0).take(len)); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # let len = 4; /// let mut vec1 = vec![0; len]; /// let mut vec2 = vec![0; len]; /// ``` diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs index 7c196ccaa8c..71f3e6b6a6e 100644 --- a/clippy_lints/src/strings.rs +++ b/clippy_lints/src/strings.rs @@ -99,11 +99,12 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad - /// let bs = "a byte string".as_bytes(); + /// let bstr = "a byte string".as_bytes(); + /// ``` /// - /// // Good - /// let bs = b"a byte string"; + /// Use instead: + /// ```rust + /// let bstr = b"a byte string"; /// ``` #[clippy::version = "pre 1.29.0"] pub STRING_LIT_AS_BYTES, @@ -223,11 +224,12 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// let _ = std::str::from_utf8(&"Hello World!".as_bytes()[6..11]).unwrap(); + /// std::str::from_utf8(&"Hello World!".as_bytes()[6..11]).unwrap(); /// ``` - /// could be written as + /// + /// Use instead: /// ```rust - /// let _ = &"Hello World!"[6..11]; + /// &"Hello World!"[6..11]; /// ``` #[clippy::version = "1.50.0"] pub STRING_FROM_UTF8_AS_BYTES, diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 1e5b646f5f0..ac63d182337 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -29,8 +29,7 @@ declare_clippy_lint! { /// pub fn foo<T>(t: T) where T: Copy, T: Clone {} /// ``` /// - /// Could be written as: - /// + /// Use instead: /// ```rust /// pub fn foo<T>(t: T) where T: Copy + Clone {} /// ``` diff --git a/clippy_lints/src/transmute/useless_transmute.rs b/clippy_lints/src/transmute/useless_transmute.rs index fc9227b76f0..8ea985a8984 100644 --- a/clippy_lints/src/transmute/useless_transmute.rs +++ b/clippy_lints/src/transmute/useless_transmute.rs @@ -61,12 +61,7 @@ pub(super) fn check<'tcx>( "transmute from an integer to a pointer", |diag| { if let Some(arg) = sugg::Sugg::hir_opt(cx, arg) { - diag.span_suggestion( - e.span, - "try", - arg.as_ty(&to_ty.to_string()), - Applicability::Unspecified, - ); + diag.span_suggestion(e.span, "try", arg.as_ty(&to_ty.to_string()), Applicability::Unspecified); } }, ); diff --git a/clippy_lints/src/unicode.rs b/clippy_lints/src/unicode.rs index afd7be89a4e..cc64d17be05 100644 --- a/clippy_lints/src/unicode.rs +++ b/clippy_lints/src/unicode.rs @@ -41,7 +41,8 @@ declare_clippy_lint! { /// ```rust /// let x = String::from("€"); /// ``` - /// Could be written as: + /// + /// Use instead: /// ```rust /// let x = String::from("\u{20ac}"); /// ``` diff --git a/clippy_lints/src/unused_async.rs b/clippy_lints/src/unused_async.rs index 41333bb2add..c8ec4442ab1 100644 --- a/clippy_lints/src/unused_async.rs +++ b/clippy_lints/src/unused_async.rs @@ -17,13 +17,14 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// async fn get_random_number() -> i64 { /// 4 // Chosen by fair dice roll. Guaranteed to be random. /// } /// let number_future = get_random_number(); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// fn get_random_number_improved() -> i64 { /// 4 // Chosen by fair dice roll. Guaranteed to be random. /// } diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs index 4a3b5383c89..fe29bf29d0c 100644 --- a/clippy_lints/src/useless_conversion.rs +++ b/clippy_lints/src/useless_conversion.rs @@ -21,11 +21,12 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// // format!() returns a `String` /// let s: String = format!("hello").into(); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// let s: String = format!("hello"); /// ``` #[clippy::version = "1.45.0"] diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index b5c5d35135f..38e5c5e5b73 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -9,6 +9,29 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; use std::{cmp, env, fmt, fs, io, iter}; +#[rustfmt::skip] +const DEFAULT_DOC_VALID_IDENTS: &[&str] = &[ + "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", + "DirectX", + "ECMAScript", + "GPLv2", "GPLv3", + "GitHub", "GitLab", + "IPv4", "IPv6", + "ClojureScript", "CoffeeScript", "JavaScript", "PureScript", "TypeScript", + "NaN", "NaNs", + "OAuth", "GraphQL", + "OCaml", + "OpenGL", "OpenMP", "OpenSSH", "OpenSSL", "OpenStreetMap", "OpenDNS", + "WebGL", + "TensorFlow", + "TrueType", + "iOS", "macOS", "FreeBSD", + "TeX", "LaTeX", "BibTeX", "BibLaTeX", + "MinGW", + "CamelCase", +]; +const DEFAULT_BLACKLISTED_NAMES: &[&str] = &["foo", "baz", "quux"]; + /// Holds information used by `MISSING_ENFORCED_IMPORT_RENAMES` lint. #[derive(Clone, Debug, Deserialize)] pub struct Rename { @@ -178,8 +201,10 @@ define_Conf! { (msrv: Option<String> = None), /// Lint: BLACKLISTED_NAME. /// - /// The list of blacklisted names to lint about. NB: `bar` is not here since it has legitimate uses - (blacklisted_names: Vec<String> = ["foo", "baz", "quux"].iter().map(ToString::to_string).collect()), + /// The list of blacklisted names to lint about. NB: `bar` is not here since it has legitimate uses. The value + /// `".."` can be used as part of the list to indicate, that the configured values should be appended to the + /// default configuration of Clippy. By default any configuraction will replace the default value. + (blacklisted_names: Vec<String> = super::DEFAULT_BLACKLISTED_NAMES.iter().map(ToString::to_string).collect()), /// Lint: COGNITIVE_COMPLEXITY. /// /// The maximum cognitive complexity a function can have @@ -191,27 +216,14 @@ define_Conf! { (cyclomatic_complexity_threshold: Option<u64> = None), /// Lint: DOC_MARKDOWN. /// - /// The list of words this lint should not consider as identifiers needing ticks - (doc_valid_idents: Vec<String> = [ - "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", - "DirectX", - "ECMAScript", - "GPLv2", "GPLv3", - "GitHub", "GitLab", - "IPv4", "IPv6", - "ClojureScript", "CoffeeScript", "JavaScript", "PureScript", "TypeScript", - "NaN", "NaNs", - "OAuth", "GraphQL", - "OCaml", - "OpenGL", "OpenMP", "OpenSSH", "OpenSSL", "OpenStreetMap", "OpenDNS", - "WebGL", - "TensorFlow", - "TrueType", - "iOS", "macOS", "FreeBSD", - "TeX", "LaTeX", "BibTeX", "BibLaTeX", - "MinGW", - "CamelCase", - ].iter().map(ToString::to_string).collect()), + /// The list of words this lint should not consider as identifiers needing ticks. The value + /// `".."` can be used as part of the list to indicate, that the configured values should be appended to the + /// default configuration of Clippy. By default any configuraction will replace the default value. For example: + /// * `doc-valid-idents = ["ClipPy"]` would replace the default list with `["ClipPy"]`. + /// * `doc-valid-idents = ["ClipPy", ".."]` would append `ClipPy` to the default list. + /// + /// Default list: + (doc_valid_idents: Vec<String> = super::DEFAULT_DOC_VALID_IDENTS.iter().map(ToString::to_string).collect()), /// Lint: TOO_MANY_ARGUMENTS. /// /// The maximum number of argument a function or method can have @@ -401,7 +413,21 @@ pub fn read(path: &Path) -> TryConf { Err(e) => return TryConf::from_error(e), Ok(content) => content, }; - toml::from_str(&content).unwrap_or_else(TryConf::from_error) + match toml::from_str::<TryConf>(&content) { + Ok(mut conf) => { + extend_vec_if_indicator_present(&mut conf.conf.doc_valid_idents, DEFAULT_DOC_VALID_IDENTS); + extend_vec_if_indicator_present(&mut conf.conf.blacklisted_names, DEFAULT_BLACKLISTED_NAMES); + + conf + }, + Err(e) => TryConf::from_error(e), + } +} + +fn extend_vec_if_indicator_present(vec: &mut Vec<String>, default: &[&str]) { + if vec.contains(&"..".to_string()) { + vec.extend(default.iter().map(ToString::to_string)); + } } const SEPARATOR_WIDTH: usize = 4; diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 60f98876994..b885e5132f1 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -89,12 +89,11 @@ declare_clippy_lint! { /// warning/error messages. /// /// ### Example - /// Bad: /// ```rust,ignore /// cx.span_lint(LINT_NAME, "message"); /// ``` /// - /// Good: + /// Use instead: /// ```rust,ignore /// utils::span_lint(cx, LINT_NAME, "message"); /// ``` @@ -112,12 +111,11 @@ declare_clippy_lint! { /// `cx.outer_expn_data()` is faster and more concise. /// /// ### Example - /// Bad: /// ```rust,ignore /// expr.span.ctxt().outer().expn_data() /// ``` /// - /// Good: + /// Use instead: /// ```rust,ignore /// expr.span.ctxt().outer_expn_data() /// ``` @@ -135,7 +133,6 @@ declare_clippy_lint! { /// ICE in large quantities can damage your teeth /// /// ### Example - /// Bad: /// ```rust,ignore /// 🍦🍦🍦🍦🍦 /// ``` @@ -153,12 +150,11 @@ declare_clippy_lint! { /// Indicates that the lint is not finished. /// /// ### Example - /// Bad: /// ```rust,ignore /// declare_lint! { pub COOL_LINT, nursery, "default lint description" } /// ``` /// - /// Good: + /// Use instead: /// ```rust,ignore /// declare_lint! { pub COOL_LINT, nursery, "a great new lint" } /// ``` @@ -183,7 +179,6 @@ declare_clippy_lint! { /// convenient, readable and less error prone. /// /// ### Example - /// Bad: /// ```rust,ignore /// span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |diag| { /// diag.span_suggestion( @@ -207,7 +202,7 @@ declare_clippy_lint! { /// }); /// ``` /// - /// Good: + /// Use instead: /// ```rust,ignore /// span_lint_and_sugg( /// cx, @@ -237,12 +232,11 @@ declare_clippy_lint! { /// `utils::is_type_diagnostic_item()` does not require hardcoded paths. /// /// ### Example - /// Bad: /// ```rust,ignore /// utils::match_type(cx, ty, &paths::VEC) /// ``` /// - /// Good: + /// Use instead: /// ```rust,ignore /// utils::is_type_diagnostic_item(cx, ty, sym::Vec) /// ``` @@ -273,12 +267,11 @@ declare_clippy_lint! { /// It's faster and easier to use the symbol constant. /// /// ### Example - /// Bad: /// ```rust,ignore /// let _ = sym!(f32); /// ``` /// - /// Good: + /// Use instead: /// ```rust,ignore /// let _ = sym::f32; /// ``` @@ -295,12 +288,11 @@ declare_clippy_lint! { /// It's faster use symbols directly instead of strings. /// /// ### Example - /// Bad: /// ```rust,ignore /// symbol.as_str() == "clippy"; /// ``` /// - /// Good: + /// Use instead: /// ```rust,ignore /// symbol == sym::clippy; /// ``` @@ -672,8 +664,8 @@ impl<'tcx> LateLintPass<'tcx> for CollapsibleCalls { if let ExprKind::Call(func, and_then_args) = expr.kind; if is_expr_path_def_path(cx, func, &["clippy_utils", "diagnostics", "span_lint_and_then"]); if and_then_args.len() == 5; - if let ExprKind::Closure(_, _, body_id, _, _) = &and_then_args[4].kind; - let body = cx.tcx.hir().body(*body_id); + if let ExprKind::Closure { body, .. } = &and_then_args[4].kind; + let body = cx.tcx.hir().body(*body); let only_expr = peel_blocks_with_stmt(&body.value); if let ExprKind::MethodCall(ps, span_call_args, _) = &only_expr.kind; then { diff --git a/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/clippy_lints/src/utils/internal_lints/metadata_collector.rs index cf2de6a42af..99e9e3275ab 100644 --- a/clippy_lints/src/utils/internal_lints/metadata_collector.rs +++ b/clippy_lints/src/utils/internal_lints/metadata_collector.rs @@ -31,6 +31,8 @@ use std::fmt::Write as _; use std::fs::{self, OpenOptions}; use std::io::prelude::*; use std::path::Path; +use std::path::PathBuf; +use std::process::Command; /// This is the output file of the lint collector. const OUTPUT_FILE: &str = "../util/gh-pages/lints.json"; @@ -180,6 +182,7 @@ pub struct MetadataCollector { lints: BinaryHeap<LintMetadata>, applicability_info: FxHashMap<String, ApplicabilityInfo>, config: Vec<ClippyConfiguration>, + clippy_project_root: PathBuf, } impl MetadataCollector { @@ -188,6 +191,7 @@ impl MetadataCollector { lints: BinaryHeap::<LintMetadata>::default(), applicability_info: FxHashMap::<String, ApplicabilityInfo>::default(), config: collect_configs(), + clippy_project_root: clippy_dev::clippy_project_root(), } } @@ -215,11 +219,13 @@ impl Drop for MetadataCollector { // Mapping the final data let mut lints = std::mem::take(&mut self.lints).into_sorted_vec(); - collect_renames(&mut lints); for x in &mut lints { x.applicability = Some(applicability_info.remove(&x.id).unwrap_or_default()); + replace_produces(&x.id, &mut x.docs, &self.clippy_project_root); } + collect_renames(&mut lints); + // Outputting if Path::new(OUTPUT_FILE).exists() { fs::remove_file(OUTPUT_FILE).unwrap(); @@ -263,14 +269,193 @@ impl LintMetadata { } } +fn replace_produces(lint_name: &str, docs: &mut String, clippy_project_root: &Path) { + let mut doc_lines = docs.lines().map(ToString::to_string).collect::<Vec<_>>(); + let mut lines = doc_lines.iter_mut(); + + 'outer: loop { + // Find the start of the example + + // ```rust + loop { + match lines.next() { + Some(line) if line.trim_start().starts_with("```rust") => { + if line.contains("ignore") || line.contains("no_run") { + // A {{produces}} marker may have been put on a ignored code block by mistake, + // just seek to the end of the code block and continue checking. + if lines.any(|line| line.trim_start().starts_with("```")) { + continue; + } + + panic!("lint `{}` has an unterminated code block", lint_name) + } + + break; + }, + Some(line) if line.trim_start() == "{{produces}}" => { + panic!( + "lint `{}` has marker {{{{produces}}}} with an ignored or missing code block", + lint_name + ) + }, + Some(line) => { + let line = line.trim(); + // These are the two most common markers of the corrections section + if line.eq_ignore_ascii_case("Use instead:") || line.eq_ignore_ascii_case("Could be written as:") { + break 'outer; + } + }, + None => break 'outer, + } + } + + // Collect the example + let mut example = Vec::new(); + loop { + match lines.next() { + Some(line) if line.trim_start() == "```" => break, + Some(line) => example.push(line), + None => panic!("lint `{}` has an unterminated code block", lint_name), + } + } + + // Find the {{produces}} and attempt to generate the output + loop { + match lines.next() { + Some(line) if line.is_empty() => {}, + Some(line) if line.trim() == "{{produces}}" => { + let output = get_lint_output(lint_name, &example, clippy_project_root); + line.replace_range( + .., + &format!( + "<details>\ + <summary>Produces</summary>\n\ + \n\ + ```text\n\ + {}\n\ + ```\n\ + </details>", + output + ), + ); + + break; + }, + // No {{produces}}, we can move on to the next example + Some(_) => break, + None => break 'outer, + } + } + } + + *docs = cleanup_docs(&doc_lines); +} + +fn get_lint_output(lint_name: &str, example: &[&mut String], clippy_project_root: &Path) -> String { + let dir = tempfile::tempdir().unwrap_or_else(|e| panic!("failed to create temp dir: {e}")); + let file = dir.path().join("lint_example.rs"); + + let mut source = String::new(); + let unhidden = example + .iter() + .map(|line| line.trim_start().strip_prefix("# ").unwrap_or(line)); + + // Get any attributes + let mut lines = unhidden.peekable(); + while let Some(line) = lines.peek() { + if line.starts_with("#!") { + source.push_str(line); + source.push('\n'); + lines.next(); + } else { + break; + } + } + + let needs_main = !example.iter().any(|line| line.contains("fn main")); + if needs_main { + source.push_str("fn main() {\n"); + } + + for line in lines { + source.push_str(line); + source.push('\n'); + } + + if needs_main { + source.push_str("}\n"); + } + + if let Err(e) = fs::write(&file, &source) { + panic!("failed to write to `{}`: {e}", file.as_path().to_string_lossy()); + } + + let prefixed_name = format!("{}{lint_name}", CLIPPY_LINT_GROUP_PREFIX); + + let mut cmd = Command::new("cargo"); + + cmd.current_dir(clippy_project_root) + .env("CARGO_INCREMENTAL", "0") + .env("CLIPPY_ARGS", "") + .env("CLIPPY_DISABLE_DOCS_LINKS", "1") + // We need to disable this to enable all lints + .env("ENABLE_METADATA_COLLECTION", "0") + .args(["run", "--bin", "clippy-driver"]) + .args(["--target-dir", "./clippy_lints/target"]) + .args(["--", "--error-format=json"]) + .args(["--edition", "2021"]) + .arg("-Cdebuginfo=0") + .args(["-A", "clippy::all"]) + .args(["-W", &prefixed_name]) + .args(["-L", "./target/debug"]) + .args(["-Z", "no-codegen"]); + + let output = cmd + .arg(file.as_path()) + .output() + .unwrap_or_else(|e| panic!("failed to run `{:?}`: {e}", cmd)); + + let tmp_file_path = file.to_string_lossy(); + let stderr = std::str::from_utf8(&output.stderr).unwrap(); + let msgs = stderr + .lines() + .filter(|line| line.starts_with('{')) + .map(|line| serde_json::from_str(line).unwrap()) + .collect::<Vec<serde_json::Value>>(); + + let mut rendered = String::new(); + let iter = msgs + .iter() + .filter(|msg| matches!(&msg["code"]["code"], serde_json::Value::String(s) if s == &prefixed_name)); + + for message in iter { + let rendered_part = message["rendered"].as_str().expect("rendered field should exist"); + rendered.push_str(rendered_part); + } + + if rendered.is_empty() { + let rendered: Vec<&str> = msgs.iter().filter_map(|msg| msg["rendered"].as_str()).collect(); + let non_json: Vec<&str> = stderr.lines().filter(|line| !line.starts_with('{')).collect(); + panic!( + "did not find lint `{}` in output of example, got:\n{}\n{}", + lint_name, + non_json.join("\n"), + rendered.join("\n") + ); + } + + // The reader doesn't need to see `/tmp/.tmpfiy2Qd/lint_example.rs` :) + rendered.trim_end().replace(&*tmp_file_path, "lint_example.rs") +} + #[derive(Debug, Clone, Serialize, PartialEq, Eq, PartialOrd, Ord)] struct SerializableSpan { path: String, line: usize, } -impl std::fmt::Display for SerializableSpan { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl fmt::Display for SerializableSpan { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}:{}", self.path.rsplit('/').next().unwrap_or_default(), self.line) } } @@ -435,10 +620,10 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector { if !BLACK_LISTED_LINTS.contains(&lint_name.as_str()); // metadata extraction if let Some((group, level)) = get_lint_group_and_level_or_lint(cx, &lint_name, item); - if let Some(mut docs) = extract_attr_docs_or_lint(cx, item); + if let Some(mut raw_docs) = extract_attr_docs_or_lint(cx, item); then { if let Some(configuration_section) = self.get_lint_configs(&lint_name) { - docs.push_str(&configuration_section); + raw_docs.push_str(&configuration_section); } let version = get_lint_version(cx, item); @@ -448,7 +633,7 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector { group, level, version, - docs, + raw_docs, )); } } @@ -459,7 +644,7 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector { let lint_name = sym_to_string(item.ident.name).to_ascii_lowercase(); if !BLACK_LISTED_LINTS.contains(&lint_name.as_str()); // Metadata the little we can get from a deprecated lint - if let Some(docs) = extract_attr_docs_or_lint(cx, item); + if let Some(raw_docs) = extract_attr_docs_or_lint(cx, item); then { let version = get_lint_version(cx, item); @@ -469,7 +654,7 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector { DEPRECATED_LINT_GROUP_STR.to_string(), DEPRECATED_LINT_LEVEL, version, - docs, + raw_docs, )); } } @@ -535,22 +720,28 @@ fn extract_attr_docs_or_lint(cx: &LateContext<'_>, item: &Item<'_>) -> Option<St /// ``` /// /// Would result in `Hello world!\n=^.^=\n` -/// -/// --- -/// +fn extract_attr_docs(cx: &LateContext<'_>, item: &Item<'_>) -> Option<String> { + let attrs = cx.tcx.hir().attrs(item.hir_id()); + let mut lines = attrs.iter().filter_map(ast::Attribute::doc_str); + + if let Some(line) = lines.next() { + let raw_docs = lines.fold(String::from(line.as_str()) + "\n", |s, line| s + line.as_str() + "\n"); + return Some(raw_docs); + } + + None +} + /// This function may modify the doc comment to ensure that the string can be displayed using a /// markdown viewer in Clippy's lint list. The following modifications could be applied: /// * Removal of leading space after a new line. (Important to display tables) /// * Ensures that code blocks only contain language information -fn extract_attr_docs(cx: &LateContext<'_>, item: &Item<'_>) -> Option<String> { - let attrs = cx.tcx.hir().attrs(item.hir_id()); - let mut lines = attrs.iter().filter_map(ast::Attribute::doc_str); - let mut docs = String::from(lines.next()?.as_str()); +fn cleanup_docs(docs_collection: &Vec<String>) -> String { let mut in_code_block = false; let mut is_code_block_rust = false; - for line in lines { - let line = line.as_str(); + let mut docs = String::new(); + for line in docs_collection { // Rustdoc hides code lines starting with `# ` and this removes them from Clippy's lint list :) if is_code_block_rust && line.trim_start().starts_with("# ") { continue; @@ -583,7 +774,8 @@ fn extract_attr_docs(cx: &LateContext<'_>, item: &Item<'_>) -> Option<String> { docs.push_str(line); } } - Some(docs) + + docs } fn get_lint_version(cx: &LateContext<'_>, item: &Item<'_>) -> String { @@ -762,9 +954,9 @@ fn resolve_applicability<'hir>(cx: &LateContext<'hir>, expr: &'hir hir::Expr<'hi } fn check_is_multi_part<'hir>(cx: &LateContext<'hir>, closure_expr: &'hir hir::Expr<'hir>) -> bool { - if let ExprKind::Closure(_, _, body_id, _, _) = closure_expr.kind { + if let ExprKind::Closure { body, .. } = closure_expr.kind { let mut scanner = IsMultiSpanScanner::new(cx); - intravisit::walk_body(&mut scanner, cx.tcx.hir().body(body_id)); + intravisit::walk_body(&mut scanner, cx.tcx.hir().body(body)); return scanner.is_multi_part(); } else if let Some(local) = get_parent_local(cx, closure_expr) { if let Some(local_init) = local.init { diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs index ba1ff65479d..297a80e5767 100644 --- a/clippy_lints/src/vec.rs +++ b/clippy_lints/src/vec.rs @@ -28,12 +28,14 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// # fn foo(my_vec: &[u8]) {} + /// fn foo(_x: &[u8]) {} /// - /// // Bad /// foo(&vec![1, 2]); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # fn foo(_x: &[u8]) {} /// foo(&[1, 2]); /// ``` #[clippy::version = "pre 1.29.0"] diff --git a/clippy_lints/src/wildcard_imports.rs b/clippy_lints/src/wildcard_imports.rs index 2f74eaf3cf5..5418eca382d 100644 --- a/clippy_lints/src/wildcard_imports.rs +++ b/clippy_lints/src/wildcard_imports.rs @@ -26,13 +26,18 @@ declare_clippy_lint! { /// still around. /// /// ### Example - /// ```rust,ignore - /// // Bad + /// ```rust /// use std::cmp::Ordering::*; + /// + /// # fn foo(_: std::cmp::Ordering) {} /// foo(Less); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// use std::cmp::Ordering; + /// + /// # fn foo(_: Ordering) {} /// foo(Ordering::Less) /// ``` #[clippy::version = "pre 1.29.0"] @@ -76,14 +81,13 @@ declare_clippy_lint! { /// /// ### Example /// ```rust,ignore - /// // Bad /// use crate1::*; /// /// foo(); /// ``` /// + /// Use instead: /// ```rust,ignore - /// // Good /// use crate1::foo; /// /// foo(); diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index d2493c055a5..67b2bc8c3f3 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -25,10 +25,11 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// println!(""); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// println!(); /// ``` #[clippy::version = "pre 1.29.0"] @@ -177,10 +178,13 @@ declare_clippy_lint! { /// ```rust /// # use std::fmt::Write; /// # let mut buf = String::new(); - /// // Bad /// writeln!(buf, ""); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # use std::fmt::Write; + /// # let mut buf = String::new(); /// writeln!(buf); /// ``` #[clippy::version = "pre 1.29.0"] @@ -204,10 +208,14 @@ declare_clippy_lint! { /// # use std::fmt::Write; /// # let mut buf = String::new(); /// # let name = "World"; - /// // Bad /// write!(buf, "Hello {}!\n", name); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # use std::fmt::Write; + /// # let mut buf = String::new(); + /// # let name = "World"; /// writeln!(buf, "Hello {}!", name); /// ``` #[clippy::version = "pre 1.29.0"] @@ -233,10 +241,13 @@ declare_clippy_lint! { /// ```rust /// # use std::fmt::Write; /// # let mut buf = String::new(); - /// // Bad /// writeln!(buf, "{}", "foo"); + /// ``` /// - /// // Good + /// Use instead: + /// ```rust + /// # use std::fmt::Write; + /// # let mut buf = String::new(); /// writeln!(buf, "foo"); /// ``` #[clippy::version = "pre 1.29.0"] diff --git a/clippy_lints/src/zero_div_zero.rs b/clippy_lints/src/zero_div_zero.rs index 641681185a2..50d3c079fe6 100644 --- a/clippy_lints/src/zero_div_zero.rs +++ b/clippy_lints/src/zero_div_zero.rs @@ -14,10 +14,11 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// // Bad /// let nan = 0.0f32 / 0.0; + /// ``` /// - /// // Good + /// Use instead: + /// ```rust /// let nan = f32::NAN; /// ``` #[clippy::version = "pre 1.29.0"] |