From 3957244120444d2ff9a5e43a0a2df97d79168174 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Thu, 18 Nov 2021 13:54:49 +0000 Subject: [PATCH 1/2] Add `needless_late_init` lint --- CHANGELOG.md | 1 + clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_style.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/needless_late_init.rs | 345 ++++++++++++++++++++ clippy_lints/src/no_effect.rs | 9 +- tests/ui/let_if_seq.stderr | 43 ++- tests/ui/min_max.rs | 7 +- tests/ui/min_max.stderr | 26 +- tests/ui/needless_late_init.rs | 133 ++++++++ tests/ui/needless_late_init.stderr | 108 ++++++ tests/ui/needless_late_init_fixable.fixed | 38 +++ tests/ui/needless_late_init_fixable.rs | 38 +++ tests/ui/needless_late_init_fixable.stderr | 103 ++++++ tests/ui/redundant_closure_call_late.rs | 1 + tests/ui/redundant_closure_call_late.stderr | 6 +- tests/ui/redundant_else.rs | 2 +- 18 files changed, 838 insertions(+), 27 deletions(-) create mode 100644 clippy_lints/src/needless_late_init.rs create mode 100644 tests/ui/needless_late_init.rs create mode 100644 tests/ui/needless_late_init.stderr create mode 100644 tests/ui/needless_late_init_fixable.fixed create mode 100644 tests/ui/needless_late_init_fixable.rs create mode 100644 tests/ui/needless_late_init_fixable.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 401557b3eacd..157ea0c963af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3033,6 +3033,7 @@ Released 2018-09-13 [`needless_continue`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_continue [`needless_doctest_main`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_doctest_main [`needless_for_each`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_for_each +[`needless_late_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_late_init [`needless_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes [`needless_option_as_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_option_as_deref [`needless_pass_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 612240135ac6..04912120e278 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -206,6 +206,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(needless_bool::BOOL_COMPARISON), 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_option_as_deref::NEEDLESS_OPTION_AS_DEREF), LintId::of(needless_question_mark::NEEDLESS_QUESTION_MARK), LintId::of(needless_update::NEEDLESS_UPDATE), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 19c35a5e5f40..bb159e50373c 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -362,6 +362,7 @@ store.register_lints(&[ needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE, needless_continue::NEEDLESS_CONTINUE, needless_for_each::NEEDLESS_FOR_EACH, + needless_late_init::NEEDLESS_LATE_INIT, needless_option_as_deref::NEEDLESS_OPTION_AS_DEREF, needless_pass_by_value::NEEDLESS_PASS_BY_VALUE, needless_question_mark::NEEDLESS_QUESTION_MARK, diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs index f336441ea842..ea87e7e7a736 100644 --- a/clippy_lints/src/lib.register_style.rs +++ b/clippy_lints/src/lib.register_style.rs @@ -82,6 +82,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![ LintId::of(misc_early::REDUNDANT_PATTERN), 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(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 3dafdf8f0d5e..bd9710ec4075 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -299,6 +299,7 @@ mod needless_bool; mod needless_borrowed_ref; mod needless_continue; mod needless_for_each; +mod needless_late_init; mod needless_option_as_deref; mod needless_pass_by_value; mod needless_question_mark; @@ -851,6 +852,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || Box::new(format_args::FormatArgs)); store.register_late_pass(|| Box::new(trailing_empty_array::TrailingEmptyArray)); store.register_early_pass(|| Box::new(octal_escapes::OctalEscapes)); + store.register_late_pass(|| Box::new(needless_late_init::NeedlessLateInit)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/needless_late_init.rs b/clippy_lints/src/needless_late_init.rs new file mode 100644 index 000000000000..d2f040dc36a9 --- /dev/null +++ b/clippy_lints/src/needless_late_init.rs @@ -0,0 +1,345 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::path_to_local; +use clippy_utils::source::snippet_opt; +use clippy_utils::visitors::{expr_visitor, is_local_used}; +use rustc_errors::Applicability; +use rustc_hir::intravisit::Visitor; +use rustc_hir::{Block, Expr, ExprKind, HirId, Local, LocalSource, MatchSource, Node, Pat, PatKind, Stmt, StmtKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::Span; + +declare_clippy_lint! { + /// ### What it does + /// Checks for late initializations that can be replaced by a let statement + /// with an initializer. + /// + /// ### Why is this bad? + /// Assigning in the let statement is less repetitive. + /// + /// ### Example + /// ```rust + /// let a; + /// a = 1; + /// + /// let b; + /// match 3 { + /// 0 => b = "zero", + /// 1 => b = "one", + /// _ => b = "many", + /// } + /// + /// let c; + /// if true { + /// c = 1; + /// } else { + /// c = -1; + /// } + /// ``` + /// Use instead: + /// ```rust + /// let a = 1; + /// + /// let b = match 3 { + /// 0 => "zero", + /// 1 => "one", + /// _ => "many", + /// }; + /// + /// let c = if true { + /// 1 + /// } else { + /// -1 + /// }; + /// ``` + #[clippy::version = "1.58.0"] + pub NEEDLESS_LATE_INIT, + style, + "late initializations that can be replaced by a let statement with an initializer" +} +declare_lint_pass!(NeedlessLateInit => [NEEDLESS_LATE_INIT]); + +fn contains_assign_expr<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'tcx>) -> bool { + let mut seen = false; + expr_visitor(cx, |expr| { + if let ExprKind::Assign(..) = expr.kind { + seen = true; + } + + !seen + }) + .visit_stmt(stmt); + + seen +} + +#[derive(Debug)] +struct LocalAssign { + lhs_id: HirId, + lhs_span: Span, + rhs_span: Span, + span: Span, +} + +impl LocalAssign { + fn from_expr(expr: &Expr<'_>, span: Span) -> Option { + if let ExprKind::Assign(lhs, rhs, _) = expr.kind { + if lhs.span.from_expansion() { + return None; + } + + Some(Self { + lhs_id: path_to_local(lhs)?, + lhs_span: lhs.span, + rhs_span: rhs.span.source_callsite(), + span, + }) + } else { + None + } + } + + fn new<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, binding_id: HirId) -> Option { + let assign = match expr.kind { + ExprKind::Block(Block { expr: Some(expr), .. }, _) => Self::from_expr(expr, expr.span), + ExprKind::Block(block, _) => { + if_chain! { + if let Some((last, other_stmts)) = block.stmts.split_last(); + if let StmtKind::Expr(expr) | StmtKind::Semi(expr) = last.kind; + + let assign = Self::from_expr(expr, last.span)?; + + // avoid visiting if not needed + if assign.lhs_id == binding_id; + if other_stmts.iter().all(|stmt| !contains_assign_expr(cx, stmt)); + + then { + Some(assign) + } else { + None + } + } + }, + ExprKind::Assign(..) => Self::from_expr(expr, expr.span), + _ => None, + }?; + + if assign.lhs_id == binding_id { + Some(assign) + } else { + None + } + } +} + +fn assignment_suggestions<'tcx>( + cx: &LateContext<'tcx>, + binding_id: HirId, + exprs: impl IntoIterator>, +) -> Option<(Applicability, Vec<(Span, String)>)> { + let mut assignments = Vec::new(); + + for expr in exprs { + let ty = cx.typeck_results().expr_ty(expr); + + if ty.is_never() { + continue; + } + if !ty.is_unit() { + return None; + } + + let assign = LocalAssign::new(cx, expr, binding_id)?; + + assignments.push(assign); + } + + let suggestions = assignments + .into_iter() + .map(|assignment| Some((assignment.span, snippet_opt(cx, assignment.rhs_span)?))) + .collect::>>()?; + + let applicability = if suggestions.len() > 1 { + // multiple suggestions don't work with rustfix in multipart_suggest + // https://github.com/rust-lang/rustfix/issues/141 + Applicability::Unspecified + } else { + Applicability::MachineApplicable + }; + Some((applicability, suggestions)) +} + +struct Usage<'tcx> { + stmt: &'tcx Stmt<'tcx>, + expr: &'tcx Expr<'tcx>, + needs_semi: bool, +} + +fn first_usage<'tcx>( + cx: &LateContext<'tcx>, + binding_id: HirId, + local_stmt_id: HirId, + block: &'tcx Block<'tcx>, +) -> Option> { + block + .stmts + .iter() + .skip_while(|stmt| stmt.hir_id != local_stmt_id) + .skip(1) + .find(|&stmt| is_local_used(cx, stmt, binding_id)) + .and_then(|stmt| match stmt.kind { + StmtKind::Expr(expr) => Some(Usage { + stmt, + expr, + needs_semi: true, + }), + StmtKind::Semi(expr) => Some(Usage { + stmt, + expr, + needs_semi: false, + }), + _ => None, + }) +} + +fn local_snippet_without_semicolon(cx: &LateContext<'_>, local: &Local<'_>) -> Option { + let span = local.span.with_hi(match local.ty { + // let : ; + // ~~~~~~~~~~~~~~~ + Some(ty) => ty.span.hi(), + // let ; + // ~~~~~~~~~ + None => local.pat.span.hi(), + }); + + snippet_opt(cx, span) +} + +fn check<'tcx>( + cx: &LateContext<'tcx>, + local: &'tcx Local<'tcx>, + local_stmt: &'tcx Stmt<'tcx>, + block: &'tcx Block<'tcx>, + binding_id: HirId, +) -> Option<()> { + let usage = first_usage(cx, binding_id, local_stmt.hir_id, block)?; + let binding_name = cx.tcx.hir().opt_name(binding_id)?; + let let_snippet = local_snippet_without_semicolon(cx, local)?; + + match usage.expr.kind { + ExprKind::Assign(..) => { + let assign = LocalAssign::new(cx, usage.expr, binding_id)?; + + span_lint_and_then( + cx, + NEEDLESS_LATE_INIT, + local_stmt.span, + "unneeded late initalization", + |diag| { + diag.tool_only_span_suggestion( + local_stmt.span, + "remove the local", + String::new(), + Applicability::MachineApplicable, + ); + + diag.span_suggestion( + assign.lhs_span, + &format!("declare `{}` here", binding_name), + let_snippet, + Applicability::MachineApplicable, + ); + }, + ); + }, + ExprKind::If(_, then_expr, Some(else_expr)) => { + let (applicability, suggestions) = assignment_suggestions(cx, binding_id, [then_expr, else_expr])?; + + span_lint_and_then( + cx, + NEEDLESS_LATE_INIT, + local_stmt.span, + "unneeded late initalization", + |diag| { + diag.tool_only_span_suggestion(local_stmt.span, "remove the local", String::new(), applicability); + + diag.span_suggestion_verbose( + usage.stmt.span.shrink_to_lo(), + &format!("declare `{}` here", binding_name), + format!("{} = ", let_snippet), + applicability, + ); + + diag.multipart_suggestion("remove the assignments from the branches", suggestions, applicability); + + if usage.needs_semi { + diag.span_suggestion( + usage.stmt.span.shrink_to_hi(), + "add a semicolon after the if expression", + ";".to_string(), + applicability, + ); + } + }, + ); + }, + ExprKind::Match(_, arms, MatchSource::Normal) => { + let (applicability, suggestions) = assignment_suggestions(cx, binding_id, arms.iter().map(|arm| arm.body))?; + + span_lint_and_then( + cx, + NEEDLESS_LATE_INIT, + local_stmt.span, + "unneeded late initalization", + |diag| { + diag.tool_only_span_suggestion(local_stmt.span, "remove the local", String::new(), applicability); + + diag.span_suggestion_verbose( + usage.stmt.span.shrink_to_lo(), + &format!("declare `{}` here", binding_name), + format!("{} = ", let_snippet), + applicability, + ); + + diag.multipart_suggestion("remove the assignments from the match arms", suggestions, applicability); + + if usage.needs_semi { + diag.span_suggestion( + usage.stmt.span.shrink_to_hi(), + "add a semicolon after the match expression", + ";".to_string(), + applicability, + ); + } + }, + ); + }, + _ => {}, + }; + + Some(()) +} + +impl LateLintPass<'tcx> for NeedlessLateInit { + fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) { + let mut parents = cx.tcx.hir().parent_iter(local.hir_id); + + if_chain! { + if let Local { + init: None, + pat: &Pat { + kind: PatKind::Binding(_, binding_id, _, None), + .. + }, + source: LocalSource::Normal, + .. + } = local; + if let Some((_, Node::Stmt(local_stmt))) = parents.next(); + if let Some((_, Node::Block(block))) = parents.next(); + + then { + check(cx, local, local_stmt, block, binding_id); + } + } + } +} diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index cd54de3ee4b7..6fcc9ca29b92 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -164,12 +164,13 @@ fn check_unnecessary_operation(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { if !&reduced.iter().any(|e| e.span.from_expansion()); then { if let ExprKind::Index(..) = &expr.kind { - let snippet; - if let (Some(arr), Some(func)) = (snippet_opt(cx, reduced[0].span), snippet_opt(cx, reduced[1].span)) { - snippet = format!("assert!({}.len() > {});", &arr, &func); + let snippet = if let (Some(arr), Some(func)) = + (snippet_opt(cx, reduced[0].span), snippet_opt(cx, reduced[1].span)) + { + format!("assert!({}.len() > {});", &arr, &func) } else { return; - } + }; span_lint_hir_and_then( cx, UNNECESSARY_OPERATION, diff --git a/tests/ui/let_if_seq.stderr b/tests/ui/let_if_seq.stderr index 9cf2e10a5ee5..3a522628fead 100644 --- a/tests/ui/let_if_seq.stderr +++ b/tests/ui/let_if_seq.stderr @@ -1,3 +1,23 @@ +error: unneeded late initalization + --> $DIR/let_if_seq.rs:48:5 + | +LL | let foo; + | ^^^^^^^^ + | + = note: `-D clippy::needless-late-init` implied by `-D warnings` +help: declare `foo` here + | +LL | let foo = if f() { + | +++++++++ +help: remove the assignments from the branches + | +LL | 0 + | +help: add a semicolon after the if expression + | +LL | }; + | + + error: `if _ { .. } else { .. }` is an expression --> $DIR/let_if_seq.rs:65:5 | @@ -46,5 +66,26 @@ LL | | } | = note: you might not need `mut` at all -error: aborting due to 4 previous errors +error: unneeded late initalization + --> $DIR/let_if_seq.rs:78:5 + | +LL | let quz; + | ^^^^^^^^ + | +help: declare `quz` here + | +LL | let quz = if f() { + | +++++++++ +help: remove the assignments from the branches + | +LL ~ 42 +LL | } else { +LL ~ 0 + | +help: add a semicolon after the if expression + | +LL | }; + | + + +error: aborting due to 6 previous errors diff --git a/tests/ui/min_max.rs b/tests/ui/min_max.rs index f7ed72a11cf6..b2bc97f4744a 100644 --- a/tests/ui/min_max.rs +++ b/tests/ui/min_max.rs @@ -19,8 +19,7 @@ impl NotOrd { } fn main() { - let x; - x = 2usize; + let x = 2usize; min(1, max(3, x)); min(max(3, x), 1); max(min(x, 1), 3); @@ -35,9 +34,7 @@ fn main() { let y = 2isize; min(max(y, -1), 3); - let s; - s = "Hello"; - + let s = "Hello"; min("Apple", max("Zoo", s)); max(min(s, "Apple"), "Zoo"); diff --git a/tests/ui/min_max.stderr b/tests/ui/min_max.stderr index 9f8e26fa406f..c70b77eabbd8 100644 --- a/tests/ui/min_max.stderr +++ b/tests/ui/min_max.stderr @@ -1,5 +1,5 @@ error: this `min`/`max` combination leads to constant result - --> $DIR/min_max.rs:24:5 + --> $DIR/min_max.rs:23:5 | LL | min(1, max(3, x)); | ^^^^^^^^^^^^^^^^^ @@ -7,73 +7,73 @@ LL | min(1, max(3, x)); = note: `-D clippy::min-max` implied by `-D warnings` error: this `min`/`max` combination leads to constant result - --> $DIR/min_max.rs:25:5 + --> $DIR/min_max.rs:24:5 | LL | min(max(3, x), 1); | ^^^^^^^^^^^^^^^^^ error: this `min`/`max` combination leads to constant result - --> $DIR/min_max.rs:26:5 + --> $DIR/min_max.rs:25:5 | LL | max(min(x, 1), 3); | ^^^^^^^^^^^^^^^^^ error: this `min`/`max` combination leads to constant result - --> $DIR/min_max.rs:27:5 + --> $DIR/min_max.rs:26:5 | LL | max(3, min(x, 1)); | ^^^^^^^^^^^^^^^^^ error: this `min`/`max` combination leads to constant result - --> $DIR/min_max.rs:29:5 + --> $DIR/min_max.rs:28:5 | LL | my_max(3, my_min(x, 1)); | ^^^^^^^^^^^^^^^^^^^^^^^ error: this `min`/`max` combination leads to constant result - --> $DIR/min_max.rs:41:5 + --> $DIR/min_max.rs:38:5 | LL | min("Apple", max("Zoo", s)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this `min`/`max` combination leads to constant result - --> $DIR/min_max.rs:42:5 + --> $DIR/min_max.rs:39:5 | LL | max(min(s, "Apple"), "Zoo"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this `min`/`max` combination leads to constant result - --> $DIR/min_max.rs:47:5 + --> $DIR/min_max.rs:44:5 | LL | x.min(1).max(3); | ^^^^^^^^^^^^^^^ error: this `min`/`max` combination leads to constant result - --> $DIR/min_max.rs:48:5 + --> $DIR/min_max.rs:45:5 | LL | x.max(3).min(1); | ^^^^^^^^^^^^^^^ error: this `min`/`max` combination leads to constant result - --> $DIR/min_max.rs:49:5 + --> $DIR/min_max.rs:46:5 | LL | f.max(3f32).min(1f32); | ^^^^^^^^^^^^^^^^^^^^^ error: this `min`/`max` combination leads to constant result - --> $DIR/min_max.rs:55:5 + --> $DIR/min_max.rs:52:5 | LL | max(x.min(1), 3); | ^^^^^^^^^^^^^^^^ error: this `min`/`max` combination leads to constant result - --> $DIR/min_max.rs:58:5 + --> $DIR/min_max.rs:55:5 | LL | s.max("Zoo").min("Apple"); | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: this `min`/`max` combination leads to constant result - --> $DIR/min_max.rs:59:5 + --> $DIR/min_max.rs:56:5 | LL | s.min("Apple").max("Zoo"); | ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/needless_late_init.rs b/tests/ui/needless_late_init.rs new file mode 100644 index 000000000000..37b3cb34e2a9 --- /dev/null +++ b/tests/ui/needless_late_init.rs @@ -0,0 +1,133 @@ +#![allow(unused)] + +fn main() { + let a; + let n = 1; + match n { + 1 => a = "one", + _ => { + a = "two"; + }, + } + + let b; + if n == 3 { + b = "four"; + } else { + b = "five" + } + + let c; + if let Some(n) = Some(5) { + c = n; + } else { + c = -50; + } + + let d; + if true { + let temp = 5; + d = temp; + } else { + d = 15; + } + + let e; + if true { + e = format!("{} {}", a, b); + } else { + e = format!("{}", c); + } + + println!("{}", a); +} + +fn does_not_lint() { + let z; + if false { + z = 1; + } + + let x; + let y; + if true { + x = 1; + } else { + y = 1; + } + + let mut x; + if true { + x = 5; + x = 10 / x; + } else { + x = 2; + } + + let x; + let _ = match 1 { + 1 => x = 10, + _ => x = 20, + }; + + // using tuples would be possible, but not always preferable + let x; + let y; + if true { + x = 1; + y = 2; + } else { + x = 3; + y = 4; + } + + // could match with a smarter heuristic to avoid multiple assignments + let x; + if true { + let mut y = 5; + y = 6; + x = y; + } else { + x = 2; + } + + let (x, y); + if true { + x = 1; + } else { + x = 2; + } + y = 3; + + macro_rules! assign { + ($i:ident) => { + $i = 1; + }; + } + let x; + assign!(x); + + let x; + if true { + assign!(x); + } else { + x = 2; + } + + macro_rules! in_macro { + () => { + let x; + x = 1; + + let x; + if true { + x = 1; + } else { + x = 2; + } + }; + } + in_macro!(); + + println!("{}", x); +} diff --git a/tests/ui/needless_late_init.stderr b/tests/ui/needless_late_init.stderr new file mode 100644 index 000000000000..ece50773f45d --- /dev/null +++ b/tests/ui/needless_late_init.stderr @@ -0,0 +1,108 @@ +error: unneeded late initalization + --> $DIR/needless_late_init.rs:4:5 + | +LL | let a; + | ^^^^^^ + | + = note: `-D clippy::needless-late-init` implied by `-D warnings` +help: declare `a` here + | +LL | let a = match n { + | +++++++ +help: remove the assignments from the match arms + | +LL ~ 1 => "one", +LL | _ => { +LL ~ "two" + | +help: add a semicolon after the match expression + | +LL | }; + | + + +error: unneeded late initalization + --> $DIR/needless_late_init.rs:13:5 + | +LL | let b; + | ^^^^^^ + | +help: declare `b` here + | +LL | let b = if n == 3 { + | +++++++ +help: remove the assignments from the branches + | +LL ~ "four" +LL | } else { +LL ~ "five" + | +help: add a semicolon after the if expression + | +LL | }; + | + + +error: unneeded late initalization + --> $DIR/needless_late_init.rs:20:5 + | +LL | let c; + | ^^^^^^ + | +help: declare `c` here + | +LL | let c = if let Some(n) = Some(5) { + | +++++++ +help: remove the assignments from the branches + | +LL ~ n +LL | } else { +LL ~ -50 + | +help: add a semicolon after the if expression + | +LL | }; + | + + +error: unneeded late initalization + --> $DIR/needless_late_init.rs:27:5 + | +LL | let d; + | ^^^^^^ + | +help: declare `d` here + | +LL | let d = if true { + | +++++++ +help: remove the assignments from the branches + | +LL ~ temp +LL | } else { +LL ~ 15 + | +help: add a semicolon after the if expression + | +LL | }; + | + + +error: unneeded late initalization + --> $DIR/needless_late_init.rs:35:5 + | +LL | let e; + | ^^^^^^ + | +help: declare `e` here + | +LL | let e = if true { + | +++++++ +help: remove the assignments from the branches + | +LL ~ format!("{} {}", a, b) +LL | } else { +LL ~ format!("{}", c) + | +help: add a semicolon after the if expression + | +LL | }; + | + + +error: aborting due to 5 previous errors + diff --git a/tests/ui/needless_late_init_fixable.fixed b/tests/ui/needless_late_init_fixable.fixed new file mode 100644 index 000000000000..32d5d04fde4d --- /dev/null +++ b/tests/ui/needless_late_init_fixable.fixed @@ -0,0 +1,38 @@ +// run-rustfix + +#![allow(unused, clippy::assign_op_pattern)] + +fn main() { + + let a = "zero"; + + + + let b = 1; + let c = 2; + + + let d: usize = 1; + + + let mut e = 1; + e = 2; + + + let f = match 1 { + 1 => "three", + _ => return, + }; // has semi + + + let g: usize = if true { + 5 + } else { + panic!(); + }; + + + let h = format!("{}", e); + + println!("{}", a); +} diff --git a/tests/ui/needless_late_init_fixable.rs b/tests/ui/needless_late_init_fixable.rs new file mode 100644 index 000000000000..6bc85f686325 --- /dev/null +++ b/tests/ui/needless_late_init_fixable.rs @@ -0,0 +1,38 @@ +// run-rustfix + +#![allow(unused, clippy::assign_op_pattern)] + +fn main() { + let a; + a = "zero"; + + let b; + let c; + b = 1; + c = 2; + + let d: usize; + d = 1; + + let mut e; + e = 1; + e = 2; + + let f; + match 1 { + 1 => f = "three", + _ => return, + }; // has semi + + let g: usize; + if true { + g = 5; + } else { + panic!(); + } + + let h; + h = format!("{}", e); + + println!("{}", a); +} diff --git a/tests/ui/needless_late_init_fixable.stderr b/tests/ui/needless_late_init_fixable.stderr new file mode 100644 index 000000000000..05491496cd76 --- /dev/null +++ b/tests/ui/needless_late_init_fixable.stderr @@ -0,0 +1,103 @@ +error: unneeded late initalization + --> $DIR/needless_late_init_fixable.rs:6:5 + | +LL | let a; + | ^^^^^^ + | + = note: `-D clippy::needless-late-init` implied by `-D warnings` +help: declare `a` here + | +LL | let a = "zero"; + | ~~~~~ + +error: unneeded late initalization + --> $DIR/needless_late_init_fixable.rs:9:5 + | +LL | let b; + | ^^^^^^ + | +help: declare `b` here + | +LL | let b = 1; + | ~~~~~ + +error: unneeded late initalization + --> $DIR/needless_late_init_fixable.rs:10:5 + | +LL | let c; + | ^^^^^^ + | +help: declare `c` here + | +LL | let c = 2; + | ~~~~~ + +error: unneeded late initalization + --> $DIR/needless_late_init_fixable.rs:14:5 + | +LL | let d: usize; + | ^^^^^^^^^^^^^ + | +help: declare `d` here + | +LL | let d: usize = 1; + | ~~~~~~~~~~~~ + +error: unneeded late initalization + --> $DIR/needless_late_init_fixable.rs:17:5 + | +LL | let mut e; + | ^^^^^^^^^^ + | +help: declare `e` here + | +LL | let mut e = 1; + | ~~~~~~~~~ + +error: unneeded late initalization + --> $DIR/needless_late_init_fixable.rs:21:5 + | +LL | let f; + | ^^^^^^ + | +help: declare `f` here + | +LL | let f = match 1 { + | +++++++ +help: remove the assignments from the match arms + | +LL | 1 => "three", + | ~~~~~~~ + +error: unneeded late initalization + --> $DIR/needless_late_init_fixable.rs:27:5 + | +LL | let g: usize; + | ^^^^^^^^^^^^^ + | +help: declare `g` here + | +LL | let g: usize = if true { + | ++++++++++++++ +help: remove the assignments from the branches + | +LL | 5 + | +help: add a semicolon after the if expression + | +LL | }; + | + + +error: unneeded late initalization + --> $DIR/needless_late_init_fixable.rs:34:5 + | +LL | let h; + | ^^^^^^ + | +help: declare `h` here + | +LL | let h = format!("{}", e); + | ~~~~~ + +error: aborting due to 8 previous errors + diff --git a/tests/ui/redundant_closure_call_late.rs b/tests/ui/redundant_closure_call_late.rs index 1f4864b72895..5612827bd393 100644 --- a/tests/ui/redundant_closure_call_late.rs +++ b/tests/ui/redundant_closure_call_late.rs @@ -1,6 +1,7 @@ // non rustfixable, see redundant_closure_call_fixable.rs #![warn(clippy::redundant_closure_call)] +#![allow(clippy::needless_late_init)] fn main() { let mut i = 1; diff --git a/tests/ui/redundant_closure_call_late.stderr b/tests/ui/redundant_closure_call_late.stderr index 7c8865f1bd37..4eca43a2b599 100644 --- a/tests/ui/redundant_closure_call_late.stderr +++ b/tests/ui/redundant_closure_call_late.stderr @@ -1,5 +1,5 @@ error: closure called just once immediately after it was declared - --> $DIR/redundant_closure_call_late.rs:15:5 + --> $DIR/redundant_closure_call_late.rs:16:5 | LL | i = redun_closure(); | ^^^^^^^^^^^^^^^^^^^ @@ -7,13 +7,13 @@ LL | i = redun_closure(); = note: `-D clippy::redundant-closure-call` implied by `-D warnings` error: closure called just once immediately after it was declared - --> $DIR/redundant_closure_call_late.rs:19:5 + --> $DIR/redundant_closure_call_late.rs:20:5 | LL | i = shadowed_closure(); | ^^^^^^^^^^^^^^^^^^^^^^ error: closure called just once immediately after it was declared - --> $DIR/redundant_closure_call_late.rs:21:5 + --> $DIR/redundant_closure_call_late.rs:22:5 | LL | i = shadowed_closure(); | ^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/redundant_else.rs b/tests/ui/redundant_else.rs index e8a6e940c01c..64f566735cd9 100644 --- a/tests/ui/redundant_else.rs +++ b/tests/ui/redundant_else.rs @@ -1,5 +1,5 @@ #![warn(clippy::redundant_else)] -#![allow(clippy::needless_return, clippy::if_same_then_else)] +#![allow(clippy::needless_return, clippy::if_same_then_else, clippy::needless_late_init)] fn main() { loop { From d346ec94fe3477ee2ee0cf41061a92c156db8701 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Fri, 26 Nov 2021 14:16:48 +0000 Subject: [PATCH 2/2] Add async/const fn tests for needless-late-init +nits --- clippy_lints/src/needless_late_init.rs | 16 ++++--- tests/ui/let_if_seq.rs | 3 +- tests/ui/let_if_seq.stderr | 51 ++------------------ tests/ui/needless_late_init.rs | 34 +++++++++++++ tests/ui/needless_late_init.stderr | 56 +++++++++++++++++++--- tests/ui/needless_late_init_fixable.stderr | 4 +- 6 files changed, 102 insertions(+), 62 deletions(-) diff --git a/clippy_lints/src/needless_late_init.rs b/clippy_lints/src/needless_late_init.rs index d2f040dc36a9..e0522f3fe0b1 100644 --- a/clippy_lints/src/needless_late_init.rs +++ b/clippy_lints/src/needless_late_init.rs @@ -11,11 +11,11 @@ use rustc_span::Span; declare_clippy_lint! { /// ### What it does - /// Checks for late initializations that can be replaced by a let statement + /// Checks for late initializations that can be replaced by a `let` statement /// with an initializer. /// /// ### Why is this bad? - /// Assigning in the let statement is less repetitive. + /// Assigning in the `let` statement is less repetitive. /// /// ### Example /// ```rust @@ -55,7 +55,7 @@ declare_clippy_lint! { #[clippy::version = "1.58.0"] pub NEEDLESS_LATE_INIT, style, - "late initializations that can be replaced by a let statement with an initializer" + "late initializations that can be replaced by a `let` statement with an initializer" } declare_lint_pass!(NeedlessLateInit => [NEEDLESS_LATE_INIT]); @@ -275,7 +275,7 @@ fn check<'tcx>( if usage.needs_semi { diag.span_suggestion( usage.stmt.span.shrink_to_hi(), - "add a semicolon after the if expression", + "add a semicolon after the `if` expression", ";".to_string(), applicability, ); @@ -301,12 +301,16 @@ fn check<'tcx>( applicability, ); - diag.multipart_suggestion("remove the assignments from the match arms", suggestions, applicability); + diag.multipart_suggestion( + "remove the assignments from the `match` arms", + suggestions, + applicability, + ); if usage.needs_semi { diag.span_suggestion( usage.stmt.span.shrink_to_hi(), - "add a semicolon after the match expression", + "add a semicolon after the `match` expression", ";".to_string(), applicability, ); diff --git a/tests/ui/let_if_seq.rs b/tests/ui/let_if_seq.rs index 2d8f3c2f0e7a..c5cb2eb1fe1c 100644 --- a/tests/ui/let_if_seq.rs +++ b/tests/ui/let_if_seq.rs @@ -3,7 +3,8 @@ unused_assignments, clippy::similar_names, clippy::blacklisted_name, - clippy::branches_sharing_code + clippy::branches_sharing_code, + clippy::needless_late_init )] #![warn(clippy::useless_let_if_seq)] diff --git a/tests/ui/let_if_seq.stderr b/tests/ui/let_if_seq.stderr index 3a522628fead..271ccce681c9 100644 --- a/tests/ui/let_if_seq.stderr +++ b/tests/ui/let_if_seq.stderr @@ -1,25 +1,5 @@ -error: unneeded late initalization - --> $DIR/let_if_seq.rs:48:5 - | -LL | let foo; - | ^^^^^^^^ - | - = note: `-D clippy::needless-late-init` implied by `-D warnings` -help: declare `foo` here - | -LL | let foo = if f() { - | +++++++++ -help: remove the assignments from the branches - | -LL | 0 - | -help: add a semicolon after the if expression - | -LL | }; - | + - error: `if _ { .. } else { .. }` is an expression - --> $DIR/let_if_seq.rs:65:5 + --> $DIR/let_if_seq.rs:66:5 | LL | / let mut foo = 0; LL | | if f() { @@ -31,7 +11,7 @@ LL | | } = note: you might not need `mut` at all error: `if _ { .. } else { .. }` is an expression - --> $DIR/let_if_seq.rs:70:5 + --> $DIR/let_if_seq.rs:71:5 | LL | / let mut bar = 0; LL | | if f() { @@ -45,7 +25,7 @@ LL | | } = note: you might not need `mut` at all error: `if _ { .. } else { .. }` is an expression - --> $DIR/let_if_seq.rs:78:5 + --> $DIR/let_if_seq.rs:79:5 | LL | / let quz; LL | | if f() { @@ -56,7 +36,7 @@ LL | | } | |_____^ help: it is more idiomatic to write: `let quz = if f() { 42 } else { 0 };` error: `if _ { .. } else { .. }` is an expression - --> $DIR/let_if_seq.rs:107:5 + --> $DIR/let_if_seq.rs:108:5 | LL | / let mut baz = 0; LL | | if f() { @@ -66,26 +46,5 @@ LL | | } | = note: you might not need `mut` at all -error: unneeded late initalization - --> $DIR/let_if_seq.rs:78:5 - | -LL | let quz; - | ^^^^^^^^ - | -help: declare `quz` here - | -LL | let quz = if f() { - | +++++++++ -help: remove the assignments from the branches - | -LL ~ 42 -LL | } else { -LL ~ 0 - | -help: add a semicolon after the if expression - | -LL | }; - | + - -error: aborting due to 6 previous errors +error: aborting due to 4 previous errors diff --git a/tests/ui/needless_late_init.rs b/tests/ui/needless_late_init.rs index 37b3cb34e2a9..6fdb4cc566f5 100644 --- a/tests/ui/needless_late_init.rs +++ b/tests/ui/needless_late_init.rs @@ -42,6 +42,40 @@ fn main() { println!("{}", a); } +async fn in_async() -> &'static str { + async fn f() -> &'static str { + "one" + } + + let a; + let n = 1; + match n { + 1 => a = f().await, + _ => { + a = "two"; + }, + } + + a +} + +const fn in_const() -> &'static str { + const fn f() -> &'static str { + "one" + } + + let a; + let n = 1; + match n { + 1 => a = f(), + _ => { + a = "two"; + }, + } + + a +} + fn does_not_lint() { let z; if false { diff --git a/tests/ui/needless_late_init.stderr b/tests/ui/needless_late_init.stderr index ece50773f45d..cbb7538c63b3 100644 --- a/tests/ui/needless_late_init.stderr +++ b/tests/ui/needless_late_init.stderr @@ -9,13 +9,13 @@ help: declare `a` here | LL | let a = match n { | +++++++ -help: remove the assignments from the match arms +help: remove the assignments from the `match` arms | LL ~ 1 => "one", LL | _ => { LL ~ "two" | -help: add a semicolon after the match expression +help: add a semicolon after the `match` expression | LL | }; | + @@ -36,7 +36,7 @@ LL ~ "four" LL | } else { LL ~ "five" | -help: add a semicolon after the if expression +help: add a semicolon after the `if` expression | LL | }; | + @@ -57,7 +57,7 @@ LL ~ n LL | } else { LL ~ -50 | -help: add a semicolon after the if expression +help: add a semicolon after the `if` expression | LL | }; | + @@ -78,7 +78,7 @@ LL ~ temp LL | } else { LL ~ 15 | -help: add a semicolon after the if expression +help: add a semicolon after the `if` expression | LL | }; | + @@ -99,10 +99,52 @@ LL ~ format!("{} {}", a, b) LL | } else { LL ~ format!("{}", c) | -help: add a semicolon after the if expression +help: add a semicolon after the `if` expression | LL | }; | + -error: aborting due to 5 previous errors +error: unneeded late initalization + --> $DIR/needless_late_init.rs:50:5 + | +LL | let a; + | ^^^^^^ + | +help: declare `a` here + | +LL | let a = match n { + | +++++++ +help: remove the assignments from the `match` arms + | +LL ~ 1 => f().await, +LL | _ => { +LL ~ "two" + | +help: add a semicolon after the `match` expression + | +LL | }; + | + + +error: unneeded late initalization + --> $DIR/needless_late_init.rs:67:5 + | +LL | let a; + | ^^^^^^ + | +help: declare `a` here + | +LL | let a = match n { + | +++++++ +help: remove the assignments from the `match` arms + | +LL ~ 1 => f(), +LL | _ => { +LL ~ "two" + | +help: add a semicolon after the `match` expression + | +LL | }; + | + + +error: aborting due to 7 previous errors diff --git a/tests/ui/needless_late_init_fixable.stderr b/tests/ui/needless_late_init_fixable.stderr index 05491496cd76..a0ce4f812f4e 100644 --- a/tests/ui/needless_late_init_fixable.stderr +++ b/tests/ui/needless_late_init_fixable.stderr @@ -64,7 +64,7 @@ help: declare `f` here | LL | let f = match 1 { | +++++++ -help: remove the assignments from the match arms +help: remove the assignments from the `match` arms | LL | 1 => "three", | ~~~~~~~ @@ -83,7 +83,7 @@ help: remove the assignments from the branches | LL | 5 | -help: add a semicolon after the if expression +help: add a semicolon after the `if` expression | LL | }; | +