From 1788cfd95d343dc59c2555ac895acb26cd5d3811 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Sun, 26 Sep 2021 17:38:05 +0100 Subject: [PATCH 01/11] Remove NullOp::Box --- clippy_utils/src/qualify_min_const_fn.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/clippy_utils/src/qualify_min_const_fn.rs b/clippy_utils/src/qualify_min_const_fn.rs index e6d8ba3f02eb0..789418c743ff8 100644 --- a/clippy_utils/src/qualify_min_const_fn.rs +++ b/clippy_utils/src/qualify_min_const_fn.rs @@ -193,7 +193,6 @@ fn check_rvalue(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, rvalue: &Rv } }, Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf, _) | Rvalue::ShallowInitBox(_, _) => Ok(()), - Rvalue::NullaryOp(NullOp::Box, _) => Err((span, "heap allocations are not allowed in const fn".into())), Rvalue::UnaryOp(_, operand) => { let ty = operand.ty(body, tcx); if ty.is_integral() || ty.is_bool() { From d5cbae90f9e3f323f18d9234b3e55b98c52fed52 Mon Sep 17 00:00:00 2001 From: lcnr Date: Thu, 23 Dec 2021 10:02:05 +0100 Subject: [PATCH 02/11] fix clippy --- clippy_lints/src/trailing_empty_array.rs | 1 + clippy_lints/src/utils/author.rs | 11 +++++-- clippy_lints/src/utils/inspector.rs | 9 ++++-- clippy_utils/src/hir_utils.rs | 39 ++++++++++++++++-------- clippy_utils/src/lib.rs | 7 +++-- tests/ui/author/repeat.stdout | 3 +- 6 files changed, 50 insertions(+), 20 deletions(-) diff --git a/clippy_lints/src/trailing_empty_array.rs b/clippy_lints/src/trailing_empty_array.rs index 47c0a84cd4630..af36f7267004d 100644 --- a/clippy_lints/src/trailing_empty_array.rs +++ b/clippy_lints/src/trailing_empty_array.rs @@ -59,6 +59,7 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx if let ItemKind::Struct(data, _) = &item.kind; if let Some(last_field) = data.fields().last(); if let rustc_hir::TyKind::Array(_, length) = last_field.ty.kind; + if let rustc_hir::ArrayLen::Body(length) = length; // Then check if that that array zero-sized let length_ldid = cx.tcx.hir().local_def_id(length.hir_id); diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index c1b811c217440..9b06ca4e82493 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -6,7 +6,7 @@ use rustc_ast::ast::{LitFloatType, LitKind}; use rustc_ast::LitIntType; use rustc_data_structures::fx::FxHashMap; use rustc_hir as hir; -use rustc_hir::{ExprKind, FnRetTy, HirId, Lit, PatKind, QPath, StmtKind, TyKind}; +use rustc_hir::{ArrayLen, ExprKind, FnRetTy, HirId, Lit, PatKind, QPath, StmtKind, TyKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::symbol::{Ident, Symbol}; @@ -567,7 +567,14 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> { bind!(self, value, length); kind!("Repeat({value}, {length})"); self.expr(value); - self.body(field!(length.body)); + match length.value { + ArrayLen::Infer(..) => out!("if let ArrayLen::Infer(..) = length;"), + ArrayLen::Body(anon_const) => { + bind!(self, anon_const); + out!("if let ArrayLen::Body({anon_const}) = {length};"); + self.body(field!(anon_const.body)); + } + } }, ExprKind::Err => kind!("Err"), ExprKind::DropTemps(expr) => { diff --git a/clippy_lints/src/utils/inspector.rs b/clippy_lints/src/utils/inspector.rs index abf4826a06917..c96766e56784f 100644 --- a/clippy_lints/src/utils/inspector.rs +++ b/clippy_lints/src/utils/inspector.rs @@ -334,12 +334,17 @@ fn print_expr(cx: &LateContext<'_>, expr: &hir::Expr<'_>, indent: usize) { println!("{}anon_const:", ind); print_expr(cx, &cx.tcx.hir().body(anon_const.body).value, indent + 1); }, - hir::ExprKind::Repeat(val, ref anon_const) => { + hir::ExprKind::Repeat(val, length) => { println!("{}Repeat", ind); println!("{}value:", ind); print_expr(cx, val, indent + 1); println!("{}repeat count:", ind); - print_expr(cx, &cx.tcx.hir().body(anon_const.body).value, indent + 1); + match length { + hir::ArrayLen::Infer(_, _) => println!("{}repeat count: _", ind), + hir::ArrayLen::Body(anon_const) => { + print_expr(cx, &cx.tcx.hir().body(anon_const.body).value, indent + 1) + } + } }, hir::ExprKind::Err => { println!("{}Err", ind); diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index ad50759effaf8..ac2b1a0259e3e 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -8,7 +8,7 @@ use rustc_hir::HirIdMap; use rustc_hir::{ BinOpKind, Block, BodyId, Expr, ExprField, ExprKind, FnRetTy, GenericArg, GenericArgs, Guard, HirId, InlineAsmOperand, Let, Lifetime, LifetimeName, ParamName, Pat, PatField, PatKind, Path, PathSegment, QPath, Stmt, - StmtKind, Ty, TyKind, TypeBinding, + StmtKind, Ty, TyKind, TypeBinding, ArrayLen }; use rustc_lexer::{tokenize, TokenKind}; use rustc_lint::LateContext; @@ -170,6 +170,14 @@ impl HirEqInterExpr<'_, '_, '_> { } } + pub fn eq_array_length(&mut self, left: ArrayLen, right: ArrayLen) -> bool { + match (left, right) { + (ArrayLen::Infer(..), ArrayLen::Infer(..)) => true, + (ArrayLen::Body(l_ct), ArrayLen::Body(r_ct)) => self.eq_body(l_ct.body, r_ct.body), + (_, _) => false, + } + } + pub fn eq_body(&mut self, left: BodyId, right: BodyId) -> bool { let cx = self.inner.cx; let eval_const = |body| constant_context(cx, cx.tcx.typeck_body(body)).expr(&cx.tcx.hir().body(body).value); @@ -194,8 +202,8 @@ impl HirEqInterExpr<'_, '_, '_> { } let is_eq = match ( - &reduce_exprkind(self.inner.cx, &left.kind), - &reduce_exprkind(self.inner.cx, &right.kind), + reduce_exprkind(self.inner.cx, &left.kind), + reduce_exprkind(self.inner.cx, &right.kind), ) { (&ExprKind::AddrOf(lb, l_mut, le), &ExprKind::AddrOf(rb, r_mut, re)) => { lb == rb && l_mut == r_mut && self.eq_expr(le, re) @@ -232,7 +240,7 @@ impl HirEqInterExpr<'_, '_, '_> { }, (&ExprKind::Index(la, li), &ExprKind::Index(ra, ri)) => self.eq_expr(la, ra) && self.eq_expr(li, ri), (&ExprKind::If(lc, lt, ref le), &ExprKind::If(rc, rt, ref re)) => { - self.eq_expr(lc, rc) && self.eq_expr(&**lt, &**rt) && both(le, re, |l, r| self.eq_expr(l, r)) + self.eq_expr(lc, rc) && self.eq_expr(lt, rt) && both(le, re, |l, r| self.eq_expr(l, r)) }, (&ExprKind::Let(l), &ExprKind::Let(r)) => { self.eq_pat(l.pat, r.pat) && both(&l.ty, &r.ty, |l, r| self.eq_ty(l, r)) && self.eq_expr(l.init, r.init) @@ -253,8 +261,8 @@ impl HirEqInterExpr<'_, '_, '_> { (&ExprKind::MethodCall(l_path, _, l_args, _), &ExprKind::MethodCall(r_path, _, r_args, _)) => { self.inner.allow_side_effects && self.eq_path_segment(l_path, r_path) && self.eq_exprs(l_args, r_args) }, - (&ExprKind::Repeat(le, ref ll_id), &ExprKind::Repeat(re, ref rl_id)) => { - self.eq_expr(le, re) && self.eq_body(ll_id.body, rl_id.body) + (&ExprKind::Repeat(le, ll), &ExprKind::Repeat(re, rl)) => { + self.eq_expr(le, re) && self.eq_array_length(ll, rl) }, (&ExprKind::Ret(ref l), &ExprKind::Ret(ref r)) => both(l, r, |l, r| self.eq_expr(l, r)), (&ExprKind::Path(ref l), &ExprKind::Path(ref r)) => self.eq_qpath(l, r), @@ -391,8 +399,8 @@ impl HirEqInterExpr<'_, '_, '_> { fn eq_ty(&mut self, left: &Ty<'_>, right: &Ty<'_>) -> bool { match (&left.kind, &right.kind) { (&TyKind::Slice(l_vec), &TyKind::Slice(r_vec)) => self.eq_ty(l_vec, r_vec), - (&TyKind::Array(lt, ref ll_id), &TyKind::Array(rt, ref rl_id)) => { - self.eq_ty(lt, rt) && self.eq_body(ll_id.body, rl_id.body) + (&TyKind::Array(lt, ll), &TyKind::Array(rt, rl)) => { + self.eq_ty(lt, rt) && self.eq_array_length(ll, rl) }, (&TyKind::Ptr(ref l_mut), &TyKind::Ptr(ref r_mut)) => { l_mut.mutbl == r_mut.mutbl && self.eq_ty(&*l_mut.ty, &*r_mut.ty) @@ -714,9 +722,9 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { ExprKind::ConstBlock(ref l_id) => { self.hash_body(l_id.body); }, - ExprKind::Repeat(e, ref l_id) => { + ExprKind::Repeat(e, len) => { self.hash_expr(e); - self.hash_body(l_id.body); + self.hash_array_length(len); }, ExprKind::Ret(ref e) => { if let Some(e) = *e { @@ -906,9 +914,9 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { TyKind::Slice(ty) => { self.hash_ty(ty); }, - TyKind::Array(ty, anon_const) => { + &TyKind::Array(ty, len) => { self.hash_ty(ty); - self.hash_body(anon_const.body); + self.hash_array_length(len); }, TyKind::Ptr(ref mut_ty) => { self.hash_ty(mut_ty.ty); @@ -953,6 +961,13 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { } } + pub fn hash_array_length(&mut self, length: ArrayLen) { + match length { + ArrayLen::Infer(..) => {} + ArrayLen::Body(anon_const) => self.hash_body(anon_const.body), + } + } + pub fn hash_body(&mut self, body_id: BodyId) { // swap out TypeckResults when hashing a body let old_maybe_typeck_results = self.maybe_typeck_results.replace(self.cx.tcx.typeck_body(body_id)); diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 7e054a54c3c02..be8558f36ca76 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -75,7 +75,7 @@ use rustc_hir::{ def, Arm, BindingAnnotation, Block, BlockCheckMode, Body, Constness, Destination, Expr, ExprKind, FnDecl, ForeignItem, GenericArgs, HirId, Impl, ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, MatchSource, Mutability, Node, Param, Pat, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, - TraitItemKind, TraitRef, TyKind, UnOp, + TraitItemKind, TraitRef, TyKind, UnOp, ArrayLen }; use rustc_lint::{LateContext, Level, Lint, LintContext}; use rustc_middle::hir::exports::Export; @@ -675,8 +675,9 @@ pub fn is_default_equivalent(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { _ => false, }, ExprKind::Tup(items) | ExprKind::Array(items) => items.iter().all(|x| is_default_equivalent(cx, x)), - ExprKind::Repeat(x, y) => if_chain! { - if let ExprKind::Lit(ref const_lit) = cx.tcx.hir().body(y.body).value.kind; + ExprKind::Repeat(x, len) => if_chain! { + if let ArrayLen::Body(len) = len; + if let ExprKind::Lit(ref const_lit) = cx.tcx.hir().body(len.body).value.kind; if let LitKind::Int(v, _) = const_lit.node; if v <= 32 && is_default_equivalent(cx, x); then { diff --git a/tests/ui/author/repeat.stdout b/tests/ui/author/repeat.stdout index f16350e4b5e6d..471bbce4f4185 100644 --- a/tests/ui/author/repeat.stdout +++ b/tests/ui/author/repeat.stdout @@ -2,7 +2,8 @@ if_chain! { if let ExprKind::Repeat(value, length) = expr.kind; if let ExprKind::Lit(ref lit) = value.kind; if let LitKind::Int(1, LitIntType::Unsigned(UintTy::U8)) = lit.node; - let expr1 = &cx.tcx.hir().body(length.body).value; + if let ArrayLen::Body(anon_const) = length; + let expr1 = &cx.tcx.hir().body(anon_const.body).value; if let ExprKind::Lit(ref lit1) = expr1.kind; if let LitKind::Int(5, LitIntType::Unsuffixed) = lit1.node; then { From dfd32544faaba6a54238533b32ecf5cb7145cb35 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 28 Dec 2021 16:19:23 +0100 Subject: [PATCH 03/11] Update pulldown-cmark version in clippy --- clippy_lints/Cargo.toml | 2 +- clippy_lints/src/doc.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index 0661c2803864c..7d2a3e4f639c3 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -13,7 +13,7 @@ cargo_metadata = "0.14" clippy_utils = { path = "../clippy_utils" } if_chain = "1.0" itertools = "0.10" -pulldown-cmark = { version = "0.8", default-features = false } +pulldown-cmark = { version = "0.9", default-features = false } quine-mc_cluskey = "0.2" regex-syntax = "0.6" serde = { version = "1.0", features = ["derive"] } diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index 3650e4f91a001..7c2717733578b 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -542,16 +542,16 @@ fn check_doc<'a, Events: Iterator, Range in_link = Some(url), End(Link(..)) => in_link = None, - Start(Heading(_) | Paragraph | Item) => { - if let Start(Heading(_)) = event { + Start(Heading(_, _, _) | Paragraph | Item) => { + if let Start(Heading(_, _, _)) = event { in_heading = true; } ticks_unbalanced = false; let (_, span) = get_current_span(spans, range.start); paragraph_span = first_line_of_span(cx, span); }, - End(Heading(_) | Paragraph | Item) => { - if let End(Heading(_)) = event { + End(Heading(_, _, _) | Paragraph | Item) => { + if let End(Heading(_, _, _)) = event { in_heading = false; } if ticks_unbalanced { From 97ab44ca974544fdf5ef598b04d3794af513e4bf Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 30 Dec 2021 15:10:43 +0100 Subject: [PATCH 04/11] Merge commit '0eff589afc83e21a03a168497bbab6b4dfbb4ef6' into clippyup --- .github/workflows/clippy.yml | 4 + .github/workflows/clippy_bors.yml | 4 + CHANGELOG.md | 123 ++++++++- CONTRIBUTING.md | 8 +- README.md | 10 +- clippy_lints/src/approx_const.rs | 2 +- clippy_lints/src/attrs.rs | 6 +- clippy_lints/src/casts/unnecessary_cast.rs | 5 +- clippy_lints/src/enum_variants.rs | 128 ++++----- clippy_lints/src/float_literal.rs | 2 +- clippy_lints/src/floating_point_arithmetic.rs | 34 ++- clippy_lints/src/identity_op.rs | 15 +- clippy_lints/src/implicit_saturating_sub.rs | 6 +- clippy_lints/src/init_numbered_fields.rs | 80 ++++++ clippy_lints/src/len_zero.rs | 2 +- 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 | 8 +- clippy_lints/src/loops/manual_memcpy.rs | 29 +- clippy_lints/src/macro_use.rs | 6 +- clippy_lints/src/match_str_case_mismatch.rs | 6 +- clippy_lints/src/methods/iter_skip_next.rs | 32 ++- clippy_lints/src/methods/mod.rs | 2 +- .../src/methods/unwrap_or_else_default.rs | 5 +- clippy_lints/src/methods/utils.rs | 2 +- clippy_lints/src/needless_bool.rs | 12 +- clippy_lints/src/neg_multiply.rs | 25 +- clippy_lints/src/non_expressive_names.rs | 8 +- clippy_lints/src/ranges.rs | 8 +- clippy_lints/src/return_self_not_must_use.rs | 11 +- clippy_lints/src/returns.rs | 14 +- clippy_lints/src/shadow.rs | 6 +- clippy_lints/src/tabs_in_doc_comments.rs | 2 +- .../src/undocumented_unsafe_blocks.rs | 6 +- clippy_lints/src/unsafe_removed_from_name.rs | 2 +- clippy_lints/src/unwrap.rs | 2 +- clippy_lints/src/upper_case_acronyms.rs | 2 +- clippy_lints/src/utils/internal_lints.rs | 20 +- clippy_utils/src/consts.rs | 10 +- clippy_utils/src/lib.rs | 123 +++++---- clippy_utils/src/str_utils.rs | 101 ++++++- clippy_utils/src/sugg.rs | 251 ++++++++++++------ rust-toolchain | 2 +- tests/ui/crashes/ice-7868.stderr | 2 +- tests/ui/enum_variants.rs | 6 + tests/ui/enum_variants.stderr | 38 ++- tests/ui/floating_point_rad.fixed | 5 + tests/ui/floating_point_rad.rs | 5 + tests/ui/floating_point_rad.stderr | 28 +- tests/ui/identity_op.rs | 15 ++ tests/ui/identity_op.stderr | 36 ++- tests/ui/iter_skip_next.fixed | 15 ++ tests/ui/iter_skip_next.rs | 15 ++ tests/ui/iter_skip_next.stderr | 28 +- tests/ui/iter_skip_next_unfixable.rs | 19 ++ tests/ui/iter_skip_next_unfixable.stderr | 39 +++ .../manual_memcpy/with_loop_counters.stderr | 2 +- tests/ui/needless_bool/fixable.fixed | 9 + tests/ui/needless_bool/fixable.rs | 33 +++ tests/ui/needless_bool/fixable.stderr | 86 +++++- tests/ui/needless_return.fixed | 15 +- tests/ui/needless_return.rs | 11 + tests/ui/needless_return.stderr | 58 ++-- tests/ui/neg_multiply.fixed | 45 ++++ tests/ui/neg_multiply.rs | 12 +- tests/ui/neg_multiply.stderr | 38 ++- tests/ui/numbered_fields.fixed | 33 +++ tests/ui/numbered_fields.rs | 41 +++ tests/ui/numbered_fields.stderr | 26 ++ tests/ui/return_self_not_must_use.rs | 27 +- tests/ui/shadow.rs | 2 + tests/ui/shadow.stderr | 18 +- tests/ui/short_circuit_statement.fixed | 2 +- tests/ui/short_circuit_statement.stderr | 2 +- tests/ui/undocumented_unsafe_blocks.rs | 38 +-- tests/ui/undocumented_unsafe_blocks.stderr | 30 +-- tests/ui/unwrap_or_else_default.fixed | 5 +- tests/ui/unwrap_or_else_default.rs | 3 + tests/ui/unwrap_or_else_default.stderr | 18 +- 80 files changed, 1493 insertions(+), 439 deletions(-) create mode 100644 clippy_lints/src/init_numbered_fields.rs create mode 100644 tests/ui/iter_skip_next_unfixable.rs create mode 100644 tests/ui/iter_skip_next_unfixable.stderr create mode 100644 tests/ui/neg_multiply.fixed create mode 100644 tests/ui/numbered_fields.fixed create mode 100644 tests/ui/numbered_fields.rs create mode 100644 tests/ui/numbered_fields.stderr diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml index 0339de77f3cec..3d8c39408a924 100644 --- a/.github/workflows/clippy.yml +++ b/.github/workflows/clippy.yml @@ -58,6 +58,10 @@ jobs: run: cargo test --features deny-warnings,internal-lints,metadata-collector-lint working-directory: clippy_lints + - name: Test clippy_utils + run: cargo test --features deny-warnings,internal-lints,metadata-collector-lint + working-directory: clippy_utils + - name: Test rustc_tools_util run: cargo test --features deny-warnings working-directory: rustc_tools_util diff --git a/.github/workflows/clippy_bors.yml b/.github/workflows/clippy_bors.yml index 1f4d666c7a92c..8b644aa28176b 100644 --- a/.github/workflows/clippy_bors.yml +++ b/.github/workflows/clippy_bors.yml @@ -121,6 +121,10 @@ jobs: run: cargo test --features deny-warnings,internal-lints,metadata-collector-lint working-directory: clippy_lints + - name: Test clippy_utils + run: cargo test --features deny-warnings,internal-lints,metadata-collector-lint + working-directory: clippy_utils + - name: Test rustc_tools_util run: cargo test --features deny-warnings working-directory: rustc_tools_util diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b5279cda6ea3..27bac4718b6c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,13 +6,127 @@ document. ## Unreleased / In Rust Nightly -[b7f3f7f...master](https://github.com/rust-lang/rust-clippy/compare/b7f3f7f...master) +[e181011...master](https://github.com/rust-lang/rust-clippy/compare/e181011...master) + +## Rust 1.58 (beta) + +Current beta, release 2022-01-13 + +[00e31fa...e181011](https://github.com/rust-lang/rust-clippy/compare/00e31fa...e181011) + +### New lints + +* [`transmute_num_to_bytes`] + [#7805](https://github.com/rust-lang/rust-clippy/pull/7805) +* [`match_str_case_mismatch`] + [#7806](https://github.com/rust-lang/rust-clippy/pull/7806) +* [`format_in_format_args`], [`to_string_in_format_args`] + [#7743](https://github.com/rust-lang/rust-clippy/pull/7743) +* [`uninit_vec`] + [#7682](https://github.com/rust-lang/rust-clippy/pull/7682) +* [`fn_to_numeric_cast_any`] + [#7705](https://github.com/rust-lang/rust-clippy/pull/7705) +* [`undocumented_unsafe_blocks`] + [#7748](https://github.com/rust-lang/rust-clippy/pull/7748) +* [`trailing_empty_array`] + [#7838](https://github.com/rust-lang/rust-clippy/pull/7838) +* [`string_slice`] + [#7878](https://github.com/rust-lang/rust-clippy/pull/7878) + +### Moves or deprecations of lints + +* Move [`non_send_fields_in_send_ty`] to `suspicious` + [#7874](https://github.com/rust-lang/rust-clippy/pull/7874) +* Move [`non_ascii_literal`] to `restriction` + [#7907](https://github.com/rust-lang/rust-clippy/pull/7907) + +### Changes that expand what code existing lints cover + +* [`question_mark`] now covers `Result` + [#7840](https://github.com/rust-lang/rust-clippy/pull/7840) +* Make [`useless_format`] recognize bare `format!("")` + [#7801](https://github.com/rust-lang/rust-clippy/pull/7801) +* Lint on underscored variables with no side effects in [`no_effect`] + [#7775](https://github.com/rust-lang/rust-clippy/pull/7775) +* Expand [`match_ref_pats`] to check for multiple reference patterns + [#7800](https://github.com/rust-lang/rust-clippy/pull/7800) + +### False positive fixes + +* Fix false positive of [`implicit_saturating_sub`] with `else` clause + [#7832](https://github.com/rust-lang/rust-clippy/pull/7832) +* Fix [`question_mark`] when there is call in conditional predicate + [#7860](https://github.com/rust-lang/rust-clippy/pull/7860) +* [`mut_mut`] no longer lints when type is defined in external macros + [#7795](https://github.com/rust-lang/rust-clippy/pull/7795) +* Avoid [`eq_op`] in test functions + [#7811](https://github.com/rust-lang/rust-clippy/pull/7811) +* [`cast_possible_truncation`] no longer lints when cast is coming from `signum` + method call [#7850](https://github.com/rust-lang/rust-clippy/pull/7850) +* [`match_str_case_mismatch`] no longer lints on uncased characters + [#7865](https://github.com/rust-lang/rust-clippy/pull/7865) +* [`ptr_arg`] no longer lints references to type aliases + [#7890](https://github.com/rust-lang/rust-clippy/pull/7890) +* [`missing_safety_doc`] now also accepts "implementation safety" headers + [#7856](https://github.com/rust-lang/rust-clippy/pull/7856) +* [`missing_safety_doc`] no longer lints if any parent has `#[doc(hidden)]` + attribute [#7849](https://github.com/rust-lang/rust-clippy/pull/7849) +* [`if_not_else`] now ignores else-if statements + [#7895](https://github.com/rust-lang/rust-clippy/pull/7895) +* Avoid linting [`cast_possible_truncation`] on bit-reducing operations + [#7819](https://github.com/rust-lang/rust-clippy/pull/7819) +* Avoid linting [`field_reassign_with_default`] when `Drop` and `Copy` are + involved [#7794](https://github.com/rust-lang/rust-clippy/pull/7794) +* [`unnecessary_sort_by`] now checks if argument implements `Ord` trait + [#7824](https://github.com/rust-lang/rust-clippy/pull/7824) +* Fix false positive in [`match_overlapping_arm`] + [#7847](https://github.com/rust-lang/rust-clippy/pull/7847) +* Prevent [`needless_lifetimes`] false positive in `async` function definition + [#7901](https://github.com/rust-lang/rust-clippy/pull/7901) + +### Suggestion fixes/improvements + +* Keep an initial `::` when [`doc_markdown`] suggests to use ticks + [#7916](https://github.com/rust-lang/rust-clippy/pull/7916) +* Add a machine applicable suggestion for the [`doc_markdown`] missing backticks + lint [#7904](https://github.com/rust-lang/rust-clippy/pull/7904) +* [`equatable_if_let`] no longer expands macros in the suggestion + [#7788](https://github.com/rust-lang/rust-clippy/pull/7788) +* Make [`shadow_reuse`] suggestion less verbose + [#7782](https://github.com/rust-lang/rust-clippy/pull/7782) + +### ICE fixes + +* Fix ICE in [`enum_variant_names`] + [#7873](https://github.com/rust-lang/rust-clippy/pull/7873) +* Fix ICE in [`undocumented_unsafe_blocks`] + [#7891](https://github.com/rust-lang/rust-clippy/pull/7891) + +### Documentation improvements + +* Fixed naive doc formatting for `#[must_use]` lints ([`must_use_unit`], + [`double_must_use`], [`must_use_candidate`], [`let_underscore_must_use`]) + [#7827](https://github.com/rust-lang/rust-clippy/pull/7827) +* Fix typo in example for [`match_result_ok`] + [#7815](https://github.com/rust-lang/rust-clippy/pull/7815) + +### Others + +* Allow giving reasons for [`disallowed_types`] + [#7791](https://github.com/rust-lang/rust-clippy/pull/7791) +* Fix [`manual_assert`] and [`match_wild_err_arm`] for `#![no_std]` and Rust + 2021. [#7851](https://github.com/rust-lang/rust-clippy/pull/7851) +* Fix regression in [`semicolon_if_nothing_returned`] on macros containing while + loops [#7789](https://github.com/rust-lang/rust-clippy/pull/7789) +* Added a new configuration `literal-suffix-style` to enforce a certain style + writing [`unseparated_literal_suffix`] + [#7726](https://github.com/rust-lang/rust-clippy/pull/7726) ## Rust 1.57 -Current beta, release 2021-12-02 +Current stable, released 2021-12-02 -[7bfc26e...b7f3f7f](https://github.com/rust-lang/rust-clippy/compare/7bfc26e...b7f3f7f) +[7bfc26e...00e31fa](https://github.com/rust-lang/rust-clippy/compare/7bfc26e...00e31fa) ### New Lints @@ -161,7 +275,7 @@ Current beta, release 2021-12-02 ## Rust 1.56 -Current stable, released 2021-10-21 +Released 2021-10-21 [74d1561...7bfc26e](https://github.com/rust-lang/rust-clippy/compare/74d1561...7bfc26e) @@ -2912,6 +3026,7 @@ Released 2018-09-13 [`infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_iter [`inherent_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string [`inherent_to_string_shadow_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string_shadow_display +[`init_numbered_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#init_numbered_fields [`inline_always`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_always [`inline_asm_x86_att_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_att_syntax [`inline_asm_x86_intel_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_intel_syntax diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 97ff31b4bc5a3..fc663de8f792d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -118,7 +118,7 @@ which `IntelliJ Rust` will be able to understand. Run `cargo dev setup intellij --repo-path ` where `` is a path to the rustc repo you just cloned. The command will add path-dependencies pointing towards rustc-crates inside the rustc repo to -Clippys `Cargo.toml`s and should allow `IntelliJ Rust` to understand most of the types that Clippy uses. +Clippy's `Cargo.toml`s and should allow `IntelliJ Rust` to understand most of the types that Clippy uses. Just make sure to remove the dependencies again before finally making a pull request! [rustc_repo]: https://github.com/rust-lang/rust/ @@ -126,8 +126,8 @@ Just make sure to remove the dependencies again before finally making a pull req ### Rust Analyzer As of [#6869][6869], [`rust-analyzer`][ra_homepage] can understand that Clippy uses compiler-internals -using `extern crate` when `package.metadata.rust-analyzer.rustc_private` is set to `true` in Clippys `Cargo.toml.` -You will required a `nightly` toolchain with the `rustc-dev` component installed. +using `extern crate` when `package.metadata.rust-analyzer.rustc_private` is set to `true` in Clippy's `Cargo.toml.` +You will require a `nightly` toolchain with the `rustc-dev` component installed. Make sure that in the `rust-analyzer` configuration, you set ``` { "rust-analyzer.rustcSource": "discover" } @@ -228,7 +228,7 @@ about `subtree`s in the Rust repository see [Rust's `CONTRIBUTING.md`][subtree]. ### Patching git-subtree to work with big repos -Currently there's a bug in `git-subtree` that prevents it from working properly +Currently, there's a bug in `git-subtree` that prevents it from working properly with the [`rust-lang/rust`] repo. There's an open PR to fix that, but it's stale. Before continuing with the following steps, we need to manually apply that fix to our local copy of `git-subtree`. diff --git a/README.md b/README.md index 73b0167d363f6..1bbd89e7822e8 100644 --- a/README.md +++ b/README.md @@ -144,7 +144,7 @@ line. (You can swap `clippy::all` with the specific lint category you are target ## Configuration Some lints can be configured in a TOML file named `clippy.toml` or `.clippy.toml`. It contains a basic `variable = -value` mapping eg. +value` mapping e.g. ```toml avoid-breaking-exported-api = false @@ -155,6 +155,10 @@ cognitive-complexity-threshold = 30 See the [list of lints](https://rust-lang.github.io/rust-clippy/master/index.html) for more information about which lints can be configured and the meaning of the variables. +Note that configuration changes will not apply for code that has already been compiled and cached under `./target/`; +for example, adding a new string to `doc-valid-idents` may still result in Clippy flagging that string. To be sure that +any configuration changes are applied, you may want to run `cargo clean` and re-compile your crate from scratch. + To deactivate the “for further information visit *lint-link*” message you can define the `CLIPPY_DISABLE_DOCS_LINKS` environment variable. @@ -193,7 +197,7 @@ And to warn on `lint_name`, run cargo clippy -- -W clippy::lint_name ``` -This also works with lint groups. For example you +This also works with lint groups. For example, you can run Clippy with warnings for all lints enabled: ```terminal cargo clippy -- -W clippy::pedantic @@ -228,7 +232,7 @@ fn main() { You can also omit the patch version when specifying the MSRV, so `msrv = 1.30` is equivalent to `msrv = 1.30.0`. -Note: `custom_inner_attributes` is an unstable feature so it has to be enabled explicitly. +Note: `custom_inner_attributes` is an unstable feature, so it has to be enabled explicitly. Lints that recognize this configuration option can be found [here](https://rust-lang.github.io/rust-clippy/master/index.html#msrv) diff --git a/clippy_lints/src/approx_const.rs b/clippy_lints/src/approx_const.rs index 12435eefbc4ee..5061c9d1eaf6f 100644 --- a/clippy_lints/src/approx_const.rs +++ b/clippy_lints/src/approx_const.rs @@ -87,7 +87,7 @@ impl ApproxConstant { let s = s.as_str(); if s.parse::().is_ok() { for &(constant, name, min_digits, msrv) in &KNOWN_CONSTS { - if is_approx_const(constant, &s, min_digits) + if is_approx_const(constant, s, min_digits) && msrv.as_ref().map_or(true, |msrv| meets_msrv(self.msrv.as_ref(), msrv)) { span_lint_and_help( diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 489945b513da4..0629674307ba7 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -310,8 +310,10 @@ impl<'tcx> LateLintPass<'tcx> for Attributes { || is_word(lint, sym::deprecated) || is_word(lint, sym!(unreachable_pub)) || is_word(lint, sym!(unused)) - || extract_clippy_lint(lint).map_or(false, |s| s.as_str() == "wildcard_imports") - || extract_clippy_lint(lint).map_or(false, |s| s.as_str() == "enum_glob_use") + || extract_clippy_lint(lint) + .map_or(false, |s| s.as_str() == "wildcard_imports") + || extract_clippy_lint(lint) + .map_or(false, |s| s.as_str() == "enum_glob_use") { return; } diff --git a/clippy_lints/src/casts/unnecessary_cast.rs b/clippy_lints/src/casts/unnecessary_cast.rs index 9ed359922fd4d..1915d990c126c 100644 --- a/clippy_lints/src/casts/unnecessary_cast.rs +++ b/clippy_lints/src/casts/unnecessary_cast.rs @@ -49,8 +49,9 @@ pub(super) fn check( if cast_from.kind() == cast_to.kind() => { if let Some(src) = snippet_opt(cx, lit.span) { - let num_lit = NumericLiteral::from_lit_kind(&src, &lit.node).unwrap(); - lint_unnecessary_cast(cx, expr, num_lit.integer, cast_from, cast_to); + if let Some(num_lit) = NumericLiteral::from_lit_kind(&src, &lit.node) { + lint_unnecessary_cast(cx, expr, num_lit.integer, cast_from, cast_to); + } } }, _ => { diff --git a/clippy_lints/src/enum_variants.rs b/clippy_lints/src/enum_variants.rs index 689ac6184bffb..4f89e56743068 100644 --- a/clippy_lints/src/enum_variants.rs +++ b/clippy_lints/src/enum_variants.rs @@ -2,8 +2,8 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_help}; use clippy_utils::source::is_present_in_source; -use clippy_utils::str_utils::{self, count_match_end, count_match_start}; -use rustc_hir::{EnumDef, Item, ItemKind}; +use clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start}; +use rustc_hir::{EnumDef, Item, ItemKind, Variant}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Span; @@ -18,6 +18,12 @@ declare_clippy_lint! { /// Enumeration variant names should specify their variant, /// not repeat the enumeration name. /// + /// ### Limitations + /// Characters with no casing will be considered when comparing prefixes/suffixes + /// This applies to numbers and non-ascii characters without casing + /// e.g. `Foo1` and `Foo2` is considered to have different prefixes + /// (the prefixes are `Foo1` and `Foo2` respectively), as also `Bar螃`, `Bar蟹` + /// /// ### Example /// ```rust /// enum Cake { @@ -120,72 +126,73 @@ impl_lint_pass!(EnumVariantNames => [ MODULE_INCEPTION ]); -fn check_variant( - cx: &LateContext<'_>, - threshold: u64, - def: &EnumDef<'_>, - item_name: &str, - item_name_chars: usize, - span: Span, -) { +fn check_enum_start(cx: &LateContext<'_>, item_name: &str, variant: &Variant<'_>) { + let name = variant.ident.name.as_str(); + let item_name_chars = item_name.chars().count(); + + if count_match_start(item_name, name).char_count == item_name_chars + && name.chars().nth(item_name_chars).map_or(false, |c| !c.is_lowercase()) + && name.chars().nth(item_name_chars + 1).map_or(false, |c| !c.is_numeric()) + { + span_lint( + cx, + ENUM_VARIANT_NAMES, + variant.span, + "variant name starts with the enum's name", + ); + } +} + +fn check_enum_end(cx: &LateContext<'_>, item_name: &str, variant: &Variant<'_>) { + let name = variant.ident.name.as_str(); + let item_name_chars = item_name.chars().count(); + + if count_match_end(item_name, name).char_count == item_name_chars { + span_lint( + cx, + ENUM_VARIANT_NAMES, + variant.span, + "variant name ends with the enum's name", + ); + } +} + +fn check_variant(cx: &LateContext<'_>, threshold: u64, def: &EnumDef<'_>, item_name: &str, span: Span) { if (def.variants.len() as u64) < threshold { return; } + + let first = &def.variants[0].ident.name.as_str(); + let mut pre = camel_case_split(first); + let mut post = pre.clone(); + post.reverse(); for var in def.variants { - let name = var.ident.name.as_str(); - if count_match_start(item_name, &name).char_count == item_name_chars - && name.chars().nth(item_name_chars).map_or(false, |c| !c.is_lowercase()) - && name.chars().nth(item_name_chars + 1).map_or(false, |c| !c.is_numeric()) - { - span_lint( - cx, - ENUM_VARIANT_NAMES, - var.span, - "variant name starts with the enum's name", - ); - } - if count_match_end(item_name, &name).char_count == item_name_chars { - span_lint( - cx, - ENUM_VARIANT_NAMES, - var.span, - "variant name ends with the enum's name", - ); - } - } - let first = def.variants[0].ident.name.as_str(); - let mut pre = &first[..str_utils::camel_case_until(&*first).byte_index]; - let mut post = &first[str_utils::camel_case_start(&*first).byte_index..]; - for var in def.variants { + check_enum_start(cx, item_name, var); + check_enum_end(cx, item_name, var); let name = var.ident.name.as_str(); - let pre_match = count_match_start(pre, &name).byte_count; - pre = &pre[..pre_match]; - let pre_camel = str_utils::camel_case_until(pre).byte_index; - pre = &pre[..pre_camel]; - while let Some((next, last)) = name[pre.len()..].chars().zip(pre.chars().rev()).next() { - if next.is_numeric() { - return; - } - if next.is_lowercase() { - let last = pre.len() - last.len_utf8(); - let last_camel = str_utils::camel_case_until(&pre[..last]); - pre = &pre[..last_camel.byte_index]; - } else { - break; - } - } + let variant_split = camel_case_split(name); - let post_match = count_match_end(post, &name); - let post_end = post.len() - post_match.byte_count; - post = &post[post_end..]; - let post_camel = str_utils::camel_case_start(post); - post = &post[post_camel.byte_index..]; + pre = pre + .iter() + .zip(variant_split.iter()) + .take_while(|(a, b)| a == b) + .map(|e| *e.0) + .collect(); + post = post + .iter() + .zip(variant_split.iter().rev()) + .take_while(|(a, b)| a == b) + .map(|e| *e.0) + .collect(); } let (what, value) = match (pre.is_empty(), post.is_empty()) { (true, true) => return, - (false, _) => ("pre", pre), - (true, false) => ("post", post), + (false, _) => ("pre", pre.join("")), + (true, false) => { + post.reverse(); + ("post", post.join("")) + }, }; span_lint_and_help( cx, @@ -233,8 +240,7 @@ impl LateLintPass<'_> for EnumVariantNames { #[allow(clippy::similar_names)] fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { let item_name = item.ident.name.as_str(); - let item_name_chars = item_name.chars().count(); - let item_camel = to_camel_case(&item_name); + let item_camel = to_camel_case(item_name); if !item.span.from_expansion() && is_present_in_source(cx, item.span) { if let Some(&(ref mod_name, ref mod_camel)) = self.modules.last() { // constants don't have surrounding modules @@ -283,7 +289,7 @@ impl LateLintPass<'_> for EnumVariantNames { } if let ItemKind::Enum(ref def, _) = item.kind { if !(self.avoid_breaking_exported_api && cx.access_levels.is_exported(item.def_id)) { - check_variant(cx, self.threshold, def, &item_name, item_name_chars, item.span); + check_variant(cx, self.threshold, def, item_name, item.span); } } self.modules.push((item.ident.name, item_camel)); diff --git a/clippy_lints/src/float_literal.rs b/clippy_lints/src/float_literal.rs index 6903073fbcd85..7a4397a7b7467 100644 --- a/clippy_lints/src/float_literal.rs +++ b/clippy_lints/src/float_literal.rs @@ -73,7 +73,7 @@ impl<'tcx> LateLintPass<'tcx> for FloatLiteral { // If its within the 2 decimal digits of being out of precision we // check if the parsed representation is the same as the string // since we'll need the truncated string anyway. - let digits = count_digits(&sym_str); + let digits = count_digits(sym_str); let max = max_digits(fty); let type_suffix = match lit_float_ty { LitFloatType::Suffixed(ast::FloatTy::F32) => Some("f32"), diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index a70e58c9c3a5e..6dcbaf68dfdbc 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -356,7 +356,7 @@ fn detect_hypot(cx: &LateContext<'_>, args: &[Expr<'_>]) -> Option { if eq_expr_value(cx, lmul_lhs, lmul_rhs); if eq_expr_value(cx, rmul_lhs, rmul_rhs); then { - return Some(format!("{}.hypot({})", Sugg::hir(cx, lmul_lhs, ".."), Sugg::hir(cx, rmul_lhs, ".."))); + return Some(format!("{}.hypot({})", Sugg::hir(cx, lmul_lhs, "..").maybe_par(), Sugg::hir(cx, rmul_lhs, ".."))); } } @@ -379,7 +379,7 @@ fn detect_hypot(cx: &LateContext<'_>, args: &[Expr<'_>]) -> Option { if let Some((rvalue, _)) = constant(cx, cx.typeck_results(), rargs_1); if Int(2) == lvalue && Int(2) == rvalue; then { - return Some(format!("{}.hypot({})", Sugg::hir(cx, largs_0, ".."), Sugg::hir(cx, rargs_0, ".."))); + return Some(format!("{}.hypot({})", Sugg::hir(cx, largs_0, "..").maybe_par(), Sugg::hir(cx, rargs_0, ".."))); } } } @@ -654,26 +654,52 @@ fn check_radians(cx: &LateContext<'_>, expr: &Expr<'_>) { if (F32(f32_consts::PI) == rvalue || F64(f64_consts::PI) == rvalue) && (F32(180_f32) == lvalue || F64(180_f64) == lvalue) { + let mut proposal = format!("{}.to_degrees()", Sugg::hir(cx, mul_lhs, "..")); + if_chain! { + if let ExprKind::Lit(ref literal) = mul_lhs.kind; + if let ast::LitKind::Float(ref value, float_type) = literal.node; + if float_type == ast::LitFloatType::Unsuffixed; + then { + if value.as_str().ends_with('.') { + proposal = format!("{}0_f64.to_degrees()", Sugg::hir(cx, mul_lhs, "..")); + } else { + proposal = format!("{}_f64.to_degrees()", Sugg::hir(cx, mul_lhs, "..")); + } + } + } span_lint_and_sugg( cx, SUBOPTIMAL_FLOPS, expr.span, "conversion to degrees can be done more accurately", "consider using", - format!("{}.to_degrees()", Sugg::hir(cx, mul_lhs, "..")), + proposal, Applicability::MachineApplicable, ); } else if (F32(180_f32) == rvalue || F64(180_f64) == rvalue) && (F32(f32_consts::PI) == lvalue || F64(f64_consts::PI) == lvalue) { + let mut proposal = format!("{}.to_radians()", Sugg::hir(cx, mul_lhs, "..")); + if_chain! { + if let ExprKind::Lit(ref literal) = mul_lhs.kind; + if let ast::LitKind::Float(ref value, float_type) = literal.node; + if float_type == ast::LitFloatType::Unsuffixed; + then { + if value.as_str().ends_with('.') { + proposal = format!("{}0_f64.to_radians()", Sugg::hir(cx, mul_lhs, "..")); + } else { + proposal = format!("{}_f64.to_radians()", Sugg::hir(cx, mul_lhs, "..")); + } + } + } span_lint_and_sugg( cx, SUBOPTIMAL_FLOPS, expr.span, "conversion to radians can be done more accurately", "consider using", - format!("{}.to_radians()", Sugg::hir(cx, mul_lhs, "..")), + proposal, Applicability::MachineApplicable, ); } diff --git a/clippy_lints/src/identity_op.rs b/clippy_lints/src/identity_op.rs index b4e7bbc767135..f824f20ca40a0 100644 --- a/clippy_lints/src/identity_op.rs +++ b/clippy_lints/src/identity_op.rs @@ -61,15 +61,18 @@ impl<'tcx> LateLintPass<'tcx> for IdentityOp { } fn is_allowed(cx: &LateContext<'_>, cmp: BinOp, left: &Expr<'_>, right: &Expr<'_>) -> bool { - // `1 << 0` is a common pattern in bit manipulation code - cmp.node == BinOpKind::Shl - && constant_simple(cx, cx.typeck_results(), right) == Some(Constant::Int(0)) - && constant_simple(cx, cx.typeck_results(), left) == Some(Constant::Int(1)) + // This lint applies to integers + !cx.typeck_results().expr_ty(left).peel_refs().is_integral() + || !cx.typeck_results().expr_ty(right).peel_refs().is_integral() + // `1 << 0` is a common pattern in bit manipulation code + || (cmp.node == BinOpKind::Shl + && constant_simple(cx, cx.typeck_results(), right) == Some(Constant::Int(0)) + && constant_simple(cx, cx.typeck_results(), left) == Some(Constant::Int(1))) } fn check(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span) { - if let Some(Constant::Int(v)) = constant_simple(cx, cx.typeck_results(), e) { - let check = match *cx.typeck_results().expr_ty(e).kind() { + if let Some(Constant::Int(v)) = constant_simple(cx, cx.typeck_results(), e).map(Constant::peel_refs) { + let check = match *cx.typeck_results().expr_ty(e).peel_refs().kind() { ty::Int(ity) => unsext(cx.tcx, -1_i128, ity), ty::Uint(uty) => clip(cx.tcx, !0, uty), _ => return, diff --git a/clippy_lints/src/implicit_saturating_sub.rs b/clippy_lints/src/implicit_saturating_sub.rs index 26a196aab5972..6515975fbffdc 100644 --- a/clippy_lints/src/implicit_saturating_sub.rs +++ b/clippy_lints/src/implicit_saturating_sub.rs @@ -96,7 +96,7 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub { if let LitKind::Int(0, _) = cond_lit.node { if cx.typeck_results().expr_ty(cond_left).is_signed() { } else { - print_lint_and_sugg(cx, &var_name, expr); + print_lint_and_sugg(cx, var_name, expr); }; } }, @@ -108,7 +108,7 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub { let mut int_ids = INT_TYPES.iter().filter_map(|&ty| cx.tcx.lang_items().require(ty).ok()); if int_ids.any(|int_id| int_id == impl_id); then { - print_lint_and_sugg(cx, &var_name, expr) + print_lint_and_sugg(cx, var_name, expr) } } }, @@ -121,7 +121,7 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub { let mut int_ids = INT_TYPES.iter().filter_map(|&ty| cx.tcx.lang_items().require(ty).ok()); if int_ids.any(|int_id| int_id == impl_id); then { - print_lint_and_sugg(cx, &var_name, expr) + print_lint_and_sugg(cx, var_name, expr) } } }, diff --git a/clippy_lints/src/init_numbered_fields.rs b/clippy_lints/src/init_numbered_fields.rs new file mode 100644 index 0000000000000..5fe6725b581dc --- /dev/null +++ b/clippy_lints/src/init_numbered_fields.rs @@ -0,0 +1,80 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::in_macro; +use clippy_utils::source::snippet_with_applicability; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use std::borrow::Cow; +use std::cmp::Reverse; +use std::collections::BinaryHeap; + +declare_clippy_lint! { + /// ### What it does + /// Checks for tuple structs initialized with field syntax. + /// It will however not lint if a base initializer is present. + /// The lint will also ignore code in macros. + /// + /// ### Why is this bad? + /// This may be confusing to the uninitiated and adds no + /// benefit as opposed to tuple initializers + /// + /// ### Example + /// ```rust + /// struct TupleStruct(u8, u16); + /// + /// let _ = TupleStruct { + /// 0: 1, + /// 1: 23, + /// }; + /// + /// // should be written as + /// let base = TupleStruct(1, 23); + /// + /// // This is OK however + /// let _ = TupleStruct { 0: 42, ..base }; + /// ``` + #[clippy::version = "1.59.0"] + pub INIT_NUMBERED_FIELDS, + style, + "numbered fields in tuple struct initializer" +} + +declare_lint_pass!(NumberedFields => [INIT_NUMBERED_FIELDS]); + +impl<'tcx> LateLintPass<'tcx> for NumberedFields { + fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { + if let ExprKind::Struct(path, fields, None) = e.kind { + if !fields.is_empty() + && !in_macro(e.span) + && fields + .iter() + .all(|f| f.ident.as_str().as_bytes().iter().all(u8::is_ascii_digit)) + { + let expr_spans = fields + .iter() + .map(|f| (Reverse(f.ident.as_str().parse::().unwrap()), f.expr.span)) + .collect::>(); + let mut appl = Applicability::MachineApplicable; + let snippet = format!( + "{}({})", + snippet_with_applicability(cx, path.span(), "..", &mut appl), + expr_spans + .into_iter_sorted() + .map(|(_, span)| snippet_with_applicability(cx, span, "..", &mut appl)) + .intersperse(Cow::Borrowed(", ")) + .collect::() + ); + span_lint_and_sugg( + cx, + INIT_NUMBERED_FIELDS, + e.span, + "used a field initializer for a tuple struct", + "try this instead", + snippet, + appl, + ); + } + } + } +} diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index 09cd0d22d8b0f..20e6220ec7d3a 100644 --- a/clippy_lints/src/len_zero.rs +++ b/clippy_lints/src/len_zero.rs @@ -441,7 +441,7 @@ fn is_empty_string(expr: &Expr<'_>) -> bool { if let ExprKind::Lit(ref lit) = expr.kind { if let LitKind::Str(lit, _) = lit.node { let lit = lit.as_str(); - return lit == ""; + return lit.is_empty(); } } false diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 3d3999d4cc0d8..944411087e951 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -81,6 +81,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(infinite_iter::INFINITE_ITER), LintId::of(inherent_to_string::INHERENT_TO_STRING), LintId::of(inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY), + LintId::of(init_numbered_fields::INIT_NUMBERED_FIELDS), LintId::of(inline_fn_without_body::INLINE_FN_WITHOUT_BODY), LintId::of(int_plus_one::INT_PLUS_ONE), LintId::of(large_const_arrays::LARGE_CONST_ARRAYS), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 766c5ba1bcb0f..002122793f3b6 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -178,6 +178,7 @@ store.register_lints(&[ inherent_impl::MULTIPLE_INHERENT_IMPL, inherent_to_string::INHERENT_TO_STRING, inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY, + init_numbered_fields::INIT_NUMBERED_FIELDS, inline_fn_without_body::INLINE_FN_WITHOUT_BODY, int_plus_one::INT_PLUS_ONE, integer_division::INTEGER_DIVISION, diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs index ea87e7e7a7368..1a0b869d40adb 100644 --- a/clippy_lints/src/lib.register_style.rs +++ b/clippy_lints/src/lib.register_style.rs @@ -29,6 +29,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![ LintId::of(functions::MUST_USE_UNIT), LintId::of(functions::RESULT_UNIT_ERR), LintId::of(inherent_to_string::INHERENT_TO_STRING), + LintId::of(init_numbered_fields::INIT_NUMBERED_FIELDS), LintId::of(len_zero::COMPARISON_TO_EMPTY), LintId::of(len_zero::LEN_WITHOUT_IS_EMPTY), LintId::of(len_zero::LEN_ZERO), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d1c7956a7a5c8..d4687a1e28797 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1,13 +1,15 @@ // error-pattern:cargo-clippy +#![feature(binary_heap_into_iter_sorted)] #![feature(box_patterns)] +#![feature(control_flow_enum)] #![feature(drain_filter)] #![feature(in_band_lifetimes)] +#![feature(iter_intersperse)] +#![feature(let_else)] #![feature(once_cell)] #![feature(rustc_private)] #![feature(stmt_expr_attributes)] -#![feature(control_flow_enum)] -#![feature(let_else)] #![recursion_limit = "512"] #![cfg_attr(feature = "deny-warnings", deny(warnings))] #![allow(clippy::missing_docs_in_private_items, clippy::must_use_candidate)] @@ -242,6 +244,7 @@ mod indexing_slicing; mod infinite_iter; mod inherent_impl; mod inherent_to_string; +mod init_numbered_fields; mod inline_fn_without_body; mod int_plus_one; mod integer_division; @@ -854,6 +857,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(|| Box::new(octal_escapes::OctalEscapes)); store.register_late_pass(|| Box::new(needless_late_init::NeedlessLateInit)); store.register_late_pass(|| Box::new(return_self_not_must_use::ReturnSelfNotMustUse)); + store.register_late_pass(|| Box::new(init_numbered_fields::NumberedFields)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/loops/manual_memcpy.rs b/clippy_lints/src/loops/manual_memcpy.rs index 6cda926853438..c62fa5e998bd4 100644 --- a/clippy_lints/src/loops/manual_memcpy.rs +++ b/clippy_lints/src/loops/manual_memcpy.rs @@ -12,6 +12,7 @@ use rustc_hir::{BinOpKind, Block, Expr, ExprKind, HirId, Pat, PatKind, StmtKind} use rustc_lint::LateContext; use rustc_middle::ty::{self, Ty}; use rustc_span::symbol::sym; +use std::fmt::Display; use std::iter::Iterator; /// Checks for for loops that sequentially copy items from one slice-like @@ -108,7 +109,7 @@ fn build_manual_memcpy_suggestion<'tcx>( src: &IndexExpr<'_>, ) -> String { fn print_offset(offset: MinifyingSugg<'static>) -> MinifyingSugg<'static> { - if offset.as_str() == "0" { + if offset.to_string() == "0" { sugg::EMPTY.into() } else { offset @@ -123,7 +124,7 @@ fn build_manual_memcpy_suggestion<'tcx>( if let Some(arg) = len_args.get(0); if path_to_local(arg) == path_to_local(base); then { - if sugg.as_str() == end_str { + if sugg.to_string() == end_str { sugg::EMPTY.into() } else { sugg @@ -147,7 +148,7 @@ fn build_manual_memcpy_suggestion<'tcx>( print_offset(apply_offset(&start_str, &idx_expr.idx_offset)).into_sugg(), print_limit( end, - end_str.as_str(), + end_str.to_string().as_str(), idx_expr.base, apply_offset(&end_str, &idx_expr.idx_offset), ) @@ -159,7 +160,7 @@ fn build_manual_memcpy_suggestion<'tcx>( print_offset(apply_offset(&counter_start, &idx_expr.idx_offset)).into_sugg(), print_limit( end, - end_str.as_str(), + end_str.to_string().as_str(), idx_expr.base, apply_offset(&end_str, &idx_expr.idx_offset) + &counter_start - &start_str, ) @@ -202,15 +203,13 @@ fn build_manual_memcpy_suggestion<'tcx>( #[derive(Clone)] struct MinifyingSugg<'a>(Sugg<'a>); -impl<'a> MinifyingSugg<'a> { - fn as_str(&self) -> &str { - // HACK: Don't sync to Clippy! Required because something with the `or_patterns` feature - // changed and this would now require parentheses. - match &self.0 { - Sugg::NonParen(s) | Sugg::MaybeParen(s) | Sugg::BinOp(_, s) => s.as_ref(), - } +impl Display for MinifyingSugg<'a> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.fmt(f) } +} +impl<'a> MinifyingSugg<'a> { fn into_sugg(self) -> Sugg<'a> { self.0 } @@ -225,7 +224,7 @@ impl<'a> From> for MinifyingSugg<'a> { impl std::ops::Add for &MinifyingSugg<'static> { type Output = MinifyingSugg<'static>; fn add(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> { - match (self.as_str(), rhs.as_str()) { + match (self.to_string().as_str(), rhs.to_string().as_str()) { ("0", _) => rhs.clone(), (_, "0") => self.clone(), (_, _) => (&self.0 + &rhs.0).into(), @@ -236,7 +235,7 @@ impl std::ops::Add for &MinifyingSugg<'static> { impl std::ops::Sub for &MinifyingSugg<'static> { type Output = MinifyingSugg<'static>; fn sub(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> { - match (self.as_str(), rhs.as_str()) { + match (self.to_string().as_str(), rhs.to_string().as_str()) { (_, "0") => self.clone(), ("0", _) => (-rhs.0.clone()).into(), (x, y) if x == y => sugg::ZERO.into(), @@ -248,7 +247,7 @@ impl std::ops::Sub for &MinifyingSugg<'static> { impl std::ops::Add<&MinifyingSugg<'static>> for MinifyingSugg<'static> { type Output = MinifyingSugg<'static>; fn add(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> { - match (self.as_str(), rhs.as_str()) { + match (self.to_string().as_str(), rhs.to_string().as_str()) { ("0", _) => rhs.clone(), (_, "0") => self, (_, _) => (self.0 + &rhs.0).into(), @@ -259,7 +258,7 @@ impl std::ops::Add<&MinifyingSugg<'static>> for MinifyingSugg<'static> { impl std::ops::Sub<&MinifyingSugg<'static>> for MinifyingSugg<'static> { type Output = MinifyingSugg<'static>; fn sub(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> { - match (self.as_str(), rhs.as_str()) { + match (self.to_string().as_str(), rhs.to_string().as_str()) { (_, "0") => self, ("0", _) => (-rhs.0.clone()).into(), (x, y) if x == y => sugg::ZERO.into(), diff --git a/clippy_lints/src/macro_use.rs b/clippy_lints/src/macro_use.rs index 5b22b64a370e2..50d80e6a1d224 100644 --- a/clippy_lints/src/macro_use.rs +++ b/clippy_lints/src/macro_use.rs @@ -1,4 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::in_macro; use clippy_utils::source::snippet; use hir::def::{DefKind, Res}; use if_chain::if_chain; @@ -8,7 +9,6 @@ use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::hygiene::ExpnKind; use rustc_span::{edition::Edition, sym, Span}; declare_clippy_lint! { @@ -213,7 +213,3 @@ impl<'tcx> LateLintPass<'tcx> for MacroUseImports { } } } - -fn in_macro(span: Span) -> bool { - span.from_expansion() && !matches!(span.ctxt().outer_expn_data().kind, ExpnKind::Desugaring(..)) -} diff --git a/clippy_lints/src/match_str_case_mismatch.rs b/clippy_lints/src/match_str_case_mismatch.rs index dbf103143d93c..2c0fc218ca07c 100644 --- a/clippy_lints/src/match_str_case_mismatch.rs +++ b/clippy_lints/src/match_str_case_mismatch.rs @@ -94,8 +94,8 @@ impl<'a, 'tcx> Visitor<'tcx> for MatchExprVisitor<'a, 'tcx> { fn visit_expr(&mut self, ex: &'tcx Expr<'_>) { match ex.kind { - ExprKind::MethodCall(segment, _, [receiver], _) - if self.case_altered(segment.ident.as_str(), receiver) => {}, + ExprKind::MethodCall(segment, _, [receiver], _) if self.case_altered(segment.ident.as_str(), receiver) => { + }, _ => walk_expr(self, ex), } } @@ -142,7 +142,7 @@ fn verify_case<'a>(case_method: &'a CaseMethod, arms: &'a [Arm<'_>]) -> Option<( }) = arm.pat.kind; if let LitKind::Str(symbol, _) = lit.node; let input = symbol.as_str(); - if !case_check(&input); + if !case_check(input); then { return Some((lit.span, symbol)); } diff --git a/clippy_lints/src/methods/iter_skip_next.rs b/clippy_lints/src/methods/iter_skip_next.rs index e32594757d0ca..f5410c7fd7fc8 100644 --- a/clippy_lints/src/methods/iter_skip_next.rs +++ b/clippy_lints/src/methods/iter_skip_next.rs @@ -1,8 +1,10 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::is_trait_method; +use clippy_utils::path_to_local; use clippy_utils::source::snippet; use rustc_errors::Applicability; use rustc_hir as hir; +use rustc_hir::{BindingAnnotation, Node, PatKind}; use rustc_lint::LateContext; use rustc_span::sym; @@ -11,14 +13,34 @@ use super::ITER_SKIP_NEXT; pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, arg: &hir::Expr<'_>) { // lint if caller of skip is an Iterator if is_trait_method(cx, expr, sym::Iterator) { - span_lint_and_sugg( + let mut application = Applicability::MachineApplicable; + span_lint_and_then( cx, ITER_SKIP_NEXT, expr.span.trim_start(recv.span).unwrap(), "called `skip(..).next()` on an iterator", - "use `nth` instead", - format!(".nth({})", snippet(cx, arg.span, "..")), - Applicability::MachineApplicable, + |diag| { + if_chain! { + if let Some(id) = path_to_local(recv); + if let Node::Binding(pat) = cx.tcx.hir().get(id); + if let PatKind::Binding(ann, _, _, _) = pat.kind; + if ann != BindingAnnotation::Mutable; + then { + application = Applicability::Unspecified; + diag.span_help( + pat.span, + &format!("for this change `{}` has to be mutable", snippet(cx, pat.span, "..")), + ); + } + } + + diag.span_suggestion( + expr.span.trim_start(recv.span).unwrap(), + "use `nth` instead", + format!(".nth({})", snippet(cx, arg.span, "..")), + application, + ); + }, ); } } diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 4934240abfc46..4e33b2ff14cde 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2112,7 +2112,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods { { wrong_self_convention::check( cx, - &name, + name, self_ty, first_arg_ty, first_arg.pat.span, diff --git a/clippy_lints/src/methods/unwrap_or_else_default.rs b/clippy_lints/src/methods/unwrap_or_else_default.rs index 276467b1dfdbf..f3af281d6cacc 100644 --- a/clippy_lints/src/methods/unwrap_or_else_default.rs +++ b/clippy_lints/src/methods/unwrap_or_else_default.rs @@ -2,7 +2,8 @@ use super::UNWRAP_OR_ELSE_DEFAULT; use clippy_utils::{ - diagnostics::span_lint_and_sugg, is_trait_item, source::snippet_with_applicability, ty::is_type_diagnostic_item, + diagnostics::span_lint_and_sugg, is_default_equivalent_call, source::snippet_with_applicability, + ty::is_type_diagnostic_item, }; use rustc_errors::Applicability; use rustc_hir as hir; @@ -24,7 +25,7 @@ pub(super) fn check<'tcx>( if_chain! { if is_option || is_result; - if is_trait_item(cx, u_arg, sym::Default); + if is_default_equivalent_call(cx, u_arg); then { let mut applicability = Applicability::MachineApplicable; diff --git a/clippy_lints/src/methods/utils.rs b/clippy_lints/src/methods/utils.rs index 11ad881ee7b95..24b44f819f419 100644 --- a/clippy_lints/src/methods/utils.rs +++ b/clippy_lints/src/methods/utils.rs @@ -57,7 +57,7 @@ pub(super) fn get_hint_if_single_char_arg( let string = r.as_str(); if string.chars().count() == 1; then { - let snip = snippet_with_applicability(cx, arg.span, &string, applicability); + let snip = snippet_with_applicability(cx, arg.span, string, applicability); let ch = if let ast::StrStyle::Raw(nhash) = style { let nhash = nhash as usize; // for raw string: r##"a"## diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs index d391fbecf82e1..778d49cb4b6ed 100644 --- a/clippy_lints/src/needless_bool.rs +++ b/clippy_lints/src/needless_bool.rs @@ -187,14 +187,14 @@ impl<'tcx> LateLintPass<'tcx> for BoolComparison { BinOpKind::Eq => { let true_case = Some((|h| h, "equality checks against true are unnecessary")); let false_case = Some(( - |h: Sugg<'_>| !h, + |h: Sugg<'tcx>| !h, "equality checks against false can be replaced by a negation", )); check_comparison(cx, e, true_case, false_case, true_case, false_case, ignore_no_literal); }, BinOpKind::Ne => { let true_case = Some(( - |h: Sugg<'_>| !h, + |h: Sugg<'tcx>| !h, "inequality checks against true can be replaced by a negation", )); let false_case = Some((|h| h, "inequality checks against false are unnecessary")); @@ -206,12 +206,12 @@ impl<'tcx> LateLintPass<'tcx> for BoolComparison { ignore_case, Some((|h| h, "greater than checks against false are unnecessary")), Some(( - |h: Sugg<'_>| !h, + |h: Sugg<'tcx>| !h, "less than comparison against true can be replaced by a negation", )), ignore_case, Some(( - |l: Sugg<'_>, r: Sugg<'_>| (!l).bit_and(&r), + |l: Sugg<'tcx>, r: Sugg<'tcx>| (!l).bit_and(&r), "order comparisons between booleans can be simplified", )), ), @@ -219,14 +219,14 @@ impl<'tcx> LateLintPass<'tcx> for BoolComparison { cx, e, Some(( - |h: Sugg<'_>| !h, + |h: Sugg<'tcx>| !h, "less than comparison against true can be replaced by a negation", )), ignore_case, ignore_case, Some((|h| h, "greater than checks against false are unnecessary")), Some(( - |l: Sugg<'_>, r: Sugg<'_>| l.bit_and(&(!r)), + |l: Sugg<'tcx>, r: Sugg<'tcx>| l.bit_and(&(!r)), "order comparisons between booleans can be simplified", )), ), diff --git a/clippy_lints/src/neg_multiply.rs b/clippy_lints/src/neg_multiply.rs index cb67fab174005..0d05c83ffe45e 100644 --- a/clippy_lints/src/neg_multiply.rs +++ b/clippy_lints/src/neg_multiply.rs @@ -1,6 +1,8 @@ use clippy_utils::consts::{self, Constant}; -use clippy_utils::diagnostics::span_lint; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_applicability; use if_chain::if_chain; +use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -18,12 +20,16 @@ declare_clippy_lint! { /// /// ### Example /// ```ignore - /// x * -1 + /// // Bad + /// let a = x * -1; + /// + /// // Good + /// let b = -x; /// ``` #[clippy::version = "pre 1.29.0"] pub NEG_MULTIPLY, style, - "multiplying integers with `-1`" + "multiplying integers by `-1`" } declare_lint_pass!(NegMultiply => [NEG_MULTIPLY]); @@ -49,8 +55,19 @@ fn check_mul(cx: &LateContext<'_>, span: Span, lit: &Expr<'_>, exp: &Expr<'_>) { if let ExprKind::Lit(ref l) = lit.kind; if consts::lit_to_constant(&l.node, cx.typeck_results().expr_ty_opt(lit)) == Constant::Int(1); if cx.typeck_results().expr_ty(exp).is_integral(); + then { - span_lint(cx, NEG_MULTIPLY, span, "negation by multiplying with `-1`"); + let mut applicability = Applicability::MachineApplicable; + let suggestion = format!("-{}", snippet_with_applicability(cx, exp.span, "..", &mut applicability)); + span_lint_and_sugg( + cx, + NEG_MULTIPLY, + span, + "this multiplication by -1 can be written more succinctly", + "consider using", + suggestion, + applicability, + ); } } } diff --git a/clippy_lints/src/non_expressive_names.rs b/clippy_lints/src/non_expressive_names.rs index 25fbcb3c6094b..39a37e3e378ed 100644 --- a/clippy_lints/src/non_expressive_names.rs +++ b/clippy_lints/src/non_expressive_names.rs @@ -218,20 +218,20 @@ impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> { return; } for existing_name in &self.0.names { - if allowed_to_be_similar(&interned_name, existing_name.exemptions) { + if allowed_to_be_similar(interned_name, existing_name.exemptions) { continue; } match existing_name.len.cmp(&count) { Ordering::Greater => { if existing_name.len - count != 1 - || levenstein_not_1(&interned_name, existing_name.interned.as_str()) + || levenstein_not_1(interned_name, existing_name.interned.as_str()) { continue; } }, Ordering::Less => { if count - existing_name.len != 1 - || levenstein_not_1(existing_name.interned.as_str(), &interned_name) + || levenstein_not_1(existing_name.interned.as_str(), interned_name) { continue; } @@ -298,7 +298,7 @@ impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> { return; } self.0.names.push(ExistingName { - exemptions: get_exemptions(&interned_name).unwrap_or(&[]), + exemptions: get_exemptions(interned_name).unwrap_or(&[]), interned: ident.name, span: ident.span, len: count, diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index 52c060bc42c7a..c8cbfefb63d65 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -378,8 +378,8 @@ fn check_exclusive_range_plus_one(cx: &LateContext<'_>, expr: &Expr<'_>) { span, "an inclusive range would be more readable", |diag| { - let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string()); - let end = Sugg::hir(cx, y, "y"); + let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").maybe_par().to_string()); + let end = Sugg::hir(cx, y, "y").maybe_par(); if let Some(is_wrapped) = &snippet_opt(cx, span) { if is_wrapped.starts_with('(') && is_wrapped.ends_with(')') { diag.span_suggestion( @@ -415,8 +415,8 @@ fn check_inclusive_range_minus_one(cx: &LateContext<'_>, expr: &Expr<'_>) { expr.span, "an exclusive range would be more readable", |diag| { - let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string()); - let end = Sugg::hir(cx, y, "y"); + let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").maybe_par().to_string()); + let end = Sugg::hir(cx, y, "y").maybe_par(); diag.span_suggestion( expr.span, "use", diff --git a/clippy_lints/src/return_self_not_must_use.rs b/clippy_lints/src/return_self_not_must_use.rs index 1118da6c8cb57..b57ec96bc7e6d 100644 --- a/clippy_lints/src/return_self_not_must_use.rs +++ b/clippy_lints/src/return_self_not_must_use.rs @@ -1,11 +1,12 @@ -use clippy_utils::{diagnostics::span_lint, must_use_attr, nth_arg, return_ty}; +use clippy_utils::ty::is_must_use_ty; +use clippy_utils::{diagnostics::span_lint, nth_arg, return_ty}; use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::FnKind; use rustc_hir::{Body, FnDecl, HirId, TraitItem, TraitItemKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::Span; +use rustc_span::{sym, Span}; declare_clippy_lint! { /// ### What it does @@ -50,9 +51,9 @@ fn check_method(cx: &LateContext<'tcx>, decl: &'tcx FnDecl<'tcx>, fn_def: LocalD if decl.implicit_self.has_implicit_self(); // We only show this warning for public exported methods. if cx.access_levels.is_exported(fn_def); + // We don't want to emit this lint if the `#[must_use]` attribute is already there. + if !cx.tcx.hir().attrs(hir_id).iter().any(|attr| attr.has_name(sym::must_use)); if cx.tcx.visibility(fn_def.to_def_id()).is_public(); - // No need to warn if the attribute is already present. - if must_use_attr(cx.tcx.hir().attrs(hir_id)).is_none(); let ret_ty = return_ty(cx, hir_id); let self_arg = nth_arg(cx, hir_id, 0); // If `Self` has the same type as the returned type, then we want to warn. @@ -60,6 +61,8 @@ fn check_method(cx: &LateContext<'tcx>, decl: &'tcx FnDecl<'tcx>, fn_def: LocalD // For this check, we don't want to remove the reference on the returned type because if // there is one, we shouldn't emit a warning! if self_arg.peel_refs() == ret_ty; + // If `Self` is already marked as `#[must_use]`, no need for the attribute here. + if !is_must_use_ty(cx, ret_ty); then { span_lint( diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index 494bc7dda18e7..112ccdcdd4202 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -73,6 +73,7 @@ declare_clippy_lint! { enum RetReplacement { Empty, Block, + Unit, } declare_lint_pass!(Return => [LET_AND_RETURN, NEEDLESS_RETURN]); @@ -212,7 +213,7 @@ fn check_final_expr<'tcx>( // (except for unit type functions) so we don't match it ExprKind::Match(_, arms, MatchSource::Normal) => { for arm in arms.iter() { - check_final_expr(cx, arm.body, Some(arm.body.span), RetReplacement::Block); + check_final_expr(cx, arm.body, Some(arm.body.span), RetReplacement::Unit); } }, ExprKind::DropTemps(expr) => check_final_expr(cx, expr, None, RetReplacement::Empty), @@ -259,6 +260,17 @@ fn emit_return_lint(cx: &LateContext<'_>, ret_span: Span, inner_span: Option { + span_lint_and_sugg( + cx, + NEEDLESS_RETURN, + ret_span, + "unneeded `return` statement", + "replace `return` with a unit value", + "()".to_string(), + Applicability::MachineApplicable, + ); + }, }, } } diff --git a/clippy_lints/src/shadow.rs b/clippy_lints/src/shadow.rs index f6880af0cab28..ce05c5a6164fd 100644 --- a/clippy_lints/src/shadow.rs +++ b/clippy_lints/src/shadow.rs @@ -5,7 +5,7 @@ use rustc_data_structures::fx::FxHashMap; use rustc_hir::def::Res; use rustc_hir::def_id::LocalDefId; use rustc_hir::hir_id::ItemLocalId; -use rustc_hir::{Block, Body, BodyOwnerKind, Expr, ExprKind, HirId, Node, Pat, PatKind, QPath, UnOp}; +use rustc_hir::{Block, Body, BodyOwnerKind, Expr, ExprKind, HirId, Let, Node, Pat, PatKind, QPath, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::{Span, Symbol}; @@ -220,14 +220,14 @@ fn is_self_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, mut expr: &Expr<'_>, hir_ } } -/// Finds the "init" expression for a pattern: `let = ;` or +/// Finds the "init" expression for a pattern: `let = ;` (or `if let`) or /// `match { .., => .., .. }` fn find_init<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Option<&'tcx Expr<'tcx>> { for (_, node) in cx.tcx.hir().parent_iter(hir_id) { let init = match node { Node::Arm(_) | Node::Pat(_) => continue, Node::Expr(expr) => match expr.kind { - ExprKind::Match(e, _, _) => Some(e), + ExprKind::Match(e, _, _) | ExprKind::Let(&Let { init: e, .. }) => Some(e), _ => None, }, Node::Local(local) => local.init, diff --git a/clippy_lints/src/tabs_in_doc_comments.rs b/clippy_lints/src/tabs_in_doc_comments.rs index c9b4b245f4cc2..15543b6a26277 100644 --- a/clippy_lints/src/tabs_in_doc_comments.rs +++ b/clippy_lints/src/tabs_in_doc_comments.rs @@ -64,7 +64,7 @@ impl TabsInDocComments { if let ast::AttrKind::DocComment(_, comment) = attr.kind { let comment = comment.as_str(); - for (lo, hi) in get_chunks_of_tabs(&comment) { + for (lo, hi) in get_chunks_of_tabs(comment) { // +3 skips the opening delimiter let new_span = Span::new( attr.span.lo() + BytePos(3 + lo), diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs index ccc49caf47c7a..3d3b4a6679dd1 100644 --- a/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -15,7 +15,7 @@ use std::borrow::Cow; declare_clippy_lint! { /// ### What it does - /// Checks for `unsafe` blocks without a `// Safety: ` comment + /// Checks for `unsafe` blocks without a `// SAFETY: ` comment /// explaining why the unsafe operations performed inside /// the block are safe. /// @@ -36,7 +36,7 @@ declare_clippy_lint! { /// use std::ptr::NonNull; /// let a = &mut 42; /// - /// // Safety: references are guaranteed to be non-null. + /// // SAFETY: references are guaranteed to be non-null. /// let ptr = unsafe { NonNull::new_unchecked(a) }; /// ``` #[clippy::version = "1.58.0"] @@ -213,7 +213,7 @@ impl UndocumentedUnsafeBlocks { ); } else { let block_indent = indent_of(cx, span); - let suggestion = format!("// Safety: ...\n{}", snippet(cx, span, "..")); + let suggestion = format!("// SAFETY: ...\n{}", snippet(cx, span, "..")); span_lint_and_sugg( cx, diff --git a/clippy_lints/src/unsafe_removed_from_name.rs b/clippy_lints/src/unsafe_removed_from_name.rs index 44b1989dbc68a..64f7a055cd9bd 100644 --- a/clippy_lints/src/unsafe_removed_from_name.rs +++ b/clippy_lints/src/unsafe_removed_from_name.rs @@ -60,7 +60,7 @@ fn check_use_tree(use_tree: &UseTree, cx: &EarlyContext<'_>, span: Span) { fn unsafe_to_safe_check(old_name: Ident, new_name: Ident, cx: &EarlyContext<'_>, span: Span) { let old_str = old_name.name.as_str(); let new_str = new_name.name.as_str(); - if contains_unsafe(&old_str) && !contains_unsafe(&new_str) { + if contains_unsafe(old_str) && !contains_unsafe(new_str) { span_lint( cx, UNSAFE_REMOVED_FROM_NAME, diff --git a/clippy_lints/src/unwrap.rs b/clippy_lints/src/unwrap.rs index 01a5691223bfc..918fa5f7dc124 100644 --- a/clippy_lints/src/unwrap.rs +++ b/clippy_lints/src/unwrap.rs @@ -161,7 +161,7 @@ fn collect_unwrap_info<'tcx>( if is_relevant_option_call(cx, ty, name) || is_relevant_result_call(cx, ty, name); then { assert!(args.len() == 1); - let unwrappable = match name.as_ref() { + let unwrappable = match name { "is_some" | "is_ok" => true, "is_err" | "is_none" => false, _ => unreachable!(), diff --git a/clippy_lints/src/upper_case_acronyms.rs b/clippy_lints/src/upper_case_acronyms.rs index 0c62161e53d43..7286d0a7bf99b 100644 --- a/clippy_lints/src/upper_case_acronyms.rs +++ b/clippy_lints/src/upper_case_acronyms.rs @@ -87,7 +87,7 @@ fn check_ident(cx: &LateContext<'_>, ident: &Ident, be_aggressive: bool) { if (ident.chars().all(|c| c.is_ascii_uppercase()) && ident.len() > 2) // otherwise, warn if we have SOmeTHING lIKE THIs but only warn with the aggressive // upper-case-acronyms-aggressive config option enabled - || (be_aggressive && ident != &corrected) + || (be_aggressive && ident != corrected) { span_lint_and_sugg( cx, diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 81e9c2e15c97a..e98dcd3cf983b 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -28,7 +28,7 @@ use rustc_middle::ty; use rustc_semver::RustcVersion; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Spanned; -use rustc_span::symbol::{Symbol, SymbolStr}; +use rustc_span::symbol::Symbol; use rustc_span::{sym, BytePos, Span}; use rustc_typeck::hir_ty_to_ty; @@ -344,11 +344,11 @@ impl EarlyLintPass for ClippyLintsInternal { if let ItemKind::Mod(_, ModKind::Loaded(ref items, ..)) = utils.kind { if let Some(paths) = items.iter().find(|item| item.ident.name.as_str() == "paths") { if let ItemKind::Mod(_, ModKind::Loaded(ref items, ..)) = paths.kind { - let mut last_name: Option = None; + let mut last_name: Option<&str> = None; for item in items { let name = item.ident.as_str(); - if let Some(ref last_name) = last_name { - if **last_name > *name { + if let Some(last_name) = last_name { + if *last_name > *name { span_lint( cx, CLIPPY_LINTS_INTERNAL, @@ -608,8 +608,7 @@ impl<'tcx> LateLintPass<'tcx> for OuterExpnDataPass { } let (method_names, arg_lists, spans) = method_calls(expr, 2); - let method_names: Vec = method_names.iter().map(|s| s.as_str()).collect(); - let method_names: Vec<&str> = method_names.iter().map(|s| &**s).collect(); + let method_names: Vec<&str> = method_names.iter().map(Symbol::as_str).collect(); if_chain! { if let ["expn_data", "outer_expn"] = method_names.as_slice(); let args = arg_lists[1]; @@ -839,7 +838,7 @@ impl<'tcx> LateLintPass<'tcx> for MatchTypeOnDiagItem { if is_expr_path_def_path(cx, fn_path, &["clippy_utils", "ty", "match_type"]); // Extract the path to the matched type if let Some(segments) = path_to_matched_type(cx, ty_path); - let segments: Vec<&str> = segments.iter().map(|sym| &**sym).collect(); + let segments: Vec<&str> = segments.iter().map(Symbol::as_str).collect(); if let Some(ty_did) = path_to_res(cx, &segments[..]).opt_def_id(); // Check if the matched type is a diagnostic item if let Some(item_name) = cx.tcx.get_diagnostic_name(ty_did); @@ -862,7 +861,7 @@ impl<'tcx> LateLintPass<'tcx> for MatchTypeOnDiagItem { } } -fn path_to_matched_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option> { +fn path_to_matched_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option> { use rustc_hir::ItemKind; match &expr.kind { @@ -887,12 +886,12 @@ fn path_to_matched_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option {}, }, ExprKind::Array(exprs) => { - let segments: Vec = exprs + let segments: Vec = exprs .iter() .filter_map(|expr| { if let ExprKind::Lit(lit) = &expr.kind { if let LitKind::Str(sym, _) = lit.node { - return Some(sym.as_str()); + return Some(sym); } } @@ -1076,7 +1075,6 @@ impl InterningDefinedSymbol { &paths::SYMBOL_TO_IDENT_STRING, &paths::TO_STRING_METHOD, ]; - // SymbolStr might be de-referenced: `&*symbol.as_str()` let call = if_chain! { if let ExprKind::AddrOf(_, _, e) = expr.kind; if let ExprKind::Unary(UnOp::Deref, e) = e.kind; diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index dc5ec5f229518..5c024612f8eba 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -168,6 +168,14 @@ impl Constant { None } } + + #[must_use] + pub fn peel_refs(mut self) -> Self { + while let Constant::Ref(r) = self { + self = *r; + } + self + } } /// Parses a `LitKind` to a `Constant`. @@ -320,7 +328,7 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { let res = self.typeck_results.qpath_res(qpath, callee.hir_id); if let Some(def_id) = res.opt_def_id(); let def_path = self.lcx.get_def_path(def_id); - let def_path: Vec<&str> = def_path.iter().take(4).map(|s| s.as_str()).collect(); + let def_path: Vec<&str> = def_path.iter().take(4).map(Symbol::as_str).collect(); if let ["core", "num", int_impl, "max_value"] = *def_path; then { let value = match int_impl { diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 7e054a54c3c02..54d470ca73820 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1,8 +1,9 @@ #![feature(box_patterns)] +#![feature(control_flow_enum)] #![feature(in_band_lifetimes)] #![feature(let_else)] +#![feature(once_cell)] #![feature(rustc_private)] -#![feature(control_flow_enum)] #![recursion_limit = "512"] #![cfg_attr(feature = "deny-warnings", deny(warnings))] #![allow(clippy::missing_errors_doc, clippy::missing_panics_doc, clippy::must_use_candidate)] @@ -60,9 +61,12 @@ pub use self::hir_utils::{both, count_eq, eq_expr_value, over, SpanlessEq, Spanl use std::collections::hash_map::Entry; use std::hash::BuildHasherDefault; +use std::lazy::SyncOnceCell; +use std::sync::{Mutex, MutexGuard}; use if_chain::if_chain; use rustc_ast::ast::{self, Attribute, LitKind}; +use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::unhash::UnhashMap; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; @@ -87,6 +91,7 @@ use rustc_middle::ty::binding::BindingMode; use rustc_middle::ty::{layout::IntegerExt, BorrowKind, DefIdTree, Ty, TyCtxt, TypeAndMut, TypeFoldable, UpvarCapture}; use rustc_semver::RustcVersion; use rustc_session::Session; +use rustc_span::def_id::LocalDefId; use rustc_span::hygiene::{ExpnKind, MacroKind}; use rustc_span::source_map::original_sp; use rustc_span::sym; @@ -142,6 +147,13 @@ macro_rules! extract_msrv_attr { }; } +/// Returns `true` if the span comes from a macro expansion, no matter if from a +/// macro by example or from a procedural macro +#[must_use] +pub fn in_macro(span: Span) -> bool { + span.from_expansion() && !matches!(span.ctxt().outer_expn_data().kind, ExpnKind::Desugaring(..)) +} + /// Returns `true` if the two spans come from differing expansions (i.e., one is /// from a macro and one isn't). #[must_use] @@ -156,18 +168,18 @@ pub fn differing_macro_contexts(lhs: Span, rhs: Span) -> bool { /// instead. /// /// Examples: -/// ```ignore +/// ``` /// let abc = 1; /// // ^ output /// let def = abc; -/// dbg!(def) +/// dbg!(def); /// // ^^^ input /// /// // or... /// let abc = 1; /// let def = abc + 2; /// // ^^^^^^^ output -/// dbg!(def) +/// dbg!(def); /// // ^^^ input /// ``` pub fn expr_or_init<'a, 'b, 'tcx: 'b>(cx: &LateContext<'tcx>, mut expr: &'a Expr<'b>) -> &'a Expr<'b> { @@ -664,6 +676,22 @@ fn is_default_equivalent_ctor(cx: &LateContext<'_>, def_id: DefId, path: &QPath< false } +/// Return true if the expr is equal to `Default::default` when evaluated. +pub fn is_default_equivalent_call(cx: &LateContext<'_>, repl_func: &Expr<'_>) -> bool { + if_chain! { + if let hir::ExprKind::Path(ref repl_func_qpath) = repl_func.kind; + if let Some(repl_def_id) = cx.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id(); + if is_diag_trait_item(cx, repl_def_id, sym::Default) + || is_default_equivalent_ctor(cx, repl_def_id, repl_func_qpath); + then { + true + } + else { + false + } + } +} + /// Returns true if the expr is equal to `Default::default()` of it's type when evaluated. /// It doesn't cover all cases, for example indirect function calls (some of std /// functions are supported) but it is the best we have. @@ -686,18 +714,7 @@ pub fn is_default_equivalent(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { false } }, - ExprKind::Call(repl_func, _) => if_chain! { - if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind; - if let Some(repl_def_id) = cx.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id(); - if is_diag_trait_item(cx, repl_def_id, sym::Default) - || is_default_equivalent_ctor(cx, repl_def_id, repl_func_qpath); - then { - true - } - else { - false - } - }, + ExprKind::Call(repl_func, _) => is_default_equivalent_call(cx, repl_func), ExprKind::Path(qpath) => is_lang_ctor(cx, qpath, OptionNone), ExprKind::AddrOf(rustc_hir::BorrowKind::Ref, _, expr) => matches!(expr.kind, ExprKind::Array([])), _ => false, @@ -1136,7 +1153,7 @@ pub fn find_macro_calls(names: &[&str], body: &Body<'_>) -> Vec { /// Extends the span to the beginning of the spans line, incl. whitespaces. /// -/// ```rust,ignore +/// ```rust /// let x = (); /// // ^^ /// // will be converted to @@ -1337,7 +1354,7 @@ pub fn is_adjusted(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { cx.typeck_results().adjustments().get(e.hir_id).is_some() } -/// Returns the pre-expansion span if is this comes from an expansion of the +/// Returns the pre-expansion span if this comes from an expansion of the /// macro `name`. /// See also [`is_direct_expn_of`]. #[must_use] @@ -1364,7 +1381,8 @@ pub fn is_expn_of(mut span: Span, name: &str) -> Option { /// of the macro `name`. /// The difference with [`is_expn_of`] is that in /// ```rust -/// # macro_rules! foo { ($e:tt) => { $e } }; macro_rules! bar { ($e:expr) => { $e } } +/// # macro_rules! foo { ($name:tt!$args:tt) => { $name!$args } } +/// # macro_rules! bar { ($e:expr) => { $e } } /// foo!(bar!(42)); /// ``` /// `42` is considered expanded from `foo!` and `bar!` by `is_expn_of` but only @@ -1905,7 +1923,9 @@ pub fn is_no_core_crate(cx: &LateContext<'_>) -> bool { /// Check if parent of a hir node is a trait implementation block. /// For example, `f` in -/// ```rust,ignore +/// ```rust +/// # struct S; +/// # trait Trait { fn f(); } /// impl Trait for S { /// fn f() {} /// } @@ -2124,17 +2144,16 @@ pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool { false } -struct VisitConstTestStruct<'tcx> { +struct TestItemNamesVisitor<'tcx> { tcx: TyCtxt<'tcx>, names: Vec, - found: bool, } -impl<'hir> ItemLikeVisitor<'hir> for VisitConstTestStruct<'hir> { + +impl<'hir> ItemLikeVisitor<'hir> for TestItemNamesVisitor<'hir> { fn visit_item(&mut self, item: &Item<'_>) { if let ItemKind::Const(ty, _body) = item.kind { if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind { // We could also check for the type name `test::TestDescAndFn` - // and the `#[rustc_test_marker]` attribute? if let Res::Def(DefKind::Struct, _) = path.res { let has_test_marker = self .tcx @@ -2142,8 +2161,8 @@ impl<'hir> ItemLikeVisitor<'hir> for VisitConstTestStruct<'hir> { .attrs(item.hir_id()) .iter() .any(|a| a.has_name(sym::rustc_test_marker)); - if has_test_marker && self.names.contains(&item.ident.name) { - self.found = true; + if has_test_marker { + self.names.push(item.ident.name); } } } @@ -2154,32 +2173,42 @@ impl<'hir> ItemLikeVisitor<'hir> for VisitConstTestStruct<'hir> { fn visit_foreign_item(&mut self, _: &ForeignItem<'_>) {} } +static TEST_ITEM_NAMES_CACHE: SyncOnceCell>>> = SyncOnceCell::new(); + +fn with_test_item_names(tcx: TyCtxt<'tcx>, module: LocalDefId, f: impl Fn(&[Symbol]) -> bool) -> bool { + let cache = TEST_ITEM_NAMES_CACHE.get_or_init(|| Mutex::new(FxHashMap::default())); + let mut map: MutexGuard<'_, FxHashMap>> = cache.lock().unwrap(); + match map.entry(module) { + Entry::Occupied(entry) => f(entry.get()), + Entry::Vacant(entry) => { + let mut visitor = TestItemNamesVisitor { tcx, names: Vec::new() }; + tcx.hir().visit_item_likes_in_module(module, &mut visitor); + visitor.names.sort_unstable(); + f(&*entry.insert(visitor.names)) + }, + } +} + /// Checks if the function containing the given `HirId` is a `#[test]` function /// /// Note: If you use this function, please add a `#[test]` case in `tests/ui_test`. pub fn is_in_test_function(tcx: TyCtxt<'_>, id: hir::HirId) -> bool { - let names: Vec<_> = tcx - .hir() - .parent_iter(id) - // Since you can nest functions we need to collect all until we leave - // function scope - .filter_map(|(_id, node)| { - if let Node::Item(item) = node { - if let ItemKind::Fn(_, _, _) = item.kind { - return Some(item.ident.name); + with_test_item_names(tcx, tcx.parent_module(id), |names| { + tcx.hir() + .parent_iter(id) + // Since you can nest functions we need to collect all until we leave + // function scope + .any(|(_id, node)| { + if let Node::Item(item) = node { + if let ItemKind::Fn(_, _, _) = item.kind { + // Note that we have sorted the item names in the visitor, + // so the binary_search gets the same as `contains`, but faster. + return names.binary_search(&item.ident.name).is_ok(); + } } - } - None - }) - .collect(); - let parent_mod = tcx.parent_module(id); - let mut vis = VisitConstTestStruct { - tcx, - names, - found: false, - }; - tcx.hir().visit_item_likes_in_module(parent_mod, &mut vis); - vis.found + false + }) + }) } /// Checks whether item either has `test` attribute applied, or diff --git a/clippy_utils/src/str_utils.rs b/clippy_utils/src/str_utils.rs index cba96e05a2412..03a9d3c25fd98 100644 --- a/clippy_utils/src/str_utils.rs +++ b/clippy_utils/src/str_utils.rs @@ -15,6 +15,7 @@ impl StrIndex { /// Returns the index of the character after the first camel-case component of `s`. /// /// ``` +/// # use clippy_utils::str_utils::{camel_case_until, StrIndex}; /// assert_eq!(camel_case_until("AbcDef"), StrIndex::new(6, 6)); /// assert_eq!(camel_case_until("ABCD"), StrIndex::new(0, 0)); /// assert_eq!(camel_case_until("AbcDD"), StrIndex::new(3, 3)); @@ -55,9 +56,10 @@ pub fn camel_case_until(s: &str) -> StrIndex { } } -/// Returns index of the last camel-case component of `s`. +/// Returns index of the first camel-case component of `s`. /// /// ``` +/// # use clippy_utils::str_utils::{camel_case_start, StrIndex}; /// assert_eq!(camel_case_start("AbcDef"), StrIndex::new(0, 0)); /// assert_eq!(camel_case_start("abcDef"), StrIndex::new(3, 3)); /// assert_eq!(camel_case_start("ABCD"), StrIndex::new(4, 4)); @@ -66,19 +68,37 @@ pub fn camel_case_until(s: &str) -> StrIndex { /// ``` #[must_use] pub fn camel_case_start(s: &str) -> StrIndex { + camel_case_start_from_idx(s, 0) +} + +/// Returns `StrIndex` of the last camel-case component of `s[idx..]`. +/// +/// ``` +/// # use clippy_utils::str_utils::{camel_case_start_from_idx, StrIndex}; +/// assert_eq!(camel_case_start_from_idx("AbcDef", 0), StrIndex::new(0, 0)); +/// assert_eq!(camel_case_start_from_idx("AbcDef", 1), StrIndex::new(3, 3)); +/// assert_eq!(camel_case_start_from_idx("AbcDefGhi", 0), StrIndex::new(0, 0)); +/// assert_eq!(camel_case_start_from_idx("AbcDefGhi", 1), StrIndex::new(3, 3)); +/// assert_eq!(camel_case_start_from_idx("Abcdefg", 1), StrIndex::new(7, 7)); +/// ``` +pub fn camel_case_start_from_idx(s: &str, start_idx: usize) -> StrIndex { let char_count = s.chars().count(); let range = 0..char_count; let mut iter = range.rev().zip(s.char_indices().rev()); - if let Some((char_index, (_, first))) = iter.next() { + if let Some((_, (_, first))) = iter.next() { if !first.is_lowercase() { - return StrIndex::new(char_index, s.len()); + return StrIndex::new(char_count, s.len()); } } else { return StrIndex::new(char_count, s.len()); } + let mut down = true; let mut last_index = StrIndex::new(char_count, s.len()); for (char_index, (byte_index, c)) in iter { + if byte_index < start_idx { + break; + } if down { if c.is_uppercase() { down = false; @@ -96,9 +116,55 @@ pub fn camel_case_start(s: &str) -> StrIndex { return last_index; } } + last_index } +/// Get the indexes of camel case components of a string `s` +/// +/// ``` +/// # use clippy_utils::str_utils::{camel_case_indices, StrIndex}; +/// assert_eq!( +/// camel_case_indices("AbcDef"), +/// vec![StrIndex::new(0, 0), StrIndex::new(3, 3), StrIndex::new(6, 6)] +/// ); +/// assert_eq!( +/// camel_case_indices("abcDef"), +/// vec![StrIndex::new(3, 3), StrIndex::new(6, 6)] +/// ); +/// ``` +pub fn camel_case_indices(s: &str) -> Vec { + let mut result = Vec::new(); + let mut str_idx = camel_case_start(s); + + while str_idx.byte_index < s.len() { + let next_idx = str_idx.byte_index + 1; + result.push(str_idx); + str_idx = camel_case_start_from_idx(s, next_idx); + } + result.push(str_idx); + + result +} + +/// Split camel case string into a vector of its components +/// +/// ``` +/// # use clippy_utils::str_utils::{camel_case_split, StrIndex}; +/// assert_eq!(camel_case_split("AbcDef"), vec!["Abc", "Def"]); +/// ``` +pub fn camel_case_split(s: &str) -> Vec<&str> { + let mut offsets = camel_case_indices(s) + .iter() + .map(|e| e.byte_index) + .collect::>(); + if offsets[0] != 0 { + offsets.insert(0, 0); + } + + offsets.windows(2).map(|w| &s[w[0]..w[1]]).collect() +} + /// Dealing with sting comparison can be complicated, this struct ensures that both the /// character and byte count are provided for correct indexing. #[derive(Debug, Default, PartialEq, Eq)] @@ -116,6 +182,7 @@ impl StrCount { /// Returns the number of chars that match from the start /// /// ``` +/// # use clippy_utils::str_utils::{count_match_start, StrCount}; /// assert_eq!(count_match_start("hello_mouse", "hello_penguin"), StrCount::new(6, 6)); /// assert_eq!(count_match_start("hello_clippy", "bye_bugs"), StrCount::new(0, 0)); /// assert_eq!(count_match_start("hello_world", "hello_world"), StrCount::new(11, 11)); @@ -141,6 +208,7 @@ pub fn count_match_start(str1: &str, str2: &str) -> StrCount { /// Returns the number of chars and bytes that match from the end /// /// ``` +/// # use clippy_utils::str_utils::{count_match_end, StrCount}; /// assert_eq!(count_match_end("hello_cat", "bye_cat"), StrCount::new(4, 4)); /// assert_eq!(count_match_end("if_item_thing", "enum_value"), StrCount::new(0, 0)); /// assert_eq!(count_match_end("Clippy", "Clippy"), StrCount::new(6, 6)); @@ -227,4 +295,31 @@ mod test { fn until_caps() { assert_eq!(camel_case_until("ABCD"), StrIndex::new(0, 0)); } + + #[test] + fn camel_case_start_from_idx_full() { + assert_eq!(camel_case_start_from_idx("AbcDef", 0), StrIndex::new(0, 0)); + assert_eq!(camel_case_start_from_idx("AbcDef", 1), StrIndex::new(3, 3)); + assert_eq!(camel_case_start_from_idx("AbcDef", 4), StrIndex::new(6, 6)); + assert_eq!(camel_case_start_from_idx("AbcDefGhi", 0), StrIndex::new(0, 0)); + assert_eq!(camel_case_start_from_idx("AbcDefGhi", 1), StrIndex::new(3, 3)); + assert_eq!(camel_case_start_from_idx("Abcdefg", 1), StrIndex::new(7, 7)); + } + + #[test] + fn camel_case_indices_full() { + assert_eq!(camel_case_indices("Abc\u{f6}\u{f6}DD"), vec![StrIndex::new(7, 9)]); + } + + #[test] + fn camel_case_split_full() { + assert_eq!(camel_case_split("A"), vec!["A"]); + assert_eq!(camel_case_split("AbcDef"), vec!["Abc", "Def"]); + assert_eq!(camel_case_split("Abc"), vec!["Abc"]); + assert_eq!(camel_case_split("abcDef"), vec!["abc", "Def"]); + assert_eq!( + camel_case_split("\u{f6}\u{f6}AabABcd"), + vec!["\u{f6}\u{f6}", "Aab", "A", "Bcd"] + ); + } } diff --git a/clippy_utils/src/sugg.rs b/clippy_utils/src/sugg.rs index 586934df46037..92662c59226a2 100644 --- a/clippy_utils/src/sugg.rs +++ b/clippy_utils/src/sugg.rs @@ -1,9 +1,7 @@ //! Contains utility functions to generate suggestions. #![deny(clippy::missing_docs_in_private_items)] -use crate::source::{ - snippet, snippet_opt, snippet_with_applicability, snippet_with_context, snippet_with_macro_callsite, -}; +use crate::source::{snippet, snippet_opt, snippet_with_applicability, snippet_with_macro_callsite}; use crate::{get_parent_expr_for_hir, higher}; use rustc_ast::util::parser::AssocOp; use rustc_ast::{ast, token}; @@ -33,7 +31,7 @@ pub enum Sugg<'a> { MaybeParen(Cow<'a, str>), /// A binary operator expression, including `as`-casts and explicit type /// coercion. - BinOp(AssocOp, Cow<'a, str>), + BinOp(AssocOp, Cow<'a, str>, Cow<'a, str>), } /// Literal constant `0`, for convenience. @@ -46,7 +44,8 @@ pub const EMPTY: Sugg<'static> = Sugg::NonParen(Cow::Borrowed("")); impl Display for Sugg<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { match *self { - Sugg::NonParen(ref s) | Sugg::MaybeParen(ref s) | Sugg::BinOp(_, ref s) => s.fmt(f), + Sugg::NonParen(ref s) | Sugg::MaybeParen(ref s) => s.fmt(f), + Sugg::BinOp(op, ref lhs, ref rhs) => binop_to_string(op, lhs, rhs).fmt(f), } } } @@ -55,10 +54,8 @@ impl Display for Sugg<'_> { impl<'a> Sugg<'a> { /// Prepare a suggestion from an expression. pub fn hir_opt(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option { - snippet_opt(cx, expr.span).map(|snippet| { - let snippet = Cow::Owned(snippet); - Self::hir_from_snippet(expr, snippet) - }) + let get_snippet = |span| snippet(cx, span, ""); + snippet_opt(cx, expr.span).map(|_| Self::hir_from_snippet(expr, get_snippet)) } /// Convenience function around `hir_opt` for suggestions with a default @@ -93,9 +90,8 @@ impl<'a> Sugg<'a> { /// Same as `hir`, but will use the pre expansion span if the `expr` was in a macro. pub fn hir_with_macro_callsite(cx: &LateContext<'_>, expr: &hir::Expr<'_>, default: &'a str) -> Self { - let snippet = snippet_with_macro_callsite(cx, expr.span, default); - - Self::hir_from_snippet(expr, snippet) + let get_snippet = |span| snippet_with_macro_callsite(cx, span, default); + Self::hir_from_snippet(expr, get_snippet) } /// Same as `hir`, but first walks the span up to the given context. This will result in the @@ -112,24 +108,26 @@ impl<'a> Sugg<'a> { default: &'a str, applicability: &mut Applicability, ) -> Self { - let (snippet, in_macro) = snippet_with_context(cx, expr.span, ctxt, default, applicability); - - if in_macro { - Sugg::NonParen(snippet) + if expr.span.ctxt() == ctxt { + Self::hir_from_snippet(expr, |span| snippet(cx, span, default)) } else { - Self::hir_from_snippet(expr, snippet) + let snip = snippet_with_applicability(cx, expr.span, default, applicability); + Sugg::NonParen(snip) } } /// Generate a suggestion for an expression with the given snippet. This is used by the `hir_*` /// function variants of `Sugg`, since these use different snippet functions. - fn hir_from_snippet(expr: &hir::Expr<'_>, snippet: Cow<'a, str>) -> Self { + fn hir_from_snippet(expr: &hir::Expr<'_>, get_snippet: impl Fn(Span) -> Cow<'a, str>) -> Self { if let Some(range) = higher::Range::hir(expr) { let op = match range.limits { ast::RangeLimits::HalfOpen => AssocOp::DotDot, ast::RangeLimits::Closed => AssocOp::DotDotEq, }; - return Sugg::BinOp(op, snippet); + let start = range.start.map_or("".into(), |expr| get_snippet(expr.span)); + let end = range.end.map_or("".into(), |expr| get_snippet(expr.span)); + + return Sugg::BinOp(op, start, end); } match expr.kind { @@ -139,7 +137,7 @@ impl<'a> Sugg<'a> { | hir::ExprKind::Let(..) | hir::ExprKind::Closure(..) | hir::ExprKind::Unary(..) - | hir::ExprKind::Match(..) => Sugg::MaybeParen(snippet), + | hir::ExprKind::Match(..) => Sugg::MaybeParen(get_snippet(expr.span)), hir::ExprKind::Continue(..) | hir::ExprKind::Yield(..) | hir::ExprKind::Array(..) @@ -160,12 +158,20 @@ impl<'a> Sugg<'a> { | hir::ExprKind::Struct(..) | hir::ExprKind::Tup(..) | hir::ExprKind::DropTemps(_) - | hir::ExprKind::Err => Sugg::NonParen(snippet), - hir::ExprKind::Assign(..) => Sugg::BinOp(AssocOp::Assign, snippet), - hir::ExprKind::AssignOp(op, ..) => Sugg::BinOp(hirbinop2assignop(op), snippet), - hir::ExprKind::Binary(op, ..) => Sugg::BinOp(AssocOp::from_ast_binop(op.node.into()), snippet), - hir::ExprKind::Cast(..) => Sugg::BinOp(AssocOp::As, snippet), - hir::ExprKind::Type(..) => Sugg::BinOp(AssocOp::Colon, snippet), + | hir::ExprKind::Err => Sugg::NonParen(get_snippet(expr.span)), + hir::ExprKind::Assign(lhs, rhs, _) => { + Sugg::BinOp(AssocOp::Assign, get_snippet(lhs.span), get_snippet(rhs.span)) + }, + hir::ExprKind::AssignOp(op, lhs, rhs) => { + Sugg::BinOp(hirbinop2assignop(op), get_snippet(lhs.span), get_snippet(rhs.span)) + }, + hir::ExprKind::Binary(op, lhs, rhs) => Sugg::BinOp( + AssocOp::from_ast_binop(op.node.into()), + get_snippet(lhs.span), + get_snippet(rhs.span), + ), + hir::ExprKind::Cast(lhs, ty) => Sugg::BinOp(AssocOp::As, get_snippet(lhs.span), get_snippet(ty.span)), + hir::ExprKind::Type(lhs, ty) => Sugg::BinOp(AssocOp::Colon, get_snippet(lhs.span), get_snippet(ty.span)), } } @@ -173,10 +179,12 @@ impl<'a> Sugg<'a> { pub fn ast(cx: &EarlyContext<'_>, expr: &ast::Expr, default: &'a str) -> Self { use rustc_ast::ast::RangeLimits; - let snippet = if expr.span.from_expansion() { - snippet_with_macro_callsite(cx, expr.span, default) - } else { - snippet(cx, expr.span, default) + let get_whole_snippet = || { + if expr.span.from_expansion() { + snippet_with_macro_callsite(cx, expr.span, default) + } else { + snippet(cx, expr.span, default) + } }; match expr.kind { @@ -186,7 +194,7 @@ impl<'a> Sugg<'a> { | ast::ExprKind::If(..) | ast::ExprKind::Let(..) | ast::ExprKind::Unary(..) - | ast::ExprKind::Match(..) => Sugg::MaybeParen(snippet), + | ast::ExprKind::Match(..) => Sugg::MaybeParen(get_whole_snippet()), ast::ExprKind::Async(..) | ast::ExprKind::Block(..) | ast::ExprKind::Break(..) @@ -215,14 +223,42 @@ impl<'a> Sugg<'a> { | ast::ExprKind::Array(..) | ast::ExprKind::While(..) | ast::ExprKind::Await(..) - | ast::ExprKind::Err => Sugg::NonParen(snippet), - ast::ExprKind::Range(.., RangeLimits::HalfOpen) => Sugg::BinOp(AssocOp::DotDot, snippet), - ast::ExprKind::Range(.., RangeLimits::Closed) => Sugg::BinOp(AssocOp::DotDotEq, snippet), - ast::ExprKind::Assign(..) => Sugg::BinOp(AssocOp::Assign, snippet), - ast::ExprKind::AssignOp(op, ..) => Sugg::BinOp(astbinop2assignop(op), snippet), - ast::ExprKind::Binary(op, ..) => Sugg::BinOp(AssocOp::from_ast_binop(op.node), snippet), - ast::ExprKind::Cast(..) => Sugg::BinOp(AssocOp::As, snippet), - ast::ExprKind::Type(..) => Sugg::BinOp(AssocOp::Colon, snippet), + | ast::ExprKind::Err => Sugg::NonParen(get_whole_snippet()), + ast::ExprKind::Range(ref lhs, ref rhs, RangeLimits::HalfOpen) => Sugg::BinOp( + AssocOp::DotDot, + lhs.as_ref().map_or("".into(), |lhs| snippet(cx, lhs.span, default)), + rhs.as_ref().map_or("".into(), |rhs| snippet(cx, rhs.span, default)), + ), + ast::ExprKind::Range(ref lhs, ref rhs, RangeLimits::Closed) => Sugg::BinOp( + AssocOp::DotDotEq, + lhs.as_ref().map_or("".into(), |lhs| snippet(cx, lhs.span, default)), + rhs.as_ref().map_or("".into(), |rhs| snippet(cx, rhs.span, default)), + ), + ast::ExprKind::Assign(ref lhs, ref rhs, _) => Sugg::BinOp( + AssocOp::Assign, + snippet(cx, lhs.span, default), + snippet(cx, rhs.span, default), + ), + ast::ExprKind::AssignOp(op, ref lhs, ref rhs) => Sugg::BinOp( + astbinop2assignop(op), + snippet(cx, lhs.span, default), + snippet(cx, rhs.span, default), + ), + ast::ExprKind::Binary(op, ref lhs, ref rhs) => Sugg::BinOp( + AssocOp::from_ast_binop(op.node), + snippet(cx, lhs.span, default), + snippet(cx, rhs.span, default), + ), + ast::ExprKind::Cast(ref lhs, ref ty) => Sugg::BinOp( + AssocOp::As, + snippet(cx, lhs.span, default), + snippet(cx, ty.span, default), + ), + ast::ExprKind::Type(ref lhs, ref ty) => Sugg::BinOp( + AssocOp::Colon, + snippet(cx, lhs.span, default), + snippet(cx, ty.span, default), + ), } } @@ -306,17 +342,51 @@ impl<'a> Sugg<'a> { Sugg::NonParen(format!("({})", sugg).into()) } }, - Sugg::BinOp(_, sugg) => { - if has_enclosing_paren(&sugg) { - Sugg::NonParen(sugg) - } else { - Sugg::NonParen(format!("({})", sugg).into()) - } + Sugg::BinOp(op, lhs, rhs) => { + let sugg = binop_to_string(op, &lhs, &rhs); + Sugg::NonParen(format!("({})", sugg).into()) }, } } } +/// Generates a string from the operator and both sides. +fn binop_to_string(op: AssocOp, lhs: &str, rhs: &str) -> String { + match op { + AssocOp::Add + | AssocOp::Subtract + | AssocOp::Multiply + | AssocOp::Divide + | AssocOp::Modulus + | AssocOp::LAnd + | AssocOp::LOr + | AssocOp::BitXor + | AssocOp::BitAnd + | AssocOp::BitOr + | AssocOp::ShiftLeft + | AssocOp::ShiftRight + | AssocOp::Equal + | AssocOp::Less + | AssocOp::LessEqual + | AssocOp::NotEqual + | AssocOp::Greater + | AssocOp::GreaterEqual => format!( + "{} {} {}", + lhs, + op.to_ast_binop().expect("Those are AST ops").to_string(), + rhs + ), + AssocOp::Assign => format!("{} = {}", lhs, rhs), + AssocOp::AssignOp(op) => { + format!("{} {}= {}", lhs, token_kind_to_string(&token::BinOp(op)), rhs) + }, + AssocOp::As => format!("{} as {}", lhs, rhs), + AssocOp::DotDot => format!("{}..{}", lhs, rhs), + AssocOp::DotDotEq => format!("{}..={}", lhs, rhs), + AssocOp::Colon => format!("{}: {}", lhs, rhs), + } +} + /// Return `true` if `sugg` is enclosed in parenthesis. fn has_enclosing_paren(sugg: impl AsRef) -> bool { let mut chars = sugg.as_ref().chars(); @@ -391,10 +461,25 @@ impl Neg for Sugg<'_> { } } -impl Not for Sugg<'_> { - type Output = Sugg<'static>; - fn not(self) -> Sugg<'static> { - make_unop("!", self) +impl Not for Sugg<'a> { + type Output = Sugg<'a>; + fn not(self) -> Sugg<'a> { + use AssocOp::{Equal, Greater, GreaterEqual, Less, LessEqual, NotEqual}; + + if let Sugg::BinOp(op, lhs, rhs) = self { + let to_op = match op { + Equal => NotEqual, + NotEqual => Equal, + Less => GreaterEqual, + GreaterEqual => Less, + Greater => LessEqual, + LessEqual => Greater, + _ => return make_unop("!", Sugg::BinOp(op, lhs, rhs)), + }; + Sugg::BinOp(to_op, lhs, rhs) + } else { + make_unop("!", self) + } } } @@ -463,53 +548,21 @@ pub fn make_assoc(op: AssocOp, lhs: &Sugg<'_>, rhs: &Sugg<'_>) -> Sugg<'static> || is_shift(other) && is_arith(op) } - let lhs_paren = if let Sugg::BinOp(lop, _) = *lhs { + let lhs_paren = if let Sugg::BinOp(lop, _, _) = *lhs { needs_paren(op, lop, Associativity::Left) } else { false }; - let rhs_paren = if let Sugg::BinOp(rop, _) = *rhs { + let rhs_paren = if let Sugg::BinOp(rop, _, _) = *rhs { needs_paren(op, rop, Associativity::Right) } else { false }; - let lhs = ParenHelper::new(lhs_paren, lhs); - let rhs = ParenHelper::new(rhs_paren, rhs); - let sugg = match op { - AssocOp::Add - | AssocOp::BitAnd - | AssocOp::BitOr - | AssocOp::BitXor - | AssocOp::Divide - | AssocOp::Equal - | AssocOp::Greater - | AssocOp::GreaterEqual - | AssocOp::LAnd - | AssocOp::LOr - | AssocOp::Less - | AssocOp::LessEqual - | AssocOp::Modulus - | AssocOp::Multiply - | AssocOp::NotEqual - | AssocOp::ShiftLeft - | AssocOp::ShiftRight - | AssocOp::Subtract => format!( - "{} {} {}", - lhs, - op.to_ast_binop().expect("Those are AST ops").to_string(), - rhs - ), - AssocOp::Assign => format!("{} = {}", lhs, rhs), - AssocOp::AssignOp(op) => format!("{} {}= {}", lhs, token_kind_to_string(&token::BinOp(op)), rhs), - AssocOp::As => format!("{} as {}", lhs, rhs), - AssocOp::DotDot => format!("{}..{}", lhs, rhs), - AssocOp::DotDotEq => format!("{}..={}", lhs, rhs), - AssocOp::Colon => format!("{}: {}", lhs, rhs), - }; - - Sugg::BinOp(op, sugg.into()) + let lhs = ParenHelper::new(lhs_paren, lhs).to_string(); + let rhs = ParenHelper::new(rhs_paren, rhs).to_string(); + Sugg::BinOp(op, lhs.into(), rhs.into()) } /// Convenience wrapper around `make_assoc` and `AssocOp::from_ast_binop`. @@ -1007,10 +1060,32 @@ mod test { #[test] fn binop_maybe_par() { - let sugg = Sugg::BinOp(AssocOp::Add, "(1 + 1)".into()); + let sugg = Sugg::BinOp(AssocOp::Add, "1".into(), "1".into()); assert_eq!("(1 + 1)", sugg.maybe_par().to_string()); - let sugg = Sugg::BinOp(AssocOp::Add, "(1 + 1) + (1 + 1)".into()); + let sugg = Sugg::BinOp(AssocOp::Add, "(1 + 1)".into(), "(1 + 1)".into()); assert_eq!("((1 + 1) + (1 + 1))", sugg.maybe_par().to_string()); } + #[test] + fn not_op() { + use AssocOp::{Add, Equal, Greater, GreaterEqual, LAnd, LOr, Less, LessEqual, NotEqual}; + + fn test_not(op: AssocOp, correct: &str) { + let sugg = Sugg::BinOp(op, "x".into(), "y".into()); + assert_eq!((!sugg).to_string(), correct); + } + + // Invert the comparison operator. + test_not(Equal, "x != y"); + test_not(NotEqual, "x == y"); + test_not(Less, "x >= y"); + test_not(LessEqual, "x > y"); + test_not(Greater, "x <= y"); + test_not(GreaterEqual, "x < y"); + + // Other operators are inverted like !(..). + test_not(Add, "!(x + y)"); + test_not(LAnd, "!(x && y)"); + test_not(LOr, "!(x || y)"); + } } diff --git a/rust-toolchain b/rust-toolchain index 3a5dfa6a8c7d8..471ae40f1ac7a 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2021-12-17" +channel = "nightly-2021-12-30" components = ["cargo", "llvm-tools-preview", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"] diff --git a/tests/ui/crashes/ice-7868.stderr b/tests/ui/crashes/ice-7868.stderr index d7b49eb89a28b..111350a6280dc 100644 --- a/tests/ui/crashes/ice-7868.stderr +++ b/tests/ui/crashes/ice-7868.stderr @@ -7,7 +7,7 @@ LL | unsafe { 0 }; = note: `-D clippy::undocumented-unsafe-blocks` implied by `-D warnings` help: consider adding a safety comment | -LL ~ // Safety: ... +LL ~ // SAFETY: ... LL ~ unsafe { 0 }; | diff --git a/tests/ui/enum_variants.rs b/tests/ui/enum_variants.rs index 083f5143e6e4d..d3662a0a213d2 100644 --- a/tests/ui/enum_variants.rs +++ b/tests/ui/enum_variants.rs @@ -145,4 +145,10 @@ enum HIDataRequest { DeleteUnpubHIData(String), } +enum North { + Normal, + NoLeft, + NoRight, +} + fn main() {} diff --git a/tests/ui/enum_variants.stderr b/tests/ui/enum_variants.stderr index add8a91e26b85..8a3265086e84f 100644 --- a/tests/ui/enum_variants.stderr +++ b/tests/ui/enum_variants.stderr @@ -6,6 +6,18 @@ LL | cFoo, | = note: `-D clippy::enum-variant-names` implied by `-D warnings` +error: all variants have the same prefix: `c` + --> $DIR/enum_variants.rs:14:1 + | +LL | / enum Foo { +LL | | cFoo, +LL | | cBar, +LL | | cBaz, +LL | | } + | |_^ + | + = help: remove the prefixes and use full paths to the variants instead of glob imports + error: variant name starts with the enum's name --> $DIR/enum_variants.rs:26:5 | @@ -60,25 +72,25 @@ LL | | } | = help: remove the prefixes and use full paths to the variants instead of glob imports -error: all variants have the same prefix: `WithOut` - --> $DIR/enum_variants.rs:81:1 +error: all variants have the same prefix: `C` + --> $DIR/enum_variants.rs:59:1 | -LL | / enum Seallll { -LL | | WithOutCake, -LL | | WithOutTea, -LL | | WithOut, +LL | / enum Something { +LL | | CCall, +LL | | CCreate, +LL | | CCryogenize, LL | | } | |_^ | = help: remove the prefixes and use full paths to the variants instead of glob imports -error: all variants have the same prefix: `Prefix` - --> $DIR/enum_variants.rs:87:1 +error: all variants have the same prefix: `WithOut` + --> $DIR/enum_variants.rs:81:1 | -LL | / enum NonCaps { -LL | | Prefix的, -LL | | PrefixTea, -LL | | PrefixCake, +LL | / enum Seallll { +LL | | WithOutCake, +LL | | WithOutTea, +LL | | WithOut, LL | | } | |_^ | @@ -108,5 +120,5 @@ LL | | } | = help: remove the postfixes and use full paths to the variants instead of glob imports -error: aborting due to 11 previous errors +error: aborting due to 12 previous errors diff --git a/tests/ui/floating_point_rad.fixed b/tests/ui/floating_point_rad.fixed index a35bb1c27f35c..ce91fe176c6fc 100644 --- a/tests/ui/floating_point_rad.fixed +++ b/tests/ui/floating_point_rad.fixed @@ -11,7 +11,12 @@ pub const fn const_context() { fn main() { let x = 3f32; let _ = x.to_degrees(); + let _ = 90.0_f64.to_degrees(); + let _ = 90.5_f64.to_degrees(); let _ = x.to_radians(); + let _ = 90.0_f64.to_radians(); + let _ = 90.5_f64.to_radians(); + // let _ = 90.5 * 80. * std::f32::consts::PI / 180f32; // Cases where the lint shouldn't be applied let _ = x * 90f32 / std::f32::consts::PI; let _ = x * std::f32::consts::PI / 90f32; diff --git a/tests/ui/floating_point_rad.rs b/tests/ui/floating_point_rad.rs index 834db4be533c0..8f3234986148b 100644 --- a/tests/ui/floating_point_rad.rs +++ b/tests/ui/floating_point_rad.rs @@ -11,7 +11,12 @@ pub const fn const_context() { fn main() { let x = 3f32; let _ = x * 180f32 / std::f32::consts::PI; + let _ = 90. * 180f64 / std::f64::consts::PI; + let _ = 90.5 * 180f64 / std::f64::consts::PI; let _ = x * std::f32::consts::PI / 180f32; + let _ = 90. * std::f32::consts::PI / 180f32; + let _ = 90.5 * std::f32::consts::PI / 180f32; + // let _ = 90.5 * 80. * std::f32::consts::PI / 180f32; // Cases where the lint shouldn't be applied let _ = x * 90f32 / std::f32::consts::PI; let _ = x * std::f32::consts::PI / 90f32; diff --git a/tests/ui/floating_point_rad.stderr b/tests/ui/floating_point_rad.stderr index acecddbca53bf..f12d3d23f3ab9 100644 --- a/tests/ui/floating_point_rad.stderr +++ b/tests/ui/floating_point_rad.stderr @@ -6,11 +6,35 @@ LL | let _ = x * 180f32 / std::f32::consts::PI; | = note: `-D clippy::suboptimal-flops` implied by `-D warnings` -error: conversion to radians can be done more accurately +error: conversion to degrees can be done more accurately --> $DIR/floating_point_rad.rs:14:13 | +LL | let _ = 90. * 180f64 / std::f64::consts::PI; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `90.0_f64.to_degrees()` + +error: conversion to degrees can be done more accurately + --> $DIR/floating_point_rad.rs:15:13 + | +LL | let _ = 90.5 * 180f64 / std::f64::consts::PI; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `90.5_f64.to_degrees()` + +error: conversion to radians can be done more accurately + --> $DIR/floating_point_rad.rs:16:13 + | LL | let _ = x * std::f32::consts::PI / 180f32; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.to_radians()` -error: aborting due to 2 previous errors +error: conversion to radians can be done more accurately + --> $DIR/floating_point_rad.rs:17:13 + | +LL | let _ = 90. * std::f32::consts::PI / 180f32; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `90.0_f64.to_radians()` + +error: conversion to radians can be done more accurately + --> $DIR/floating_point_rad.rs:18:13 + | +LL | let _ = 90.5 * std::f32::consts::PI / 180f32; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `90.5_f64.to_radians()` + +error: aborting due to 6 previous errors diff --git a/tests/ui/identity_op.rs b/tests/ui/identity_op.rs index ceaacaaf6bd70..2ed4b5db574d4 100644 --- a/tests/ui/identity_op.rs +++ b/tests/ui/identity_op.rs @@ -2,10 +2,20 @@ const ONE: i64 = 1; const NEG_ONE: i64 = -1; const ZERO: i64 = 0; +struct A(String); + +impl std::ops::Shl for A { + type Output = A; + fn shl(mut self, other: i32) -> Self { + self.0.push_str(&format!("{}", other)); + self + } +} #[allow( clippy::eq_op, clippy::no_effect, clippy::unnecessary_operation, + clippy::op_ref, clippy::double_parens )] #[warn(clippy::identity_op)] @@ -38,4 +48,9 @@ fn main() { 42 << 0; 1 >> 0; 42 >> 0; + &x >> 0; + x >> &0; + + let mut a = A("".into()); + let b = a << 0; // no error: non-integer } diff --git a/tests/ui/identity_op.stderr b/tests/ui/identity_op.stderr index d8d44a74f9ab0..ff34b38db015b 100644 --- a/tests/ui/identity_op.stderr +++ b/tests/ui/identity_op.stderr @@ -1,5 +1,5 @@ error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:16:5 + --> $DIR/identity_op.rs:26:5 | LL | x + 0; | ^^^^^ @@ -7,64 +7,76 @@ LL | x + 0; = note: `-D clippy::identity-op` implied by `-D warnings` error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:17:5 + --> $DIR/identity_op.rs:27:5 | LL | x + (1 - 1); | ^^^^^^^^^^^ error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:19:5 + --> $DIR/identity_op.rs:29:5 | LL | 0 + x; | ^^^^^ error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:22:5 + --> $DIR/identity_op.rs:32:5 | LL | x | (0); | ^^^^^^^ error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:25:5 + --> $DIR/identity_op.rs:35:5 | LL | x * 1; | ^^^^^ error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:26:5 + --> $DIR/identity_op.rs:36:5 | LL | 1 * x; | ^^^^^ error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:32:5 + --> $DIR/identity_op.rs:42:5 | LL | -1 & x; | ^^^^^^ error: the operation is ineffective. Consider reducing it to `u` - --> $DIR/identity_op.rs:35:5 + --> $DIR/identity_op.rs:45:5 | LL | u & 255; | ^^^^^^^ error: the operation is ineffective. Consider reducing it to `42` - --> $DIR/identity_op.rs:38:5 + --> $DIR/identity_op.rs:48:5 | LL | 42 << 0; | ^^^^^^^ error: the operation is ineffective. Consider reducing it to `1` - --> $DIR/identity_op.rs:39:5 + --> $DIR/identity_op.rs:49:5 | LL | 1 >> 0; | ^^^^^^ error: the operation is ineffective. Consider reducing it to `42` - --> $DIR/identity_op.rs:40:5 + --> $DIR/identity_op.rs:50:5 | LL | 42 >> 0; | ^^^^^^^ -error: aborting due to 11 previous errors +error: the operation is ineffective. Consider reducing it to `&x` + --> $DIR/identity_op.rs:51:5 + | +LL | &x >> 0; + | ^^^^^^^ + +error: the operation is ineffective. Consider reducing it to `x` + --> $DIR/identity_op.rs:52:5 + | +LL | x >> &0; + | ^^^^^^^ + +error: aborting due to 13 previous errors diff --git a/tests/ui/iter_skip_next.fixed b/tests/ui/iter_skip_next.fixed index 928b6acb95101..2db4c2bee7f2b 100644 --- a/tests/ui/iter_skip_next.fixed +++ b/tests/ui/iter_skip_next.fixed @@ -4,6 +4,7 @@ #![warn(clippy::iter_skip_next)] #![allow(clippy::blacklisted_name)] #![allow(clippy::iter_nth)] +#![allow(unused_mut, dead_code)] extern crate option_helpers; @@ -19,4 +20,18 @@ fn main() { let foo = IteratorFalsePositives { foo: 0 }; let _ = foo.skip(42).next(); let _ = foo.filter().skip(42).next(); + + // fix #8128 + let test_string = "1|1 2"; + let mut sp = test_string.split('|').map(|s| s.trim()); + let _: Vec<&str> = sp.nth(1).unwrap().split(' ').collect(); + if let Some(mut s) = Some(test_string.split('|').map(|s| s.trim())) { + let _: Vec<&str> = s.nth(1).unwrap().split(' ').collect(); + }; + fn check(mut s: T) + where + T: Iterator, + { + let _: Vec<&str> = s.nth(1).unwrap().split(' ').collect(); + } } diff --git a/tests/ui/iter_skip_next.rs b/tests/ui/iter_skip_next.rs index 7075e2598ebee..692edb9aed939 100644 --- a/tests/ui/iter_skip_next.rs +++ b/tests/ui/iter_skip_next.rs @@ -4,6 +4,7 @@ #![warn(clippy::iter_skip_next)] #![allow(clippy::blacklisted_name)] #![allow(clippy::iter_nth)] +#![allow(unused_mut, dead_code)] extern crate option_helpers; @@ -19,4 +20,18 @@ fn main() { let foo = IteratorFalsePositives { foo: 0 }; let _ = foo.skip(42).next(); let _ = foo.filter().skip(42).next(); + + // fix #8128 + let test_string = "1|1 2"; + let mut sp = test_string.split('|').map(|s| s.trim()); + let _: Vec<&str> = sp.skip(1).next().unwrap().split(' ').collect(); + if let Some(mut s) = Some(test_string.split('|').map(|s| s.trim())) { + let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect(); + }; + fn check(mut s: T) + where + T: Iterator, + { + let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect(); + } } diff --git a/tests/ui/iter_skip_next.stderr b/tests/ui/iter_skip_next.stderr index 486de718bb563..ca6970b27f16c 100644 --- a/tests/ui/iter_skip_next.stderr +++ b/tests/ui/iter_skip_next.stderr @@ -1,5 +1,5 @@ error: called `skip(..).next()` on an iterator - --> $DIR/iter_skip_next.rs:15:28 + --> $DIR/iter_skip_next.rs:16:28 | LL | let _ = some_vec.iter().skip(42).next(); | ^^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(42)` @@ -7,22 +7,40 @@ LL | let _ = some_vec.iter().skip(42).next(); = note: `-D clippy::iter-skip-next` implied by `-D warnings` error: called `skip(..).next()` on an iterator - --> $DIR/iter_skip_next.rs:16:36 + --> $DIR/iter_skip_next.rs:17:36 | LL | let _ = some_vec.iter().cycle().skip(42).next(); | ^^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(42)` error: called `skip(..).next()` on an iterator - --> $DIR/iter_skip_next.rs:17:20 + --> $DIR/iter_skip_next.rs:18:20 | LL | let _ = (1..10).skip(10).next(); | ^^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(10)` error: called `skip(..).next()` on an iterator - --> $DIR/iter_skip_next.rs:18:33 + --> $DIR/iter_skip_next.rs:19:33 | LL | let _ = &some_vec[..].iter().skip(3).next(); | ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(3)` -error: aborting due to 4 previous errors +error: called `skip(..).next()` on an iterator + --> $DIR/iter_skip_next.rs:27:26 + | +LL | let _: Vec<&str> = sp.skip(1).next().unwrap().split(' ').collect(); + | ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)` + +error: called `skip(..).next()` on an iterator + --> $DIR/iter_skip_next.rs:29:29 + | +LL | let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect(); + | ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)` + +error: called `skip(..).next()` on an iterator + --> $DIR/iter_skip_next.rs:35:29 + | +LL | let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect(); + | ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)` + +error: aborting due to 7 previous errors diff --git a/tests/ui/iter_skip_next_unfixable.rs b/tests/ui/iter_skip_next_unfixable.rs new file mode 100644 index 0000000000000..3607330cfa0d3 --- /dev/null +++ b/tests/ui/iter_skip_next_unfixable.rs @@ -0,0 +1,19 @@ +#![warn(clippy::iter_skip_next)] +#![allow(dead_code)] + +/// Checks implementation of `ITER_SKIP_NEXT` lint +fn main() { + // fix #8128 + let test_string = "1|1 2"; + let sp = test_string.split('|').map(|s| s.trim()); + let _: Vec<&str> = sp.skip(1).next().unwrap().split(' ').collect(); + if let Some(s) = Some(test_string.split('|').map(|s| s.trim())) { + let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect(); + }; + fn check(s: T) + where + T: Iterator, + { + let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect(); + } +} diff --git a/tests/ui/iter_skip_next_unfixable.stderr b/tests/ui/iter_skip_next_unfixable.stderr new file mode 100644 index 0000000000000..74c327c748361 --- /dev/null +++ b/tests/ui/iter_skip_next_unfixable.stderr @@ -0,0 +1,39 @@ +error: called `skip(..).next()` on an iterator + --> $DIR/iter_skip_next_unfixable.rs:9:26 + | +LL | let _: Vec<&str> = sp.skip(1).next().unwrap().split(' ').collect(); + | ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)` + | + = note: `-D clippy::iter-skip-next` implied by `-D warnings` +help: for this change `sp` has to be mutable + --> $DIR/iter_skip_next_unfixable.rs:8:9 + | +LL | let sp = test_string.split('|').map(|s| s.trim()); + | ^^ + +error: called `skip(..).next()` on an iterator + --> $DIR/iter_skip_next_unfixable.rs:11:29 + | +LL | let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect(); + | ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)` + | +help: for this change `s` has to be mutable + --> $DIR/iter_skip_next_unfixable.rs:10:17 + | +LL | if let Some(s) = Some(test_string.split('|').map(|s| s.trim())) { + | ^ + +error: called `skip(..).next()` on an iterator + --> $DIR/iter_skip_next_unfixable.rs:17:29 + | +LL | let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect(); + | ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)` + | +help: for this change `s` has to be mutable + --> $DIR/iter_skip_next_unfixable.rs:13:17 + | +LL | fn check(s: T) + | ^ + +error: aborting due to 3 previous errors + diff --git a/tests/ui/manual_memcpy/with_loop_counters.stderr b/tests/ui/manual_memcpy/with_loop_counters.stderr index 0243158dec507..2e3ebadd7b5d2 100644 --- a/tests/ui/manual_memcpy/with_loop_counters.stderr +++ b/tests/ui/manual_memcpy/with_loop_counters.stderr @@ -43,7 +43,7 @@ LL | / for i in 3..(3 + src.len()) { LL | | dst[i] = src[count]; LL | | count += 1; LL | | } - | |_____^ help: try replacing the loop by: `dst[3..(3 + src.len())].clone_from_slice(&src[..((3 + src.len()) - 3)]);` + | |_____^ help: try replacing the loop by: `dst[3..(3 + src.len())].clone_from_slice(&src[..(3 + src.len() - 3)]);` error: it looks like you're manually copying between slices --> $DIR/with_loop_counters.rs:35:5 diff --git a/tests/ui/needless_bool/fixable.fixed b/tests/ui/needless_bool/fixable.fixed index 85da1f4e10437..89dc13fd5b13d 100644 --- a/tests/ui/needless_bool/fixable.fixed +++ b/tests/ui/needless_bool/fixable.fixed @@ -41,6 +41,15 @@ fn main() { x; !x; !(x && y); + let a = 0; + let b = 1; + + a != b; + a == b; + a >= b; + a > b; + a <= b; + a < b; if x { x } else { diff --git a/tests/ui/needless_bool/fixable.rs b/tests/ui/needless_bool/fixable.rs index add606302511b..c11d9472e8d06 100644 --- a/tests/ui/needless_bool/fixable.rs +++ b/tests/ui/needless_bool/fixable.rs @@ -53,6 +53,39 @@ fn main() { } else { true }; + let a = 0; + let b = 1; + + if a == b { + false + } else { + true + }; + if a != b { + false + } else { + true + }; + if a < b { + false + } else { + true + }; + if a <= b { + false + } else { + true + }; + if a > b { + false + } else { + true + }; + if a >= b { + false + } else { + true + }; if x { x } else { diff --git a/tests/ui/needless_bool/fixable.stderr b/tests/ui/needless_bool/fixable.stderr index 22c0a7bb491c6..d2c48376f7662 100644 --- a/tests/ui/needless_bool/fixable.stderr +++ b/tests/ui/needless_bool/fixable.stderr @@ -31,7 +31,67 @@ LL | | }; | |_____^ help: you can reduce it to: `!(x && y)` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:72:5 + --> $DIR/fixable.rs:59:5 + | +LL | / if a == b { +LL | | false +LL | | } else { +LL | | true +LL | | }; + | |_____^ help: you can reduce it to: `a != b` + +error: this if-then-else expression returns a bool literal + --> $DIR/fixable.rs:64:5 + | +LL | / if a != b { +LL | | false +LL | | } else { +LL | | true +LL | | }; + | |_____^ help: you can reduce it to: `a == b` + +error: this if-then-else expression returns a bool literal + --> $DIR/fixable.rs:69:5 + | +LL | / if a < b { +LL | | false +LL | | } else { +LL | | true +LL | | }; + | |_____^ help: you can reduce it to: `a >= b` + +error: this if-then-else expression returns a bool literal + --> $DIR/fixable.rs:74:5 + | +LL | / if a <= b { +LL | | false +LL | | } else { +LL | | true +LL | | }; + | |_____^ help: you can reduce it to: `a > b` + +error: this if-then-else expression returns a bool literal + --> $DIR/fixable.rs:79:5 + | +LL | / if a > b { +LL | | false +LL | | } else { +LL | | true +LL | | }; + | |_____^ help: you can reduce it to: `a <= b` + +error: this if-then-else expression returns a bool literal + --> $DIR/fixable.rs:84:5 + | +LL | / if a >= b { +LL | | false +LL | | } else { +LL | | true +LL | | }; + | |_____^ help: you can reduce it to: `a < b` + +error: this if-then-else expression returns a bool literal + --> $DIR/fixable.rs:105:5 | LL | / if x { LL | | return true; @@ -41,7 +101,7 @@ LL | | }; | |_____^ help: you can reduce it to: `return x` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:80:5 + --> $DIR/fixable.rs:113:5 | LL | / if x { LL | | return false; @@ -51,7 +111,7 @@ LL | | }; | |_____^ help: you can reduce it to: `return !x` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:88:5 + --> $DIR/fixable.rs:121:5 | LL | / if x && y { LL | | return true; @@ -61,7 +121,7 @@ LL | | }; | |_____^ help: you can reduce it to: `return x && y` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:96:5 + --> $DIR/fixable.rs:129:5 | LL | / if x && y { LL | | return false; @@ -71,7 +131,7 @@ LL | | }; | |_____^ help: you can reduce it to: `return !(x && y)` error: equality checks against true are unnecessary - --> $DIR/fixable.rs:104:8 + --> $DIR/fixable.rs:137:8 | LL | if x == true {}; | ^^^^^^^^^ help: try simplifying it as shown: `x` @@ -79,25 +139,25 @@ LL | if x == true {}; = note: `-D clippy::bool-comparison` implied by `-D warnings` error: equality checks against false can be replaced by a negation - --> $DIR/fixable.rs:108:8 + --> $DIR/fixable.rs:141:8 | LL | if x == false {}; | ^^^^^^^^^^ help: try simplifying it as shown: `!x` error: equality checks against true are unnecessary - --> $DIR/fixable.rs:118:8 + --> $DIR/fixable.rs:151:8 | LL | if x == true {}; | ^^^^^^^^^ help: try simplifying it as shown: `x` error: equality checks against false can be replaced by a negation - --> $DIR/fixable.rs:119:8 + --> $DIR/fixable.rs:152:8 | LL | if x == false {}; | ^^^^^^^^^^ help: try simplifying it as shown: `!x` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:128:12 + --> $DIR/fixable.rs:161:12 | LL | } else if returns_bool() { | ____________^ @@ -108,7 +168,7 @@ LL | | }; | |_____^ help: you can reduce it to: `{ !returns_bool() }` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:141:5 + --> $DIR/fixable.rs:174:5 | LL | / if unsafe { no(4) } & 1 != 0 { LL | | true @@ -118,16 +178,16 @@ LL | | }; | |_____^ help: you can reduce it to: `(unsafe { no(4) } & 1 != 0)` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:146:30 + --> $DIR/fixable.rs:179:30 | LL | let _brackets_unneeded = if unsafe { no(4) } & 1 != 0 { true } else { false }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `unsafe { no(4) } & 1 != 0` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:149:9 + --> $DIR/fixable.rs:182:9 | LL | if unsafe { no(4) } & 1 != 0 { true } else { false } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `(unsafe { no(4) } & 1 != 0)` -error: aborting due to 15 previous errors +error: aborting due to 21 previous errors diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed index 83f467c840026..603d438d55889 100644 --- a/tests/ui/needless_return.fixed +++ b/tests/ui/needless_return.fixed @@ -71,7 +71,18 @@ fn test_void_if_fun(b: bool) { fn test_void_match(x: u32) { match x { 0 => (), - _ => {}, + _ => (), + } +} + +fn test_nested_match(x: u32) { + match x { + 0 => (), + 1 => { + let _ = 42; + + }, + _ => (), } } @@ -182,7 +193,7 @@ async fn async_test_void_if_fun(b: bool) { async fn async_test_void_match(x: u32) { match x { 0 => (), - _ => {}, + _ => (), } } diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs index 341caf18bd60c..c6c8cb9ec1520 100644 --- a/tests/ui/needless_return.rs +++ b/tests/ui/needless_return.rs @@ -75,6 +75,17 @@ fn test_void_match(x: u32) { } } +fn test_nested_match(x: u32) { + match x { + 0 => (), + 1 => { + let _ = 42; + return; + }, + _ => return, + } +} + fn read_line() -> String { use std::io::BufRead; let stdin = ::std::io::stdin(); diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr index c0abc2c63dde1..5bc787c56a65b 100644 --- a/tests/ui/needless_return.stderr +++ b/tests/ui/needless_return.stderr @@ -70,127 +70,139 @@ error: unneeded `return` statement --> $DIR/needless_return.rs:74:14 | LL | _ => return, - | ^^^^^^ help: replace `return` with an empty block: `{}` + | ^^^^^^ help: replace `return` with a unit value: `()` error: unneeded `return` statement - --> $DIR/needless_return.rs:89:9 + --> $DIR/needless_return.rs:83:13 + | +LL | return; + | ^^^^^^^ help: remove `return` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:85:14 + | +LL | _ => return, + | ^^^^^^ help: replace `return` with a unit value: `()` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:100:9 | LL | return String::from("test"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::from("test")` error: unneeded `return` statement - --> $DIR/needless_return.rs:91:9 + --> $DIR/needless_return.rs:102:9 | LL | return String::new(); | ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()` error: unneeded `return` statement - --> $DIR/needless_return.rs:113:32 + --> $DIR/needless_return.rs:124:32 | LL | bar.unwrap_or_else(|_| return) | ^^^^^^ help: replace `return` with an empty block: `{}` error: unneeded `return` statement - --> $DIR/needless_return.rs:118:13 + --> $DIR/needless_return.rs:129:13 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:120:20 + --> $DIR/needless_return.rs:131:20 | LL | let _ = || return; | ^^^^^^ help: replace `return` with an empty block: `{}` error: unneeded `return` statement - --> $DIR/needless_return.rs:126:32 + --> $DIR/needless_return.rs:137:32 | LL | res.unwrap_or_else(|_| return Foo) | ^^^^^^^^^^ help: remove `return`: `Foo` error: unneeded `return` statement - --> $DIR/needless_return.rs:135:5 + --> $DIR/needless_return.rs:146:5 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:139:5 + --> $DIR/needless_return.rs:150:5 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:144:9 + --> $DIR/needless_return.rs:155:9 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:146:9 + --> $DIR/needless_return.rs:157:9 | LL | return false; | ^^^^^^^^^^^^^ help: remove `return`: `false` error: unneeded `return` statement - --> $DIR/needless_return.rs:152:17 + --> $DIR/needless_return.rs:163:17 | LL | true => return false, | ^^^^^^^^^^^^ help: remove `return`: `false` error: unneeded `return` statement - --> $DIR/needless_return.rs:154:13 + --> $DIR/needless_return.rs:165:13 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:161:9 + --> $DIR/needless_return.rs:172:9 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:163:16 + --> $DIR/needless_return.rs:174:16 | LL | let _ = || return true; | ^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:171:5 + --> $DIR/needless_return.rs:182:5 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:176:9 + --> $DIR/needless_return.rs:187:9 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:178:9 + --> $DIR/needless_return.rs:189:9 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:185:14 + --> $DIR/needless_return.rs:196:14 | LL | _ => return, - | ^^^^^^ help: replace `return` with an empty block: `{}` + | ^^^^^^ help: replace `return` with a unit value: `()` error: unneeded `return` statement - --> $DIR/needless_return.rs:200:9 + --> $DIR/needless_return.rs:211:9 | LL | return String::from("test"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::from("test")` error: unneeded `return` statement - --> $DIR/needless_return.rs:202:9 + --> $DIR/needless_return.rs:213:9 | LL | return String::new(); | ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()` -error: aborting due to 32 previous errors +error: aborting due to 34 previous errors diff --git a/tests/ui/neg_multiply.fixed b/tests/ui/neg_multiply.fixed new file mode 100644 index 0000000000000..35af9d6ae317f --- /dev/null +++ b/tests/ui/neg_multiply.fixed @@ -0,0 +1,45 @@ +// run-rustfix +#![warn(clippy::neg_multiply)] +#![allow(clippy::no_effect, clippy::unnecessary_operation, clippy::precedence)] +#![allow(unused)] + +use std::ops::Mul; + +struct X; + +impl Mul for X { + type Output = X; + + fn mul(self, _r: isize) -> Self { + self + } +} + +impl Mul for isize { + type Output = X; + + fn mul(self, _r: X) -> X { + X + } +} + +fn main() { + let x = 0; + + -x; + + -x; + + 100 + -x; + + -(100 + x); + + -17; + + 0xcafe | -0xff00; + + -1 * -1; // should be ok + + X * -1; // should be ok + -1 * X; // should also be ok +} diff --git a/tests/ui/neg_multiply.rs b/tests/ui/neg_multiply.rs index d4a20ce9db1c8..7dbdb0906ceeb 100644 --- a/tests/ui/neg_multiply.rs +++ b/tests/ui/neg_multiply.rs @@ -1,5 +1,7 @@ +// run-rustfix #![warn(clippy::neg_multiply)] -#![allow(clippy::no_effect, clippy::unnecessary_operation)] +#![allow(clippy::no_effect, clippy::unnecessary_operation, clippy::precedence)] +#![allow(unused)] use std::ops::Mul; @@ -28,6 +30,14 @@ fn main() { -1 * x; + 100 + x * -1; + + (100 + x) * -1; + + -1 * 17; + + 0xcafe | 0xff00 * -1; + -1 * -1; // should be ok X * -1; // should be ok diff --git a/tests/ui/neg_multiply.stderr b/tests/ui/neg_multiply.stderr index ad677f6d6fb9b..dbf8fb36938cb 100644 --- a/tests/ui/neg_multiply.stderr +++ b/tests/ui/neg_multiply.stderr @@ -1,16 +1,40 @@ -error: negation by multiplying with `-1` - --> $DIR/neg_multiply.rs:27:5 +error: this multiplication by -1 can be written more succinctly + --> $DIR/neg_multiply.rs:29:5 | LL | x * -1; - | ^^^^^^ + | ^^^^^^ help: consider using: `-x` | = note: `-D clippy::neg-multiply` implied by `-D warnings` -error: negation by multiplying with `-1` - --> $DIR/neg_multiply.rs:29:5 +error: this multiplication by -1 can be written more succinctly + --> $DIR/neg_multiply.rs:31:5 | LL | -1 * x; - | ^^^^^^ + | ^^^^^^ help: consider using: `-x` + +error: this multiplication by -1 can be written more succinctly + --> $DIR/neg_multiply.rs:33:11 + | +LL | 100 + x * -1; + | ^^^^^^ help: consider using: `-x` + +error: this multiplication by -1 can be written more succinctly + --> $DIR/neg_multiply.rs:35:5 + | +LL | (100 + x) * -1; + | ^^^^^^^^^^^^^^ help: consider using: `-(100 + x)` + +error: this multiplication by -1 can be written more succinctly + --> $DIR/neg_multiply.rs:37:5 + | +LL | -1 * 17; + | ^^^^^^^ help: consider using: `-17` + +error: this multiplication by -1 can be written more succinctly + --> $DIR/neg_multiply.rs:39:14 + | +LL | 0xcafe | 0xff00 * -1; + | ^^^^^^^^^^^ help: consider using: `-0xff00` -error: aborting due to 2 previous errors +error: aborting due to 6 previous errors diff --git a/tests/ui/numbered_fields.fixed b/tests/ui/numbered_fields.fixed new file mode 100644 index 0000000000000..1da97e9687988 --- /dev/null +++ b/tests/ui/numbered_fields.fixed @@ -0,0 +1,33 @@ +//run-rustfix +#![warn(clippy::init_numbered_fields)] + +#[derive(Default)] +struct TupleStruct(u32, u32, u8); + +// This shouldn't lint because it's in a macro +macro_rules! tuple_struct_init { + () => { + TupleStruct { 0: 0, 1: 1, 2: 2 } + }; +} + +fn main() { + let tuple_struct = TupleStruct::default(); + + // This should lint + let _ = TupleStruct(1u32, 42, 23u8); + + // This should also lint and order the fields correctly + let _ = TupleStruct(1u32, 3u32, 2u8); + + // Ok because of default initializer + let _ = TupleStruct { 0: 42, ..tuple_struct }; + + let _ = TupleStruct { + 1: 23, + ..TupleStruct::default() + }; + + // Ok because it's in macro + let _ = tuple_struct_init!(); +} diff --git a/tests/ui/numbered_fields.rs b/tests/ui/numbered_fields.rs new file mode 100644 index 0000000000000..08ec405a5606e --- /dev/null +++ b/tests/ui/numbered_fields.rs @@ -0,0 +1,41 @@ +//run-rustfix +#![warn(clippy::init_numbered_fields)] + +#[derive(Default)] +struct TupleStruct(u32, u32, u8); + +// This shouldn't lint because it's in a macro +macro_rules! tuple_struct_init { + () => { + TupleStruct { 0: 0, 1: 1, 2: 2 } + }; +} + +fn main() { + let tuple_struct = TupleStruct::default(); + + // This should lint + let _ = TupleStruct { + 0: 1u32, + 1: 42, + 2: 23u8, + }; + + // This should also lint and order the fields correctly + let _ = TupleStruct { + 0: 1u32, + 2: 2u8, + 1: 3u32, + }; + + // Ok because of default initializer + let _ = TupleStruct { 0: 42, ..tuple_struct }; + + let _ = TupleStruct { + 1: 23, + ..TupleStruct::default() + }; + + // Ok because it's in macro + let _ = tuple_struct_init!(); +} diff --git a/tests/ui/numbered_fields.stderr b/tests/ui/numbered_fields.stderr new file mode 100644 index 0000000000000..01691c8b141e8 --- /dev/null +++ b/tests/ui/numbered_fields.stderr @@ -0,0 +1,26 @@ +error: used a field initializer for a tuple struct + --> $DIR/numbered_fields.rs:18:13 + | +LL | let _ = TupleStruct { + | _____________^ +LL | | 0: 1u32, +LL | | 1: 42, +LL | | 2: 23u8, +LL | | }; + | |_____^ help: try this instead: `TupleStruct(1u32, 42, 23u8)` + | + = note: `-D clippy::init-numbered-fields` implied by `-D warnings` + +error: used a field initializer for a tuple struct + --> $DIR/numbered_fields.rs:25:13 + | +LL | let _ = TupleStruct { + | _____________^ +LL | | 0: 1u32, +LL | | 2: 2u8, +LL | | 1: 3u32, +LL | | }; + | |_____^ help: try this instead: `TupleStruct(1u32, 3u32, 2u8)` + +error: aborting due to 2 previous errors + diff --git a/tests/ui/return_self_not_must_use.rs b/tests/ui/return_self_not_must_use.rs index bdf3f3d799582..7dd5742dae9f2 100644 --- a/tests/ui/return_self_not_must_use.rs +++ b/tests/ui/return_self_not_must_use.rs @@ -5,12 +5,12 @@ pub struct Bar; pub trait Whatever { fn what(&self) -> Self; - // There should be no warning here! + // There should be no warning here! (returns a reference) fn what2(&self) -> &Self; } impl Bar { - // There should be no warning here! + // There should be no warning here! (note taking a self argument) pub fn not_new() -> Self { Self } @@ -20,23 +20,38 @@ impl Bar { pub fn bar(self) -> Self { self } - // There should be no warning here! + // There should be no warning here! (private method) fn foo2(&self) -> Self { Self } - // There should be no warning here! + // There should be no warning here! (returns a reference) pub fn foo3(&self) -> &Self { self } + // There should be no warning here! (already a `must_use` attribute) + #[must_use] + pub fn foo4(&self) -> Self { + Self + } } impl Whatever for Bar { - // There should be no warning here! + // There should be no warning here! (comes from the trait) fn what(&self) -> Self { self.foo2() } - // There should be no warning here! + // There should be no warning here! (comes from the trait) fn what2(&self) -> &Self { self } } + +#[must_use] +pub struct Foo; + +impl Foo { + // There should be no warning here! (`Foo` already implements `#[must_use]`) + fn foo(&self) -> Self { + Self + } +} diff --git a/tests/ui/shadow.rs b/tests/ui/shadow.rs index 06f6949b66f5a..0321f8c4cdf84 100644 --- a/tests/ui/shadow.rs +++ b/tests/ui/shadow.rs @@ -47,6 +47,8 @@ fn syntax() { let _ = |[x]: [u32; 1]| { let x = 1; }; + let y = Some(1); + if let Some(y) = y {} } fn negative() { diff --git a/tests/ui/shadow.stderr b/tests/ui/shadow.stderr index dcc7d4e6b2ff9..f8b9221d55587 100644 --- a/tests/ui/shadow.stderr +++ b/tests/ui/shadow.stderr @@ -241,17 +241,29 @@ note: previous binding is here LL | let _ = |[x]: [u32; 1]| { | ^ +error: `y` is shadowed + --> $DIR/shadow.rs:51:17 + | +LL | if let Some(y) = y {} + | ^ + | +note: previous binding is here + --> $DIR/shadow.rs:50:9 + | +LL | let y = Some(1); + | ^ + error: `_b` shadows a previous, unrelated binding - --> $DIR/shadow.rs:85:9 + --> $DIR/shadow.rs:87:9 | LL | let _b = _a; | ^^ | note: previous binding is here - --> $DIR/shadow.rs:84:28 + --> $DIR/shadow.rs:86:28 | LL | pub async fn foo2(_a: i32, _b: i64) { | ^^ -error: aborting due to 21 previous errors +error: aborting due to 22 previous errors diff --git a/tests/ui/short_circuit_statement.fixed b/tests/ui/short_circuit_statement.fixed index af0a397bd1aff..dd22ecab0b551 100644 --- a/tests/ui/short_circuit_statement.fixed +++ b/tests/ui/short_circuit_statement.fixed @@ -6,7 +6,7 @@ fn main() { if f() { g(); } if !f() { g(); } - if !(1 == 2) { g(); } + if 1 != 2 { g(); } } fn f() -> bool { diff --git a/tests/ui/short_circuit_statement.stderr b/tests/ui/short_circuit_statement.stderr index 0a3f60c3d132d..aa84ac3a7925f 100644 --- a/tests/ui/short_circuit_statement.stderr +++ b/tests/ui/short_circuit_statement.stderr @@ -16,7 +16,7 @@ error: boolean short circuit operator in statement may be clearer using an expli --> $DIR/short_circuit_statement.rs:9:5 | LL | 1 == 2 || g(); - | ^^^^^^^^^^^^^^ help: replace it with: `if !(1 == 2) { g(); }` + | ^^^^^^^^^^^^^^ help: replace it with: `if 1 != 2 { g(); }` error: aborting due to 3 previous errors diff --git a/tests/ui/undocumented_unsafe_blocks.rs b/tests/ui/undocumented_unsafe_blocks.rs index 7e510d8947522..380303d8152aa 100644 --- a/tests/ui/undocumented_unsafe_blocks.rs +++ b/tests/ui/undocumented_unsafe_blocks.rs @@ -5,7 +5,7 @@ fn nested_local() { let _ = { let _ = { - // Safety: + // SAFETY: let _ = unsafe {}; }; }; @@ -14,7 +14,7 @@ fn nested_local() { fn deep_nest() { let _ = { let _ = { - // Safety: + // SAFETY: let _ = unsafe {}; // Safety: @@ -28,7 +28,7 @@ fn deep_nest() { // Safety: let _ = unsafe {}; - // Safety: + // SAFETY: unsafe {}; }; }; @@ -44,7 +44,7 @@ fn deep_nest() { unsafe {}; }; - // Safety: + // SAFETY: unsafe {}; } @@ -59,7 +59,7 @@ fn line_comment() { } fn line_comment_newlines() { - // Safety: + // SAFETY: unsafe {} } @@ -84,7 +84,7 @@ fn block_comment() { } fn block_comment_newlines() { - /* Safety: */ + /* SAFETY: */ unsafe {} } @@ -96,7 +96,7 @@ fn inline_block_comment() { fn block_comment_with_extras() { /* This is a description - * Safety: + * SAFETY: */ unsafe {} } @@ -122,7 +122,7 @@ fn buried_safety() { } fn safety_with_prepended_text() { - // This is a test. Safety: + // This is a test. safety: unsafe {} } @@ -132,7 +132,7 @@ fn local_line_comment() { } fn local_block_comment() { - /* Safety: */ + /* SAFETY: */ let _ = unsafe {}; } @@ -142,18 +142,18 @@ fn comment_array() { } fn comment_tuple() { - // Safety: + // sAFETY: let _ = (42, unsafe {}, "test", unsafe {}); } fn comment_unary() { - // Safety: + // SAFETY: let _ = *unsafe { &42 }; } #[allow(clippy::match_single_binding)] fn comment_match() { - // Safety: + // SAFETY: let _ = match unsafe {} { _ => {}, }; @@ -177,7 +177,7 @@ fn comment_macro_call() { } t!( - // Safety: + // SAFETY: unsafe {} ); } @@ -194,18 +194,18 @@ fn comment_macro_def() { } fn non_ascii_comment() { - // ॐ᧻໒ Safety: ௵∰ + // ॐ᧻໒ SaFeTy: ௵∰ unsafe {}; } fn local_commented_block() { let _ = - // Safety: + // safety: unsafe {}; } fn local_nest() { - // Safety: + // safety: let _ = [(42, unsafe {}, unsafe {}), (52, unsafe {}, unsafe {})]; } @@ -267,17 +267,17 @@ fn no_comment_macro_def() { } fn trailing_comment() { - unsafe {} // Safety: + unsafe {} // SAFETY: } fn internal_comment() { unsafe { - // Safety: + // SAFETY: } } fn interference() { - // Safety + // SAFETY let _ = 42; diff --git a/tests/ui/undocumented_unsafe_blocks.stderr b/tests/ui/undocumented_unsafe_blocks.stderr index ebe589001a1fe..f69d0da54e0d6 100644 --- a/tests/ui/undocumented_unsafe_blocks.stderr +++ b/tests/ui/undocumented_unsafe_blocks.stderr @@ -7,7 +7,7 @@ LL | unsafe {} = note: `-D clippy::undocumented-unsafe-blocks` implied by `-D warnings` help: consider adding a safety comment | -LL ~ // Safety: ... +LL ~ // SAFETY: ... LL + unsafe {} | @@ -19,7 +19,7 @@ LL | let _ = [unsafe { 14 }, unsafe { 15 }, 42, unsafe { 16 }]; | help: consider adding a safety comment | -LL ~ // Safety: ... +LL ~ // SAFETY: ... LL + let _ = [unsafe { 14 }, unsafe { 15 }, 42, unsafe { 16 }]; | @@ -31,7 +31,7 @@ LL | let _ = (42, unsafe {}, "test", unsafe {}); | help: consider adding a safety comment | -LL ~ // Safety: ... +LL ~ // SAFETY: ... LL + let _ = (42, unsafe {}, "test", unsafe {}); | @@ -43,7 +43,7 @@ LL | let _ = *unsafe { &42 }; | help: consider adding a safety comment | -LL ~ // Safety: ... +LL ~ // SAFETY: ... LL + let _ = *unsafe { &42 }; | @@ -55,7 +55,7 @@ LL | let _ = match unsafe {} { | help: consider adding a safety comment | -LL ~ // Safety: ... +LL ~ // SAFETY: ... LL + let _ = match unsafe {} { | @@ -67,7 +67,7 @@ LL | let _ = &unsafe {}; | help: consider adding a safety comment | -LL ~ // Safety: ... +LL ~ // SAFETY: ... LL + let _ = &unsafe {}; | @@ -79,7 +79,7 @@ LL | let _ = [unsafe {}; 5]; | help: consider adding a safety comment | -LL ~ // Safety: ... +LL ~ // SAFETY: ... LL + let _ = [unsafe {}; 5]; | @@ -91,7 +91,7 @@ LL | let _ = unsafe {}; | help: consider adding a safety comment | -LL ~ // Safety: ... +LL ~ // SAFETY: ... LL + let _ = unsafe {}; | @@ -103,7 +103,7 @@ LL | t!(unsafe {}); | help: consider adding a safety comment | -LL ~ t!(// Safety: ... +LL ~ t!(// SAFETY: ... LL ~ unsafe {}); | @@ -122,13 +122,13 @@ LL | t!(); error: unsafe block missing a safety comment --> $DIR/undocumented_unsafe_blocks.rs:270:5 | -LL | unsafe {} // Safety: +LL | unsafe {} // SAFETY: | ^^^^^^^^^ | help: consider adding a safety comment | -LL ~ // Safety: ... -LL ~ unsafe {} // Safety: +LL ~ // SAFETY: ... +LL ~ unsafe {} // SAFETY: | error: unsafe block missing a safety comment @@ -139,7 +139,7 @@ LL | unsafe { | help: consider adding a safety comment | -LL ~ // Safety: ... +LL ~ // SAFETY: ... LL + unsafe { | @@ -151,7 +151,7 @@ LL | unsafe {}; | help: consider adding a safety comment | -LL ~ // Safety: ... +LL ~ // SAFETY: ... LL ~ unsafe {}; | @@ -163,7 +163,7 @@ LL | println!("{}", unsafe { String::from_utf8_unchecked(vec![]) }); | help: consider adding a safety comment | -LL ~ println!("{}", // Safety: ... +LL ~ println!("{}", // SAFETY: ... LL ~ unsafe { String::from_utf8_unchecked(vec![]) }); | diff --git a/tests/ui/unwrap_or_else_default.fixed b/tests/ui/unwrap_or_else_default.fixed index 7ac3f426c9775..c2b9bd2c881fe 100644 --- a/tests/ui/unwrap_or_else_default.fixed +++ b/tests/ui/unwrap_or_else_default.fixed @@ -45,7 +45,7 @@ fn unwrap_or_else_default() { with_enum.unwrap_or_else(Enum::A); let with_new = Some(vec![1]); - with_new.unwrap_or_else(Vec::new); + with_new.unwrap_or_default(); let with_err: Result<_, ()> = Ok(vec![1]); with_err.unwrap_or_else(make); @@ -66,6 +66,9 @@ fn unwrap_or_else_default() { let with_default_type = Some(1); with_default_type.unwrap_or_default(); + + let with_default_type: Option> = None; + with_default_type.unwrap_or_default(); } fn main() {} diff --git a/tests/ui/unwrap_or_else_default.rs b/tests/ui/unwrap_or_else_default.rs index 82b727a039ed4..d55664990aeb9 100644 --- a/tests/ui/unwrap_or_else_default.rs +++ b/tests/ui/unwrap_or_else_default.rs @@ -66,6 +66,9 @@ fn unwrap_or_else_default() { let with_default_type = Some(1); with_default_type.unwrap_or_else(u64::default); + + let with_default_type: Option> = None; + with_default_type.unwrap_or_else(Vec::new); } fn main() {} diff --git a/tests/ui/unwrap_or_else_default.stderr b/tests/ui/unwrap_or_else_default.stderr index feb215b09f662..53e31d85edfca 100644 --- a/tests/ui/unwrap_or_else_default.stderr +++ b/tests/ui/unwrap_or_else_default.stderr @@ -1,10 +1,16 @@ +error: use of `.unwrap_or_else(..)` to construct default value + --> $DIR/unwrap_or_else_default.rs:48:5 + | +LL | with_new.unwrap_or_else(Vec::new); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `with_new.unwrap_or_default()` + | + = note: `-D clippy::unwrap-or-else-default` implied by `-D warnings` + error: use of `.unwrap_or_else(..)` to construct default value --> $DIR/unwrap_or_else_default.rs:62:5 | LL | with_real_default.unwrap_or_else(::default); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `with_real_default.unwrap_or_default()` - | - = note: `-D clippy::unwrap-or-else-default` implied by `-D warnings` error: use of `.unwrap_or_else(..)` to construct default value --> $DIR/unwrap_or_else_default.rs:65:5 @@ -18,5 +24,11 @@ error: use of `.unwrap_or_else(..)` to construct default value LL | with_default_type.unwrap_or_else(u64::default); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `with_default_type.unwrap_or_default()` -error: aborting due to 3 previous errors +error: use of `.unwrap_or_else(..)` to construct default value + --> $DIR/unwrap_or_else_default.rs:71:5 + | +LL | with_default_type.unwrap_or_else(Vec::new); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `with_default_type.unwrap_or_default()` + +error: aborting due to 5 previous errors From f5bbd1b5299da31532d00abb0502feb5b60f2606 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Fri, 31 Dec 2021 21:13:07 -0800 Subject: [PATCH 05/11] Make tidy check for magic numbers that spell things Remove existing problematic cases. --- tests/ui/unreadable_literal.fixed | 2 +- tests/ui/unreadable_literal.rs | 2 +- tests/ui/unreadable_literal.stderr | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/ui/unreadable_literal.fixed b/tests/ui/unreadable_literal.fixed index c2e38037addd2..e726b652ef1ed 100644 --- a/tests/ui/unreadable_literal.fixed +++ b/tests/ui/unreadable_literal.fixed @@ -30,7 +30,7 @@ fn main() { 1_234.123_f32, 1.123_4_f32, ); - let _bad = (0b11_0110_i64, 0xcafe_babe_usize, 123_456_f32, 1.234_567_f32); + let _bad = (0b11_0110_i64, 0x1234_5678_usize, 123_456_f32, 1.234_567_f32); let _good_sci = 1.1234e1; let _bad_sci = 1.123_456e1; diff --git a/tests/ui/unreadable_literal.rs b/tests/ui/unreadable_literal.rs index 8296945b25eb4..5bbb2fc9dc137 100644 --- a/tests/ui/unreadable_literal.rs +++ b/tests/ui/unreadable_literal.rs @@ -30,7 +30,7 @@ fn main() { 1_234.123_f32, 1.123_4_f32, ); - let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32); + let _bad = (0b110110_i64, 0x12345678_usize, 123456_f32, 1.234567_f32); let _good_sci = 1.1234e1; let _bad_sci = 1.123456e1; diff --git a/tests/ui/unreadable_literal.stderr b/tests/ui/unreadable_literal.stderr index 8436aac17acfe..ee5466fd517fd 100644 --- a/tests/ui/unreadable_literal.stderr +++ b/tests/ui/unreadable_literal.stderr @@ -9,7 +9,7 @@ LL | 0x1_234_567, error: long literal lacking separators --> $DIR/unreadable_literal.rs:33:17 | -LL | let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32); +LL | let _bad = (0b110110_i64, 0x12345678_usize, 123456_f32, 1.234567_f32); | ^^^^^^^^^^^^ help: consider: `0b11_0110_i64` | = note: `-D clippy::unreadable-literal` implied by `-D warnings` @@ -17,19 +17,19 @@ LL | let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32); error: long literal lacking separators --> $DIR/unreadable_literal.rs:33:31 | -LL | let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32); - | ^^^^^^^^^^^^^^^^ help: consider: `0xcafe_babe_usize` +LL | let _bad = (0b110110_i64, 0x12345678_usize, 123456_f32, 1.234567_f32); + | ^^^^^^^^^^^^^^^^ help: consider: `0x1234_5678_usize` error: long literal lacking separators --> $DIR/unreadable_literal.rs:33:49 | -LL | let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32); +LL | let _bad = (0b110110_i64, 0x12345678_usize, 123456_f32, 1.234567_f32); | ^^^^^^^^^^ help: consider: `123_456_f32` error: long literal lacking separators --> $DIR/unreadable_literal.rs:33:61 | -LL | let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32); +LL | let _bad = (0b110110_i64, 0x12345678_usize, 123456_f32, 1.234567_f32); | ^^^^^^^^^^^^ help: consider: `1.234_567_f32` error: long literal lacking separators From c34e3f0f83660e02a7545ac52f4c8023044868c2 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 18 Nov 2021 22:33:49 +0000 Subject: [PATCH 06/11] Update clippy for associated item changes --- clippy_lints/src/len_zero.rs | 12 +++++++----- clippy_lints/src/non_copy_const.rs | 11 ++++++----- clippy_lints/src/use_self.rs | 7 +++---- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index 20e6220ec7d3a..64f6d62fbdcd8 100644 --- a/clippy_lints/src/len_zero.rs +++ b/clippy_lints/src/len_zero.rs @@ -214,14 +214,14 @@ fn check_trait_items(cx: &LateContext<'_>, visited_trait: &Item<'_>, trait_items { let mut current_and_super_traits = DefIdSet::default(); fill_trait_set(visited_trait.def_id.to_def_id(), &mut current_and_super_traits, cx); + let is_empty = sym!(is_empty); let is_empty_method_found = current_and_super_traits .iter() - .flat_map(|&i| cx.tcx.associated_items(i).in_definition_order()) + .flat_map(|&i| cx.tcx.associated_items(i).filter_by_name_unhygienic(is_empty)) .any(|i| { i.kind == ty::AssocKind::Fn && i.fn_has_self_parameter - && i.ident.name == sym!(is_empty) && cx.tcx.fn_sig(i.def_id).inputs().skip_binder().len() == 1 }); @@ -458,7 +458,7 @@ fn is_empty_array(expr: &Expr<'_>) -> bool { fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { /// Gets an `AssocItem` and return true if it matches `is_empty(self)`. fn is_is_empty(cx: &LateContext<'_>, item: &ty::AssocItem) -> bool { - if item.kind == ty::AssocKind::Fn && item.ident.name.as_str() == "is_empty" { + if item.kind == ty::AssocKind::Fn { let sig = cx.tcx.fn_sig(item.def_id); let ty = sig.skip_binder(); ty.inputs().len() == 1 @@ -469,10 +469,11 @@ fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { /// Checks the inherent impl's items for an `is_empty(self)` method. fn has_is_empty_impl(cx: &LateContext<'_>, id: DefId) -> bool { + let is_empty = sym!(is_empty); cx.tcx.inherent_impls(id).iter().any(|imp| { cx.tcx .associated_items(*imp) - .in_definition_order() + .filter_by_name_unhygienic(is_empty) .any(|item| is_is_empty(cx, item)) }) } @@ -480,9 +481,10 @@ fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { let ty = &cx.typeck_results().expr_ty(expr).peel_refs(); match ty.kind() { ty::Dynamic(tt, ..) => tt.principal().map_or(false, |principal| { + let is_empty = sym!(is_empty); cx.tcx .associated_items(principal.def_id()) - .in_definition_order() + .filter_by_name_unhygienic(is_empty) .any(|item| is_is_empty(cx, item)) }), ty::Projection(ref proj) => has_is_empty_impl(cx, proj.item_def_id), diff --git a/clippy_lints/src/non_copy_const.rs b/clippy_lints/src/non_copy_const.rs index 074ba9e92ba4d..7d2ff083b7e07 100644 --- a/clippy_lints/src/non_copy_const.rs +++ b/clippy_lints/src/non_copy_const.rs @@ -12,11 +12,10 @@ use rustc_hir::def_id::DefId; use rustc_hir::{ BodyId, Expr, ExprKind, HirId, Impl, ImplItem, ImplItemKind, Item, ItemKind, Node, TraitItem, TraitItemKind, UnOp, }; -use rustc_infer::traits::specialization_graph; use rustc_lint::{LateContext, LateLintPass, Lint}; use rustc_middle::mir::interpret::{ConstValue, ErrorHandled}; use rustc_middle::ty::adjustment::Adjust; -use rustc_middle::ty::{self, AssocKind, Const, Ty}; +use rustc_middle::ty::{self, Const, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::{InnerSpan, Span, DUMMY_SP}; use rustc_typeck::hir_ty_to_ty; @@ -293,8 +292,10 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst { // Lint a trait impl item only when the definition is a generic type, // assuming an assoc const is not meant to be an interior mutable type. if let Some(of_trait_def_id) = of_trait_ref.trait_def_id(); - if let Some(of_assoc_item) = specialization_graph::Node::Trait(of_trait_def_id) - .item(cx.tcx, impl_item.ident, AssocKind::Const, of_trait_def_id); + if let Some(of_assoc_item) = cx + .tcx + .associated_item(impl_item.def_id) + .trait_item_def_id; if cx .tcx .layout_of(cx.tcx.param_env(of_trait_def_id).and( @@ -303,7 +304,7 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst { // and, in that case, the definition is *not* generic. cx.tcx.normalize_erasing_regions( cx.tcx.param_env(of_trait_def_id), - cx.tcx.type_of(of_assoc_item.def_id), + cx.tcx.type_of(of_assoc_item), ), )) .is_err(); diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index 059f7f647f88f..a86db58741eb6 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -13,7 +13,6 @@ use rustc_hir::{ }; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::hir::map::Map; -use rustc_middle::ty::AssocKind; use rustc_semver::RustcVersion; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::Span; @@ -143,10 +142,10 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf { // trait, not in the impl of the trait. let trait_method = cx .tcx - .associated_items(impl_trait_ref.def_id) - .find_by_name_and_kind(cx.tcx, impl_item.ident, AssocKind::Fn, impl_trait_ref.def_id) + .associated_item(impl_item.def_id) + .trait_item_def_id .expect("impl method matches a trait method"); - let trait_method_sig = cx.tcx.fn_sig(trait_method.def_id); + let trait_method_sig = cx.tcx.fn_sig(trait_method); let trait_method_sig = cx.tcx.erase_late_bound_regions(trait_method_sig); // `impl_inputs_outputs` is an iterator over the types (`hir::Ty`) declared in the From 1288b80affc300819a38cf56de9d4776f3e50e1f Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 18 Dec 2021 20:07:58 +0800 Subject: [PATCH 07/11] rustc_metadata: Optimize and document module children decoding --- clippy_utils/src/lib.rs | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 9179e67c4f4ee..bd6851d1fbba2 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -82,7 +82,6 @@ use rustc_hir::{ TraitItemKind, TraitRef, TyKind, UnOp, ArrayLen }; use rustc_lint::{LateContext, Level, Lint, LintContext}; -use rustc_middle::hir::exports::Export; use rustc_middle::hir::map::Map; use rustc_middle::hir::place::PlaceBase; use rustc_middle::ty as rustc_ty; @@ -523,10 +522,21 @@ pub fn path_to_res(cx: &LateContext<'_>, path: &[&str]) -> Res { } }; } - fn item_child_by_name<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, name: &str) -> Option<&'tcx Export> { - tcx.item_children(def_id) - .iter() - .find(|item| item.ident.name.as_str() == name) + fn item_child_by_name<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, name: &str) -> Option { + match tcx.def_kind(def_id) { + DefKind::Mod | DefKind::Enum | DefKind::Trait => tcx + .item_children(def_id) + .iter() + .find(|item| item.ident.name.as_str() == name) + .map(|child| child.res.expect_non_local()), + DefKind::Impl => tcx + .associated_item_def_ids(def_id) + .iter() + .copied() + .find(|assoc_def_id| tcx.item_name(*assoc_def_id).as_str() == name) + .map(|assoc_def_id| Res::Def(tcx.def_kind(assoc_def_id), assoc_def_id)), + _ => None, + } } let (krate, first, path) = match *path { @@ -543,15 +553,12 @@ pub fn path_to_res(cx: &LateContext<'_>, path: &[&str]) -> Res { let last = path .iter() .copied() - // `get_def_path` seems to generate these empty segments for extern blocks. - // We can just ignore them. - .filter(|segment| !segment.is_empty()) // for each segment, find the child item - .try_fold(first, |item, segment| { - let def_id = item.res.def_id(); + .try_fold(first, |res, segment| { + let def_id = res.def_id(); if let Some(item) = item_child_by_name(tcx, def_id, segment) { Some(item) - } else if matches!(item.res, Res::Def(DefKind::Enum | DefKind::Struct, _)) { + } else if matches!(res, Res::Def(DefKind::Enum | DefKind::Struct, _)) { // it is not a child item so check inherent impl items tcx.inherent_impls(def_id) .iter() @@ -560,7 +567,7 @@ pub fn path_to_res(cx: &LateContext<'_>, path: &[&str]) -> Res { None } }); - try_res!(last).res.expect_non_local() + try_res!(last).expect_non_local() } /// Convenience function to get the `DefId` of a trait by path. From c8ea0420cb6ce2262a93a919c2bb70378f5cd3ae Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 23 Dec 2021 16:12:34 +0800 Subject: [PATCH 08/11] rustc_metadata: Rename `item_children(_untracked)` to `module_children(_untracked)` And `each_child_of_item` to `for_each_module_child` --- clippy_lints/src/macro_use.rs | 2 +- clippy_lints/src/utils/internal_lints.rs | 4 ++-- clippy_utils/src/lib.rs | 2 +- tests/ui/macro_use_imports.fixed | 2 +- tests/ui/macro_use_imports.rs | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/macro_use.rs b/clippy_lints/src/macro_use.rs index 50d80e6a1d224..41f5a913b316e 100644 --- a/clippy_lints/src/macro_use.rs +++ b/clippy_lints/src/macro_use.rs @@ -96,7 +96,7 @@ impl<'tcx> LateLintPass<'tcx> for MacroUseImports { if let Res::Def(DefKind::Mod, id) = path.res; if !id.is_local(); then { - for kid in cx.tcx.item_children(id).iter() { + for kid in cx.tcx.module_children(id).iter() { if let Res::Def(DefKind::Macro(_mac_type), mac_id) = kid.res { let span = mac_attr.span; let def_path = cx.tcx.def_path_str(mac_id); diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index e98dcd3cf983b..7d196af7a53f4 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -924,7 +924,7 @@ pub fn check_path(cx: &LateContext<'_>, path: &[&str]) -> bool { let lang_item_path = cx.get_def_path(*item_def_id); if path_syms.starts_with(&lang_item_path) { if let [item] = &path_syms[lang_item_path.len()..] { - for child in cx.tcx.item_children(*item_def_id) { + for child in cx.tcx.module_children(*item_def_id) { if child.ident.name == *item { return true; } @@ -984,7 +984,7 @@ impl<'tcx> LateLintPass<'tcx> for InterningDefinedSymbol { for &module in &[&paths::KW_MODULE, &paths::SYM_MODULE] { if let Some(def_id) = path_to_res(cx, module).opt_def_id() { - for item in cx.tcx.item_children(def_id).iter() { + for item in cx.tcx.module_children(def_id).iter() { if_chain! { if let Res::Def(DefKind::Const, item_def_id) = item.res; let ty = cx.tcx.type_of(item_def_id); diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index bd6851d1fbba2..91ebc7ea89cc0 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -525,7 +525,7 @@ pub fn path_to_res(cx: &LateContext<'_>, path: &[&str]) -> Res { fn item_child_by_name<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, name: &str) -> Option { match tcx.def_kind(def_id) { DefKind::Mod | DefKind::Enum | DefKind::Trait => tcx - .item_children(def_id) + .module_children(def_id) .iter() .find(|item| item.ident.name.as_str() == name) .map(|child| child.res.expect_non_local()), diff --git a/tests/ui/macro_use_imports.fixed b/tests/ui/macro_use_imports.fixed index 9171558f3a2d7..306ea50258da0 100644 --- a/tests/ui/macro_use_imports.fixed +++ b/tests/ui/macro_use_imports.fixed @@ -40,7 +40,7 @@ mod a { } } -// issue #7015, ICE due to calling `item_children` with local `DefId` +// issue #7015, ICE due to calling `module_children` with local `DefId` #[macro_use] use a as b; diff --git a/tests/ui/macro_use_imports.rs b/tests/ui/macro_use_imports.rs index cd01fd43f6d32..e26a7545ea6f8 100644 --- a/tests/ui/macro_use_imports.rs +++ b/tests/ui/macro_use_imports.rs @@ -40,7 +40,7 @@ mod a { } } -// issue #7015, ICE due to calling `item_children` with local `DefId` +// issue #7015, ICE due to calling `module_children` with local `DefId` #[macro_use] use a as b; From dda2aef64fb5b4903a28e5d4fb8d63483642cc6f Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 2 Jan 2022 22:37:05 -0500 Subject: [PATCH 09/11] Store a `Symbol` instead of an `Ident` in `VariantDef`/`FieldDef` The field is also renamed from `ident` to `name. In most cases, we don't actually need the `Span`. A new `ident` method is added to `VariantDef` and `FieldDef`, which constructs the full `Ident` using `tcx.def_ident_span()`. This method is used in the cases where we actually need an `Ident`. This makes incremental compilation properly track changes to the `Span`, without all of the invalidations caused by storing a `Span` directly via an `Ident`. --- clippy_lints/src/default.rs | 2 +- clippy_lints/src/default_numeric_fallback.rs | 2 +- clippy_lints/src/inconsistent_struct_constructor.rs | 2 +- clippy_lints/src/matches.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/default.rs b/clippy_lints/src/default.rs index a0b137efe221a..6422f5aabe5e2 100644 --- a/clippy_lints/src/default.rs +++ b/clippy_lints/src/default.rs @@ -198,7 +198,7 @@ impl LateLintPass<'_> for Default { let ext_with_default = !variant .fields .iter() - .all(|field| assigned_fields.iter().any(|(a, _)| a == &field.ident.name)); + .all(|field| assigned_fields.iter().any(|(a, _)| a == &field.name)); let field_list = assigned_fields .into_iter() diff --git a/clippy_lints/src/default_numeric_fallback.rs b/clippy_lints/src/default_numeric_fallback.rs index 3573ea5f02671..15215ac15cdb9 100644 --- a/clippy_lints/src/default_numeric_fallback.rs +++ b/clippy_lints/src/default_numeric_fallback.rs @@ -161,7 +161,7 @@ impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> { fields_def .iter() .find_map(|f_def| { - if f_def.ident == field.ident + if f_def.ident(self.cx.tcx) == field.ident { Some(self.cx.tcx.type_of(f_def.did)) } else { None } }); diff --git a/clippy_lints/src/inconsistent_struct_constructor.rs b/clippy_lints/src/inconsistent_struct_constructor.rs index 1debdef9d86c7..388bb3727f96c 100644 --- a/clippy_lints/src/inconsistent_struct_constructor.rs +++ b/clippy_lints/src/inconsistent_struct_constructor.rs @@ -76,7 +76,7 @@ impl LateLintPass<'_> for InconsistentStructConstructor { then { let mut def_order_map = FxHashMap::default(); for (idx, field) in variant.fields.iter().enumerate() { - def_order_map.insert(field.ident.name, idx); + def_order_map.insert(field.name, idx); } if is_consistent_order(fields, &def_order_map) { diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 22970507f964c..5fa8f249e701e 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -1136,7 +1136,7 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) s.push_str("::"); s }, - variant.ident.name, + variant.name, match variant.ctor_kind { CtorKind::Fn if variant.fields.len() == 1 => "(_)", CtorKind::Fn => "(..)", From 8a2141bae408050e3178c802c8e9557d314000d5 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 13 Jan 2022 12:48:08 +0100 Subject: [PATCH 10/11] Bump Clippy Version -> 0.1.60 --- Cargo.toml | 2 +- clippy_lints/Cargo.toml | 2 +- clippy_utils/Cargo.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index cb49dd84c39a9..e445889a58f77 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy" -version = "0.1.59" +version = "0.1.60" description = "A bunch of helpful lints to avoid common pitfalls in Rust" repository = "https://github.com/rust-lang/rust-clippy" readme = "README.md" diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index 49171e21d10e7..2053ca64ba23d 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy_lints" -version = "0.1.59" +version = "0.1.60" description = "A bunch of helpful lints to avoid common pitfalls in Rust" repository = "https://github.com/rust-lang/rust-clippy" readme = "README.md" diff --git a/clippy_utils/Cargo.toml b/clippy_utils/Cargo.toml index 565dd1faa3d7f..afff6491aba64 100644 --- a/clippy_utils/Cargo.toml +++ b/clippy_utils/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy_utils" -version = "0.1.59" +version = "0.1.60" edition = "2021" publish = false From 6ad05bcbbe821c3643e2cecf659fe091818fb7e8 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 13 Jan 2022 12:48:17 +0100 Subject: [PATCH 11/11] Bump nightly version -> 2022-01-13 --- rust-toolchain | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-toolchain b/rust-toolchain index 471ae40f1ac7a..e6a58e9207250 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2021-12-30" +channel = "nightly-2022-01-13" components = ["cargo", "llvm-tools-preview", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"]