diff --git a/.github/driver.sh b/.github/driver.sh index 40a2aad0f537..11fd6b5c79ed 100755 --- a/.github/driver.sh +++ b/.github/driver.sh @@ -32,7 +32,7 @@ test "$sysroot" = $desired_sysroot ) # Check that the --sysroot argument is only passed once via arg_file.txt (SYSROOT is ignored) -( +( echo "fn main() {}" > target/driver_test.rs echo "--sysroot="$(./target/debug/clippy-driver --print sysroot)"" > arg_file.txt echo "--verbose" >> arg_file.txt @@ -45,7 +45,7 @@ unset CARGO_MANIFEST_DIR # Run a lint and make sure it produces the expected output. It's also expected to exit with code 1 # FIXME: How to match the clippy invocation in compile-test.rs? ./target/debug/clippy-driver -Dwarnings -Aunused -Zui-testing --emit metadata --crate-type bin tests/ui/double_neg.rs 2>double_neg.stderr && exit 1 -sed -e "s,tests/ui,\$DIR," -e "/= help: for/d" double_neg.stderr > normalized.stderr +sed -e "/= help: for/d" double_neg.stderr > normalized.stderr diff -u normalized.stderr tests/ui/double_neg.stderr # make sure "clippy-driver --rustc --arg" and "rustc --arg" behave the same diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b853567219c..c2f47ad97203 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5125,6 +5125,7 @@ Released 2018-09-13 [`default_trait_access`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_trait_access [`default_union_representation`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_union_representation [`deprecated_cfg_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_cfg_attr +[`deprecated_clippy_cfg_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_clippy_cfg_attr [`deprecated_semver`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_semver [`deref_addrof`]: https://rust-lang.github.io/rust-clippy/master/index.html#deref_addrof [`deref_by_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#deref_by_slicing @@ -5725,6 +5726,7 @@ Released 2018-09-13 [`unknown_clippy_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#unknown_clippy_lints [`unnecessary_box_returns`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_box_returns [`unnecessary_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast +[`unnecessary_clippy_cfg`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_clippy_cfg [`unnecessary_fallible_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fallible_conversions [`unnecessary_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map [`unnecessary_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_find_map diff --git a/Cargo.toml b/Cargo.toml index 321424880d1e..c83a2e349e46 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,10 +27,10 @@ rustc_tools_util = "0.3.0" tempfile = { version = "3.2", optional = true } termize = "0.1" color-print = "0.3.4" -anstream = "0.5.0" +anstream = "0.6.0" [dev-dependencies] -ui_test = "0.21.2" +ui_test = "0.22.1" tester = "0.9" regex = "1.5" toml = "0.7.3" diff --git a/book/src/configuration.md b/book/src/configuration.md index e8274bc4575d..055203464569 100644 --- a/book/src/configuration.md +++ b/book/src/configuration.md @@ -113,17 +113,14 @@ found [here](https://rust-lang.github.io/rust-clippy/master/index.html#msrv) Very rarely, you may wish to prevent Clippy from evaluating certain sections of code entirely. You can do this with [conditional compilation](https://doc.rust-lang.org/reference/conditional-compilation.html) by checking that the -`cargo-clippy` feature is not set. You may need to provide a stub so that the code compiles: +`clippy` cfg is not set. You may need to provide a stub so that the code compiles: ```rust -#[cfg(not(feature = "cargo-clippy"))] +#[cfg(not(clippy)] include!(concat!(env!("OUT_DIR"), "/my_big_function-generated.rs")); -#[cfg(feature = "cargo-clippy")] +#[cfg(clippy)] fn my_big_function(_input: &str) -> Option { None } ``` - -This feature is not actually part of your crate, so specifying `--all-features` to other tools, e.g. `cargo test ---all-features`, will not disable it. diff --git a/book/src/development/emitting_lints.md b/book/src/development/emitting_lints.md index a12f6aa91b30..d70f4fc17ebf 100644 --- a/book/src/development/emitting_lints.md +++ b/book/src/development/emitting_lints.md @@ -82,7 +82,7 @@ The output looks something like this (from the example earlier): ```text error: an inclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:37:14 + --> tests/ui/range_plus_minus_one.rs:37:14 | LL | for _ in 1..1 + 1 {} | ^^^^^^^^ help: use: `1..=1` @@ -135,14 +135,14 @@ Examples: ```text error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing. - --> $DIR/drop_forget_ref.rs:10:5 + --> tests/ui/drop_forget_ref.rs:10:5 | 10 | forget(&SomeStruct); | ^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::forget-ref` implied by `-D warnings` note: argument has type &SomeStruct - --> $DIR/drop_forget_ref.rs:10:12 + --> tests/ui/drop_forget_ref.rs:10:12 | 10 | forget(&SomeStruct); | ^^^^^^^^^^^ @@ -158,7 +158,7 @@ Example: ```text error: constant division of 0.0 with 0.0 will always result in NaN - --> $DIR/zero_div_zero.rs:6:25 + --> tests/ui/zero_div_zero.rs:6:25 | 6 | let other_f64_nan = 0.0f64 / 0.0; | ^^^^^^^^^^^^ @@ -176,7 +176,7 @@ Example: ```text error: This `.fold` can be more succinctly expressed as `.any` ---> $DIR/methods.rs:390:13 +--> tests/ui/methods.rs:390:13 | 390 | let _ = (0..3).fold(false, |acc, x| acc || x > 2); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.any(|x| x > 2)` diff --git a/book/src/development/writing_tests.md b/book/src/development/writing_tests.md index 8937e0d8e946..39a5ad966885 100644 --- a/book/src/development/writing_tests.md +++ b/book/src/development/writing_tests.md @@ -97,19 +97,19 @@ failures: ---- compile_test stdout ---- normalized stderr: error: function called "foo" - --> $DIR/foo_functions.rs:6:12 + --> tests/ui/foo_functions.rs:6:12 | LL | pub fn foo(&self) {} | ^^^ | = note: `-D clippy::foo-functions` implied by `-D warnings` error: function called "foo" - --> $DIR/foo_functions.rs:13:8 + --> tests/ui/foo_functions.rs:13:8 | LL | fn foo(&self) {} | ^^^ error: function called "foo" - --> $DIR/foo_functions.rs:19:4 + --> tests/ui/foo_functions.rs:19:4 | LL | fn foo() {} | ^^^ diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index f2357e2b5de9..214a60d3bfdf 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -278,7 +278,7 @@ The minimum number of struct fields for the lints about field names to trigger --- **Affected lints:** -* [`struct_variant_names`](https://rust-lang.github.io/rust-clippy/master/index.html#struct_variant_names) +* [`struct_field_names`](https://rust-lang.github.io/rust-clippy/master/index.html#struct_field_names) ## `enum-variant-size-threshold` diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 9741b94d5041..b781259ad969 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -325,7 +325,7 @@ define_Conf! { /// /// The minimum number of enum variants for the lints about variant names to trigger (enum_variant_name_threshold: u64 = 3), - /// Lint: STRUCT_VARIANT_NAMES. + /// Lint: STRUCT_FIELD_NAMES. /// /// The minimum number of struct fields for the lints about field names to trigger (struct_field_name_threshold: u64 = 3), @@ -648,7 +648,7 @@ fn deserialize(file: &SourceFile) -> TryConf { extend_vec_if_indicator_present(&mut conf.conf.doc_valid_idents, DEFAULT_DOC_VALID_IDENTS); extend_vec_if_indicator_present(&mut conf.conf.disallowed_names, DEFAULT_DISALLOWED_NAMES); // TODO: THIS SHOULD BE TESTED, this comment will be gone soon - if conf.conf.allowed_idents_below_min_chars.contains(&"..".to_owned()) { + if conf.conf.allowed_idents_below_min_chars.contains("..") { conf.conf .allowed_idents_below_min_chars .extend(DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS.iter().map(ToString::to_string)); diff --git a/clippy_config/src/lib.rs b/clippy_config/src/lib.rs index 5449feed090a..dab3119894a4 100644 --- a/clippy_config/src/lib.rs +++ b/clippy_config/src/lib.rs @@ -6,7 +6,7 @@ clippy::missing_panics_doc, rustc::diagnostic_outside_of_impl, rustc::untranslatable_diagnostic, - rustc::untranslatable_diagnostic_trivial, + rustc::untranslatable_diagnostic_trivial )] extern crate rustc_ast; diff --git a/clippy_lints/src/asm_syntax.rs b/clippy_lints/src/asm_syntax.rs index feb6437ee26a..c2fa56e13603 100644 --- a/clippy_lints/src/asm_syntax.rs +++ b/clippy_lints/src/asm_syntax.rs @@ -2,8 +2,11 @@ use std::fmt; use clippy_utils::diagnostics::span_lint_and_help; use rustc_ast::ast::{Expr, ExprKind, InlineAsmOptions}; -use rustc_lint::{EarlyContext, EarlyLintPass, Lint}; +use rustc_ast::{InlineAsm, Item, ItemKind}; +use rustc_lint::{EarlyContext, EarlyLintPass, Lint, LintContext}; use rustc_session::declare_lint_pass; +use rustc_span::Span; +use rustc_target::asm::InlineAsmArch; #[derive(Clone, Copy, PartialEq, Eq)] enum AsmStyle { @@ -31,8 +34,14 @@ impl std::ops::Not for AsmStyle { } } -fn check_expr_asm_syntax(lint: &'static Lint, cx: &EarlyContext<'_>, expr: &Expr, check_for: AsmStyle) { - if let ExprKind::InlineAsm(ref inline_asm) = expr.kind { +fn check_asm_syntax( + lint: &'static Lint, + cx: &EarlyContext<'_>, + inline_asm: &InlineAsm, + span: Span, + check_for: AsmStyle, +) { + if matches!(cx.sess().asm_arch, Some(InlineAsmArch::X86 | InlineAsmArch::X86_64)) { let style = if inline_asm.options.contains(InlineAsmOptions::ATT_SYNTAX) { AsmStyle::Att } else { @@ -43,7 +52,7 @@ fn check_expr_asm_syntax(lint: &'static Lint, cx: &EarlyContext<'_>, expr: &Expr span_lint_and_help( cx, lint, - expr.span, + span, &format!("{style} x86 assembly syntax used"), None, &format!("use {} x86 assembly syntax", !style), @@ -89,7 +98,15 @@ declare_lint_pass!(InlineAsmX86IntelSyntax => [INLINE_ASM_X86_INTEL_SYNTAX]); impl EarlyLintPass for InlineAsmX86IntelSyntax { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { - check_expr_asm_syntax(Self::get_lints()[0], cx, expr, AsmStyle::Intel); + if let ExprKind::InlineAsm(inline_asm) = &expr.kind { + check_asm_syntax(Self::get_lints()[0], cx, inline_asm, expr.span, AsmStyle::Intel); + } + } + + fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { + if let ItemKind::GlobalAsm(inline_asm) = &item.kind { + check_asm_syntax(Self::get_lints()[0], cx, inline_asm, item.span, AsmStyle::Intel); + } } } @@ -130,6 +147,14 @@ declare_lint_pass!(InlineAsmX86AttSyntax => [INLINE_ASM_X86_ATT_SYNTAX]); impl EarlyLintPass for InlineAsmX86AttSyntax { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { - check_expr_asm_syntax(Self::get_lints()[0], cx, expr, AsmStyle::Att); + if let ExprKind::InlineAsm(inline_asm) = &expr.kind { + check_asm_syntax(Self::get_lints()[0], cx, inline_asm, expr.span, AsmStyle::Att); + } + } + + fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { + if let ItemKind::GlobalAsm(inline_asm) = &item.kind { + check_asm_syntax(Self::get_lints()[0], cx, inline_asm, item.span, AsmStyle::Att); + } } } diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index da38422874b4..f90a7dbbfe51 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -1,7 +1,9 @@ //! checks for attributes use clippy_config::msrvs::{self, Msrv}; -use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then}; +use clippy_utils::diagnostics::{ + span_lint, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then, +}; use clippy_utils::is_from_proc_macro; use clippy_utils::macros::{is_panic, macro_backtrace}; use clippy_utils::source::{first_line_of_span, is_present_in_source, snippet_opt, without_block_comments}; @@ -433,6 +435,56 @@ declare_clippy_lint! { "prevent from misusing the wrong attr name" } +declare_clippy_lint! { + /// ### What it does + /// Checks for `#[cfg_attr(feature = "cargo-clippy", ...)]` and for + /// `#[cfg(feature = "cargo-clippy")]` and suggests to replace it with + /// `#[cfg_attr(clippy, ...)]` or `#[cfg(clippy)]`. + /// + /// ### Why is this bad? + /// This feature has been deprecated for years and shouldn't be used anymore. + /// + /// ### Example + /// ```no_run + /// #[cfg(feature = "cargo-clippy")] + /// struct Bar; + /// ``` + /// + /// Use instead: + /// ```no_run + /// #[cfg(clippy)] + /// struct Bar; + /// ``` + #[clippy::version = "1.78.0"] + pub DEPRECATED_CLIPPY_CFG_ATTR, + suspicious, + "usage of `cfg(feature = \"cargo-clippy\")` instead of `cfg(clippy)`" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for `#[cfg_attr(clippy, allow(clippy::lint))]` + /// and suggests to replace it with `#[allow(clippy::lint)]`. + /// + /// ### Why is this bad? + /// There is no reason to put clippy attributes behind a clippy `cfg` as they are not + /// run by anything else than clippy. + /// + /// ### Example + /// ```no_run + /// #![cfg_attr(clippy, allow(clippy::deprecated_cfg_attr))] + /// ``` + /// + /// Use instead: + /// ```no_run + /// #![allow(clippy::deprecated_cfg_attr)] + /// ``` + #[clippy::version = "1.78.0"] + pub UNNECESSARY_CLIPPY_CFG, + suspicious, + "usage of `cfg_attr(clippy, allow(clippy::lint))` instead of `allow(clippy::lint)`" +} + declare_lint_pass!(Attributes => [ ALLOW_ATTRIBUTES_WITHOUT_REASON, INLINE_ALWAYS, @@ -794,6 +846,8 @@ impl_lint_pass!(EarlyAttributes => [ EMPTY_LINE_AFTER_DOC_COMMENTS, NON_MINIMAL_CFG, MAYBE_MISUSED_CFG, + DEPRECATED_CLIPPY_CFG_ATTR, + UNNECESSARY_CLIPPY_CFG, ]); impl EarlyLintPass for EarlyAttributes { @@ -803,6 +857,7 @@ impl EarlyLintPass for EarlyAttributes { fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) { check_deprecated_cfg_attr(cx, attr, &self.msrv); + check_deprecated_cfg(cx, attr); check_mismatched_target_os(cx, attr); check_minimal_cfg_condition(cx, attr); check_misused_cfg(cx, attr); @@ -857,39 +912,148 @@ fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::It } } +fn check_cargo_clippy_attr(cx: &EarlyContext<'_>, item: &rustc_ast::MetaItem) { + if item.has_name(sym::feature) && item.value_str().is_some_and(|v| v.as_str() == "cargo-clippy") { + span_lint_and_sugg( + cx, + DEPRECATED_CLIPPY_CFG_ATTR, + item.span, + "`feature = \"cargo-clippy\"` was replaced by `clippy`", + "replace with", + "clippy".to_string(), + Applicability::MachineApplicable, + ); + } +} + +fn check_deprecated_cfg_recursively(cx: &EarlyContext<'_>, attr: &rustc_ast::MetaItem) { + if let Some(ident) = attr.ident() { + if ["any", "all", "not"].contains(&ident.name.as_str()) { + let Some(list) = attr.meta_item_list() else { return }; + for item in list.iter().filter_map(|item| item.meta_item()) { + check_deprecated_cfg_recursively(cx, item); + } + } else { + check_cargo_clippy_attr(cx, attr); + } + } +} + +fn check_deprecated_cfg(cx: &EarlyContext<'_>, attr: &Attribute) { + if attr.has_name(sym::cfg) + && let Some(list) = attr.meta_item_list() + { + for item in list.iter().filter_map(|item| item.meta_item()) { + check_deprecated_cfg_recursively(cx, item); + } + } +} + fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &Msrv) { - if msrv.meets(msrvs::TOOL_ATTRIBUTES) - // check cfg_attr - && attr.has_name(sym::cfg_attr) + // check cfg_attr + if attr.has_name(sym::cfg_attr) && let Some(items) = attr.meta_item_list() && items.len() == 2 - // check for `rustfmt` && let Some(feature_item) = items[0].meta_item() - && feature_item.has_name(sym::rustfmt) - // check for `rustfmt_skip` and `rustfmt::skip` - && let Some(skip_item) = &items[1].meta_item() - && (skip_item.has_name(sym!(rustfmt_skip)) - || skip_item - .path - .segments - .last() - .expect("empty path in attribute") - .ident - .name - == sym::skip) - // Only lint outer attributes, because custom inner attributes are unstable - // Tracking issue: https://github.com/rust-lang/rust/issues/54726 - && attr.style == AttrStyle::Outer { - span_lint_and_sugg( - cx, - DEPRECATED_CFG_ATTR, - attr.span, - "`cfg_attr` is deprecated for rustfmt and got replaced by tool attributes", - "use", - "#[rustfmt::skip]".to_string(), - Applicability::MachineApplicable, - ); + // check for `rustfmt` + if feature_item.has_name(sym::rustfmt) + && msrv.meets(msrvs::TOOL_ATTRIBUTES) + // check for `rustfmt_skip` and `rustfmt::skip` + && let Some(skip_item) = &items[1].meta_item() + && (skip_item.has_name(sym!(rustfmt_skip)) + || skip_item + .path + .segments + .last() + .expect("empty path in attribute") + .ident + .name + == sym::skip) + // Only lint outer attributes, because custom inner attributes are unstable + // Tracking issue: https://github.com/rust-lang/rust/issues/54726 + && attr.style == AttrStyle::Outer + { + span_lint_and_sugg( + cx, + DEPRECATED_CFG_ATTR, + attr.span, + "`cfg_attr` is deprecated for rustfmt and got replaced by tool attributes", + "use", + "#[rustfmt::skip]".to_string(), + Applicability::MachineApplicable, + ); + } else { + check_deprecated_cfg_recursively(cx, feature_item); + if let Some(behind_cfg_attr) = items[1].meta_item() { + check_clippy_cfg_attr(cx, feature_item, behind_cfg_attr, attr); + } + } + } +} + +fn check_clippy_cfg_attr( + cx: &EarlyContext<'_>, + cfg_attr: &rustc_ast::MetaItem, + behind_cfg_attr: &rustc_ast::MetaItem, + attr: &Attribute, +) { + if cfg_attr.has_name(sym::clippy) + && let Some(ident) = behind_cfg_attr.ident() + // FIXME: replace with `from_symbol` once https://github.com/rust-lang/rust/pull/121230 + // is merged. + && Level::from_str(ident.name.as_str()).is_some() + && let Some(items) = behind_cfg_attr.meta_item_list() + { + let nb_items = items.len(); + let mut clippy_lints = Vec::with_capacity(items.len()); + for item in items { + if let Some(meta_item) = item.meta_item() + && let [part1, _] = meta_item.path.segments.as_slice() + && part1.ident.name == sym::clippy + { + clippy_lints.push(item.span()); + } + } + if clippy_lints.is_empty() { + return; + } + if nb_items == clippy_lints.len() { + if let Some(snippet) = snippet_opt(cx, behind_cfg_attr.span) { + span_lint_and_sugg( + cx, + UNNECESSARY_CLIPPY_CFG, + attr.span, + "no need to put clippy lints behind a `clippy` cfg", + "replace with", + format!( + "#{}[{}]", + if attr.style == AttrStyle::Inner { "!" } else { "" }, + snippet + ), + Applicability::MachineApplicable, + ); + } + } else { + let snippet = clippy_lints + .iter() + .filter_map(|sp| snippet_opt(cx, *sp)) + .collect::>() + .join(","); + span_lint_and_note( + cx, + UNNECESSARY_CLIPPY_CFG, + clippy_lints, + "no need to put clippy lints behind a `clippy` cfg", + None, + &format!( + "write instead: `#{}[{}({})]`", + if attr.style == AttrStyle::Inner { "!" } else { "" }, + ident.name, + snippet + ), + ); + } } } diff --git a/clippy_lints/src/blocks_in_conditions.rs b/clippy_lints/src/blocks_in_conditions.rs index ff4dffd06079..2eb0dac97425 100644 --- a/clippy_lints/src/blocks_in_conditions.rs +++ b/clippy_lints/src/blocks_in_conditions.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg}; use clippy_utils::source::snippet_block_with_applicability; use clippy_utils::ty::implements_trait; use clippy_utils::visitors::{for_each_expr, Descend}; -use clippy_utils::{get_parent_expr, higher}; +use clippy_utils::{get_parent_expr, higher, is_from_proc_macro}; use core::ops::ControlFlow; use rustc_errors::Applicability; use rustc_hir::{BlockCheckMode, Expr, ExprKind, MatchSource}; @@ -13,7 +13,7 @@ use rustc_span::sym; declare_clippy_lint! { /// ### What it does - /// Checks for `if` conditions that use blocks containing an + /// Checks for `if` and `match` conditions that use blocks containing an /// expression, statements or conditions that use closures with blocks. /// /// ### Why is this bad? @@ -25,6 +25,11 @@ declare_clippy_lint! { /// if { true } { /* ... */ } /// /// if { let x = somefunc(); x } { /* ... */ } + /// + /// match { let e = somefunc(); e } { + /// // ... + /// # _ => {} + /// } /// ``` /// /// Use instead: @@ -34,6 +39,12 @@ declare_clippy_lint! { /// /// let res = { let x = somefunc(); x }; /// if res { /* ... */ } + /// + /// let res = { let e = somefunc(); e }; + /// match res { + /// // ... + /// # _ => {} + /// } /// ``` #[clippy::version = "1.45.0"] pub BLOCKS_IN_CONDITIONS, @@ -94,7 +105,7 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInConditions { } } else { let span = block.expr.as_ref().map_or_else(|| block.stmts[0].span, |e| e.span); - if span.from_expansion() || expr.span.from_expansion() { + if span.from_expansion() || expr.span.from_expansion() || is_from_proc_macro(cx, cond) { return; } // move block higher diff --git a/clippy_lints/src/booleans.rs b/clippy_lints/src/booleans.rs index 2d1c250ace90..0d66f2d644d9 100644 --- a/clippy_lints/src/booleans.rs +++ b/clippy_lints/src/booleans.rs @@ -85,7 +85,117 @@ impl<'tcx> LateLintPass<'tcx> for NonminimalBool { ) { NonminimalBoolVisitor { cx }.visit_body(body); } + + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + match expr.kind { + ExprKind::Unary(UnOp::Not, sub) => check_inverted_condition(cx, expr.span, sub), + // This check the case where an element in a boolean comparison is inverted, like: + // + // ``` + // let a = true; + // !a == false; + // ``` + ExprKind::Binary(op, left, right) if matches!(op.node, BinOpKind::Eq | BinOpKind::Ne) => { + check_inverted_bool_in_condition(cx, expr.span, op.node, left, right); + }, + _ => {}, + } + } +} + +fn inverted_bin_op_eq_str(op: BinOpKind) -> Option<&'static str> { + match op { + BinOpKind::Eq => Some("!="), + BinOpKind::Ne => Some("=="), + _ => None, + } +} + +fn bin_op_eq_str(op: BinOpKind) -> Option<&'static str> { + match op { + BinOpKind::Eq => Some("=="), + BinOpKind::Ne => Some("!="), + _ => None, + } +} + +fn check_inverted_condition(cx: &LateContext<'_>, expr_span: Span, sub_expr: &Expr<'_>) { + if !expr_span.from_expansion() + && let ExprKind::Binary(op, left, right) = sub_expr.kind + && let Some(left) = snippet_opt(cx, left.span) + && let Some(right) = snippet_opt(cx, right.span) + { + let Some(op) = inverted_bin_op_eq_str(op.node) else { + return; + }; + span_lint_and_sugg( + cx, + NONMINIMAL_BOOL, + expr_span, + "this boolean expression can be simplified", + "try", + format!("{left} {op} {right}",), + Applicability::MachineApplicable, + ); + } +} + +fn check_inverted_bool_in_condition( + cx: &LateContext<'_>, + expr_span: Span, + op: BinOpKind, + left: &Expr<'_>, + right: &Expr<'_>, +) { + if expr_span.from_expansion() + && (!cx.typeck_results().node_types()[left.hir_id].is_bool() + || !cx.typeck_results().node_types()[right.hir_id].is_bool()) + { + return; + } + + let suggestion = match (left.kind, right.kind) { + (ExprKind::Unary(UnOp::Not, left_sub), ExprKind::Unary(UnOp::Not, right_sub)) => { + let Some(left) = snippet_opt(cx, left_sub.span) else { + return; + }; + let Some(right) = snippet_opt(cx, right_sub.span) else { + return; + }; + let Some(op) = bin_op_eq_str(op) else { return }; + format!("{left} {op} {right}") + }, + (ExprKind::Unary(UnOp::Not, left_sub), _) => { + let Some(left) = snippet_opt(cx, left_sub.span) else { + return; + }; + let Some(right) = snippet_opt(cx, right.span) else { + return; + }; + let Some(op) = inverted_bin_op_eq_str(op) else { return }; + format!("{left} {op} {right}") + }, + (_, ExprKind::Unary(UnOp::Not, right_sub)) => { + let Some(left) = snippet_opt(cx, left.span) else { return }; + let Some(right) = snippet_opt(cx, right_sub.span) else { + return; + }; + let Some(op) = inverted_bin_op_eq_str(op) else { return }; + format!("{left} {op} {right}") + }, + _ => return, + }; + span_lint_and_sugg( + cx, + NONMINIMAL_BOOL, + expr_span, + "this boolean expression can be simplified", + "try", + suggestion, + Applicability::MachineApplicable, + ); } + struct NonminimalBoolVisitor<'a, 'tcx> { cx: &'a LateContext<'tcx>, } diff --git a/clippy_lints/src/box_default.rs b/clippy_lints/src/box_default.rs index ef12fe344e40..d71cca6aa4f4 100644 --- a/clippy_lints/src/box_default.rs +++ b/clippy_lints/src/box_default.rs @@ -101,25 +101,22 @@ impl<'tcx> Visitor<'tcx> for InferVisitor { fn given_type(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { match get_parent_node(cx.tcx, expr.hir_id) { - Some(Node::Local(Local { ty: Some(ty), .. })) => { + Node::Local(Local { ty: Some(ty), .. }) => { let mut v = InferVisitor::default(); v.visit_ty(ty); !v.0 }, - Some( - Node::Expr(Expr { + Node::Expr(Expr { + kind: ExprKind::Call(path, args), + .. + }) + | Node::Block(Block { + expr: Some(Expr { kind: ExprKind::Call(path, args), .. - }) - | Node::Block(Block { - expr: - Some(Expr { - kind: ExprKind::Call(path, args), - .. - }), - .. }), - ) => { + .. + }) => { if let Some(index) = args.iter().position(|arg| arg.hir_id == expr.hir_id) && let Some(sig) = expr_sig(cx, path) && let Some(input) = sig.input(index) diff --git a/clippy_lints/src/casts/cast_possible_truncation.rs b/clippy_lints/src/casts/cast_possible_truncation.rs index 1543ae803996..ab89bb2f5f15 100644 --- a/clippy_lints/src/casts/cast_possible_truncation.rs +++ b/clippy_lints/src/casts/cast_possible_truncation.rs @@ -131,8 +131,7 @@ pub(super) fn check( let cast_from_ptr_size = def.repr().int.map_or(true, |ty| matches!(ty, IntegerType::Pointer(_),)); let suffix = match (cast_from_ptr_size, is_isize_or_usize(cast_to)) { - (false, false) if from_nbits > to_nbits => "", - (true, false) if from_nbits > to_nbits => "", + (_, false) if from_nbits > to_nbits => "", (false, true) if from_nbits > 64 => "", (false, true) if from_nbits > 32 => " on targets with 32-bit wide pointers", _ => return, diff --git a/clippy_lints/src/casts/unnecessary_cast.rs b/clippy_lints/src/casts/unnecessary_cast.rs index b4a23d0d4db4..3800d32eedc7 100644 --- a/clippy_lints/src/casts/unnecessary_cast.rs +++ b/clippy_lints/src/casts/unnecessary_cast.rs @@ -264,8 +264,7 @@ fn is_cast_from_ty_alias<'tcx>(cx: &LateContext<'tcx>, expr: impl Visitable<'tcx } // Local usage } else if let Res::Local(hir_id) = res - && let Some(parent) = get_parent_node(cx.tcx, hir_id) - && let Node::Local(l) = parent + && let Node::Local(l) = get_parent_node(cx.tcx, hir_id) { if let Some(e) = l.init && is_cast_from_ty_alias(cx, e, cast_from) diff --git a/clippy_lints/src/collection_is_never_read.rs b/clippy_lints/src/collection_is_never_read.rs index d0c989cfff30..5ab52312843e 100644 --- a/clippy_lints/src/collection_is_never_read.rs +++ b/clippy_lints/src/collection_is_never_read.rs @@ -94,7 +94,7 @@ fn has_no_read_access<'tcx>(cx: &LateContext<'tcx>, id: HirId, block: &'tcx Bloc // `id` appearing in the left-hand side of an assignment is not a read access: // // id = ...; // Not reading `id`. - if let Some(Node::Expr(parent)) = get_parent_node(cx.tcx, expr.hir_id) + if let Node::Expr(parent) = get_parent_node(cx.tcx, expr.hir_id) && let ExprKind::Assign(lhs, ..) = parent.kind && path_to_local_id(lhs, id) { @@ -108,7 +108,7 @@ fn has_no_read_access<'tcx>(cx: &LateContext<'tcx>, id: HirId, block: &'tcx Bloc // Only assuming this for "official" methods defined on the type. For methods defined in extension // traits (identified as local, based on the orphan rule), pessimistically assume that they might // have side effects, so consider them a read. - if let Some(Node::Expr(parent)) = get_parent_node(cx.tcx, expr.hir_id) + if let Node::Expr(parent) = get_parent_node(cx.tcx, expr.hir_id) && let ExprKind::MethodCall(_, receiver, _, _) = parent.kind && path_to_local_id(receiver, id) && let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(parent.hir_id) @@ -117,7 +117,7 @@ fn has_no_read_access<'tcx>(cx: &LateContext<'tcx>, id: HirId, block: &'tcx Bloc // The method call is a statement, so the return value is not used. That's not a read access: // // id.foo(args); - if let Some(Node::Stmt(..)) = get_parent_node(cx.tcx, parent.hir_id) { + if let Node::Stmt(..) = get_parent_node(cx.tcx, parent.hir_id) { return ControlFlow::Continue(()); } diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 0a5baabd973e..76595e2e26e3 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -51,6 +51,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::attrs::ALLOW_ATTRIBUTES_WITHOUT_REASON_INFO, crate::attrs::BLANKET_CLIPPY_RESTRICTION_LINTS_INFO, crate::attrs::DEPRECATED_CFG_ATTR_INFO, + crate::attrs::DEPRECATED_CLIPPY_CFG_ATTR_INFO, crate::attrs::DEPRECATED_SEMVER_INFO, crate::attrs::EMPTY_LINE_AFTER_DOC_COMMENTS_INFO, crate::attrs::EMPTY_LINE_AFTER_OUTER_ATTR_INFO, @@ -59,6 +60,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::attrs::MISMATCHED_TARGET_OS_INFO, crate::attrs::NON_MINIMAL_CFG_INFO, crate::attrs::SHOULD_PANIC_WITHOUT_EXPECT_INFO, + crate::attrs::UNNECESSARY_CLIPPY_CFG_INFO, crate::attrs::USELESS_ATTRIBUTE_INFO, crate::await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE_INFO, crate::await_holding_invalid::AWAIT_HOLDING_LOCK_INFO, diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index 0ddfeaa0ae05..e6eb5088c2b2 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -1009,7 +1009,7 @@ fn report<'tcx>( state.msg, |diag| { let (precedence, calls_field) = match get_parent_node(cx.tcx, data.first_expr.hir_id) { - Some(Node::Expr(e)) => match e.kind { + Node::Expr(e) => match e.kind { ExprKind::Call(callee, _) if callee.hir_id != data.first_expr.hir_id => (0, false), ExprKind::Call(..) => (PREC_POSTFIX, matches!(expr.kind, ExprKind::Field(..))), _ => (e.precedence().order(), false), diff --git a/clippy_lints/src/disallowed_macros.rs b/clippy_lints/src/disallowed_macros.rs index 656b3d9bfaf1..75379cb4e545 100644 --- a/clippy_lints/src/disallowed_macros.rs +++ b/clippy_lints/src/disallowed_macros.rs @@ -1,13 +1,16 @@ use clippy_config::types::DisallowedPath; -use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then}; use clippy_utils::macros::macro_backtrace; use rustc_ast::Attribute; use rustc_data_structures::fx::FxHashSet; +use rustc_errors::DiagnosticBuilder; use rustc_hir::def_id::DefIdMap; -use rustc_hir::{Expr, ExprKind, ForeignItem, HirId, ImplItem, Item, Pat, Path, Stmt, TraitItem, Ty}; +use rustc_hir::{ + Expr, ExprKind, ForeignItem, HirId, ImplItem, Item, ItemKind, OwnerId, Pat, Path, Stmt, TraitItem, Ty, +}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::impl_lint_pass; -use rustc_span::{ExpnId, Span}; +use rustc_span::{ExpnId, MacroKind, Span}; declare_clippy_lint! { /// ### What it does @@ -57,6 +60,10 @@ pub struct DisallowedMacros { conf_disallowed: Vec, disallowed: DefIdMap, seen: FxHashSet, + + // Track the most recently seen node that can have a `derive` attribute. + // Needed to use the correct lint level. + derive_src: Option, } impl DisallowedMacros { @@ -65,10 +72,11 @@ impl DisallowedMacros { conf_disallowed, disallowed: DefIdMap::default(), seen: FxHashSet::default(), + derive_src: None, } } - fn check(&mut self, cx: &LateContext<'_>, span: Span) { + fn check(&mut self, cx: &LateContext<'_>, span: Span, derive_src: Option) { if self.conf_disallowed.is_empty() { return; } @@ -80,18 +88,26 @@ impl DisallowedMacros { if let Some(&index) = self.disallowed.get(&mac.def_id) { let conf = &self.conf_disallowed[index]; - - span_lint_and_then( - cx, - DISALLOWED_MACROS, - mac.span, - &format!("use of a disallowed macro `{}`", conf.path()), - |diag| { - if let Some(reason) = conf.reason() { - diag.note(reason); - } - }, - ); + let msg = format!("use of a disallowed macro `{}`", conf.path()); + let add_note = |diag: &mut DiagnosticBuilder<'_, _>| { + if let Some(reason) = conf.reason() { + diag.note(reason); + } + }; + if matches!(mac.kind, MacroKind::Derive) + && let Some(derive_src) = derive_src + { + span_lint_hir_and_then( + cx, + DISALLOWED_MACROS, + cx.tcx.local_def_id_to_hir_id(derive_src.def_id), + mac.span, + &msg, + add_note, + ); + } else { + span_lint_and_then(cx, DISALLOWED_MACROS, mac.span, &msg, add_note); + } } } } @@ -110,49 +126,57 @@ impl LateLintPass<'_> for DisallowedMacros { } fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { - self.check(cx, expr.span); + self.check(cx, expr.span, None); // `$t + $t` can have the context of $t, check also the span of the binary operator if let ExprKind::Binary(op, ..) = expr.kind { - self.check(cx, op.span); + self.check(cx, op.span, None); } } fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) { - self.check(cx, stmt.span); + self.check(cx, stmt.span, None); } fn check_ty(&mut self, cx: &LateContext<'_>, ty: &Ty<'_>) { - self.check(cx, ty.span); + self.check(cx, ty.span, None); } fn check_pat(&mut self, cx: &LateContext<'_>, pat: &Pat<'_>) { - self.check(cx, pat.span); + self.check(cx, pat.span, None); } fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { - self.check(cx, item.span); - self.check(cx, item.vis_span); + self.check(cx, item.span, self.derive_src); + self.check(cx, item.vis_span, None); + + if matches!( + item.kind, + ItemKind::Struct(..) | ItemKind::Enum(..) | ItemKind::Union(..) + ) && macro_backtrace(item.span).all(|m| !matches!(m.kind, MacroKind::Derive)) + { + self.derive_src = Some(item.owner_id); + } } fn check_foreign_item(&mut self, cx: &LateContext<'_>, item: &ForeignItem<'_>) { - self.check(cx, item.span); - self.check(cx, item.vis_span); + self.check(cx, item.span, None); + self.check(cx, item.vis_span, None); } fn check_impl_item(&mut self, cx: &LateContext<'_>, item: &ImplItem<'_>) { - self.check(cx, item.span); - self.check(cx, item.vis_span); + self.check(cx, item.span, None); + self.check(cx, item.vis_span, None); } fn check_trait_item(&mut self, cx: &LateContext<'_>, item: &TraitItem<'_>) { - self.check(cx, item.span); + self.check(cx, item.span, None); } fn check_path(&mut self, cx: &LateContext<'_>, path: &Path<'_>, _: HirId) { - self.check(cx, path.span); + self.check(cx, path.span, None); } fn check_attribute(&mut self, cx: &LateContext<'_>, attr: &Attribute) { - self.check(cx, attr.span); + self.check(cx, attr.span, self.derive_src); } } diff --git a/clippy_lints/src/drop_forget_ref.rs b/clippy_lints/src/drop_forget_ref.rs index 124d78fc4ffd..da1dcba7d339 100644 --- a/clippy_lints/src/drop_forget_ref.rs +++ b/clippy_lints/src/drop_forget_ref.rs @@ -144,8 +144,7 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetRef { // } fn is_single_call_in_arm<'tcx>(cx: &LateContext<'tcx>, arg: &'tcx Expr<'_>, drop_expr: &'tcx Expr<'_>) -> bool { if matches!(arg.kind, ExprKind::Call(..) | ExprKind::MethodCall(..)) { - let parent_node = get_parent_node(cx.tcx, drop_expr.hir_id); - if let Some(Node::Arm(Arm { body, .. })) = &parent_node { + if let Node::Arm(Arm { body, .. }) = get_parent_node(cx.tcx, drop_expr.hir_id) { return body.hir_id == drop_expr.hir_id; } } diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index 8af321e4d553..61f550ce0beb 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -4,7 +4,7 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use clippy_utils::is_diag_trait_item; use clippy_utils::macros::{ find_format_arg_expr, find_format_args, format_arg_removal_span, format_placeholder_format_span, is_assert_macro, - is_format_macro, is_panic, root_macro_call, root_macro_call_first_node, FormatParamUsage, + is_format_macro, is_panic, root_macro_call, root_macro_call_first_node, FormatParamUsage, MacroCall, }; use clippy_utils::source::snippet_opt; use clippy_utils::ty::{implements_trait, is_type_lang_item}; @@ -20,7 +20,6 @@ use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::ty::adjustment::{Adjust, Adjustment}; use rustc_middle::ty::Ty; use rustc_session::impl_lint_pass; -use rustc_span::def_id::DefId; use rustc_span::edition::Edition::Edition2021; use rustc_span::{sym, Span, Symbol}; @@ -189,32 +188,18 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs { && is_format_macro(cx, macro_call.def_id) && let Some(format_args) = find_format_args(cx, expr, macro_call.expn) { - for piece in &format_args.template { - if let FormatArgsPiece::Placeholder(placeholder) = piece - && let Ok(index) = placeholder.argument.index - && let Some(arg) = format_args.arguments.all_args().get(index) - { - let arg_expr = find_format_arg_expr(expr, arg); - - check_unused_format_specifier(cx, placeholder, arg_expr); - - if placeholder.format_trait != FormatTrait::Display - || placeholder.format_options != FormatOptions::default() - || is_aliased(&format_args, index) - { - continue; - } + let linter = FormatArgsExpr { + cx, + expr, + macro_call: ¯o_call, + format_args: &format_args, + ignore_mixed: self.ignore_mixed, + }; - if let Ok(arg_hir_expr) = arg_expr { - let name = cx.tcx.item_name(macro_call.def_id); - check_format_in_format_args(cx, macro_call.span, name, arg_hir_expr); - check_to_string_in_format_args(cx, name, arg_hir_expr); - } - } - } + linter.check_templates(); if self.msrv.meets(msrvs::FORMAT_ARGS_CAPTURE) { - check_uninlined_args(cx, &format_args, macro_call.span, macro_call.def_id, self.ignore_mixed); + linter.check_uninlined_args(); } } } @@ -222,255 +207,279 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs { extract_msrv_attr!(LateContext); } -fn check_unused_format_specifier( - cx: &LateContext<'_>, - placeholder: &FormatPlaceholder, - arg_expr: Result<&Expr<'_>, &rustc_ast::Expr>, -) { - let ty_or_ast_expr = arg_expr.map(|expr| cx.typeck_results().expr_ty(expr).peel_refs()); - - let is_format_args = match ty_or_ast_expr { - Ok(ty) => is_type_lang_item(cx, ty, LangItem::FormatArguments), - Err(expr) => matches!(expr.peel_parens_and_refs().kind, rustc_ast::ExprKind::FormatArgs(_)), - }; - - let options = &placeholder.format_options; - - let arg_span = match arg_expr { - Ok(expr) => expr.span, - Err(expr) => expr.span, - }; - - if let Some(placeholder_span) = placeholder.span - && is_format_args - && *options != FormatOptions::default() - { - span_lint_and_then( - cx, - UNUSED_FORMAT_SPECS, - placeholder_span, - "format specifiers have no effect on `format_args!()`", - |diag| { - let mut suggest_format = |spec| { - let message = format!("for the {spec} to apply consider using `format!()`"); - - if let Some(mac_call) = root_macro_call(arg_span) - && cx.tcx.is_diagnostic_item(sym::format_args_macro, mac_call.def_id) - { - diag.span_suggestion( - cx.sess().source_map().span_until_char(mac_call.span, '!'), - message, - "format", - Applicability::MaybeIncorrect, - ); - } else { - diag.help(message); - } - }; +struct FormatArgsExpr<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, + macro_call: &'a MacroCall, + format_args: &'a rustc_ast::FormatArgs, + ignore_mixed: bool, +} - if options.width.is_some() { - suggest_format("width"); +impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> { + fn check_templates(&self) { + for piece in &self.format_args.template { + if let FormatArgsPiece::Placeholder(placeholder) = piece + && let Ok(index) = placeholder.argument.index + && let Some(arg) = self.format_args.arguments.all_args().get(index) + { + let arg_expr = find_format_arg_expr(self.expr, arg); + + self.check_unused_format_specifier(placeholder, arg_expr); + + if let Ok(arg_expr) = arg_expr + && placeholder.format_trait == FormatTrait::Display + && placeholder.format_options == FormatOptions::default() + && !self.is_aliased(index) + { + let name = self.cx.tcx.item_name(self.macro_call.def_id); + self.check_format_in_format_args(name, arg_expr); + self.check_to_string_in_format_args(name, arg_expr); } + } + } + } - if options.precision.is_some() { - suggest_format("precision"); - } + fn check_unused_format_specifier( + &self, + placeholder: &FormatPlaceholder, + arg_expr: Result<&Expr<'_>, &rustc_ast::Expr>, + ) { + let ty_or_ast_expr = arg_expr.map(|expr| self.cx.typeck_results().expr_ty(expr).peel_refs()); - if let Some(format_span) = format_placeholder_format_span(placeholder) { - diag.span_suggestion_verbose( - format_span, - "if the current behavior is intentional, remove the format specifiers", - "", - Applicability::MaybeIncorrect, - ); - } - }, - ); - } -} + let is_format_args = match ty_or_ast_expr { + Ok(ty) => is_type_lang_item(self.cx, ty, LangItem::FormatArguments), + Err(expr) => matches!(expr.peel_parens_and_refs().kind, rustc_ast::ExprKind::FormatArgs(_)), + }; -fn check_uninlined_args( - cx: &LateContext<'_>, - args: &rustc_ast::FormatArgs, - call_site: Span, - def_id: DefId, - ignore_mixed: bool, -) { - if args.span.from_expansion() { - return; - } - if call_site.edition() < Edition2021 && (is_panic(cx, def_id) || is_assert_macro(cx, def_id)) { - // panic!, assert!, and debug_assert! before 2021 edition considers a single string argument as - // non-format - return; + let options = &placeholder.format_options; + + let arg_span = match arg_expr { + Ok(expr) => expr.span, + Err(expr) => expr.span, + }; + + if let Some(placeholder_span) = placeholder.span + && is_format_args + && *options != FormatOptions::default() + { + span_lint_and_then( + self.cx, + UNUSED_FORMAT_SPECS, + placeholder_span, + "format specifiers have no effect on `format_args!()`", + |diag| { + let mut suggest_format = |spec| { + let message = format!("for the {spec} to apply consider using `format!()`"); + + if let Some(mac_call) = root_macro_call(arg_span) + && self.cx.tcx.is_diagnostic_item(sym::format_args_macro, mac_call.def_id) + { + diag.span_suggestion( + self.cx.sess().source_map().span_until_char(mac_call.span, '!'), + message, + "format", + Applicability::MaybeIncorrect, + ); + } else { + diag.help(message); + } + }; + + if options.width.is_some() { + suggest_format("width"); + } + + if options.precision.is_some() { + suggest_format("precision"); + } + + if let Some(format_span) = format_placeholder_format_span(placeholder) { + diag.span_suggestion_verbose( + format_span, + "if the current behavior is intentional, remove the format specifiers", + "", + Applicability::MaybeIncorrect, + ); + } + }, + ); + } } - let mut fixes = Vec::new(); - // If any of the arguments are referenced by an index number, - // and that argument is not a simple variable and cannot be inlined, - // we cannot remove any other arguments in the format string, - // because the index numbers might be wrong after inlining. - // Example of an un-inlinable format: print!("{}{1}", foo, 2) - for (pos, usage) in format_arg_positions(args) { - if !check_one_arg(args, pos, usage, &mut fixes, ignore_mixed) { + fn check_uninlined_args(&self) { + if self.format_args.span.from_expansion() { + return; + } + if self.macro_call.span.edition() < Edition2021 + && (is_panic(self.cx, self.macro_call.def_id) || is_assert_macro(self.cx, self.macro_call.def_id)) + { + // panic!, assert!, and debug_assert! before 2021 edition considers a single string argument as + // non-format return; } - } - if fixes.is_empty() { - return; - } + let mut fixes = Vec::new(); + // If any of the arguments are referenced by an index number, + // and that argument is not a simple variable and cannot be inlined, + // we cannot remove any other arguments in the format string, + // because the index numbers might be wrong after inlining. + // Example of an un-inlinable format: print!("{}{1}", foo, 2) + for (pos, usage) in self.format_arg_positions() { + if !self.check_one_arg(pos, usage, &mut fixes) { + return; + } + } - // multiline span display suggestion is sometimes broken: https://github.com/rust-lang/rust/pull/102729#discussion_r988704308 - // in those cases, make the code suggestion hidden - let multiline_fix = fixes.iter().any(|(span, _)| cx.sess().source_map().is_multiline(*span)); - - // Suggest removing each argument only once, for example in `format!("{0} {0}", arg)`. - fixes.sort_unstable_by_key(|(span, _)| *span); - fixes.dedup_by_key(|(span, _)| *span); - - span_lint_and_then( - cx, - UNINLINED_FORMAT_ARGS, - call_site, - "variables can be used directly in the `format!` string", - |diag| { - diag.multipart_suggestion_with_style( - "change this to", - fixes, - Applicability::MachineApplicable, - if multiline_fix { CompletelyHidden } else { ShowCode }, - ); - }, - ); -} + if fixes.is_empty() { + return; + } -fn check_one_arg( - args: &rustc_ast::FormatArgs, - pos: &FormatArgPosition, - usage: FormatParamUsage, - fixes: &mut Vec<(Span, String)>, - ignore_mixed: bool, -) -> bool { - let index = pos.index.unwrap(); - let arg = &args.arguments.all_args()[index]; - - if !matches!(arg.kind, FormatArgumentKind::Captured(_)) - && let rustc_ast::ExprKind::Path(None, path) = &arg.expr.kind - && let [segment] = path.segments.as_slice() - && segment.args.is_none() - && let Some(arg_span) = format_arg_removal_span(args, index) - && let Some(pos_span) = pos.span - { - let replacement = match usage { - FormatParamUsage::Argument => segment.ident.name.to_string(), - FormatParamUsage::Width => format!("{}$", segment.ident.name), - FormatParamUsage::Precision => format!(".{}$", segment.ident.name), - }; - fixes.push((pos_span, replacement)); - fixes.push((arg_span, String::new())); - true // successful inlining, continue checking - } else { - // Do not continue inlining (return false) in case - // * if we can't inline a numbered argument, e.g. `print!("{0} ...", foo.bar, ...)` - // * if allow_mixed_uninlined_format_args is false and this arg hasn't been inlined already - pos.kind != FormatArgPositionKind::Number - && (!ignore_mixed || matches!(arg.kind, FormatArgumentKind::Captured(_))) - } -} + // multiline span display suggestion is sometimes broken: https://github.com/rust-lang/rust/pull/102729#discussion_r988704308 + // in those cases, make the code suggestion hidden + let multiline_fix = fixes + .iter() + .any(|(span, _)| self.cx.sess().source_map().is_multiline(*span)); -fn check_format_in_format_args(cx: &LateContext<'_>, call_site: Span, name: Symbol, arg: &Expr<'_>) { - let expn_data = arg.span.ctxt().outer_expn_data(); - if expn_data.call_site.from_expansion() { - return; - } - let Some(mac_id) = expn_data.macro_def_id else { return }; - if !cx.tcx.is_diagnostic_item(sym::format_macro, mac_id) { - return; + // Suggest removing each argument only once, for example in `format!("{0} {0}", arg)`. + fixes.sort_unstable_by_key(|(span, _)| *span); + fixes.dedup_by_key(|(span, _)| *span); + + span_lint_and_then( + self.cx, + UNINLINED_FORMAT_ARGS, + self.macro_call.span, + "variables can be used directly in the `format!` string", + |diag| { + diag.multipart_suggestion_with_style( + "change this to", + fixes, + Applicability::MachineApplicable, + if multiline_fix { CompletelyHidden } else { ShowCode }, + ); + }, + ); } - span_lint_and_then( - cx, - FORMAT_IN_FORMAT_ARGS, - call_site, - &format!("`format!` in `{name}!` args"), - |diag| { - diag.help(format!( - "combine the `format!(..)` arguments with the outer `{name}!(..)` call" - )); - diag.help("or consider changing `format!` to `format_args!`"); - }, - ); -} -fn check_to_string_in_format_args(cx: &LateContext<'_>, name: Symbol, value: &Expr<'_>) { - if !value.span.from_expansion() - && let ExprKind::MethodCall(_, receiver, [], to_string_span) = value.kind - && let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(value.hir_id) - && is_diag_trait_item(cx, method_def_id, sym::ToString) - && let receiver_ty = cx.typeck_results().expr_ty(receiver) - && let Some(display_trait_id) = cx.tcx.get_diagnostic_item(sym::Display) - && let (n_needed_derefs, target) = - count_needed_derefs(receiver_ty, cx.typeck_results().expr_adjustments(receiver).iter()) - && implements_trait(cx, target, display_trait_id, &[]) - && let Some(sized_trait_id) = cx.tcx.lang_items().sized_trait() - && let Some(receiver_snippet) = snippet_opt(cx, receiver.span) - { - let needs_ref = !implements_trait(cx, receiver_ty, sized_trait_id, &[]); - if n_needed_derefs == 0 && !needs_ref { - span_lint_and_sugg( - cx, - TO_STRING_IN_FORMAT_ARGS, - to_string_span.with_lo(receiver.span.hi()), - &format!("`to_string` applied to a type that implements `Display` in `{name}!` args"), - "remove this", - String::new(), - Applicability::MachineApplicable, - ); + fn check_one_arg(&self, pos: &FormatArgPosition, usage: FormatParamUsage, fixes: &mut Vec<(Span, String)>) -> bool { + let index = pos.index.unwrap(); + let arg = &self.format_args.arguments.all_args()[index]; + + if !matches!(arg.kind, FormatArgumentKind::Captured(_)) + && let rustc_ast::ExprKind::Path(None, path) = &arg.expr.kind + && let [segment] = path.segments.as_slice() + && segment.args.is_none() + && let Some(arg_span) = format_arg_removal_span(self.format_args, index) + && let Some(pos_span) = pos.span + { + let replacement = match usage { + FormatParamUsage::Argument => segment.ident.name.to_string(), + FormatParamUsage::Width => format!("{}$", segment.ident.name), + FormatParamUsage::Precision => format!(".{}$", segment.ident.name), + }; + fixes.push((pos_span, replacement)); + fixes.push((arg_span, String::new())); + true // successful inlining, continue checking } else { - span_lint_and_sugg( - cx, - TO_STRING_IN_FORMAT_ARGS, - value.span, - &format!("`to_string` applied to a type that implements `Display` in `{name}!` args"), - "use this", - format!( - "{}{:*>n_needed_derefs$}{receiver_snippet}", - if needs_ref { "&" } else { "" }, - "" - ), - Applicability::MachineApplicable, - ); + // Do not continue inlining (return false) in case + // * if we can't inline a numbered argument, e.g. `print!("{0} ...", foo.bar, ...)` + // * if allow_mixed_uninlined_format_args is false and this arg hasn't been inlined already + pos.kind != FormatArgPositionKind::Number + && (!self.ignore_mixed || matches!(arg.kind, FormatArgumentKind::Captured(_))) } } -} -fn format_arg_positions( - format_args: &rustc_ast::FormatArgs, -) -> impl Iterator { - format_args.template.iter().flat_map(|piece| match piece { - FormatArgsPiece::Placeholder(placeholder) => { - let mut positions = ArrayVec::<_, 3>::new(); + fn check_format_in_format_args(&self, name: Symbol, arg: &Expr<'_>) { + let expn_data = arg.span.ctxt().outer_expn_data(); + if expn_data.call_site.from_expansion() { + return; + } + let Some(mac_id) = expn_data.macro_def_id else { return }; + if !self.cx.tcx.is_diagnostic_item(sym::format_macro, mac_id) { + return; + } + span_lint_and_then( + self.cx, + FORMAT_IN_FORMAT_ARGS, + self.macro_call.span, + &format!("`format!` in `{name}!` args"), + |diag| { + diag.help(format!( + "combine the `format!(..)` arguments with the outer `{name}!(..)` call" + )); + diag.help("or consider changing `format!` to `format_args!`"); + }, + ); + } - positions.push((&placeholder.argument, FormatParamUsage::Argument)); - if let Some(FormatCount::Argument(position)) = &placeholder.format_options.width { - positions.push((position, FormatParamUsage::Width)); - } - if let Some(FormatCount::Argument(position)) = &placeholder.format_options.precision { - positions.push((position, FormatParamUsage::Precision)); + fn check_to_string_in_format_args(&self, name: Symbol, value: &Expr<'_>) { + let cx = self.cx; + if !value.span.from_expansion() + && let ExprKind::MethodCall(_, receiver, [], to_string_span) = value.kind + && let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(value.hir_id) + && is_diag_trait_item(cx, method_def_id, sym::ToString) + && let receiver_ty = cx.typeck_results().expr_ty(receiver) + && let Some(display_trait_id) = cx.tcx.get_diagnostic_item(sym::Display) + && let (n_needed_derefs, target) = + count_needed_derefs(receiver_ty, cx.typeck_results().expr_adjustments(receiver).iter()) + && implements_trait(cx, target, display_trait_id, &[]) + && let Some(sized_trait_id) = cx.tcx.lang_items().sized_trait() + && let Some(receiver_snippet) = snippet_opt(cx, receiver.span) + { + let needs_ref = !implements_trait(cx, receiver_ty, sized_trait_id, &[]); + if n_needed_derefs == 0 && !needs_ref { + span_lint_and_sugg( + cx, + TO_STRING_IN_FORMAT_ARGS, + to_string_span.with_lo(receiver.span.hi()), + &format!("`to_string` applied to a type that implements `Display` in `{name}!` args"), + "remove this", + String::new(), + Applicability::MachineApplicable, + ); + } else { + span_lint_and_sugg( + cx, + TO_STRING_IN_FORMAT_ARGS, + value.span, + &format!("`to_string` applied to a type that implements `Display` in `{name}!` args"), + "use this", + format!( + "{}{:*>n_needed_derefs$}{receiver_snippet}", + if needs_ref { "&" } else { "" }, + "" + ), + Applicability::MachineApplicable, + ); } + } + } - positions - }, - FormatArgsPiece::Literal(_) => ArrayVec::new(), - }) -} + fn format_arg_positions(&self) -> impl Iterator { + self.format_args.template.iter().flat_map(|piece| match piece { + FormatArgsPiece::Placeholder(placeholder) => { + let mut positions = ArrayVec::<_, 3>::new(); -/// Returns true if the format argument at `index` is referred to by multiple format params -fn is_aliased(format_args: &rustc_ast::FormatArgs, index: usize) -> bool { - format_arg_positions(format_args) - .filter(|(position, _)| position.index == Ok(index)) - .at_most_one() - .is_err() + positions.push((&placeholder.argument, FormatParamUsage::Argument)); + if let Some(FormatCount::Argument(position)) = &placeholder.format_options.width { + positions.push((position, FormatParamUsage::Width)); + } + if let Some(FormatCount::Argument(position)) = &placeholder.format_options.precision { + positions.push((position, FormatParamUsage::Precision)); + } + + positions + }, + FormatArgsPiece::Literal(_) => ArrayVec::new(), + }) + } + + /// Returns true if the format argument at `index` is referred to by multiple format params + fn is_aliased(&self, index: usize) -> bool { + self.format_arg_positions() + .filter(|(position, _)| position.index == Ok(index)) + .at_most_one() + .is_err() + } } fn count_needed_derefs<'tcx, I>(mut ty: Ty<'tcx>, mut iter: I) -> (usize, Ty<'tcx>) diff --git a/clippy_lints/src/format_impl.rs b/clippy_lints/src/format_impl.rs index 9360eb1fa91a..93517076cda0 100644 --- a/clippy_lints/src/format_impl.rs +++ b/clippy_lints/src/format_impl.rs @@ -7,7 +7,7 @@ use rustc_hir::{Expr, ExprKind, Impl, ImplItem, ImplItemKind, QPath}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::impl_lint_pass; use rustc_span::symbol::kw; -use rustc_span::{sym, Span, Symbol}; +use rustc_span::{sym, Symbol}; declare_clippy_lint! { /// ### What it does @@ -119,123 +119,132 @@ impl<'tcx> LateLintPass<'tcx> for FormatImpl { } fn check_impl_item_post(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) { - // Assume no nested Impl of Debug and Display within eachother + // Assume no nested Impl of Debug and Display within each other if is_format_trait_impl(cx, impl_item).is_some() { self.format_trait_impl = None; } } fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - let Some(format_trait_impl) = self.format_trait_impl else { - return; - }; - - if format_trait_impl.name == sym::Display { - check_to_string_in_display(cx, expr); + if let Some(format_trait_impl) = self.format_trait_impl { + let linter = FormatImplExpr { + cx, + expr, + format_trait_impl, + }; + linter.check_to_string_in_display(); + linter.check_self_in_format_args(); + linter.check_print_in_format_impl(); } - - check_self_in_format_args(cx, expr, format_trait_impl); - check_print_in_format_impl(cx, expr, format_trait_impl); } } -fn check_to_string_in_display(cx: &LateContext<'_>, expr: &Expr<'_>) { - if let ExprKind::MethodCall(path, self_arg, ..) = expr.kind - // Get the hir_id of the object we are calling the method on - // Is the method to_string() ? - && path.ident.name == sym::to_string - // Is the method a part of the ToString trait? (i.e. not to_string() implemented - // separately) - && let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) - && is_diag_trait_item(cx, expr_def_id, sym::ToString) - // Is the method is called on self - && let ExprKind::Path(QPath::Resolved(_, path)) = self_arg.kind - && let [segment] = path.segments - && segment.ident.name == kw::SelfLower - { - span_lint( - cx, - RECURSIVE_FORMAT_IMPL, - expr.span, - "using `self.to_string` in `fmt::Display` implementation will cause infinite recursion", - ); - } +struct FormatImplExpr<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, + format_trait_impl: FormatTraitNames, } -fn check_self_in_format_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, impl_trait: FormatTraitNames) { - // Check each arg in format calls - do we ever use Display on self (directly or via deref)? - if let Some(outer_macro) = root_macro_call_first_node(cx, expr) - && let macro_def_id = outer_macro.def_id - && is_format_macro(cx, macro_def_id) - && let Some(format_args) = find_format_args(cx, expr, outer_macro.expn) - { - for piece in &format_args.template { - if let FormatArgsPiece::Placeholder(placeholder) = piece - && let trait_name = match placeholder.format_trait { - FormatTrait::Display => sym::Display, - FormatTrait::Debug => sym::Debug, - FormatTrait::LowerExp => sym!(LowerExp), - FormatTrait::UpperExp => sym!(UpperExp), - FormatTrait::Octal => sym!(Octal), - FormatTrait::Pointer => sym::Pointer, - FormatTrait::Binary => sym!(Binary), - FormatTrait::LowerHex => sym!(LowerHex), - FormatTrait::UpperHex => sym!(UpperHex), +impl<'a, 'tcx> FormatImplExpr<'a, 'tcx> { + fn check_to_string_in_display(&self) { + if self.format_trait_impl.name == sym::Display + && let ExprKind::MethodCall(path, self_arg, ..) = self.expr.kind + // Get the hir_id of the object we are calling the method on + // Is the method to_string() ? + && path.ident.name == sym::to_string + // Is the method a part of the ToString trait? (i.e. not to_string() implemented + // separately) + && let Some(expr_def_id) = self.cx.typeck_results().type_dependent_def_id(self.expr.hir_id) + && is_diag_trait_item(self.cx, expr_def_id, sym::ToString) + // Is the method is called on self + && let ExprKind::Path(QPath::Resolved(_, path)) = self_arg.kind + && let [segment] = path.segments + && segment.ident.name == kw::SelfLower + { + span_lint( + self.cx, + RECURSIVE_FORMAT_IMPL, + self.expr.span, + "using `self.to_string` in `fmt::Display` implementation will cause infinite recursion", + ); + } + } + + fn check_self_in_format_args(&self) { + // Check each arg in format calls - do we ever use Display on self (directly or via deref)? + if let Some(outer_macro) = root_macro_call_first_node(self.cx, self.expr) + && let macro_def_id = outer_macro.def_id + && is_format_macro(self.cx, macro_def_id) + && let Some(format_args) = find_format_args(self.cx, self.expr, outer_macro.expn) + { + for piece in &format_args.template { + if let FormatArgsPiece::Placeholder(placeholder) = piece + && let trait_name = match placeholder.format_trait { + FormatTrait::Display => sym::Display, + FormatTrait::Debug => sym::Debug, + FormatTrait::LowerExp => sym!(LowerExp), + FormatTrait::UpperExp => sym!(UpperExp), + FormatTrait::Octal => sym!(Octal), + FormatTrait::Pointer => sym::Pointer, + FormatTrait::Binary => sym!(Binary), + FormatTrait::LowerHex => sym!(LowerHex), + FormatTrait::UpperHex => sym!(UpperHex), + } + && trait_name == self.format_trait_impl.name + && let Ok(index) = placeholder.argument.index + && let Some(arg) = format_args.arguments.all_args().get(index) + && let Ok(arg_expr) = find_format_arg_expr(self.expr, arg) + { + self.check_format_arg_self(arg_expr); } - && trait_name == impl_trait.name - && let Ok(index) = placeholder.argument.index - && let Some(arg) = format_args.arguments.all_args().get(index) - && let Ok(arg_expr) = find_format_arg_expr(expr, arg) - { - check_format_arg_self(cx, expr.span, arg_expr, impl_trait); } } } -} -fn check_format_arg_self(cx: &LateContext<'_>, span: Span, arg: &Expr<'_>, impl_trait: FormatTraitNames) { - // Handle multiple dereferencing of references e.g. &&self - // Handle dereference of &self -> self that is equivalent (i.e. via *self in fmt() impl) - // Since the argument to fmt is itself a reference: &self - let reference = peel_ref_operators(cx, arg); - let map = cx.tcx.hir(); - // Is the reference self? - if path_to_local(reference).map(|x| map.name(x)) == Some(kw::SelfLower) { - let FormatTraitNames { name, .. } = impl_trait; - span_lint( - cx, - RECURSIVE_FORMAT_IMPL, - span, - &format!("using `self` as `{name}` in `impl {name}` will cause infinite recursion"), - ); + fn check_format_arg_self(&self, arg: &Expr<'_>) { + // Handle multiple dereferencing of references e.g. &&self + // Handle dereference of &self -> self that is equivalent (i.e. via *self in fmt() impl) + // Since the argument to fmt is itself a reference: &self + let reference = peel_ref_operators(self.cx, arg); + let map = self.cx.tcx.hir(); + // Is the reference self? + if path_to_local(reference).map(|x| map.name(x)) == Some(kw::SelfLower) { + let FormatTraitNames { name, .. } = self.format_trait_impl; + span_lint( + self.cx, + RECURSIVE_FORMAT_IMPL, + self.expr.span, + &format!("using `self` as `{name}` in `impl {name}` will cause infinite recursion"), + ); + } } -} -fn check_print_in_format_impl(cx: &LateContext<'_>, expr: &Expr<'_>, impl_trait: FormatTraitNames) { - if let Some(macro_call) = root_macro_call_first_node(cx, expr) - && let Some(name) = cx.tcx.get_diagnostic_name(macro_call.def_id) - { - let replacement = match name { - sym::print_macro | sym::eprint_macro => "write", - sym::println_macro | sym::eprintln_macro => "writeln", - _ => return, - }; + fn check_print_in_format_impl(&self) { + if let Some(macro_call) = root_macro_call_first_node(self.cx, self.expr) + && let Some(name) = self.cx.tcx.get_diagnostic_name(macro_call.def_id) + { + let replacement = match name { + sym::print_macro | sym::eprint_macro => "write", + sym::println_macro | sym::eprintln_macro => "writeln", + _ => return, + }; - let name = name.as_str().strip_suffix("_macro").unwrap(); + let name = name.as_str().strip_suffix("_macro").unwrap(); - span_lint_and_sugg( - cx, - PRINT_IN_FORMAT_IMPL, - macro_call.span, - &format!("use of `{name}!` in `{}` impl", impl_trait.name), - "replace with", - if let Some(formatter_name) = impl_trait.formatter_name { - format!("{replacement}!({formatter_name}, ..)") - } else { - format!("{replacement}!(..)") - }, - Applicability::HasPlaceholders, - ); + span_lint_and_sugg( + self.cx, + PRINT_IN_FORMAT_IMPL, + macro_call.span, + &format!("use of `{name}!` in `{}` impl", self.format_trait_impl.name), + "replace with", + if let Some(formatter_name) = self.format_trait_impl.formatter_name { + format!("{replacement}!({formatter_name}, ..)") + } else { + format!("{replacement}!(..)") + }, + Applicability::HasPlaceholders, + ); + } } } diff --git a/clippy_lints/src/from_over_into.rs b/clippy_lints/src/from_over_into.rs index 1933a00891b0..93527bcdf5ce 100644 --- a/clippy_lints/src/from_over_into.rs +++ b/clippy_lints/src/from_over_into.rs @@ -181,6 +181,9 @@ fn convert_to_from( let from = snippet_opt(cx, self_ty.span)?; let into = snippet_opt(cx, target_ty.span)?; + let return_type = matches!(sig.decl.output, FnRetTy::Return(_)) + .then_some(String::from("Self")) + .unwrap_or_default(); let mut suggestions = vec![ // impl Into for U -> impl From for U // ~~~~ ~~~~ @@ -197,13 +200,10 @@ fn convert_to_from( // fn into([mut] self) -> T -> fn into([mut] v: T) -> T // ~~~~ ~~~~ (self_ident.span, format!("val: {from}")), - ]; - - if let FnRetTy::Return(_) = sig.decl.output { // fn into(self) -> T -> fn into(self) -> Self // ~ ~~~~ - suggestions.push((sig.decl.output.span(), String::from("Self"))); - } + (sig.decl.output.span(), return_type), + ]; let mut finder = SelfFinder { cx, diff --git a/clippy_lints/src/implied_bounds_in_impls.rs b/clippy_lints/src/implied_bounds_in_impls.rs index fab8ffedb949..174db30a7cd4 100644 --- a/clippy_lints/src/implied_bounds_in_impls.rs +++ b/clippy_lints/src/implied_bounds_in_impls.rs @@ -1,16 +1,17 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet; use rustc_errors::{Applicability, SuggestionStyle}; -use rustc_hir::def_id::{DefId, LocalDefId}; +use rustc_hir::def_id::DefId; use rustc_hir::intravisit::FnKind; use rustc_hir::{ - Body, FnDecl, FnRetTy, GenericArg, GenericBound, ImplItem, ImplItemKind, ItemKind, TraitBoundModifier, TraitItem, - TraitItemKind, TyKind, + Body, FnDecl, FnRetTy, GenericArg, GenericBound, ImplItem, ImplItemKind, ItemKind, OpaqueTy, TraitBoundModifier, + TraitItem, TraitItemKind, TyKind, TypeBinding, }; use rustc_hir_analysis::hir_ty_to_ty; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{self, ClauseKind, Generics, Ty, TyCtxt}; use rustc_session::declare_lint_pass; +use rustc_span::def_id::LocalDefId; use rustc_span::Span; declare_clippy_lint! { @@ -50,20 +51,17 @@ declare_clippy_lint! { } declare_lint_pass!(ImpliedBoundsInImpls => [IMPLIED_BOUNDS_IN_IMPLS]); -#[allow(clippy::too_many_arguments)] fn emit_lint( cx: &LateContext<'_>, poly_trait: &rustc_hir::PolyTraitRef<'_>, opaque_ty: &rustc_hir::OpaqueTy<'_>, index: usize, - // The bindings that were implied + // The bindings that were implied, used for suggestion purposes since removing a bound with associated types + // means we might need to then move it to a different bound implied_bindings: &[rustc_hir::TypeBinding<'_>], - // The original bindings that `implied_bindings` are implied from - implied_by_bindings: &[rustc_hir::TypeBinding<'_>], - implied_by_args: &[GenericArg<'_>], - implied_by_span: Span, + bound: &ImplTraitBound<'_>, ) { - let implied_by = snippet(cx, implied_by_span, ".."); + let implied_by = snippet(cx, bound.span, ".."); span_lint_and_then( cx, @@ -93,17 +91,17 @@ fn emit_lint( // If we're going to suggest removing `Deref<..>`, we'll need to put `` on `DerefMut` let omitted_assoc_tys: Vec<_> = implied_bindings .iter() - .filter(|binding| !implied_by_bindings.iter().any(|b| b.ident == binding.ident)) + .filter(|binding| !bound.bindings.iter().any(|b| b.ident == binding.ident)) .collect(); if !omitted_assoc_tys.is_empty() { // `<>` needs to be added if there aren't yet any generic arguments or bindings - let needs_angle_brackets = implied_by_args.is_empty() && implied_by_bindings.is_empty(); - let insert_span = match (implied_by_args, implied_by_bindings) { + let needs_angle_brackets = bound.args.is_empty() && bound.bindings.is_empty(); + let insert_span = match (bound.args, bound.bindings) { ([.., arg], [.., binding]) => arg.span().max(binding.span).shrink_to_hi(), ([.., arg], []) => arg.span().shrink_to_hi(), ([], [.., binding]) => binding.span.shrink_to_hi(), - ([], []) => implied_by_span.shrink_to_hi(), + ([], []) => bound.span.shrink_to_hi(), }; let mut associated_tys_sugg = if needs_angle_brackets { @@ -223,42 +221,93 @@ fn is_same_generics<'tcx>( }) } +struct ImplTraitBound<'tcx> { + /// The span of the bound in the `impl Trait` type + span: Span, + /// The predicates defined in the trait referenced by this bound. This also contains the actual + /// supertrait bounds + predicates: &'tcx [(ty::Clause<'tcx>, Span)], + /// The `DefId` of the trait being referenced by this bound + trait_def_id: DefId, + /// The generic arguments on the `impl Trait` bound + args: &'tcx [GenericArg<'tcx>], + /// The associated types on this bound + bindings: &'tcx [TypeBinding<'tcx>], +} + +/// Given an `impl Trait` type, gets all the supertraits from each bound ("implied bounds"). +/// +/// For `impl Deref + DerefMut + Eq` this returns `[Deref, PartialEq]`. +/// The `Deref` comes from `DerefMut` because `trait DerefMut: Deref {}`, and `PartialEq` comes from +/// `Eq`. +fn collect_supertrait_bounds<'tcx>(cx: &LateContext<'tcx>, opaque_ty: &OpaqueTy<'tcx>) -> Vec> { + opaque_ty + .bounds + .iter() + .filter_map(|bound| { + if let GenericBound::Trait(poly_trait, TraitBoundModifier::None) = bound + && let [.., path] = poly_trait.trait_ref.path.segments + && poly_trait.bound_generic_params.is_empty() + && let Some(trait_def_id) = path.res.opt_def_id() + && let predicates = cx.tcx.super_predicates_of(trait_def_id).predicates + // If the trait has no supertrait, there is no need to collect anything from that bound + && !predicates.is_empty() + { + Some(ImplTraitBound { + predicates, + args: path.args.map_or([].as_slice(), |p| p.args), + bindings: path.args.map_or([].as_slice(), |p| p.bindings), + trait_def_id, + span: bound.span(), + }) + } else { + None + } + }) + .collect() +} + +/// Given a bound in an `impl Trait` type, looks for a trait in the set of supertraits (previously +/// collected in [`collect_supertrait_bounds`]) that matches (same trait and generic arguments). +fn find_bound_in_supertraits<'a, 'tcx>( + cx: &LateContext<'tcx>, + trait_def_id: DefId, + args: &'tcx [GenericArg<'tcx>], + bounds: &'a [ImplTraitBound<'tcx>], +) -> Option<&'a ImplTraitBound<'tcx>> { + bounds.iter().find(|bound| { + bound.predicates.iter().any(|(clause, _)| { + if let ClauseKind::Trait(tr) = clause.kind().skip_binder() + && tr.def_id() == trait_def_id + { + is_same_generics( + cx.tcx, + tr.trait_ref.args, + bound.args, + args, + bound.trait_def_id, + trait_def_id, + ) + } else { + false + } + }) + }) +} + fn check(cx: &LateContext<'_>, decl: &FnDecl<'_>) { if let FnRetTy::Return(ty) = decl.output - &&let TyKind::OpaqueDef(item_id, ..) = ty.kind + && let TyKind::OpaqueDef(item_id, ..) = ty.kind && let item = cx.tcx.hir().item(item_id) && let ItemKind::OpaqueTy(opaque_ty) = item.kind // Very often there is only a single bound, e.g. `impl Deref<..>`, in which case // we can avoid doing a bunch of stuff unnecessarily. && opaque_ty.bounds.len() > 1 { - // Get all the (implied) trait predicates in the bounds. - // For `impl Deref + DerefMut` this will contain [`Deref`]. - // The implied `Deref` comes from `DerefMut` because `trait DerefMut: Deref {}`. - // N.B. (G)ATs are fine to disregard, because they must be the same for all of its supertraits. - // Example: - // `impl Deref + DerefMut` is not allowed. - // `DerefMut::Target` needs to match `Deref::Target`. - let implied_bounds: Vec<_> = opaque_ty - .bounds - .iter() - .filter_map(|bound| { - if let GenericBound::Trait(poly_trait, TraitBoundModifier::None) = bound - && let [.., path] = poly_trait.trait_ref.path.segments - && poly_trait.bound_generic_params.is_empty() - && let Some(trait_def_id) = path.res.opt_def_id() - && let predicates = cx.tcx.super_predicates_of(trait_def_id).predicates - && !predicates.is_empty() - // If the trait has no supertrait, there is nothing to add. - { - Some((bound.span(), path, predicates, trait_def_id)) - } else { - None - } - }) - .collect(); + let supertraits = collect_supertrait_bounds(cx, opaque_ty); - // Lint all bounds in the `impl Trait` type that are also in the `implied_bounds` vec. + // Lint all bounds in the `impl Trait` type that we've previously also seen in the set of + // supertraits of each of the bounds. // This involves some extra logic when generic arguments are present, since // simply comparing trait `DefId`s won't be enough. We also need to compare the generics. for (index, bound) in opaque_ty.bounds.iter().enumerate() { @@ -267,42 +316,18 @@ fn check(cx: &LateContext<'_>, decl: &FnDecl<'_>) { && let implied_args = path.args.map_or([].as_slice(), |a| a.args) && let implied_bindings = path.args.map_or([].as_slice(), |a| a.bindings) && let Some(def_id) = poly_trait.trait_ref.path.res.opt_def_id() - && let Some((implied_by_span, implied_by_args, implied_by_bindings)) = - implied_bounds - .iter() - .find_map(|&(span, implied_by_path, preds, implied_by_def_id)| { - let implied_by_args = implied_by_path.args.map_or([].as_slice(), |a| a.args); - let implied_by_bindings = implied_by_path.args.map_or([].as_slice(), |a| a.bindings); - - preds.iter().find_map(|(clause, _)| { - if let ClauseKind::Trait(tr) = clause.kind().skip_binder() - && tr.def_id() == def_id - && is_same_generics( - cx.tcx, - tr.trait_ref.args, - implied_by_args, - implied_args, - implied_by_def_id, - def_id, - ) - { - Some((span, implied_by_args, implied_by_bindings)) - } else { - None - } - }) - }) + && let Some(bound) = find_bound_in_supertraits(cx, def_id, implied_args, &supertraits) + // If the implied bound has a type binding that also exists in the implied-by trait, + // then we shouldn't lint. See #11880 for an example. + && let assocs = cx.tcx.associated_items(bound.trait_def_id) + && !implied_bindings.iter().any(|binding| { + assocs + .filter_by_name_unhygienic(binding.ident.name) + .next() + .is_some_and(|assoc| assoc.kind == ty::AssocKind::Type) + }) { - emit_lint( - cx, - poly_trait, - opaque_ty, - index, - implied_bindings, - implied_by_bindings, - implied_by_args, - implied_by_span, - ); + emit_lint(cx, poly_trait, opaque_ty, index, implied_bindings, bound); } } } diff --git a/clippy_lints/src/incompatible_msrv.rs b/clippy_lints/src/incompatible_msrv.rs index f2f0e7d42662..cd000fcd1844 100644 --- a/clippy_lints/src/incompatible_msrv.rs +++ b/clippy_lints/src/incompatible_msrv.rs @@ -1,14 +1,15 @@ use clippy_config::msrvs::Msrv; use clippy_utils::diagnostics::span_lint; +use clippy_utils::is_in_test_function; use rustc_attr::{StabilityLevel, StableSince}; use rustc_data_structures::fx::FxHashMap; -use rustc_hir::{Expr, ExprKind}; +use rustc_hir::{Expr, ExprKind, HirId}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::TyCtxt; use rustc_semver::RustcVersion; use rustc_session::impl_lint_pass; use rustc_span::def_id::DefId; -use rustc_span::Span; +use rustc_span::{ExpnKind, Span}; declare_clippy_lint! { /// ### What it does @@ -81,13 +82,18 @@ impl IncompatibleMsrv { version } - fn emit_lint_if_under_msrv(&mut self, cx: &LateContext<'_>, def_id: DefId, span: Span) { + fn emit_lint_if_under_msrv(&mut self, cx: &LateContext<'_>, def_id: DefId, node: HirId, span: Span) { if def_id.is_local() { // We don't check local items since their MSRV is supposed to always be valid. return; } let version = self.get_def_id_version(cx.tcx, def_id); - if self.msrv.meets(version) { + if self.msrv.meets(version) || is_in_test_function(cx.tcx, node) { + return; + } + if let ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) = span.ctxt().outer_expn_data().kind { + // Desugared expressions get to cheat and stability is ignored. + // Intentionally not using `.from_expansion()`, since we do still care about macro expansions return; } self.emit_lint_for(cx, span, version); @@ -117,14 +123,14 @@ impl<'tcx> LateLintPass<'tcx> for IncompatibleMsrv { match expr.kind { ExprKind::MethodCall(_, _, _, span) => { if let Some(method_did) = cx.typeck_results().type_dependent_def_id(expr.hir_id) { - self.emit_lint_if_under_msrv(cx, method_did, span); + self.emit_lint_if_under_msrv(cx, method_did, expr.hir_id, span); } }, ExprKind::Call(call, [_]) => { if let ExprKind::Path(qpath) = call.kind && let Some(path_def_id) = cx.qpath_res(&qpath, call.hir_id).opt_def_id() { - self.emit_lint_if_under_msrv(cx, path_def_id, call.span); + self.emit_lint_if_under_msrv(cx, path_def_id, expr.hir_id, call.span); } }, _ => {}, diff --git a/clippy_lints/src/index_refutable_slice.rs b/clippy_lints/src/index_refutable_slice.rs index 41e9d5b1c2e9..5b5eb355f86c 100644 --- a/clippy_lints/src/index_refutable_slice.rs +++ b/clippy_lints/src/index_refutable_slice.rs @@ -255,7 +255,9 @@ impl<'a, 'tcx> Visitor<'tcx> for SliceIndexLintingVisitor<'a, 'tcx> { && let hir::Node::Expr(maybe_addrof_expr) = cx.tcx.parent_hir_node(parent_id) && let hir::ExprKind::AddrOf(_kind, hir::Mutability::Not, _inner_expr) = maybe_addrof_expr.kind { - use_info.index_use.push((index_value, cx.tcx.hir().span(parent_expr.hir_id))); + use_info + .index_use + .push((index_value, cx.tcx.hir().span(parent_expr.hir_id))); return; } diff --git a/clippy_lints/src/indexing_slicing.rs b/clippy_lints/src/indexing_slicing.rs index 391db0b0df72..35fcd8cdd354 100644 --- a/clippy_lints/src/indexing_slicing.rs +++ b/clippy_lints/src/indexing_slicing.rs @@ -174,6 +174,7 @@ impl<'tcx> LateLintPass<'tcx> for IndexingSlicing { // only `usize` index is legal in rust array index // leave other type to rustc if let Constant::Int(off) = constant + && off <= usize::MAX as u128 && let ty::Uint(utype) = cx.typeck_results().expr_ty(index).kind() && *utype == ty::UintTy::Usize && let ty::Array(_, s) = ty.kind() diff --git a/clippy_lints/src/item_name_repetitions.rs b/clippy_lints/src/item_name_repetitions.rs index 276c1abb60cd..0b4c416d94db 100644 --- a/clippy_lints/src/item_name_repetitions.rs +++ b/clippy_lints/src/item_name_repetitions.rs @@ -385,7 +385,6 @@ impl LateLintPass<'_> for ItemNameRepetitions { assert!(last.is_some()); } - #[expect(clippy::similar_names)] fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { let item_name = item.ident.name.as_str(); let item_camel = to_camel_case(item_name); diff --git a/clippy_lints/src/iter_not_returning_iterator.rs b/clippy_lints/src/iter_not_returning_iterator.rs index b9fad726511c..2344f040522b 100644 --- a/clippy_lints/src/iter_not_returning_iterator.rs +++ b/clippy_lints/src/iter_not_returning_iterator.rs @@ -57,7 +57,7 @@ impl<'tcx> LateLintPass<'tcx> for IterNotReturningIterator { if matches!(name, "iter" | "iter_mut") && !matches!( get_parent_node(cx.tcx, item.hir_id()), - Some(Node::Item(Item { kind: ItemKind::Impl(i), .. })) if i.of_trait.is_some() + Node::Item(Item { kind: ItemKind::Impl(i), .. }) if i.of_trait.is_some() ) { if let ImplItemKind::Fn(fn_sig, _) = &item.kind { diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 14bd82f9c97e..1bca1f2bd6fe 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -14,7 +14,7 @@ clippy::missing_docs_in_private_items, clippy::must_use_candidate, rustc::diagnostic_outside_of_impl, - rustc::untranslatable_diagnostic, + rustc::untranslatable_diagnostic )] #![warn(trivial_casts, trivial_numeric_casts)] // warn on lints, that are included in `rust-lang/rust`s bootstrap diff --git a/clippy_lints/src/loops/infinite_loop.rs b/clippy_lints/src/loops/infinite_loop.rs index 5e099f1e76f8..5b5bb88c1790 100644 --- a/clippy_lints/src/loops/infinite_loop.rs +++ b/clippy_lints/src/loops/infinite_loop.rs @@ -1,17 +1,18 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::{fn_def_id, is_lint_allowed}; +use clippy_utils::{fn_def_id, is_from_proc_macro, is_lint_allowed}; use hir::intravisit::{walk_expr, Visitor}; use hir::{Expr, ExprKind, FnRetTy, FnSig, Node}; use rustc_ast::Label; use rustc_errors::Applicability; use rustc_hir as hir; -use rustc_lint::LateContext; +use rustc_lint::{LateContext, LintContext}; +use rustc_middle::lint::in_external_macro; use super::INFINITE_LOOP; pub(super) fn check<'tcx>( cx: &LateContext<'tcx>, - expr: &Expr<'_>, + expr: &Expr<'tcx>, loop_block: &'tcx hir::Block<'_>, label: Option