diff --git a/src/tools/clippy/.github/driver.sh b/src/tools/clippy/.github/driver.sh index 40a2aad0f5379..11fd6b5c79ed8 100755 --- a/src/tools/clippy/.github/driver.sh +++ b/src/tools/clippy/.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/src/tools/clippy/CHANGELOG.md b/src/tools/clippy/CHANGELOG.md index 9b853567219c9..f0b01742deb05 100644 --- a/src/tools/clippy/CHANGELOG.md +++ b/src/tools/clippy/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 @@ -5157,6 +5158,7 @@ Released 2018-09-13 [`duration_subsec`]: https://rust-lang.github.io/rust-clippy/master/index.html#duration_subsec [`eager_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#eager_transmute [`else_if_without_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#else_if_without_else +[`empty_docs`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_docs [`empty_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_drop [`empty_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum [`empty_enum_variants_with_brackets`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum_variants_with_brackets @@ -5429,6 +5431,7 @@ Released 2018-09-13 [`modulo_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#modulo_arithmetic [`modulo_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#modulo_one [`multi_assignments`]: https://rust-lang.github.io/rust-clippy/master/index.html#multi_assignments +[`multiple_bound_locations`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_bound_locations [`multiple_crate_versions`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_crate_versions [`multiple_inherent_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_inherent_impl [`multiple_unsafe_ops_per_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_unsafe_ops_per_block @@ -5725,10 +5728,12 @@ 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 [`unnecessary_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fold +[`unnecessary_get_then_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_get_then_check [`unnecessary_join`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_join [`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations [`unnecessary_literal_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_literal_unwrap diff --git a/src/tools/clippy/Cargo.toml b/src/tools/clippy/Cargo.toml index 321424880d1e2..5d1d0ce2c42fb 100644 --- a/src/tools/clippy/Cargo.toml +++ b/src/tools/clippy/Cargo.toml @@ -27,17 +27,17 @@ 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.2" tester = "0.9" regex = "1.5" toml = "0.7.3" walkdir = "2.3" # This is used by the `collect-metadata` alias. filetime = "0.2" -itertools = "0.11" +itertools = "0.12" # UI test dependencies clippy_utils = { path = "clippy_utils" } diff --git a/src/tools/clippy/book/src/configuration.md b/src/tools/clippy/book/src/configuration.md index e8274bc4575d0..9eb067abd91ec 100644 --- a/src/tools/clippy/book/src/configuration.md +++ b/src/tools/clippy/book/src/configuration.md @@ -33,26 +33,29 @@ disallowed-names = ["bar", ".."] # -> ["bar", "foo", "baz", "quux"] To deactivate the "for further information visit *lint-link*" message you can define the `CLIPPY_DISABLE_DOCS_LINKS` environment variable. -### Allowing/denying lints +### Allowing/Denying Lints -You can add options to your code to `allow`/`warn`/`deny` Clippy lints: +#### Attributes in Code -* the whole set of `Warn` lints using the `clippy` lint group (`#![deny(clippy::all)]`) +You can add attributes to your code to `allow`/`warn`/`deny` Clippy lints: -* all lints using both the `clippy` and `clippy::pedantic` lint groups (`#![deny(clippy::all)]`, - `#![deny(clippy::pedantic)]`). Note that `clippy::pedantic` contains some very aggressive lints prone to false - positives. +* the whole set of `warn`-by-default lints using the `clippy` lint group (`#![allow(clippy::all)]`) + +* all lints using both the `clippy` and `clippy::pedantic` lint groups (`#![warn(clippy::all, clippy::pedantic)]`. Note + that `clippy::pedantic` contains some very aggressive lints prone to false positives. * only some lints (`#![deny(clippy::single_match, clippy::box_vec)]`, etc.) * `allow`/`warn`/`deny` can be limited to a single function or module using `#[allow(...)]`, etc. Note: `allow` means to suppress the lint for your code. With `warn` the lint will only emit a warning, while with `deny` -the lint will emit an error, when triggering for your code. An error causes clippy to exit with an error code, so is -useful in scripts like CI/CD. +the lint will emit an error, when triggering for your code. An error causes Clippy to exit with an error code, so is +most useful in scripts used in CI/CD. + +#### Command Line Flags -If you do not want to include your lint levels in your code, you can globally enable/disable lints by passing extra -flags to Clippy during the run: +If you do not want to include your lint levels in the code, you can globally enable/disable lints by passing extra flags +to Clippy during the run: To allow `lint_name`, run @@ -66,19 +69,33 @@ And to warn on `lint_name`, run cargo clippy -- -W clippy::lint_name ``` -This also works with lint groups. For example, you can run Clippy with warnings for all lints enabled: +This also works with lint groups. For example, you can run Clippy with warnings for all pedantic lints enabled: ```terminal cargo clippy -- -W clippy::pedantic ``` -If you care only about a single lint, you can allow all others and then explicitly warn on the lint(s) you are +If you care only about a certain lints, you can allow all others and then explicitly warn on the lints you are interested in: ```terminal cargo clippy -- -A clippy::all -W clippy::useless_format -W clippy::... ``` +#### Lints Section in `Cargo.toml` + +Finally, lints can be allowed/denied using [the lints +section](https://doc.rust-lang.org/nightly/cargo/reference/manifest.html#the-lints-section)) in the `Cargo.toml` file: + +To deny `clippy::enum_glob_use`, put the following in the `Cargo.toml`: + +```toml +[lints.clippy] +enum_glob_use = "deny" +``` + +For more details and options, refer to the Cargo documentation. + ### Specifying the minimum supported Rust version Projects that intend to support old versions of Rust can disable lints pertaining to newer features by specifying the @@ -113,17 +130,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/src/tools/clippy/book/src/development/emitting_lints.md b/src/tools/clippy/book/src/development/emitting_lints.md index a12f6aa91b307..d70f4fc17ebf0 100644 --- a/src/tools/clippy/book/src/development/emitting_lints.md +++ b/src/tools/clippy/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/src/tools/clippy/book/src/development/writing_tests.md b/src/tools/clippy/book/src/development/writing_tests.md index 8937e0d8e9469..39a5ad9668851 100644 --- a/src/tools/clippy/book/src/development/writing_tests.md +++ b/src/tools/clippy/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/src/tools/clippy/book/src/lint_configuration.md b/src/tools/clippy/book/src/lint_configuration.md index f2357e2b5de92..214a60d3bfdf7 100644 --- a/src/tools/clippy/book/src/lint_configuration.md +++ b/src/tools/clippy/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/src/tools/clippy/clippy_config/src/conf.rs b/src/tools/clippy/clippy_config/src/conf.rs index 9741b94d50413..b781259ad9695 100644 --- a/src/tools/clippy/clippy_config/src/conf.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_config/src/lib.rs b/src/tools/clippy/clippy_config/src/lib.rs index 5449feed090a9..dab3119894a4d 100644 --- a/src/tools/clippy/clippy_config/src/lib.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_dev/Cargo.toml b/src/tools/clippy/clippy_dev/Cargo.toml index 5ec67554e7d89..42a953039b1c0 100644 --- a/src/tools/clippy/clippy_dev/Cargo.toml +++ b/src/tools/clippy/clippy_dev/Cargo.toml @@ -7,7 +7,7 @@ edition = "2021" aho-corasick = "1.0" clap = "4.1.4" indoc = "1.0" -itertools = "0.11" +itertools = "0.12" opener = "0.6" shell-escape = "0.1" walkdir = "2.3" diff --git a/src/tools/clippy/clippy_lints/Cargo.toml b/src/tools/clippy/clippy_lints/Cargo.toml index 6e6e315bb6561..6ae089b3e0321 100644 --- a/src/tools/clippy/clippy_lints/Cargo.toml +++ b/src/tools/clippy/clippy_lints/Cargo.toml @@ -14,7 +14,7 @@ cargo_metadata = "0.18" clippy_config = { path = "../clippy_config" } clippy_utils = { path = "../clippy_utils" } declare_clippy_lint = { path = "../declare_clippy_lint" } -itertools = "0.11" +itertools = "0.12" quine-mc_cluskey = "0.2" regex-syntax = "0.8" serde = { version = "1.0", features = ["derive"] } diff --git a/src/tools/clippy/clippy_lints/src/asm_syntax.rs b/src/tools/clippy/clippy_lints/src/asm_syntax.rs index feb6437ee26ac..c2fa56e136031 100644 --- a/src/tools/clippy/clippy_lints/src/asm_syntax.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/attrs.rs b/src/tools/clippy/clippy_lints/src/attrs.rs index da38422874b44..f2937d51340b8 100644 --- a/src/tools/clippy/clippy_lints/src/attrs.rs +++ b/src/tools/clippy/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, @@ -512,6 +564,7 @@ impl<'tcx> LateLintPass<'tcx> for Attributes { || is_word(lint, sym::deprecated) || is_word(lint, sym!(unreachable_pub)) || is_word(lint, sym!(unused)) + || is_word(lint, sym!(unused_import_braces)) || extract_clippy_lint(lint).map_or(false, |s| { matches!( s.as_str(), @@ -794,6 +847,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 +858,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 +913,146 @@ 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() + && Level::from_symbol(ident.name, Some(attr.id)).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/src/tools/clippy/clippy_lints/src/blocks_in_conditions.rs b/src/tools/clippy/clippy_lints/src/blocks_in_conditions.rs index ff4dffd06079f..2eb0dac974258 100644 --- a/src/tools/clippy/clippy_lints/src/blocks_in_conditions.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/booleans.rs b/src/tools/clippy/clippy_lints/src/booleans.rs index 2d1c250ace905..0d66f2d644d98 100644 --- a/src/tools/clippy/clippy_lints/src/booleans.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/box_default.rs b/src/tools/clippy/clippy_lints/src/box_default.rs index ef12fe344e409..779ae03c4640b 100644 --- a/src/tools/clippy/clippy_lints/src/box_default.rs +++ b/src/tools/clippy/clippy_lints/src/box_default.rs @@ -1,11 +1,12 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::macros::macro_backtrace; +use clippy_utils::source::snippet_opt; use clippy_utils::ty::expr_sig; -use clippy_utils::{get_parent_node, is_default_equivalent, path_def_id}; +use clippy_utils::{is_default_equivalent, path_def_id}; use rustc_errors::Applicability; use rustc_hir::def::Res; use rustc_hir::intravisit::{walk_ty, Visitor}; -use rustc_hir::{Block, Expr, ExprKind, Local, Node, QPath, TyKind}; +use rustc_hir::{Block, Expr, ExprKind, Local, Node, QPath, Ty, TyKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::print::with_forced_trimmed_paths; @@ -41,13 +42,24 @@ declare_lint_pass!(BoxDefault => [BOX_DEFAULT]); impl LateLintPass<'_> for BoxDefault { fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + // If the expression is a call (`Box::new(...)`) if let ExprKind::Call(box_new, [arg]) = expr.kind + // And call is of the form `::something` + // Here, it would be `::new` && let ExprKind::Path(QPath::TypeRelative(ty, seg)) = box_new.kind - && let ExprKind::Call(arg_path, ..) = arg.kind - && !in_external_macro(cx.sess(), expr.span) - && (expr.span.eq_ctxt(arg.span) || is_local_vec_expn(cx, arg, expr)) + // And that method is `new` && seg.ident.name == sym::new + // And the call is that of a `Box` method && path_def_id(cx, ty).map_or(false, |id| Some(id) == cx.tcx.lang_items().owned_box()) + // And the single argument to the call is another function call + // This is the `T::default()` of `Box::new(T::default())` + && let ExprKind::Call(arg_path, inner_call_args) = arg.kind + // And we are not in a foreign crate's macro + && !in_external_macro(cx.sess(), expr.span) + // And the argument expression has the same context as the outer call expression + // or that we are inside a `vec!` macro expansion + && (expr.span.eq_ctxt(arg.span) || is_local_vec_expn(cx, arg, expr)) + // And the argument is equivalent to `Default::default()` && is_default_equivalent(cx, arg) { span_lint_and_sugg( @@ -59,7 +71,17 @@ impl LateLintPass<'_> for BoxDefault { if is_plain_default(cx, arg_path) || given_type(cx, expr) { "Box::default()".into() } else if let Some(arg_ty) = cx.typeck_results().expr_ty(arg).make_suggestable(cx.tcx, true) { - with_forced_trimmed_paths!(format!("Box::<{arg_ty}>::default()")) + // Check if we can copy from the source expression in the replacement. + // We need the call to have no argument (see `explicit_default_type`). + if inner_call_args.is_empty() + && let Some(ty) = explicit_default_type(arg_path) + && let Some(s) = snippet_opt(cx, ty.span) + { + format!("Box::<{s}>::default()") + } else { + // Otherwise, use the inferred type's formatting. + with_forced_trimmed_paths!(format!("Box::<{arg_ty}>::default()")) + } } else { return; }, @@ -81,6 +103,20 @@ fn is_plain_default(cx: &LateContext<'_>, arg_path: &Expr<'_>) -> bool { } } +// Checks whether the call is of the form `A::B::f()`. Returns `A::B` if it is. +// +// In the event we have this kind of construct, it's easy to use `A::B` as a replacement in the +// quickfix. `f` must however have no parameter. Should `f` have some, then some of the type of +// `A::B` may be inferred from the arguments. This would be the case for `Vec::from([0; false])`, +// where the argument to `from` allows inferring this is a `Vec` +fn explicit_default_type<'a>(arg_path: &'a Expr<'_>) -> Option<&'a Ty<'a>> { + if let ExprKind::Path(QPath::TypeRelative(ty, _)) = &arg_path.kind { + Some(ty) + } else { + None + } +} + fn is_local_vec_expn(cx: &LateContext<'_>, expr: &Expr<'_>, ref_expr: &Expr<'_>) -> bool { macro_backtrace(expr.span).next().map_or(false, |call| { cx.tcx.is_diagnostic_item(sym::vec_macro, call.def_id) && call.span.eq_ctxt(ref_expr.span) @@ -100,26 +136,23 @@ 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), .. })) => { + match cx.tcx.parent_hir_node(expr.hir_id) { + 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/src/tools/clippy/clippy_lints/src/casts/cast_possible_truncation.rs b/src/tools/clippy/clippy_lints/src/casts/cast_possible_truncation.rs index 1543ae8039968..ab89bb2f5f159 100644 --- a/src/tools/clippy/clippy_lints/src/casts/cast_possible_truncation.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/casts/cast_sign_loss.rs b/src/tools/clippy/clippy_lints/src/casts/cast_sign_loss.rs index 1df5a25f674d7..8fd95d9654cf0 100644 --- a/src/tools/clippy/clippy_lints/src/casts/cast_sign_loss.rs +++ b/src/tools/clippy/clippy_lints/src/casts/cast_sign_loss.rs @@ -1,15 +1,47 @@ +use std::convert::Infallible; +use std::ops::ControlFlow; + use clippy_utils::consts::{constant, Constant}; use clippy_utils::diagnostics::span_lint; -use clippy_utils::{clip, method_chain_args, sext}; +use clippy_utils::visitors::{for_each_expr, Descend}; +use clippy_utils::{method_chain_args, sext}; use rustc_hir::{BinOpKind, Expr, ExprKind}; use rustc_lint::LateContext; -use rustc_middle::ty::{self, Ty, UintTy}; +use rustc_middle::ty::{self, Ty}; use super::CAST_SIGN_LOSS; -const METHODS_RET_POSITIVE: &[&str] = &["abs", "checked_abs", "rem_euclid", "checked_rem_euclid"]; +/// A list of methods that can never return a negative value. +/// Includes methods that panic rather than returning a negative value. +/// +/// Methods that can overflow and return a negative value must not be included in this list, +/// because casting their return values can still result in sign loss. +const METHODS_RET_POSITIVE: &[&str] = &[ + "checked_abs", + "saturating_abs", + "isqrt", + "checked_isqrt", + "rem_euclid", + "checked_rem_euclid", + "wrapping_rem_euclid", +]; + +/// A list of methods that act like `pow()`. See `pow_call_result_sign()` for details. +/// +/// Methods that can overflow and return a negative value must not be included in this list, +/// because casting their return values can still result in sign loss. +const METHODS_POW: &[&str] = &["pow", "saturating_pow", "checked_pow"]; -pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { +/// A list of methods that act like `unwrap()`, and don't change the sign of the inner value. +const METHODS_UNWRAP: &[&str] = &["unwrap", "unwrap_unchecked", "expect", "into_ok"]; + +pub(super) fn check<'cx>( + cx: &LateContext<'cx>, + expr: &Expr<'_>, + cast_op: &Expr<'_>, + cast_from: Ty<'cx>, + cast_to: Ty<'_>, +) { if should_lint(cx, cast_op, cast_from, cast_to) { span_lint( cx, @@ -20,35 +52,27 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_op: &Expr<'_>, c } } -fn should_lint(cx: &LateContext<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) -> bool { +fn should_lint<'cx>(cx: &LateContext<'cx>, cast_op: &Expr<'_>, cast_from: Ty<'cx>, cast_to: Ty<'_>) -> bool { match (cast_from.is_integral(), cast_to.is_integral()) { (true, true) => { if !cast_from.is_signed() || cast_to.is_signed() { return false; } - // Don't lint if `cast_op` is known to be positive. + // Don't lint if `cast_op` is known to be positive, ignoring overflow. if let Sign::ZeroOrPositive = expr_sign(cx, cast_op, cast_from) { return false; } - let (mut uncertain_count, mut negative_count) = (0, 0); - // Peel off possible binary expressions, e.g. x * x * y => [x, x, y] - let Some(exprs) = exprs_with_selected_binop_peeled(cast_op) else { - // Assume cast sign lose if we cannot determine the sign of `cast_op` - return true; - }; - for expr in exprs { - let ty = cx.typeck_results().expr_ty(expr); - match expr_sign(cx, expr, ty) { - Sign::Negative => negative_count += 1, - Sign::Uncertain => uncertain_count += 1, - Sign::ZeroOrPositive => (), - }; + if let Sign::ZeroOrPositive = expr_muldiv_sign(cx, cast_op) { + return false; + } + + if let Sign::ZeroOrPositive = expr_add_sign(cx, cast_op) { + return false; } - // Lint if there are odd number of uncertain or negative results - uncertain_count % 2 == 1 || negative_count % 2 == 1 + true }, (false, true) => !cast_to.is_signed(), @@ -57,7 +81,13 @@ fn should_lint(cx: &LateContext<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast } } -fn get_const_int_eval(cx: &LateContext<'_>, expr: &Expr<'_>, ty: Ty<'_>) -> Option { +fn get_const_signed_int_eval<'cx>( + cx: &LateContext<'cx>, + expr: &Expr<'_>, + ty: impl Into>>, +) -> Option { + let ty = ty.into().unwrap_or_else(|| cx.typeck_results().expr_ty(expr)); + if let Constant::Int(n) = constant(cx, cx.typeck_results(), expr)? && let ty::Int(ity) = *ty.kind() { @@ -66,29 +96,52 @@ fn get_const_int_eval(cx: &LateContext<'_>, expr: &Expr<'_>, ty: Ty<'_>) -> Opti None } +fn get_const_unsigned_int_eval<'cx>( + cx: &LateContext<'cx>, + expr: &Expr<'_>, + ty: impl Into>>, +) -> Option { + let ty = ty.into().unwrap_or_else(|| cx.typeck_results().expr_ty(expr)); + + if let Constant::Int(n) = constant(cx, cx.typeck_results(), expr)? + && let ty::Uint(_ity) = *ty.kind() + { + return Some(n); + } + None +} + +#[derive(Copy, Clone, Debug, Eq, PartialEq)] enum Sign { ZeroOrPositive, Negative, Uncertain, } -fn expr_sign(cx: &LateContext<'_>, expr: &Expr<'_>, ty: Ty<'_>) -> Sign { +fn expr_sign<'cx>(cx: &LateContext<'cx>, expr: &Expr<'_>, ty: impl Into>>) -> Sign { // Try evaluate this expr first to see if it's positive - if let Some(val) = get_const_int_eval(cx, expr, ty) { + if let Some(val) = get_const_signed_int_eval(cx, expr, ty) { return if val >= 0 { Sign::ZeroOrPositive } else { Sign::Negative }; } + if let Some(_val) = get_const_unsigned_int_eval(cx, expr, None) { + return Sign::ZeroOrPositive; + } + // Calling on methods that always return non-negative values. if let ExprKind::MethodCall(path, caller, args, ..) = expr.kind { let mut method_name = path.ident.name.as_str(); - if method_name == "unwrap" - && let Some(arglist) = method_chain_args(expr, &["unwrap"]) + // Peel unwrap(), expect(), etc. + while let Some(&found_name) = METHODS_UNWRAP.iter().find(|&name| &method_name == name) + && let Some(arglist) = method_chain_args(expr, &[found_name]) && let ExprKind::MethodCall(inner_path, ..) = &arglist[0].0.kind { + // The original type has changed, but we can't use `ty` here anyway, because it has been + // moved. method_name = inner_path.ident.name.as_str(); } - if method_name == "pow" + if METHODS_POW.iter().any(|&name| method_name == name) && let [arg] = args { return pow_call_result_sign(cx, caller, arg); @@ -100,53 +153,182 @@ fn expr_sign(cx: &LateContext<'_>, expr: &Expr<'_>, ty: Ty<'_>) -> Sign { Sign::Uncertain } -/// Return the sign of the `pow` call's result. +/// Return the sign of the `pow` call's result, ignoring overflow. /// -/// If the caller is a positive number, the result is always positive, -/// If the `power_of` is a even number, the result is always positive as well, -/// Otherwise a [`Sign::Uncertain`] will be returned. -fn pow_call_result_sign(cx: &LateContext<'_>, caller: &Expr<'_>, power_of: &Expr<'_>) -> Sign { - let caller_ty = cx.typeck_results().expr_ty(caller); - if let Some(caller_val) = get_const_int_eval(cx, caller, caller_ty) - && caller_val >= 0 - { - return Sign::ZeroOrPositive; +/// If the base is positive, the result is always positive. +/// If the exponent is a even number, the result is always positive, +/// Otherwise, if the base is negative, and the exponent is an odd number, the result is always +/// negative. +/// +/// Otherwise, returns [`Sign::Uncertain`]. +fn pow_call_result_sign(cx: &LateContext<'_>, base: &Expr<'_>, exponent: &Expr<'_>) -> Sign { + let base_sign = expr_sign(cx, base, None); + + // Rust's integer pow() functions take an unsigned exponent. + let exponent_val = get_const_unsigned_int_eval(cx, exponent, None); + let exponent_is_even = exponent_val.map(|val| val % 2 == 0); + + match (base_sign, exponent_is_even) { + // Non-negative bases always return non-negative results, ignoring overflow. + (Sign::ZeroOrPositive, _) | + // Any base raised to an even exponent is non-negative. + // These both hold even if we don't know the value of the base. + (_, Some(true)) + => Sign::ZeroOrPositive, + + // A negative base raised to an odd exponent is non-negative. + (Sign::Negative, Some(false)) => Sign::Negative, + + // Negative/unknown base to an unknown exponent, or unknown base to an odd exponent. + // Could be negative or positive depending on the actual values. + (Sign::Negative | Sign::Uncertain, None) | + (Sign::Uncertain, Some(false)) => Sign::Uncertain, } +} - if let Some(Constant::Int(n)) = constant(cx, cx.typeck_results(), power_of) - && clip(cx.tcx, n, UintTy::U32) % 2 == 0 - { - return Sign::ZeroOrPositive; +/// Peels binary operators such as [`BinOpKind::Mul`] or [`BinOpKind::Rem`], +/// where the result could always be positive. See [`exprs_with_muldiv_binop_peeled()`] for details. +/// +/// Returns the sign of the list of peeled expressions. +fn expr_muldiv_sign(cx: &LateContext<'_>, expr: &Expr<'_>) -> Sign { + let mut negative_count = 0; + + // Peel off possible binary expressions, for example: + // x * x / y => [x, x, y] + // a % b => [a] + let exprs = exprs_with_muldiv_binop_peeled(expr); + for expr in exprs { + match expr_sign(cx, expr, None) { + Sign::Negative => negative_count += 1, + // A mul/div is: + // - uncertain if there are any uncertain values (because they could be negative or positive), + Sign::Uncertain => return Sign::Uncertain, + Sign::ZeroOrPositive => (), + }; } - Sign::Uncertain + // A mul/div is: + // - negative if there are an odd number of negative values, + // - positive or zero otherwise. + if negative_count % 2 == 1 { + Sign::Negative + } else { + Sign::ZeroOrPositive + } +} + +/// Peels binary operators such as [`BinOpKind::Add`], where the result could always be positive. +/// See [`exprs_with_add_binop_peeled()`] for details. +/// +/// Returns the sign of the list of peeled expressions. +fn expr_add_sign(cx: &LateContext<'_>, expr: &Expr<'_>) -> Sign { + let mut negative_count = 0; + let mut positive_count = 0; + + // Peel off possible binary expressions, for example: + // a + b + c => [a, b, c] + let exprs = exprs_with_add_binop_peeled(expr); + for expr in exprs { + match expr_sign(cx, expr, None) { + Sign::Negative => negative_count += 1, + // A sum is: + // - uncertain if there are any uncertain values (because they could be negative or positive), + Sign::Uncertain => return Sign::Uncertain, + Sign::ZeroOrPositive => positive_count += 1, + }; + } + + // A sum is: + // - positive or zero if there are only positive (or zero) values, + // - negative if there are only negative (or zero) values, or + // - uncertain if there are both. + // We could split Zero out into its own variant, but we don't yet. + if negative_count == 0 { + Sign::ZeroOrPositive + } else if positive_count == 0 { + Sign::Negative + } else { + Sign::Uncertain + } } /// Peels binary operators such as [`BinOpKind::Mul`], [`BinOpKind::Div`] or [`BinOpKind::Rem`], -/// which the result could always be positive under certain condition. +/// where the result depends on: +/// - the number of negative values in the entire expression, or +/// - the number of negative values on the left hand side of the expression. +/// Ignores overflow. /// -/// Other operators such as `+`/`-` causing the result's sign hard to determine, which we will -/// return `None` -fn exprs_with_selected_binop_peeled<'a>(expr: &'a Expr<'_>) -> Option>> { - #[inline] - fn collect_operands<'a>(expr: &'a Expr<'a>, operands: &mut Vec<&'a Expr<'a>>) -> Option<()> { - match expr.kind { - ExprKind::Binary(op, lhs, rhs) => { - if matches!(op.node, BinOpKind::Mul | BinOpKind::Div | BinOpKind::Rem) { - collect_operands(lhs, operands); - operands.push(rhs); - } else { - // Things are complicated when there are other binary ops exist, - // abort checking by returning `None` for now. - return None; - } - }, - _ => operands.push(expr), +/// +/// Expressions using other operators are preserved, so we can try to evaluate them later. +fn exprs_with_muldiv_binop_peeled<'e>(expr: &'e Expr<'_>) -> Vec<&'e Expr<'e>> { + let mut res = vec![]; + + for_each_expr(expr, |sub_expr| -> ControlFlow { + // We don't check for mul/div/rem methods here, but we could. + if let ExprKind::Binary(op, lhs, _rhs) = sub_expr.kind { + if matches!(op.node, BinOpKind::Mul | BinOpKind::Div) { + // For binary operators where both sides contribute to the sign of the result, + // collect all their operands, recursively. This ignores overflow. + ControlFlow::Continue(Descend::Yes) + } else if matches!(op.node, BinOpKind::Rem | BinOpKind::Shr) { + // For binary operators where the left hand side determines the sign of the result, + // only collect that side, recursively. Overflow panics, so this always holds. + // + // Large left shifts turn negatives into zeroes, so we can't use it here. + // + // > Given remainder = dividend % divisor, the remainder will have the same sign as the dividend + // > ... + // > Arithmetic right shift on signed integer types + // https://doc.rust-lang.org/reference/expressions/operator-expr.html#arithmetic-and-logical-binary-operators + + // We want to descend into the lhs, but skip the rhs. + // That's tricky to do using for_each_expr(), so we just keep the lhs intact. + res.push(lhs); + ControlFlow::Continue(Descend::No) + } else { + // The sign of the result of other binary operators depends on the values of the operands, + // so try to evaluate the expression. + res.push(sub_expr); + ControlFlow::Continue(Descend::No) + } + } else { + // For other expressions, including unary operators and constants, try to evaluate the expression. + res.push(sub_expr); + ControlFlow::Continue(Descend::No) } - Some(()) - } + }); + res +} + +/// Peels binary operators such as [`BinOpKind::Add`], where the result depends on: +/// - all the expressions being positive, or +/// - all the expressions being negative. +/// Ignores overflow. +/// +/// Expressions using other operators are preserved, so we can try to evaluate them later. +fn exprs_with_add_binop_peeled<'e>(expr: &'e Expr<'_>) -> Vec<&'e Expr<'e>> { let mut res = vec![]; - collect_operands(expr, &mut res)?; - Some(res) + + for_each_expr(expr, |sub_expr| -> ControlFlow { + // We don't check for add methods here, but we could. + if let ExprKind::Binary(op, _lhs, _rhs) = sub_expr.kind { + if matches!(op.node, BinOpKind::Add) { + // For binary operators where both sides contribute to the sign of the result, + // collect all their operands, recursively. This ignores overflow. + ControlFlow::Continue(Descend::Yes) + } else { + // The sign of the result of other binary operators depends on the values of the operands, + // so try to evaluate the expression. + res.push(sub_expr); + ControlFlow::Continue(Descend::No) + } + } else { + // For other expressions, including unary operators and constants, try to evaluate the expression. + res.push(sub_expr); + ControlFlow::Continue(Descend::No) + } + }); + + res } diff --git a/src/tools/clippy/clippy_lints/src/casts/ref_as_ptr.rs b/src/tools/clippy/clippy_lints/src/casts/ref_as_ptr.rs index d600d2aec1b5e..9d5a486336d5f 100644 --- a/src/tools/clippy/clippy_lints/src/casts/ref_as_ptr.rs +++ b/src/tools/clippy/clippy_lints/src/casts/ref_as_ptr.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::is_no_std_crate; use clippy_utils::source::snippet_with_applicability; use clippy_utils::sugg::Sugg; +use clippy_utils::{expr_use_ctxt, is_no_std_crate, ExprUseNode}; use rustc_errors::Applicability; use rustc_hir::{Expr, Mutability, Ty, TyKind}; use rustc_lint::LateContext; @@ -9,7 +9,12 @@ use rustc_middle::ty::{self, TypeAndMut}; use super::REF_AS_PTR; -pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_to_hir_ty: &Ty<'_>) { +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'_>, + cast_expr: &'tcx Expr<'_>, + cast_to_hir_ty: &Ty<'_>, +) { let (cast_from, cast_to) = ( cx.typeck_results().expr_ty(cast_expr), cx.typeck_results().expr_ty(expr), @@ -17,6 +22,9 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, if matches!(cast_from.kind(), ty::Ref(..)) && let ty::RawPtr(TypeAndMut { mutbl: to_mutbl, .. }) = cast_to.kind() + && let Some(use_cx) = expr_use_ctxt(cx, expr) + // TODO: only block the lint if `cast_expr` is a temporary + && !matches!(use_cx.node, ExprUseNode::Local(_) | ExprUseNode::ConstStatic(_)) { let core_or_std = if is_no_std_crate(cx) { "core" } else { "std" }; let fn_name = match to_mutbl { diff --git a/src/tools/clippy/clippy_lints/src/casts/unnecessary_cast.rs b/src/tools/clippy/clippy_lints/src/casts/unnecessary_cast.rs index b4a23d0d4db41..cc513d46bf4ec 100644 --- a/src/tools/clippy/clippy_lints/src/casts/unnecessary_cast.rs +++ b/src/tools/clippy/clippy_lints/src/casts/unnecessary_cast.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::numeric_literal::NumericLiteral; use clippy_utils::source::snippet_opt; use clippy_utils::visitors::{for_each_expr, Visitable}; -use clippy_utils::{get_parent_expr, get_parent_node, is_hir_ty_cfg_dependant, is_ty_alias, path_to_local}; +use clippy_utils::{get_parent_expr, is_hir_ty_cfg_dependant, is_ty_alias, path_to_local}; use rustc_ast::{LitFloatType, LitIntType, LitKind}; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; @@ -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) = cx.tcx.parent_hir_node(hir_id) { if let Some(e) = l.init && is_cast_from_ty_alias(cx, e, cast_from) diff --git a/src/tools/clippy/clippy_lints/src/collection_is_never_read.rs b/src/tools/clippy/clippy_lints/src/collection_is_never_read.rs index d0c989cfff304..d820413e11129 100644 --- a/src/tools/clippy/clippy_lints/src/collection_is_never_read.rs +++ b/src/tools/clippy/clippy_lints/src/collection_is_never_read.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint; use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item}; use clippy_utils::visitors::for_each_expr_with_closures; -use clippy_utils::{get_enclosing_block, get_parent_node, path_to_local_id}; +use clippy_utils::{get_enclosing_block, path_to_local_id}; use core::ops::ControlFlow; use rustc_hir::{Block, ExprKind, HirId, LangItem, Local, Node, PatKind}; use rustc_lint::{LateContext, LateLintPass}; @@ -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) = cx.tcx.parent_hir_node(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) = cx.tcx.parent_hir_node(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(..) = cx.tcx.parent_hir_node(parent.hir_id) { return ControlFlow::Continue(()); } diff --git a/src/tools/clippy/clippy_lints/src/declared_lints.rs b/src/tools/clippy/clippy_lints/src/declared_lints.rs index 0a5baabd973ea..fb3ae2457e3c4 100644 --- a/src/tools/clippy/clippy_lints/src/declared_lints.rs +++ b/src/tools/clippy/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, @@ -137,6 +139,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::disallowed_types::DISALLOWED_TYPES_INFO, crate::doc::DOC_LINK_WITH_QUOTES_INFO, crate::doc::DOC_MARKDOWN_INFO, + crate::doc::EMPTY_DOCS_INFO, crate::doc::MISSING_ERRORS_DOC_INFO, crate::doc::MISSING_PANICS_DOC_INFO, crate::doc::MISSING_SAFETY_DOC_INFO, @@ -453,6 +456,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::UNNECESSARY_FILTER_MAP_INFO, crate::methods::UNNECESSARY_FIND_MAP_INFO, crate::methods::UNNECESSARY_FOLD_INFO, + crate::methods::UNNECESSARY_GET_THEN_CHECK_INFO, crate::methods::UNNECESSARY_JOIN_INFO, crate::methods::UNNECESSARY_LAZY_EVALUATIONS_INFO, crate::methods::UNNECESSARY_LITERAL_UNWRAP_INFO, @@ -497,6 +501,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::module_style::MOD_MODULE_FILES_INFO, crate::module_style::SELF_NAMED_MODULE_FILES_INFO, crate::multi_assignments::MULTI_ASSIGNMENTS_INFO, + crate::multiple_bound_locations::MULTIPLE_BOUND_LOCATIONS_INFO, crate::multiple_unsafe_ops_per_block::MULTIPLE_UNSAFE_OPS_PER_BLOCK_INFO, crate::mut_key::MUTABLE_KEY_TYPE_INFO, crate::mut_mut::MUT_MUT_INFO, diff --git a/src/tools/clippy/clippy_lints/src/dereference.rs b/src/tools/clippy/clippy_lints/src/dereference.rs index 0ddfeaa0ae059..560b2acc1c702 100644 --- a/src/tools/clippy/clippy_lints/src/dereference.rs +++ b/src/tools/clippy/clippy_lints/src/dereference.rs @@ -2,9 +2,7 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then}; use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; use clippy_utils::sugg::has_enclosing_paren; use clippy_utils::ty::{implements_trait, is_manually_drop, peel_mid_ty_refs}; -use clippy_utils::{ - expr_use_ctxt, get_parent_expr, get_parent_node, is_lint_allowed, path_to_local, DefinedTy, ExprUseNode, -}; +use clippy_utils::{expr_use_ctxt, get_parent_expr, is_lint_allowed, path_to_local, DefinedTy, ExprUseNode}; use core::mem; use rustc_ast::util::parser::{PREC_POSTFIX, PREC_PREFIX}; use rustc_data_structures::fx::FxIndexMap; @@ -1008,8 +1006,8 @@ fn report<'tcx>( data.first_expr.span, 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 { + let (precedence, calls_field) = match cx.tcx.parent_hir_node(data.first_expr.hir_id) { + 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/src/tools/clippy/clippy_lints/src/disallowed_macros.rs b/src/tools/clippy/clippy_lints/src/disallowed_macros.rs index 656b3d9bfaf1b..75379cb4e5458 100644 --- a/src/tools/clippy/clippy_lints/src/disallowed_macros.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/doc/mod.rs b/src/tools/clippy/clippy_lints/src/doc/mod.rs index 2b4ce6ddfaaa3..9af34c3a7bf0b 100644 --- a/src/tools/clippy/clippy_lints/src/doc/mod.rs +++ b/src/tools/clippy/clippy_lints/src/doc/mod.rs @@ -19,7 +19,8 @@ use rustc_middle::hir::nested_filter; use rustc_middle::lint::in_external_macro; use rustc_middle::ty; use rustc_resolve::rustdoc::{ - add_doc_fragment, attrs_to_doc_fragments, main_body_opts, source_span_for_markdown_range, DocFragment, + add_doc_fragment, attrs_to_doc_fragments, main_body_opts, source_span_for_markdown_range, span_of_fragments, + DocFragment, }; use rustc_session::impl_lint_pass; use rustc_span::edition::Edition; @@ -338,6 +339,30 @@ declare_clippy_lint! { "suspicious usage of (outer) doc comments" } +declare_clippy_lint! { + /// ### What it does + /// Detects documentation that is empty. + /// ### Why is this bad? + /// Empty docs clutter code without adding value, reducing readability and maintainability. + /// ### Example + /// ```no_run + /// /// + /// fn returns_true() -> bool { + /// true + /// } + /// ``` + /// Use instead: + /// ```no_run + /// fn returns_true() -> bool { + /// true + /// } + /// ``` + #[clippy::version = "1.78.0"] + pub EMPTY_DOCS, + suspicious, + "docstrings exist but documentation is empty" +} + #[derive(Clone)] pub struct Documentation { valid_idents: FxHashSet, @@ -364,7 +389,8 @@ impl_lint_pass!(Documentation => [ NEEDLESS_DOCTEST_MAIN, TEST_ATTR_IN_DOCTEST, UNNECESSARY_SAFETY_DOC, - SUSPICIOUS_DOC_COMMENTS + SUSPICIOUS_DOC_COMMENTS, + EMPTY_DOCS, ]); impl<'tcx> LateLintPass<'tcx> for Documentation { @@ -373,11 +399,22 @@ impl<'tcx> LateLintPass<'tcx> for Documentation { check_attrs(cx, &self.valid_idents, attrs); } + fn check_variant(&mut self, cx: &LateContext<'tcx>, variant: &'tcx hir::Variant<'tcx>) { + let attrs = cx.tcx.hir().attrs(variant.hir_id); + check_attrs(cx, &self.valid_idents, attrs); + } + + fn check_field_def(&mut self, cx: &LateContext<'tcx>, variant: &'tcx hir::FieldDef<'tcx>) { + let attrs = cx.tcx.hir().attrs(variant.hir_id); + check_attrs(cx, &self.valid_idents, attrs); + } + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) { let attrs = cx.tcx.hir().attrs(item.hir_id()); let Some(headers) = check_attrs(cx, &self.valid_idents, attrs) else { return; }; + match item.kind { hir::ItemKind::Fn(ref sig, _, body_id) => { if !(is_entrypoint_fn(cx, item.owner_id.to_def_id()) || in_external_macro(cx.tcx.sess, item.span)) { @@ -502,13 +539,23 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet, attrs: &[ suspicious_doc_comments::check(cx, attrs); let (fragments, _) = attrs_to_doc_fragments(attrs.iter().map(|attr| (attr, None)), true); - let mut doc = String::new(); - for fragment in &fragments { - add_doc_fragment(&mut doc, fragment); - } + let mut doc = fragments.iter().fold(String::new(), |mut acc, fragment| { + add_doc_fragment(&mut acc, fragment); + acc + }); doc.pop(); - if doc.is_empty() { + if doc.trim().is_empty() { + if let Some(span) = span_of_fragments(&fragments) { + span_lint_and_help( + cx, + EMPTY_DOCS, + span, + "empty doc comment", + None, + "consider removing or filling it", + ); + } return Some(DocHeaders::default()); } diff --git a/src/tools/clippy/clippy_lints/src/drop_forget_ref.rs b/src/tools/clippy/clippy_lints/src/drop_forget_ref.rs index 124d78fc4ffdc..bf6f54c1e72a3 100644 --- a/src/tools/clippy/clippy_lints/src/drop_forget_ref.rs +++ b/src/tools/clippy/clippy_lints/src/drop_forget_ref.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_note; +use clippy_utils::is_must_use_func_call; use clippy_utils::ty::{is_copy, is_must_use_ty, is_type_lang_item}; -use clippy_utils::{get_parent_node, is_must_use_func_call}; use rustc_hir::{Arm, Expr, ExprKind, LangItem, Node}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; @@ -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, .. }) = cx.tcx.parent_hir_node(drop_expr.hir_id) { return body.hir_id == drop_expr.hir_id; } } diff --git a/src/tools/clippy/clippy_lints/src/format_args.rs b/src/tools/clippy/clippy_lints/src/format_args.rs index 8af321e4d553b..61f550ce0beb4 100644 --- a/src/tools/clippy/clippy_lints/src/format_args.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/format_impl.rs b/src/tools/clippy/clippy_lints/src/format_impl.rs index 9360eb1fa91ad..93517076cda01 100644 --- a/src/tools/clippy/clippy_lints/src/format_impl.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/implied_bounds_in_impls.rs b/src/tools/clippy/clippy_lints/src/implied_bounds_in_impls.rs index fab8ffedb9493..74582f7f1de23 100644 --- a/src/tools/clippy/clippy_lints/src/implied_bounds_in_impls.rs +++ b/src/tools/clippy/clippy_lints/src/implied_bounds_in_impls.rs @@ -1,11 +1,10 @@ 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::intravisit::FnKind; +use rustc_hir::def_id::DefId; use rustc_hir::{ - Body, FnDecl, FnRetTy, GenericArg, GenericBound, ImplItem, ImplItemKind, ItemKind, TraitBoundModifier, TraitItem, - TraitItemKind, TyKind, + GenericArg, GenericBound, GenericBounds, ItemKind, PredicateOrigin, TraitBoundModifier, TyKind, TypeBinding, + WherePredicate, }; use rustc_hir_analysis::hir_ty_to_ty; use rustc_lint::{LateContext, LateLintPass}; @@ -50,20 +49,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<'_>, + bounds: GenericBounds<'_>, 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, @@ -75,10 +71,10 @@ fn emit_lint( // to include the `+` token that is ahead or behind, // so we don't end up with something like `impl + B` or `impl A + ` - let implied_span_extended = if let Some(next_bound) = opaque_ty.bounds.get(index + 1) { + let implied_span_extended = if let Some(next_bound) = bounds.get(index + 1) { poly_trait.span.to(next_bound.span().shrink_to_lo()) } else if index > 0 - && let Some(prev_bound) = opaque_ty.bounds.get(index - 1) + && let Some(prev_bound) = bounds.get(index - 1) { prev_bound.span().shrink_to_hi().to(poly_trait.span.shrink_to_hi()) } else { @@ -93,17 +89,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,111 +219,135 @@ fn is_same_generics<'tcx>( }) } -fn check(cx: &LateContext<'_>, decl: &FnDecl<'_>) { - if let FnRetTy::Return(ty) = decl.output - &&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(); +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>], +} - // Lint all bounds in the `impl Trait` type that are also in the `implied_bounds` vec. - // 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() { +/// 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>, bounds: GenericBounds<'tcx>) -> Vec> { + bounds + .iter() + .filter_map(|bound| { if let GenericBound::Trait(poly_trait, TraitBoundModifier::None) = bound && let [.., path] = poly_trait.trait_ref.path.segments - && 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); + && 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() +} - 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 - } - }) - }) +/// 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 { - emit_lint( - cx, - poly_trait, - opaque_ty, - index, - implied_bindings, - implied_by_bindings, - implied_by_args, - implied_by_span, - ); + is_same_generics( + cx.tcx, + tr.trait_ref.args, + bound.args, + args, + bound.trait_def_id, + trait_def_id, + ) + } else { + false } + }) + }) +} + +fn check<'tcx>(cx: &LateContext<'tcx>, bounds: GenericBounds<'tcx>) { + if bounds.len() == 1 { + // Very often there is only a single bound, e.g. `impl Deref<..>`, in which case + // we can avoid doing a bunch of stuff unnecessarily; there will trivially be + // no duplicate bounds + return; + } + + let supertraits = collect_supertrait_bounds(cx, bounds); + + // 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 bounds.iter().enumerate() { + if let GenericBound::Trait(poly_trait, TraitBoundModifier::None) = bound + && let [.., path] = poly_trait.trait_ref.path.segments + && 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(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, bounds, index, implied_bindings, bound); } } } -impl LateLintPass<'_> for ImpliedBoundsInImpls { - fn check_fn( - &mut self, - cx: &LateContext<'_>, - _: FnKind<'_>, - decl: &FnDecl<'_>, - _: &Body<'_>, - _: Span, - _: LocalDefId, - ) { - check(cx, decl); - } - fn check_trait_item(&mut self, cx: &LateContext<'_>, item: &TraitItem<'_>) { - if let TraitItemKind::Fn(sig, ..) = &item.kind { - check(cx, sig.decl); +impl<'tcx> LateLintPass<'tcx> for ImpliedBoundsInImpls { + fn check_generics(&mut self, cx: &LateContext<'tcx>, generics: &rustc_hir::Generics<'tcx>) { + for predicate in generics.predicates { + if let WherePredicate::BoundPredicate(predicate) = predicate + // In theory, the origin doesn't really matter, + // we *could* also lint on explicit where clauses written out by the user, + // not just impl trait desugared ones, but that contradicts with the lint name... + && let PredicateOrigin::ImplTrait = predicate.origin + { + check(cx, predicate.bounds); + } } } - fn check_impl_item(&mut self, cx: &LateContext<'_>, item: &ImplItem<'_>) { - if let ImplItemKind::Fn(sig, ..) = &item.kind { - check(cx, sig.decl); + + fn check_ty(&mut self, cx: &LateContext<'_>, ty: &rustc_hir::Ty<'_>) { + if let TyKind::OpaqueDef(item_id, ..) = ty.kind + && let item = cx.tcx.hir().item(item_id) + && let ItemKind::OpaqueTy(opaque_ty) = item.kind + { + check(cx, opaque_ty.bounds); } } } diff --git a/src/tools/clippy/clippy_lints/src/incompatible_msrv.rs b/src/tools/clippy/clippy_lints/src/incompatible_msrv.rs index f2f0e7d426628..cd000fcd18449 100644 --- a/src/tools/clippy/clippy_lints/src/incompatible_msrv.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/index_refutable_slice.rs b/src/tools/clippy/clippy_lints/src/index_refutable_slice.rs index 41e9d5b1c2e9d..5b5eb355f86cd 100644 --- a/src/tools/clippy/clippy_lints/src/index_refutable_slice.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/indexing_slicing.rs b/src/tools/clippy/clippy_lints/src/indexing_slicing.rs index 391db0b0df726..35fcd8cdd3547 100644 --- a/src/tools/clippy/clippy_lints/src/indexing_slicing.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/item_name_repetitions.rs b/src/tools/clippy/clippy_lints/src/item_name_repetitions.rs index 276c1abb60cd5..0b4c416d94db0 100644 --- a/src/tools/clippy/clippy_lints/src/item_name_repetitions.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/iter_not_returning_iterator.rs b/src/tools/clippy/clippy_lints/src/iter_not_returning_iterator.rs index b9fad726511c5..32ae6be568735 100644 --- a/src/tools/clippy/clippy_lints/src/iter_not_returning_iterator.rs +++ b/src/tools/clippy/clippy_lints/src/iter_not_returning_iterator.rs @@ -1,5 +1,4 @@ use clippy_utils::diagnostics::span_lint; -use clippy_utils::get_parent_node; use clippy_utils::ty::implements_trait; use rustc_hir::def_id::LocalDefId; use rustc_hir::{FnSig, ImplItem, ImplItemKind, Item, ItemKind, Node, TraitItem, TraitItemKind}; @@ -56,8 +55,8 @@ impl<'tcx> LateLintPass<'tcx> for IterNotReturningIterator { let name = item.ident.name.as_str(); 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() + cx.tcx.parent_hir_node(item.hir_id()), + Node::Item(Item { kind: ItemKind::Impl(i), .. }) if i.of_trait.is_some() ) { if let ImplItemKind::Fn(fn_sig, _) = &item.kind { diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index 14bd82f9c97ea..76e759683145e 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/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 @@ -231,6 +231,7 @@ mod missing_trait_methods; mod mixed_read_write_in_expression; mod module_style; mod multi_assignments; +mod multiple_bound_locations; mod multiple_unsafe_ops_per_block; mod mut_key; mod mut_mut; @@ -1067,7 +1068,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(move |_| { Box::new(single_call_fn::SingleCallFn { avoid_breaking_exported_api, - def_id_to_usage: rustc_data_structures::fx::FxHashMap::default(), + def_id_to_usage: rustc_data_structures::fx::FxIndexMap::default(), }) }); store.register_early_pass(move || { @@ -1116,6 +1117,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { }); store.register_late_pass(move |_| Box::new(incompatible_msrv::IncompatibleMsrv::new(msrv()))); store.register_late_pass(|_| Box::new(to_string_trait_impl::ToStringTraitImpl)); + store.register_early_pass(|| Box::new(multiple_bound_locations::MultipleBoundLocations)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/src/tools/clippy/clippy_lints/src/loops/infinite_loop.rs b/src/tools/clippy/clippy_lints/src/loops/infinite_loop.rs index 5e099f1e76f86..5b5bb88c17908 100644 --- a/src/tools/clippy/clippy_lints/src/loops/infinite_loop.rs +++ b/src/tools/clippy/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