diff options
Diffstat (limited to 'clippy_lints/src')
39 files changed, 1999 insertions, 1431 deletions
diff --git a/clippy_lints/src/assigning_clones.rs b/clippy_lints/src/assigning_clones.rs new file mode 100644 index 00000000000..b1c552c7a8d --- /dev/null +++ b/clippy_lints/src/assigning_clones.rs @@ -0,0 +1,322 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::macros::HirNode; +use clippy_utils::sugg::Sugg; +use clippy_utils::{is_trait_method, path_to_local}; +use rustc_errors::Applicability; +use rustc_hir::{self as hir, Expr, ExprKind, Node}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{self, Instance, Mutability}; +use rustc_session::declare_lint_pass; +use rustc_span::def_id::DefId; +use rustc_span::symbol::sym; +use rustc_span::ExpnKind; + +declare_clippy_lint! { + /// ### What it does + /// Checks for code like `foo = bar.clone();` + /// + /// ### Why is this bad? + /// Custom `Clone::clone_from()` or `ToOwned::clone_into` implementations allow the objects + /// to share resources and therefore avoid allocations. + /// + /// ### Example + /// ```rust + /// struct Thing; + /// + /// impl Clone for Thing { + /// fn clone(&self) -> Self { todo!() } + /// fn clone_from(&mut self, other: &Self) { todo!() } + /// } + /// + /// pub fn assign_to_ref(a: &mut Thing, b: Thing) { + /// *a = b.clone(); + /// } + /// ``` + /// Use instead: + /// ```rust + /// struct Thing; + /// impl Clone for Thing { + /// fn clone(&self) -> Self { todo!() } + /// fn clone_from(&mut self, other: &Self) { todo!() } + /// } + /// + /// pub fn assign_to_ref(a: &mut Thing, b: Thing) { + /// a.clone_from(&b); + /// } + /// ``` + #[clippy::version = "1.77.0"] + pub ASSIGNING_CLONES, + perf, + "assigning the result of cloning may be inefficient" +} +declare_lint_pass!(AssigningClones => [ASSIGNING_CLONES]); + +impl<'tcx> LateLintPass<'tcx> for AssigningClones { + fn check_expr(&mut self, cx: &LateContext<'tcx>, assign_expr: &'tcx hir::Expr<'_>) { + // Do not fire the lint in macros + let expn_data = assign_expr.span().ctxt().outer_expn_data(); + match expn_data.kind { + ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) | ExpnKind::Macro(..) => return, + ExpnKind::Root => {}, + } + + let ExprKind::Assign(lhs, rhs, _span) = assign_expr.kind else { + return; + }; + + let Some(call) = extract_call(cx, rhs) else { + return; + }; + + if is_ok_to_suggest(cx, lhs, &call) { + suggest(cx, assign_expr, lhs, &call); + } + } +} + +// Try to resolve the call to `Clone::clone` or `ToOwned::to_owned`. +fn extract_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<CallCandidate<'tcx>> { + let fn_def_id = clippy_utils::fn_def_id(cx, expr)?; + + // Fast paths to only check method calls without arguments or function calls with a single argument + let (target, kind, resolved_method) = match expr.kind { + ExprKind::MethodCall(path, receiver, [], _span) => { + let args = cx.typeck_results().node_args(expr.hir_id); + + // If we could not resolve the method, don't apply the lint + let Ok(Some(resolved_method)) = Instance::resolve(cx.tcx, cx.param_env, fn_def_id, args) else { + return None; + }; + if is_trait_method(cx, expr, sym::Clone) && path.ident.name == sym::clone { + (TargetTrait::Clone, CallKind::MethodCall { receiver }, resolved_method) + } else if is_trait_method(cx, expr, sym::ToOwned) && path.ident.name.as_str() == "to_owned" { + (TargetTrait::ToOwned, CallKind::MethodCall { receiver }, resolved_method) + } else { + return None; + } + }, + ExprKind::Call(function, [arg]) => { + let kind = cx.typeck_results().node_type(function.hir_id).kind(); + + // If we could not resolve the method, don't apply the lint + let Ok(Some(resolved_method)) = (match kind { + ty::FnDef(_, args) => Instance::resolve(cx.tcx, cx.param_env, fn_def_id, args), + _ => Ok(None), + }) else { + return None; + }; + if cx.tcx.is_diagnostic_item(sym::to_owned_method, fn_def_id) { + ( + TargetTrait::ToOwned, + CallKind::FunctionCall { self_arg: arg }, + resolved_method, + ) + } else if let Some(trait_did) = cx.tcx.trait_of_item(fn_def_id) + && cx.tcx.is_diagnostic_item(sym::Clone, trait_did) + { + ( + TargetTrait::Clone, + CallKind::FunctionCall { self_arg: arg }, + resolved_method, + ) + } else { + return None; + } + }, + _ => return None, + }; + + Some(CallCandidate { + target, + kind, + method_def_id: resolved_method.def_id(), + }) +} + +// Return true if we find that the called method has a custom implementation and isn't derived or +// provided by default by the corresponding trait. +fn is_ok_to_suggest<'tcx>(cx: &LateContext<'tcx>, lhs: &Expr<'tcx>, call: &CallCandidate<'tcx>) -> bool { + // If the left-hand side is a local variable, it might be uninitialized at this point. + // In that case we do not want to suggest the lint. + if let Some(local) = path_to_local(lhs) { + // TODO: This check currently bails if the local variable has no initializer. + // That is overly conservative - the lint should fire even if there was no initializer, + // but the variable has been initialized before `lhs` was evaluated. + if let Some(Node::Local(local)) = cx.tcx.hir().parent_id_iter(local).next().map(|p| cx.tcx.hir_node(p)) + && local.init.is_none() + { + return false; + } + } + + let Some(impl_block) = cx.tcx.impl_of_method(call.method_def_id) else { + return false; + }; + + // If the method implementation comes from #[derive(Clone)], then don't suggest the lint. + // Automatically generated Clone impls do not currently override `clone_from`. + // See e.g. https://github.com/rust-lang/rust/pull/98445#issuecomment-1190681305 for more details. + if cx.tcx.is_builtin_derived(impl_block) { + return false; + } + + // Find the function for which we want to check that it is implemented. + let provided_fn = match call.target { + TargetTrait::Clone => cx.tcx.get_diagnostic_item(sym::Clone).and_then(|clone| { + cx.tcx + .provided_trait_methods(clone) + .find(|item| item.name == sym::clone_from) + }), + TargetTrait::ToOwned => cx.tcx.get_diagnostic_item(sym::ToOwned).and_then(|to_owned| { + cx.tcx + .provided_trait_methods(to_owned) + .find(|item| item.name.as_str() == "clone_into") + }), + }; + let Some(provided_fn) = provided_fn else { + return false; + }; + + // Now take a look if the impl block defines an implementation for the method that we're interested + // in. If not, then we're using a default implementation, which is not interesting, so we will + // not suggest the lint. + let implemented_fns = cx.tcx.impl_item_implementor_ids(impl_block); + implemented_fns.contains_key(&provided_fn.def_id) +} + +fn suggest<'tcx>( + cx: &LateContext<'tcx>, + assign_expr: &hir::Expr<'tcx>, + lhs: &hir::Expr<'tcx>, + call: &CallCandidate<'tcx>, +) { + span_lint_and_then(cx, ASSIGNING_CLONES, assign_expr.span, call.message(), |diag| { + let mut applicability = Applicability::MachineApplicable; + + diag.span_suggestion( + assign_expr.span, + call.suggestion_msg(), + call.suggested_replacement(cx, lhs, &mut applicability), + applicability, + ); + }); +} + +#[derive(Copy, Clone, Debug)] +enum CallKind<'tcx> { + MethodCall { receiver: &'tcx Expr<'tcx> }, + FunctionCall { self_arg: &'tcx Expr<'tcx> }, +} + +#[derive(Copy, Clone, Debug)] +enum TargetTrait { + Clone, + ToOwned, +} + +#[derive(Debug)] +struct CallCandidate<'tcx> { + target: TargetTrait, + kind: CallKind<'tcx>, + // DefId of the called method from an impl block that implements the target trait + method_def_id: DefId, +} + +impl<'tcx> CallCandidate<'tcx> { + #[inline] + fn message(&self) -> &'static str { + match self.target { + TargetTrait::Clone => "assigning the result of `Clone::clone()` may be inefficient", + TargetTrait::ToOwned => "assigning the result of `ToOwned::to_owned()` may be inefficient", + } + } + + #[inline] + fn suggestion_msg(&self) -> &'static str { + match self.target { + TargetTrait::Clone => "use `clone_from()`", + TargetTrait::ToOwned => "use `clone_into()`", + } + } + + fn suggested_replacement( + &self, + cx: &LateContext<'tcx>, + lhs: &hir::Expr<'tcx>, + applicability: &mut Applicability, + ) -> String { + match self.target { + TargetTrait::Clone => { + match self.kind { + CallKind::MethodCall { receiver } => { + let receiver_sugg = if let ExprKind::Unary(hir::UnOp::Deref, ref_expr) = lhs.kind { + // `*lhs = self_expr.clone();` -> `lhs.clone_from(self_expr)` + Sugg::hir_with_applicability(cx, ref_expr, "_", applicability) + } else { + // `lhs = self_expr.clone();` -> `lhs.clone_from(self_expr)` + Sugg::hir_with_applicability(cx, lhs, "_", applicability) + } + .maybe_par(); + + // Determine whether we need to reference the argument to clone_from(). + let clone_receiver_type = cx.typeck_results().expr_ty(receiver); + let clone_receiver_adj_type = cx.typeck_results().expr_ty_adjusted(receiver); + let mut arg_sugg = Sugg::hir_with_applicability(cx, receiver, "_", applicability); + if clone_receiver_type != clone_receiver_adj_type { + // The receiver may have been a value type, so we need to add an `&` to + // be sure the argument to clone_from will be a reference. + arg_sugg = arg_sugg.addr(); + }; + + format!("{receiver_sugg}.clone_from({arg_sugg})") + }, + CallKind::FunctionCall { self_arg, .. } => { + let self_sugg = if let ExprKind::Unary(hir::UnOp::Deref, ref_expr) = lhs.kind { + // `*lhs = Clone::clone(self_expr);` -> `Clone::clone_from(lhs, self_expr)` + Sugg::hir_with_applicability(cx, ref_expr, "_", applicability) + } else { + // `lhs = Clone::clone(self_expr);` -> `Clone::clone_from(&mut lhs, self_expr)` + Sugg::hir_with_applicability(cx, lhs, "_", applicability).mut_addr() + }; + // The RHS had to be exactly correct before the call, there is no auto-deref for function calls. + let rhs_sugg = Sugg::hir_with_applicability(cx, self_arg, "_", applicability); + + format!("Clone::clone_from({self_sugg}, {rhs_sugg})") + }, + } + }, + TargetTrait::ToOwned => { + let rhs_sugg = if let ExprKind::Unary(hir::UnOp::Deref, ref_expr) = lhs.kind { + // `*lhs = rhs.to_owned()` -> `rhs.clone_into(lhs)` + // `*lhs = ToOwned::to_owned(rhs)` -> `ToOwned::clone_into(rhs, lhs)` + let sugg = Sugg::hir_with_applicability(cx, ref_expr, "_", applicability).maybe_par(); + let inner_type = cx.typeck_results().expr_ty(ref_expr); + // If after unwrapping the dereference, the type is not a mutable reference, we add &mut to make it + // deref to a mutable reference. + if matches!(inner_type.kind(), ty::Ref(_, _, Mutability::Mut)) { + sugg + } else { + sugg.mut_addr() + } + } else { + // `lhs = rhs.to_owned()` -> `rhs.clone_into(&mut lhs)` + // `lhs = ToOwned::to_owned(rhs)` -> `ToOwned::clone_into(rhs, &mut lhs)` + Sugg::hir_with_applicability(cx, lhs, "_", applicability) + .maybe_par() + .mut_addr() + }; + + match self.kind { + CallKind::MethodCall { receiver } => { + let receiver_sugg = Sugg::hir_with_applicability(cx, receiver, "_", applicability); + format!("{receiver_sugg}.clone_into({rhs_sugg})") + }, + CallKind::FunctionCall { self_arg, .. } => { + let self_sugg = Sugg::hir_with_applicability(cx, self_arg, "_", applicability); + format!("ToOwned::clone_into({self_sugg}, {rhs_sugg})") + }, + } + }, + } + } +} diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs deleted file mode 100644 index f2937d51340..00000000000 --- a/clippy_lints/src/attrs.rs +++ /dev/null @@ -1,1210 +0,0 @@ -//! checks for attributes - -use clippy_config::msrvs::{self, Msrv}; -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}; -use rustc_ast::token::{Token, TokenKind}; -use rustc_ast::tokenstream::TokenTree; -use rustc_ast::{ - AttrArgs, AttrArgsEq, AttrKind, AttrStyle, Attribute, LitKind, MetaItemKind, MetaItemLit, NestedMetaItem, -}; -use rustc_errors::Applicability; -use rustc_hir::{ - Block, Expr, ExprKind, ImplItem, ImplItemKind, Item, ItemKind, StmtKind, TraitFn, TraitItem, TraitItemKind, -}; -use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, Level, LintContext}; -use rustc_middle::lint::in_external_macro; -use rustc_middle::ty; -use rustc_session::{declare_lint_pass, impl_lint_pass}; -use rustc_span::symbol::Symbol; -use rustc_span::{sym, Span, DUMMY_SP}; -use semver::Version; - -static UNIX_SYSTEMS: &[&str] = &[ - "android", - "dragonfly", - "emscripten", - "freebsd", - "fuchsia", - "haiku", - "illumos", - "ios", - "l4re", - "linux", - "macos", - "netbsd", - "openbsd", - "redox", - "solaris", - "vxworks", -]; - -// NOTE: windows is excluded from the list because it's also a valid target family. -static NON_UNIX_SYSTEMS: &[&str] = &["hermit", "none", "wasi"]; - -declare_clippy_lint! { - /// ### What it does - /// Checks for items annotated with `#[inline(always)]`, - /// unless the annotated function is empty or simply panics. - /// - /// ### Why is this bad? - /// While there are valid uses of this annotation (and once - /// you know when to use it, by all means `allow` this lint), it's a common - /// newbie-mistake to pepper one's code with it. - /// - /// As a rule of thumb, before slapping `#[inline(always)]` on a function, - /// measure if that additional function call really affects your runtime profile - /// sufficiently to make up for the increase in compile time. - /// - /// ### Known problems - /// False positives, big time. This lint is meant to be - /// deactivated by everyone doing serious performance work. This means having - /// done the measurement. - /// - /// ### Example - /// ```ignore - /// #[inline(always)] - /// fn not_quite_hot_code(..) { ... } - /// ``` - #[clippy::version = "pre 1.29.0"] - pub INLINE_ALWAYS, - pedantic, - "use of `#[inline(always)]`" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for `extern crate` and `use` items annotated with - /// lint attributes. - /// - /// This lint permits lint attributes for lints emitted on the items themself. - /// For `use` items these lints are: - /// * deprecated - /// * unreachable_pub - /// * unused_imports - /// * clippy::enum_glob_use - /// * clippy::macro_use_imports - /// * clippy::wildcard_imports - /// - /// For `extern crate` items these lints are: - /// * `unused_imports` on items with `#[macro_use]` - /// - /// ### Why is this bad? - /// Lint attributes have no effect on crate imports. Most - /// likely a `!` was forgotten. - /// - /// ### Example - /// ```ignore - /// #[deny(dead_code)] - /// extern crate foo; - /// #[forbid(dead_code)] - /// use foo::bar; - /// ``` - /// - /// Use instead: - /// ```rust,ignore - /// #[allow(unused_imports)] - /// use foo::baz; - /// #[allow(unused_imports)] - /// #[macro_use] - /// extern crate baz; - /// ``` - #[clippy::version = "pre 1.29.0"] - pub USELESS_ATTRIBUTE, - correctness, - "use of lint attributes on `extern crate` items" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for `#[deprecated]` annotations with a `since` - /// field that is not a valid semantic version. Also allows "TBD" to signal - /// future deprecation. - /// - /// ### Why is this bad? - /// For checking the version of the deprecation, it must be - /// a valid semver. Failing that, the contained information is useless. - /// - /// ### Example - /// ```no_run - /// #[deprecated(since = "forever")] - /// fn something_else() { /* ... */ } - /// ``` - #[clippy::version = "pre 1.29.0"] - pub DEPRECATED_SEMVER, - correctness, - "use of `#[deprecated(since = \"x\")]` where x is not semver" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for empty lines after outer attributes - /// - /// ### Why is this bad? - /// Most likely the attribute was meant to be an inner attribute using a '!'. - /// If it was meant to be an outer attribute, then the following item - /// should not be separated by empty lines. - /// - /// ### Known problems - /// Can cause false positives. - /// - /// From the clippy side it's difficult to detect empty lines between an attributes and the - /// following item because empty lines and comments are not part of the AST. The parsing - /// currently works for basic cases but is not perfect. - /// - /// ### Example - /// ```no_run - /// #[allow(dead_code)] - /// - /// fn not_quite_good_code() { } - /// ``` - /// - /// Use instead: - /// ```no_run - /// // Good (as inner attribute) - /// #![allow(dead_code)] - /// - /// fn this_is_fine() { } - /// - /// // or - /// - /// // Good (as outer attribute) - /// #[allow(dead_code)] - /// fn this_is_fine_too() { } - /// ``` - #[clippy::version = "pre 1.29.0"] - pub EMPTY_LINE_AFTER_OUTER_ATTR, - nursery, - "empty line after outer attribute" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for empty lines after documentation comments. - /// - /// ### Why is this bad? - /// The documentation comment was most likely meant to be an inner attribute or regular comment. - /// If it was intended to be a documentation comment, then the empty line should be removed to - /// be more idiomatic. - /// - /// ### Known problems - /// Only detects empty lines immediately following the documentation. If the doc comment is followed - /// by an attribute and then an empty line, this lint will not trigger. Use `empty_line_after_outer_attr` - /// in combination with this lint to detect both cases. - /// - /// Does not detect empty lines after doc attributes (e.g. `#[doc = ""]`). - /// - /// ### Example - /// ```no_run - /// /// Some doc comment with a blank line after it. - /// - /// fn not_quite_good_code() { } - /// ``` - /// - /// Use instead: - /// ```no_run - /// /// Good (no blank line) - /// fn this_is_fine() { } - /// ``` - /// - /// ```no_run - /// // Good (convert to a regular comment) - /// - /// fn this_is_fine_too() { } - /// ``` - /// - /// ```no_run - /// //! Good (convert to a comment on an inner attribute) - /// - /// fn this_is_fine_as_well() { } - /// ``` - #[clippy::version = "1.70.0"] - pub EMPTY_LINE_AFTER_DOC_COMMENTS, - nursery, - "empty line after documentation comments" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for `warn`/`deny`/`forbid` attributes targeting the whole clippy::restriction category. - /// - /// ### Why is this bad? - /// Restriction lints sometimes are in contrast with other lints or even go against idiomatic rust. - /// These lints should only be enabled on a lint-by-lint basis and with careful consideration. - /// - /// ### Example - /// ```no_run - /// #![deny(clippy::restriction)] - /// ``` - /// - /// Use instead: - /// ```no_run - /// #![deny(clippy::as_conversions)] - /// ``` - #[clippy::version = "1.47.0"] - pub BLANKET_CLIPPY_RESTRICTION_LINTS, - suspicious, - "enabling the complete restriction group" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for `#[cfg_attr(rustfmt, rustfmt_skip)]` and suggests to replace it - /// with `#[rustfmt::skip]`. - /// - /// ### Why is this bad? - /// Since tool_attributes ([rust-lang/rust#44690](https://github.com/rust-lang/rust/issues/44690)) - /// are stable now, they should be used instead of the old `cfg_attr(rustfmt)` attributes. - /// - /// ### Known problems - /// This lint doesn't detect crate level inner attributes, because they get - /// processed before the PreExpansionPass lints get executed. See - /// [#3123](https://github.com/rust-lang/rust-clippy/pull/3123#issuecomment-422321765) - /// - /// ### Example - /// ```no_run - /// #[cfg_attr(rustfmt, rustfmt_skip)] - /// fn main() { } - /// ``` - /// - /// Use instead: - /// ```no_run - /// #[rustfmt::skip] - /// fn main() { } - /// ``` - #[clippy::version = "1.32.0"] - pub DEPRECATED_CFG_ATTR, - complexity, - "usage of `cfg_attr(rustfmt)` instead of tool attributes" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for cfg attributes having operating systems used in target family position. - /// - /// ### Why is this bad? - /// The configuration option will not be recognised and the related item will not be included - /// by the conditional compilation engine. - /// - /// ### Example - /// ```no_run - /// #[cfg(linux)] - /// fn conditional() { } - /// ``` - /// - /// Use instead: - /// ```no_run - /// # mod hidden { - /// #[cfg(target_os = "linux")] - /// fn conditional() { } - /// # } - /// - /// // or - /// - /// #[cfg(unix)] - /// fn conditional() { } - /// ``` - /// Check the [Rust Reference](https://doc.rust-lang.org/reference/conditional-compilation.html#target_os) for more details. - #[clippy::version = "1.45.0"] - pub MISMATCHED_TARGET_OS, - correctness, - "usage of `cfg(operating_system)` instead of `cfg(target_os = \"operating_system\")`" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for attributes that allow lints without a reason. - /// - /// (This requires the `lint_reasons` feature) - /// - /// ### Why is this bad? - /// Allowing a lint should always have a reason. This reason should be documented to - /// ensure that others understand the reasoning - /// - /// ### Example - /// ```no_run - /// #![feature(lint_reasons)] - /// - /// #![allow(clippy::some_lint)] - /// ``` - /// - /// Use instead: - /// ```no_run - /// #![feature(lint_reasons)] - /// - /// #![allow(clippy::some_lint, reason = "False positive rust-lang/rust-clippy#1002020")] - /// ``` - #[clippy::version = "1.61.0"] - pub ALLOW_ATTRIBUTES_WITHOUT_REASON, - restriction, - "ensures that all `allow` and `expect` attributes have a reason" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for `#[should_panic]` attributes without specifying the expected panic message. - /// - /// ### Why is this bad? - /// The expected panic message should be specified to ensure that the test is actually - /// panicking with the expected message, and not another unrelated panic. - /// - /// ### Example - /// ```no_run - /// fn random() -> i32 { 0 } - /// - /// #[should_panic] - /// #[test] - /// fn my_test() { - /// let _ = 1 / random(); - /// } - /// ``` - /// - /// Use instead: - /// ```no_run - /// fn random() -> i32 { 0 } - /// - /// #[should_panic = "attempt to divide by zero"] - /// #[test] - /// fn my_test() { - /// let _ = 1 / random(); - /// } - /// ``` - #[clippy::version = "1.74.0"] - pub SHOULD_PANIC_WITHOUT_EXPECT, - pedantic, - "ensures that all `should_panic` attributes specify its expected panic message" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for `any` and `all` combinators in `cfg` with only one condition. - /// - /// ### Why is this bad? - /// If there is only one condition, no need to wrap it into `any` or `all` combinators. - /// - /// ### Example - /// ```no_run - /// #[cfg(any(unix))] - /// pub struct Bar; - /// ``` - /// - /// Use instead: - /// ```no_run - /// #[cfg(unix)] - /// pub struct Bar; - /// ``` - #[clippy::version = "1.71.0"] - pub NON_MINIMAL_CFG, - style, - "ensure that all `cfg(any())` and `cfg(all())` have more than one condition" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for `#[cfg(features = "...")]` and suggests to replace it with - /// `#[cfg(feature = "...")]`. - /// - /// It also checks if `cfg(test)` was misspelled. - /// - /// ### Why is this bad? - /// Misspelling `feature` as `features` or `test` as `tests` can be sometimes hard to spot. It - /// may cause conditional compilation not work quietly. - /// - /// ### Example - /// ```no_run - /// #[cfg(features = "some-feature")] - /// fn conditional() { } - /// #[cfg(tests)] - /// mod tests { } - /// ``` - /// - /// Use instead: - /// ```no_run - /// #[cfg(feature = "some-feature")] - /// fn conditional() { } - /// #[cfg(test)] - /// mod tests { } - /// ``` - #[clippy::version = "1.69.0"] - pub MAYBE_MISUSED_CFG, - suspicious, - "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, - DEPRECATED_SEMVER, - USELESS_ATTRIBUTE, - BLANKET_CLIPPY_RESTRICTION_LINTS, - SHOULD_PANIC_WITHOUT_EXPECT, -]); - -impl<'tcx> LateLintPass<'tcx> for Attributes { - fn check_crate(&mut self, cx: &LateContext<'tcx>) { - for (name, level) in &cx.sess().opts.lint_opts { - if name == "clippy::restriction" && *level > Level::Allow { - span_lint_and_then( - cx, - BLANKET_CLIPPY_RESTRICTION_LINTS, - DUMMY_SP, - "`clippy::restriction` is not meant to be enabled as a group", - |diag| { - diag.note(format!( - "because of the command line `--{} clippy::restriction`", - level.as_str() - )); - diag.help("enable the restriction lints you need individually"); - }, - ); - } - } - } - - fn check_attribute(&mut self, cx: &LateContext<'tcx>, attr: &'tcx Attribute) { - if let Some(items) = &attr.meta_item_list() { - if let Some(ident) = attr.ident() { - if is_lint_level(ident.name) { - check_clippy_lint_names(cx, ident.name, items); - } - if matches!(ident.name, sym::allow | sym::expect) { - check_lint_reason(cx, ident.name, items, attr); - } - if items.is_empty() || !attr.has_name(sym::deprecated) { - return; - } - for item in items { - if let NestedMetaItem::MetaItem(mi) = &item - && let MetaItemKind::NameValue(lit) = &mi.kind - && mi.has_name(sym::since) - { - check_deprecated_since(cx, item.span(), lit); - } - } - } - } - if attr.has_name(sym::should_panic) { - check_should_panic_reason(cx, attr); - } - } - - fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { - let attrs = cx.tcx.hir().attrs(item.hir_id()); - if is_relevant_item(cx, item) { - check_attrs(cx, item.span, item.ident.name, attrs); - } - match item.kind { - ItemKind::ExternCrate(..) | ItemKind::Use(..) => { - let skip_unused_imports = attrs.iter().any(|attr| attr.has_name(sym::macro_use)); - - for attr in attrs { - if in_external_macro(cx.sess(), attr.span) { - return; - } - if let Some(lint_list) = &attr.meta_item_list() { - if attr.ident().map_or(false, |ident| is_lint_level(ident.name)) { - for lint in lint_list { - match item.kind { - ItemKind::Use(..) => { - if is_word(lint, sym::unused_imports) - || 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(), - "wildcard_imports" - | "enum_glob_use" - | "redundant_pub_crate" - | "macro_use_imports" - | "unsafe_removed_from_name" - | "module_name_repetitions" - | "single_component_path_imports" - ) - }) - { - return; - } - }, - ItemKind::ExternCrate(..) => { - if is_word(lint, sym::unused_imports) && skip_unused_imports { - return; - } - if is_word(lint, sym!(unused_extern_crates)) { - return; - } - }, - _ => {}, - } - } - let line_span = first_line_of_span(cx, attr.span); - - if let Some(mut sugg) = snippet_opt(cx, line_span) { - if sugg.contains("#[") { - span_lint_and_then( - cx, - USELESS_ATTRIBUTE, - line_span, - "useless lint attribute", - |diag| { - sugg = sugg.replacen("#[", "#![", 1); - diag.span_suggestion( - line_span, - "if you just forgot a `!`, use", - sugg, - Applicability::MaybeIncorrect, - ); - }, - ); - } - } - } - } - } - }, - _ => {}, - } - } - - fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) { - if is_relevant_impl(cx, item) { - check_attrs(cx, item.span, item.ident.name, cx.tcx.hir().attrs(item.hir_id())); - } - } - - fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) { - if is_relevant_trait(cx, item) { - check_attrs(cx, item.span, item.ident.name, cx.tcx.hir().attrs(item.hir_id())); - } - } -} - -/// Returns the lint name if it is clippy lint. -fn extract_clippy_lint(lint: &NestedMetaItem) -> Option<Symbol> { - if let Some(meta_item) = lint.meta_item() - && meta_item.path.segments.len() > 1 - && let tool_name = meta_item.path.segments[0].ident - && tool_name.name == sym::clippy - { - let lint_name = meta_item.path.segments.last().unwrap().ident.name; - return Some(lint_name); - } - None -} - -fn check_should_panic_reason(cx: &LateContext<'_>, attr: &Attribute) { - if let AttrKind::Normal(normal_attr) = &attr.kind { - if let AttrArgs::Eq(_, AttrArgsEq::Hir(_)) = &normal_attr.item.args { - // `#[should_panic = ".."]` found, good - return; - } - - if let AttrArgs::Delimited(args) = &normal_attr.item.args - && let mut tt_iter = args.tokens.trees() - && let Some(TokenTree::Token( - Token { - kind: TokenKind::Ident(sym::expected, _), - .. - }, - _, - )) = tt_iter.next() - && let Some(TokenTree::Token( - Token { - kind: TokenKind::Eq, .. - }, - _, - )) = tt_iter.next() - && let Some(TokenTree::Token( - Token { - kind: TokenKind::Literal(_), - .. - }, - _, - )) = tt_iter.next() - { - // `#[should_panic(expected = "..")]` found, good - return; - } - - span_lint_and_sugg( - cx, - SHOULD_PANIC_WITHOUT_EXPECT, - attr.span, - "#[should_panic] attribute without a reason", - "consider specifying the expected panic", - "#[should_panic(expected = /* panic message */)]".into(), - Applicability::HasPlaceholders, - ); - } -} - -fn check_clippy_lint_names(cx: &LateContext<'_>, name: Symbol, items: &[NestedMetaItem]) { - for lint in items { - if let Some(lint_name) = extract_clippy_lint(lint) { - if lint_name.as_str() == "restriction" && name != sym::allow { - span_lint_and_help( - cx, - BLANKET_CLIPPY_RESTRICTION_LINTS, - lint.span(), - "`clippy::restriction` is not meant to be enabled as a group", - None, - "enable the restriction lints you need individually", - ); - } - } - } -} - -fn check_lint_reason<'cx>(cx: &LateContext<'cx>, name: Symbol, items: &[NestedMetaItem], attr: &'cx Attribute) { - // Check for the feature - if !cx.tcx.features().lint_reasons { - return; - } - - // Check if the reason is present - if let Some(item) = items.last().and_then(NestedMetaItem::meta_item) - && let MetaItemKind::NameValue(_) = &item.kind - && item.path == sym::reason - { - return; - } - - // Check if the attribute is in an external macro and therefore out of the developer's control - if in_external_macro(cx.sess(), attr.span) || is_from_proc_macro(cx, &attr) { - return; - } - - span_lint_and_help( - cx, - ALLOW_ATTRIBUTES_WITHOUT_REASON, - attr.span, - &format!("`{}` attribute without specifying a reason", name.as_str()), - None, - "try adding a reason at the end with `, reason = \"..\"`", - ); -} - -fn is_relevant_item(cx: &LateContext<'_>, item: &Item<'_>) -> bool { - if let ItemKind::Fn(_, _, eid) = item.kind { - is_relevant_expr(cx, cx.tcx.typeck_body(eid), cx.tcx.hir().body(eid).value) - } else { - true - } -} - -fn is_relevant_impl(cx: &LateContext<'_>, item: &ImplItem<'_>) -> bool { - match item.kind { - ImplItemKind::Fn(_, eid) => is_relevant_expr(cx, cx.tcx.typeck_body(eid), cx.tcx.hir().body(eid).value), - _ => false, - } -} - -fn is_relevant_trait(cx: &LateContext<'_>, item: &TraitItem<'_>) -> bool { - match item.kind { - TraitItemKind::Fn(_, TraitFn::Required(_)) => true, - TraitItemKind::Fn(_, TraitFn::Provided(eid)) => { - is_relevant_expr(cx, cx.tcx.typeck_body(eid), cx.tcx.hir().body(eid).value) - }, - _ => false, - } -} - -fn is_relevant_block(cx: &LateContext<'_>, typeck_results: &ty::TypeckResults<'_>, block: &Block<'_>) -> bool { - block.stmts.first().map_or( - block - .expr - .as_ref() - .map_or(false, |e| is_relevant_expr(cx, typeck_results, e)), - |stmt| match &stmt.kind { - StmtKind::Local(_) => true, - StmtKind::Expr(expr) | StmtKind::Semi(expr) => is_relevant_expr(cx, typeck_results, expr), - StmtKind::Item(_) => false, - }, - ) -} - -fn is_relevant_expr(cx: &LateContext<'_>, typeck_results: &ty::TypeckResults<'_>, expr: &Expr<'_>) -> bool { - if macro_backtrace(expr.span).last().map_or(false, |macro_call| { - is_panic(cx, macro_call.def_id) || cx.tcx.item_name(macro_call.def_id) == sym::unreachable - }) { - return false; - } - match &expr.kind { - ExprKind::Block(block, _) => is_relevant_block(cx, typeck_results, block), - ExprKind::Ret(Some(e)) => is_relevant_expr(cx, typeck_results, e), - ExprKind::Ret(None) | ExprKind::Break(_, None) => false, - _ => true, - } -} - -fn check_attrs(cx: &LateContext<'_>, span: Span, name: Symbol, attrs: &[Attribute]) { - if span.from_expansion() { - return; - } - - for attr in attrs { - if let Some(values) = attr.meta_item_list() { - if values.len() != 1 || !attr.has_name(sym::inline) { - continue; - } - if is_word(&values[0], sym::always) { - span_lint( - cx, - INLINE_ALWAYS, - attr.span, - &format!("you have declared `#[inline(always)]` on `{name}`. This is usually a bad idea"), - ); - } - } - } -} - -fn check_deprecated_since(cx: &LateContext<'_>, span: Span, lit: &MetaItemLit) { - if let LitKind::Str(is, _) = lit.kind { - if is.as_str() == "TBD" || Version::parse(is.as_str()).is_ok() { - return; - } - } - span_lint( - cx, - DEPRECATED_SEMVER, - span, - "the since field must contain a semver-compliant version", - ); -} - -fn is_word(nmi: &NestedMetaItem, expected: Symbol) -> bool { - if let NestedMetaItem::MetaItem(mi) = &nmi { - mi.is_word() && mi.has_name(expected) - } else { - false - } -} - -pub struct EarlyAttributes { - pub msrv: Msrv, -} - -impl_lint_pass!(EarlyAttributes => [ - DEPRECATED_CFG_ATTR, - MISMATCHED_TARGET_OS, - EMPTY_LINE_AFTER_OUTER_ATTR, - EMPTY_LINE_AFTER_DOC_COMMENTS, - NON_MINIMAL_CFG, - MAYBE_MISUSED_CFG, - DEPRECATED_CLIPPY_CFG_ATTR, - UNNECESSARY_CLIPPY_CFG, -]); - -impl EarlyLintPass for EarlyAttributes { - fn check_item(&mut self, cx: &EarlyContext<'_>, item: &rustc_ast::Item) { - check_empty_line_after_outer_attr(cx, item); - } - - 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); - } - - extract_msrv_attr!(EarlyContext); -} - -/// Check for empty lines after outer attributes. -/// -/// Attributes and documentation comments are both considered outer attributes -/// by the AST. However, the average user likely considers them to be different. -/// Checking for empty lines after each of these attributes is split into two different -/// lints but can share the same logic. -fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::Item) { - let mut iter = item.attrs.iter().peekable(); - while let Some(attr) = iter.next() { - if (matches!(attr.kind, AttrKind::Normal(..)) || matches!(attr.kind, AttrKind::DocComment(..))) - && attr.style == AttrStyle::Outer - && is_present_in_source(cx, attr.span) - { - let begin_of_attr_to_item = Span::new(attr.span.lo(), item.span.lo(), item.span.ctxt(), item.span.parent()); - let end_of_attr_to_next_attr_or_item = Span::new( - attr.span.hi(), - iter.peek().map_or(item.span.lo(), |next_attr| next_attr.span.lo()), - item.span.ctxt(), - item.span.parent(), - ); - - if let Some(snippet) = snippet_opt(cx, end_of_attr_to_next_attr_or_item) { - let lines = snippet.split('\n').collect::<Vec<_>>(); - let lines = without_block_comments(lines); - - if lines.iter().filter(|l| l.trim().is_empty()).count() > 2 { - let (lint_msg, lint_type) = match attr.kind { - AttrKind::DocComment(..) => ( - "found an empty line after a doc comment. \ - Perhaps you need to use `//!` to make a comment on a module, remove the empty line, or make a regular comment with `//`?", - EMPTY_LINE_AFTER_DOC_COMMENTS, - ), - AttrKind::Normal(..) => ( - "found an empty line after an outer attribute. \ - Perhaps you forgot to add a `!` to make it an inner attribute?", - EMPTY_LINE_AFTER_OUTER_ATTR, - ), - }; - - span_lint(cx, lint_type, begin_of_attr_to_item, lint_msg); - } - } - } - } -} - -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) { - // check cfg_attr - if attr.has_name(sym::cfg_attr) - && let Some(items) = attr.meta_item_list() - && items.len() == 2 - && let Some(feature_item) = items[0].meta_item() - { - // 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::<Vec<_>>() - .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 - ), - ); - } - } -} - -fn check_nested_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) { - for item in items { - if let NestedMetaItem::MetaItem(meta) = item { - if !meta.has_name(sym::any) && !meta.has_name(sym::all) { - continue; - } - if let MetaItemKind::List(list) = &meta.kind { - check_nested_cfg(cx, list); - if list.len() == 1 { - span_lint_and_then( - cx, - NON_MINIMAL_CFG, - meta.span, - "unneeded sub `cfg` when there is only one condition", - |diag| { - if let Some(snippet) = snippet_opt(cx, list[0].span()) { - diag.span_suggestion(meta.span, "try", snippet, Applicability::MaybeIncorrect); - } - }, - ); - } else if list.is_empty() && meta.has_name(sym::all) { - span_lint_and_then( - cx, - NON_MINIMAL_CFG, - meta.span, - "unneeded sub `cfg` when there is no condition", - |_| {}, - ); - } - } - } - } -} - -fn check_nested_misused_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) { - for item in items { - if let NestedMetaItem::MetaItem(meta) = item { - if let Some(ident) = meta.ident() - && ident.name.as_str() == "features" - && let Some(val) = meta.value_str() - { - span_lint_and_sugg( - cx, - MAYBE_MISUSED_CFG, - meta.span, - "'feature' may be misspelled as 'features'", - "did you mean", - format!("feature = \"{val}\""), - Applicability::MaybeIncorrect, - ); - } - if let MetaItemKind::List(list) = &meta.kind { - check_nested_misused_cfg(cx, list); - // If this is not a list, then we check for `cfg(test)`. - } else if let Some(ident) = meta.ident() - && matches!(ident.name.as_str(), "tests" | "Test") - { - span_lint_and_sugg( - cx, - MAYBE_MISUSED_CFG, - meta.span, - &format!("'test' may be misspelled as '{}'", ident.name.as_str()), - "did you mean", - "test".to_string(), - Applicability::MaybeIncorrect, - ); - } - } - } -} - -fn check_minimal_cfg_condition(cx: &EarlyContext<'_>, attr: &Attribute) { - if attr.has_name(sym::cfg) - && let Some(items) = attr.meta_item_list() - { - check_nested_cfg(cx, &items); - } -} - -fn check_misused_cfg(cx: &EarlyContext<'_>, attr: &Attribute) { - if attr.has_name(sym::cfg) - && let Some(items) = attr.meta_item_list() - { - check_nested_misused_cfg(cx, &items); - } -} - -fn check_mismatched_target_os(cx: &EarlyContext<'_>, attr: &Attribute) { - fn find_os(name: &str) -> Option<&'static str> { - UNIX_SYSTEMS - .iter() - .chain(NON_UNIX_SYSTEMS.iter()) - .find(|&&os| os == name) - .copied() - } - - fn is_unix(name: &str) -> bool { - UNIX_SYSTEMS.iter().any(|&os| os == name) - } - - fn find_mismatched_target_os(items: &[NestedMetaItem]) -> Vec<(&str, Span)> { - let mut mismatched = Vec::new(); - - for item in items { - if let NestedMetaItem::MetaItem(meta) = item { - match &meta.kind { - MetaItemKind::List(list) => { - mismatched.extend(find_mismatched_target_os(list)); - }, - MetaItemKind::Word => { - if let Some(ident) = meta.ident() - && let Some(os) = find_os(ident.name.as_str()) - { - mismatched.push((os, ident.span)); - } - }, - MetaItemKind::NameValue(..) => {}, - } - } - } - - mismatched - } - - if attr.has_name(sym::cfg) - && let Some(list) = attr.meta_item_list() - && let mismatched = find_mismatched_target_os(&list) - && !mismatched.is_empty() - { - let mess = "operating system used in target family position"; - - span_lint_and_then(cx, MISMATCHED_TARGET_OS, attr.span, mess, |diag| { - // Avoid showing the unix suggestion multiple times in case - // we have more than one mismatch for unix-like systems - let mut unix_suggested = false; - - for (os, span) in mismatched { - let sugg = format!("target_os = \"{os}\""); - diag.span_suggestion(span, "try", sugg, Applicability::MaybeIncorrect); - - if !unix_suggested && is_unix(os) { - diag.help("did you mean `unix`?"); - unix_suggested = true; - } - } - }); - } -} - -fn is_lint_level(symbol: Symbol) -> bool { - matches!(symbol, sym::allow | sym::expect | sym::warn | sym::deny | sym::forbid) -} diff --git a/clippy_lints/src/attrs/allow_attributes_without_reason.rs b/clippy_lints/src/attrs/allow_attributes_without_reason.rs new file mode 100644 index 00000000000..df00f23e37e --- /dev/null +++ b/clippy_lints/src/attrs/allow_attributes_without_reason.rs @@ -0,0 +1,37 @@ +use super::{Attribute, ALLOW_ATTRIBUTES_WITHOUT_REASON}; +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::is_from_proc_macro; +use rustc_ast::{MetaItemKind, NestedMetaItem}; +use rustc_lint::{LateContext, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_span::sym; +use rustc_span::symbol::Symbol; + +pub(super) fn check<'cx>(cx: &LateContext<'cx>, name: Symbol, items: &[NestedMetaItem], attr: &'cx Attribute) { + // Check for the feature + if !cx.tcx.features().lint_reasons { + return; + } + + // Check if the reason is present + if let Some(item) = items.last().and_then(NestedMetaItem::meta_item) + && let MetaItemKind::NameValue(_) = &item.kind + && item.path == sym::reason + { + return; + } + + // Check if the attribute is in an external macro and therefore out of the developer's control + if in_external_macro(cx.sess(), attr.span) || is_from_proc_macro(cx, &attr) { + return; + } + + span_lint_and_help( + cx, + ALLOW_ATTRIBUTES_WITHOUT_REASON, + attr.span, + &format!("`{}` attribute without specifying a reason", name.as_str()), + None, + "try adding a reason at the end with `, reason = \"..\"`", + ); +} diff --git a/clippy_lints/src/attrs/blanket_clippy_restriction_lints.rs b/clippy_lints/src/attrs/blanket_clippy_restriction_lints.rs new file mode 100644 index 00000000000..9b08fd6d654 --- /dev/null +++ b/clippy_lints/src/attrs/blanket_clippy_restriction_lints.rs @@ -0,0 +1,44 @@ +use super::utils::extract_clippy_lint; +use super::BLANKET_CLIPPY_RESTRICTION_LINTS; +use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then}; +use rustc_ast::NestedMetaItem; +use rustc_lint::{LateContext, Level, LintContext}; +use rustc_span::symbol::Symbol; +use rustc_span::{sym, DUMMY_SP}; + +pub(super) fn check(cx: &LateContext<'_>, name: Symbol, items: &[NestedMetaItem]) { + for lint in items { + if let Some(lint_name) = extract_clippy_lint(lint) { + if lint_name.as_str() == "restriction" && name != sym::allow { + span_lint_and_help( + cx, + BLANKET_CLIPPY_RESTRICTION_LINTS, + lint.span(), + "`clippy::restriction` is not meant to be enabled as a group", + None, + "enable the restriction lints you need individually", + ); + } + } + } +} + +pub(super) fn check_command_line(cx: &LateContext<'_>) { + for (name, level) in &cx.sess().opts.lint_opts { + if name == "clippy::restriction" && *level > Level::Allow { + span_lint_and_then( + cx, + BLANKET_CLIPPY_RESTRICTION_LINTS, + DUMMY_SP, + "`clippy::restriction` is not meant to be enabled as a group", + |diag| { + diag.note(format!( + "because of the command line `--{} clippy::restriction`", + level.as_str() + )); + diag.help("enable the restriction lints you need individually"); + }, + ); + } + } +} diff --git a/clippy_lints/src/attrs/deprecated_cfg_attr.rs b/clippy_lints/src/attrs/deprecated_cfg_attr.rs new file mode 100644 index 00000000000..e872ab6b4b5 --- /dev/null +++ b/clippy_lints/src/attrs/deprecated_cfg_attr.rs @@ -0,0 +1,87 @@ +use super::{unnecessary_clippy_cfg, Attribute, DEPRECATED_CFG_ATTR, DEPRECATED_CLIPPY_CFG_ATTR}; +use clippy_config::msrvs::{self, Msrv}; +use clippy_utils::diagnostics::span_lint_and_sugg; +use rustc_ast::AttrStyle; +use rustc_errors::Applicability; +use rustc_lint::EarlyContext; +use rustc_span::sym; + +pub(super) fn check(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &Msrv) { + // check cfg_attr + if attr.has_name(sym::cfg_attr) + && let Some(items) = attr.meta_item_list() + && items.len() == 2 + && let Some(feature_item) = items[0].meta_item() + { + // 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() { + unnecessary_clippy_cfg::check(cx, feature_item, behind_cfg_attr, attr); + } + } + } +} + +pub(super) fn check_clippy(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_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_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, + ); + } +} diff --git a/clippy_lints/src/attrs/deprecated_semver.rs b/clippy_lints/src/attrs/deprecated_semver.rs new file mode 100644 index 00000000000..1898c145c76 --- /dev/null +++ b/clippy_lints/src/attrs/deprecated_semver.rs @@ -0,0 +1,20 @@ +use super::DEPRECATED_SEMVER; +use clippy_utils::diagnostics::span_lint; +use rustc_ast::{LitKind, MetaItemLit}; +use rustc_lint::LateContext; +use rustc_span::Span; +use semver::Version; + +pub(super) fn check(cx: &LateContext<'_>, span: Span, lit: &MetaItemLit) { + if let LitKind::Str(is, _) = lit.kind { + if is.as_str() == "TBD" || Version::parse(is.as_str()).is_ok() { + return; + } + } + span_lint( + cx, + DEPRECATED_SEMVER, + span, + "the since field must contain a semver-compliant version", + ); +} diff --git a/clippy_lints/src/attrs/empty_line_after.rs b/clippy_lints/src/attrs/empty_line_after.rs new file mode 100644 index 00000000000..ca43e76ac57 --- /dev/null +++ b/clippy_lints/src/attrs/empty_line_after.rs @@ -0,0 +1,52 @@ +use super::{EMPTY_LINE_AFTER_DOC_COMMENTS, EMPTY_LINE_AFTER_OUTER_ATTR}; +use clippy_utils::diagnostics::span_lint; +use clippy_utils::source::{is_present_in_source, snippet_opt, without_block_comments}; +use rustc_ast::{AttrKind, AttrStyle}; +use rustc_lint::EarlyContext; +use rustc_span::Span; + +/// Check for empty lines after outer attributes. +/// +/// Attributes and documentation comments are both considered outer attributes +/// by the AST. However, the average user likely considers them to be different. +/// Checking for empty lines after each of these attributes is split into two different +/// lints but can share the same logic. +pub(super) fn check(cx: &EarlyContext<'_>, item: &rustc_ast::Item) { + let mut iter = item.attrs.iter().peekable(); + while let Some(attr) = iter.next() { + if (matches!(attr.kind, AttrKind::Normal(..)) || matches!(attr.kind, AttrKind::DocComment(..))) + && attr.style == AttrStyle::Outer + && is_present_in_source(cx, attr.span) + { + let begin_of_attr_to_item = Span::new(attr.span.lo(), item.span.lo(), item.span.ctxt(), item.span.parent()); + let end_of_attr_to_next_attr_or_item = Span::new( + attr.span.hi(), + iter.peek().map_or(item.span.lo(), |next_attr| next_attr.span.lo()), + item.span.ctxt(), + item.span.parent(), + ); + + if let Some(snippet) = snippet_opt(cx, end_of_attr_to_next_attr_or_item) { + let lines = snippet.split('\n').collect::<Vec<_>>(); + let lines = without_block_comments(lines); + + if lines.iter().filter(|l| l.trim().is_empty()).count() > 2 { + let (lint_msg, lint_type) = match attr.kind { + AttrKind::DocComment(..) => ( + "found an empty line after a doc comment. \ + Perhaps you need to use `//!` to make a comment on a module, remove the empty line, or make a regular comment with `//`?", + EMPTY_LINE_AFTER_DOC_COMMENTS, + ), + AttrKind::Normal(..) => ( + "found an empty line after an outer attribute. \ + Perhaps you forgot to add a `!` to make it an inner attribute?", + EMPTY_LINE_AFTER_OUTER_ATTR, + ), + }; + + span_lint(cx, lint_type, begin_of_attr_to_item, lint_msg); + } + } + } + } +} diff --git a/clippy_lints/src/attrs/inline_always.rs b/clippy_lints/src/attrs/inline_always.rs new file mode 100644 index 00000000000..cfcd2cc6a00 --- /dev/null +++ b/clippy_lints/src/attrs/inline_always.rs @@ -0,0 +1,29 @@ +use super::utils::is_word; +use super::INLINE_ALWAYS; +use clippy_utils::diagnostics::span_lint; +use rustc_ast::Attribute; +use rustc_lint::LateContext; +use rustc_span::symbol::Symbol; +use rustc_span::{sym, Span}; + +pub(super) fn check(cx: &LateContext<'_>, span: Span, name: Symbol, attrs: &[Attribute]) { + if span.from_expansion() { + return; + } + + for attr in attrs { + if let Some(values) = attr.meta_item_list() { + if values.len() != 1 || !attr.has_name(sym::inline) { + continue; + } + if is_word(&values[0], sym::always) { + span_lint( + cx, + INLINE_ALWAYS, + attr.span, + &format!("you have declared `#[inline(always)]` on `{name}`. This is usually a bad idea"), + ); + } + } + } +} diff --git a/clippy_lints/src/attrs/maybe_misused_cfg.rs b/clippy_lints/src/attrs/maybe_misused_cfg.rs new file mode 100644 index 00000000000..5a70866eda5 --- /dev/null +++ b/clippy_lints/src/attrs/maybe_misused_cfg.rs @@ -0,0 +1,51 @@ +use super::{Attribute, MAYBE_MISUSED_CFG}; +use clippy_utils::diagnostics::span_lint_and_sugg; +use rustc_ast::{MetaItemKind, NestedMetaItem}; +use rustc_errors::Applicability; +use rustc_lint::EarlyContext; +use rustc_span::sym; + +pub(super) fn check(cx: &EarlyContext<'_>, attr: &Attribute) { + if attr.has_name(sym::cfg) + && let Some(items) = attr.meta_item_list() + { + check_nested_misused_cfg(cx, &items); + } +} + +fn check_nested_misused_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) { + for item in items { + if let NestedMetaItem::MetaItem(meta) = item { + if let Some(ident) = meta.ident() + && ident.name.as_str() == "features" + && let Some(val) = meta.value_str() + { + span_lint_and_sugg( + cx, + MAYBE_MISUSED_CFG, + meta.span, + "'feature' may be misspelled as 'features'", + "did you mean", + format!("feature = \"{val}\""), + Applicability::MaybeIncorrect, + ); + } + if let MetaItemKind::List(list) = &meta.kind { + check_nested_misused_cfg(cx, list); + // If this is not a list, then we check for `cfg(test)`. + } else if let Some(ident) = meta.ident() + && matches!(ident.name.as_str(), "tests" | "Test") + { + span_lint_and_sugg( + cx, + MAYBE_MISUSED_CFG, + meta.span, + &format!("'test' may be misspelled as '{}'", ident.name.as_str()), + "did you mean", + "test".to_string(), + Applicability::MaybeIncorrect, + ); + } + } + } +} diff --git a/clippy_lints/src/attrs/mismatched_target_os.rs b/clippy_lints/src/attrs/mismatched_target_os.rs new file mode 100644 index 00000000000..b1cc0a763c5 --- /dev/null +++ b/clippy_lints/src/attrs/mismatched_target_os.rs @@ -0,0 +1,90 @@ +use super::{Attribute, MISMATCHED_TARGET_OS}; +use clippy_utils::diagnostics::span_lint_and_then; +use rustc_ast::{MetaItemKind, NestedMetaItem}; +use rustc_errors::Applicability; +use rustc_lint::EarlyContext; +use rustc_span::{sym, Span}; + +static UNIX_SYSTEMS: &[&str] = &[ + "android", + "dragonfly", + "emscripten", + "freebsd", + "fuchsia", + "haiku", + "illumos", + "ios", + "l4re", + "linux", + "macos", + "netbsd", + "openbsd", + "redox", + "solaris", + "vxworks", +]; + +// NOTE: windows is excluded from the list because it's also a valid target family. +static NON_UNIX_SYSTEMS: &[&str] = &["hermit", "none", "wasi"]; + +pub(super) fn check(cx: &EarlyContext<'_>, attr: &Attribute) { + fn find_os(name: &str) -> Option<&'static str> { + UNIX_SYSTEMS + .iter() + .chain(NON_UNIX_SYSTEMS.iter()) + .find(|&&os| os == name) + .copied() + } + + fn is_unix(name: &str) -> bool { + UNIX_SYSTEMS.iter().any(|&os| os == name) + } + + fn find_mismatched_target_os(items: &[NestedMetaItem]) -> Vec<(&str, Span)> { + let mut mismatched = Vec::new(); + + for item in items { + if let NestedMetaItem::MetaItem(meta) = item { + match &meta.kind { + MetaItemKind::List(list) => { + mismatched.extend(find_mismatched_target_os(list)); + }, + MetaItemKind::Word => { + if let Some(ident) = meta.ident() + && let Some(os) = find_os(ident.name.as_str()) + { + mismatched.push((os, ident.span)); + } + }, + MetaItemKind::NameValue(..) => {}, + } + } + } + + mismatched + } + + if attr.has_name(sym::cfg) + && let Some(list) = attr.meta_item_list() + && let mismatched = find_mismatched_target_os(&list) + && !mismatched.is_empty() + { + let mess = "operating system used in target family position"; + + span_lint_and_then(cx, MISMATCHED_TARGET_OS, attr.span, mess, |diag| { + // Avoid showing the unix suggestion multiple times in case + // we have more than one mismatch for unix-like systems + let mut unix_suggested = false; + + for (os, span) in mismatched { + let sugg = format!("target_os = \"{os}\""); + diag.span_suggestion(span, "try", sugg, Applicability::MaybeIncorrect); + + if !unix_suggested && is_unix(os) { + diag.help("did you mean `unix`?"); + unix_suggested = true; + } + } + }); + } +} diff --git a/clippy_lints/src/attrs/mixed_attributes_style.rs b/clippy_lints/src/attrs/mixed_attributes_style.rs new file mode 100644 index 00000000000..c2e21cfd330 --- /dev/null +++ b/clippy_lints/src/attrs/mixed_attributes_style.rs @@ -0,0 +1,30 @@ +use super::MIXED_ATTRIBUTES_STYLE; +use clippy_utils::diagnostics::span_lint; +use rustc_ast::AttrStyle; +use rustc_lint::EarlyContext; + +pub(super) fn check(cx: &EarlyContext<'_>, item: &rustc_ast::Item) { + let mut has_outer = false; + let mut has_inner = false; + + for attr in &item.attrs { + if attr.span.from_expansion() { + continue; + } + match attr.style { + AttrStyle::Inner => has_inner = true, + AttrStyle::Outer => has_outer = true, + } + } + if !has_outer || !has_inner { + return; + } + let mut attrs_iter = item.attrs.iter().filter(|attr| !attr.span.from_expansion()); + let span = attrs_iter.next().unwrap().span; + span_lint( + cx, + MIXED_ATTRIBUTES_STYLE, + span.with_hi(attrs_iter.last().unwrap().span.hi()), + "item has both inner and outer attributes", + ); +} diff --git a/clippy_lints/src/attrs/mod.rs b/clippy_lints/src/attrs/mod.rs new file mode 100644 index 00000000000..c4c65d3248a --- /dev/null +++ b/clippy_lints/src/attrs/mod.rs @@ -0,0 +1,588 @@ +//! checks for attributes + +mod allow_attributes_without_reason; +mod blanket_clippy_restriction_lints; +mod deprecated_cfg_attr; +mod deprecated_semver; +mod empty_line_after; +mod inline_always; +mod maybe_misused_cfg; +mod mismatched_target_os; +mod mixed_attributes_style; +mod non_minimal_cfg; +mod should_panic_without_expect; +mod unnecessary_clippy_cfg; +mod useless_attribute; +mod utils; + +use clippy_config::msrvs::Msrv; +use rustc_ast::{Attribute, MetaItemKind, NestedMetaItem}; +use rustc_hir::{ImplItem, Item, ItemKind, TraitItem}; +use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, impl_lint_pass}; +use rustc_span::sym; +use utils::{is_lint_level, is_relevant_impl, is_relevant_item, is_relevant_trait}; + +declare_clippy_lint! { + /// ### What it does + /// Checks for items annotated with `#[inline(always)]`, + /// unless the annotated function is empty or simply panics. + /// + /// ### Why is this bad? + /// While there are valid uses of this annotation (and once + /// you know when to use it, by all means `allow` this lint), it's a common + /// newbie-mistake to pepper one's code with it. + /// + /// As a rule of thumb, before slapping `#[inline(always)]` on a function, + /// measure if that additional function call really affects your runtime profile + /// sufficiently to make up for the increase in compile time. + /// + /// ### Known problems + /// False positives, big time. This lint is meant to be + /// deactivated by everyone doing serious performance work. This means having + /// done the measurement. + /// + /// ### Example + /// ```ignore + /// #[inline(always)] + /// fn not_quite_hot_code(..) { ... } + /// ``` + #[clippy::version = "pre 1.29.0"] + pub INLINE_ALWAYS, + pedantic, + "use of `#[inline(always)]`" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for `extern crate` and `use` items annotated with + /// lint attributes. + /// + /// This lint permits lint attributes for lints emitted on the items themself. + /// For `use` items these lints are: + /// * deprecated + /// * unreachable_pub + /// * unused_imports + /// * clippy::enum_glob_use + /// * clippy::macro_use_imports + /// * clippy::wildcard_imports + /// + /// For `extern crate` items these lints are: + /// * `unused_imports` on items with `#[macro_use]` + /// + /// ### Why is this bad? + /// Lint attributes have no effect on crate imports. Most + /// likely a `!` was forgotten. + /// + /// ### Example + /// ```ignore + /// #[deny(dead_code)] + /// extern crate foo; + /// #[forbid(dead_code)] + /// use foo::bar; + /// ``` + /// + /// Use instead: + /// ```rust,ignore + /// #[allow(unused_imports)] + /// use foo::baz; + /// #[allow(unused_imports)] + /// #[macro_use] + /// extern crate baz; + /// ``` + #[clippy::version = "pre 1.29.0"] + pub USELESS_ATTRIBUTE, + correctness, + "use of lint attributes on `extern crate` items" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for `#[deprecated]` annotations with a `since` + /// field that is not a valid semantic version. Also allows "TBD" to signal + /// future deprecation. + /// + /// ### Why is this bad? + /// For checking the version of the deprecation, it must be + /// a valid semver. Failing that, the contained information is useless. + /// + /// ### Example + /// ```no_run + /// #[deprecated(since = "forever")] + /// fn something_else() { /* ... */ } + /// ``` + #[clippy::version = "pre 1.29.0"] + pub DEPRECATED_SEMVER, + correctness, + "use of `#[deprecated(since = \"x\")]` where x is not semver" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for empty lines after outer attributes + /// + /// ### Why is this bad? + /// Most likely the attribute was meant to be an inner attribute using a '!'. + /// If it was meant to be an outer attribute, then the following item + /// should not be separated by empty lines. + /// + /// ### Known problems + /// Can cause false positives. + /// + /// From the clippy side it's difficult to detect empty lines between an attributes and the + /// following item because empty lines and comments are not part of the AST. The parsing + /// currently works for basic cases but is not perfect. + /// + /// ### Example + /// ```no_run + /// #[allow(dead_code)] + /// + /// fn not_quite_good_code() { } + /// ``` + /// + /// Use instead: + /// ```no_run + /// // Good (as inner attribute) + /// #![allow(dead_code)] + /// + /// fn this_is_fine() { } + /// + /// // or + /// + /// // Good (as outer attribute) + /// #[allow(dead_code)] + /// fn this_is_fine_too() { } + /// ``` + #[clippy::version = "pre 1.29.0"] + pub EMPTY_LINE_AFTER_OUTER_ATTR, + nursery, + "empty line after outer attribute" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for empty lines after documentation comments. + /// + /// ### Why is this bad? + /// The documentation comment was most likely meant to be an inner attribute or regular comment. + /// If it was intended to be a documentation comment, then the empty line should be removed to + /// be more idiomatic. + /// + /// ### Known problems + /// Only detects empty lines immediately following the documentation. If the doc comment is followed + /// by an attribute and then an empty line, this lint will not trigger. Use `empty_line_after_outer_attr` + /// in combination with this lint to detect both cases. + /// + /// Does not detect empty lines after doc attributes (e.g. `#[doc = ""]`). + /// + /// ### Example + /// ```no_run + /// /// Some doc comment with a blank line after it. + /// + /// fn not_quite_good_code() { } + /// ``` + /// + /// Use instead: + /// ```no_run + /// /// Good (no blank line) + /// fn this_is_fine() { } + /// ``` + /// + /// ```no_run + /// // Good (convert to a regular comment) + /// + /// fn this_is_fine_too() { } + /// ``` + /// + /// ```no_run + /// //! Good (convert to a comment on an inner attribute) + /// + /// fn this_is_fine_as_well() { } + /// ``` + #[clippy::version = "1.70.0"] + pub EMPTY_LINE_AFTER_DOC_COMMENTS, + nursery, + "empty line after documentation comments" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for `warn`/`deny`/`forbid` attributes targeting the whole clippy::restriction category. + /// + /// ### Why is this bad? + /// Restriction lints sometimes are in contrast with other lints or even go against idiomatic rust. + /// These lints should only be enabled on a lint-by-lint basis and with careful consideration. + /// + /// ### Example + /// ```no_run + /// #![deny(clippy::restriction)] + /// ``` + /// + /// Use instead: + /// ```no_run + /// #![deny(clippy::as_conversions)] + /// ``` + #[clippy::version = "1.47.0"] + pub BLANKET_CLIPPY_RESTRICTION_LINTS, + suspicious, + "enabling the complete restriction group" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for `#[cfg_attr(rustfmt, rustfmt_skip)]` and suggests to replace it + /// with `#[rustfmt::skip]`. + /// + /// ### Why is this bad? + /// Since tool_attributes ([rust-lang/rust#44690](https://github.com/rust-lang/rust/issues/44690)) + /// are stable now, they should be used instead of the old `cfg_attr(rustfmt)` attributes. + /// + /// ### Known problems + /// This lint doesn't detect crate level inner attributes, because they get + /// processed before the PreExpansionPass lints get executed. See + /// [#3123](https://github.com/rust-lang/rust-clippy/pull/3123#issuecomment-422321765) + /// + /// ### Example + /// ```no_run + /// #[cfg_attr(rustfmt, rustfmt_skip)] + /// fn main() { } + /// ``` + /// + /// Use instead: + /// ```no_run + /// #[rustfmt::skip] + /// fn main() { } + /// ``` + #[clippy::version = "1.32.0"] + pub DEPRECATED_CFG_ATTR, + complexity, + "usage of `cfg_attr(rustfmt)` instead of tool attributes" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for cfg attributes having operating systems used in target family position. + /// + /// ### Why is this bad? + /// The configuration option will not be recognised and the related item will not be included + /// by the conditional compilation engine. + /// + /// ### Example + /// ```no_run + /// #[cfg(linux)] + /// fn conditional() { } + /// ``` + /// + /// Use instead: + /// ```no_run + /// # mod hidden { + /// #[cfg(target_os = "linux")] + /// fn conditional() { } + /// # } + /// + /// // or + /// + /// #[cfg(unix)] + /// fn conditional() { } + /// ``` + /// Check the [Rust Reference](https://doc.rust-lang.org/reference/conditional-compilation.html#target_os) for more details. + #[clippy::version = "1.45.0"] + pub MISMATCHED_TARGET_OS, + correctness, + "usage of `cfg(operating_system)` instead of `cfg(target_os = \"operating_system\")`" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for attributes that allow lints without a reason. + /// + /// (This requires the `lint_reasons` feature) + /// + /// ### Why is this bad? + /// Allowing a lint should always have a reason. This reason should be documented to + /// ensure that others understand the reasoning + /// + /// ### Example + /// ```no_run + /// #![feature(lint_reasons)] + /// + /// #![allow(clippy::some_lint)] + /// ``` + /// + /// Use instead: + /// ```no_run + /// #![feature(lint_reasons)] + /// + /// #![allow(clippy::some_lint, reason = "False positive rust-lang/rust-clippy#1002020")] + /// ``` + #[clippy::version = "1.61.0"] + pub ALLOW_ATTRIBUTES_WITHOUT_REASON, + restriction, + "ensures that all `allow` and `expect` attributes have a reason" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for `#[should_panic]` attributes without specifying the expected panic message. + /// + /// ### Why is this bad? + /// The expected panic message should be specified to ensure that the test is actually + /// panicking with the expected message, and not another unrelated panic. + /// + /// ### Example + /// ```no_run + /// fn random() -> i32 { 0 } + /// + /// #[should_panic] + /// #[test] + /// fn my_test() { + /// let _ = 1 / random(); + /// } + /// ``` + /// + /// Use instead: + /// ```no_run + /// fn random() -> i32 { 0 } + /// + /// #[should_panic = "attempt to divide by zero"] + /// #[test] + /// fn my_test() { + /// let _ = 1 / random(); + /// } + /// ``` + #[clippy::version = "1.74.0"] + pub SHOULD_PANIC_WITHOUT_EXPECT, + pedantic, + "ensures that all `should_panic` attributes specify its expected panic message" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for `any` and `all` combinators in `cfg` with only one condition. + /// + /// ### Why is this bad? + /// If there is only one condition, no need to wrap it into `any` or `all` combinators. + /// + /// ### Example + /// ```no_run + /// #[cfg(any(unix))] + /// pub struct Bar; + /// ``` + /// + /// Use instead: + /// ```no_run + /// #[cfg(unix)] + /// pub struct Bar; + /// ``` + #[clippy::version = "1.71.0"] + pub NON_MINIMAL_CFG, + style, + "ensure that all `cfg(any())` and `cfg(all())` have more than one condition" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for `#[cfg(features = "...")]` and suggests to replace it with + /// `#[cfg(feature = "...")]`. + /// + /// It also checks if `cfg(test)` was misspelled. + /// + /// ### Why is this bad? + /// Misspelling `feature` as `features` or `test` as `tests` can be sometimes hard to spot. It + /// may cause conditional compilation not work quietly. + /// + /// ### Example + /// ```no_run + /// #[cfg(features = "some-feature")] + /// fn conditional() { } + /// #[cfg(tests)] + /// mod tests { } + /// ``` + /// + /// Use instead: + /// ```no_run + /// #[cfg(feature = "some-feature")] + /// fn conditional() { } + /// #[cfg(test)] + /// mod tests { } + /// ``` + #[clippy::version = "1.69.0"] + pub MAYBE_MISUSED_CFG, + suspicious, + "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_clippy_lint! { + /// ### What it does + /// Checks that an item has only one kind of attributes. + /// + /// ### Why is this bad? + /// Having both kinds of attributes makes it more complicated to read code. + /// + /// ### Example + /// ```no_run + /// #[cfg(linux)] + /// pub fn foo() { + /// #![cfg(windows)] + /// } + /// ``` + /// Use instead: + /// ```no_run + /// #[cfg(linux)] + /// #[cfg(windows)] + /// pub fn foo() { + /// } + /// ``` + #[clippy::version = "1.78.0"] + pub MIXED_ATTRIBUTES_STYLE, + suspicious, + "item has both inner and outer attributes" +} + +declare_lint_pass!(Attributes => [ + ALLOW_ATTRIBUTES_WITHOUT_REASON, + INLINE_ALWAYS, + DEPRECATED_SEMVER, + USELESS_ATTRIBUTE, + BLANKET_CLIPPY_RESTRICTION_LINTS, + SHOULD_PANIC_WITHOUT_EXPECT, +]); + +impl<'tcx> LateLintPass<'tcx> for Attributes { + fn check_crate(&mut self, cx: &LateContext<'tcx>) { + blanket_clippy_restriction_lints::check_command_line(cx); + } + + fn check_attribute(&mut self, cx: &LateContext<'tcx>, attr: &'tcx Attribute) { + if let Some(items) = &attr.meta_item_list() { + if let Some(ident) = attr.ident() { + if is_lint_level(ident.name, attr.id) { + blanket_clippy_restriction_lints::check(cx, ident.name, items); + } + if matches!(ident.name, sym::allow | sym::expect) { + allow_attributes_without_reason::check(cx, ident.name, items, attr); + } + if items.is_empty() || !attr.has_name(sym::deprecated) { + return; + } + for item in items { + if let NestedMetaItem::MetaItem(mi) = &item + && let MetaItemKind::NameValue(lit) = &mi.kind + && mi.has_name(sym::since) + { + deprecated_semver::check(cx, item.span(), lit); + } + } + } + } + if attr.has_name(sym::should_panic) { + should_panic_without_expect::check(cx, attr); + } + } + + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { + let attrs = cx.tcx.hir().attrs(item.hir_id()); + if is_relevant_item(cx, item) { + inline_always::check(cx, item.span, item.ident.name, attrs); + } + match item.kind { + ItemKind::ExternCrate(..) | ItemKind::Use(..) => useless_attribute::check(cx, item, attrs), + _ => {}, + } + } + + fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) { + if is_relevant_impl(cx, item) { + inline_always::check(cx, item.span, item.ident.name, cx.tcx.hir().attrs(item.hir_id())); + } + } + + fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) { + if is_relevant_trait(cx, item) { + inline_always::check(cx, item.span, item.ident.name, cx.tcx.hir().attrs(item.hir_id())); + } + } +} + +pub struct EarlyAttributes { + pub msrv: Msrv, +} + +impl_lint_pass!(EarlyAttributes => [ + DEPRECATED_CFG_ATTR, + MISMATCHED_TARGET_OS, + EMPTY_LINE_AFTER_OUTER_ATTR, + EMPTY_LINE_AFTER_DOC_COMMENTS, + NON_MINIMAL_CFG, + MAYBE_MISUSED_CFG, + DEPRECATED_CLIPPY_CFG_ATTR, + UNNECESSARY_CLIPPY_CFG, + MIXED_ATTRIBUTES_STYLE, +]); + +impl EarlyLintPass for EarlyAttributes { + fn check_item(&mut self, cx: &EarlyContext<'_>, item: &rustc_ast::Item) { + empty_line_after::check(cx, item); + mixed_attributes_style::check(cx, item); + } + + fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) { + deprecated_cfg_attr::check(cx, attr, &self.msrv); + deprecated_cfg_attr::check_clippy(cx, attr); + mismatched_target_os::check(cx, attr); + non_minimal_cfg::check(cx, attr); + maybe_misused_cfg::check(cx, attr); + } + + extract_msrv_attr!(EarlyContext); +} diff --git a/clippy_lints/src/attrs/non_minimal_cfg.rs b/clippy_lints/src/attrs/non_minimal_cfg.rs new file mode 100644 index 00000000000..3fde7061585 --- /dev/null +++ b/clippy_lints/src/attrs/non_minimal_cfg.rs @@ -0,0 +1,49 @@ +use super::{Attribute, NON_MINIMAL_CFG}; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::snippet_opt; +use rustc_ast::{MetaItemKind, NestedMetaItem}; +use rustc_errors::Applicability; +use rustc_lint::EarlyContext; +use rustc_span::sym; + +pub(super) fn check(cx: &EarlyContext<'_>, attr: &Attribute) { + if attr.has_name(sym::cfg) + && let Some(items) = attr.meta_item_list() + { + check_nested_cfg(cx, &items); + } +} + +fn check_nested_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) { + for item in items { + if let NestedMetaItem::MetaItem(meta) = item { + if !meta.has_name(sym::any) && !meta.has_name(sym::all) { + continue; + } + if let MetaItemKind::List(list) = &meta.kind { + check_nested_cfg(cx, list); + if list.len() == 1 { + span_lint_and_then( + cx, + NON_MINIMAL_CFG, + meta.span, + "unneeded sub `cfg` when there is only one condition", + |diag| { + if let Some(snippet) = snippet_opt(cx, list[0].span()) { + diag.span_suggestion(meta.span, "try", snippet, Applicability::MaybeIncorrect); + } + }, + ); + } else if list.is_empty() && meta.has_name(sym::all) { + span_lint_and_then( + cx, + NON_MINIMAL_CFG, + meta.span, + "unneeded sub `cfg` when there is no condition", + |_| {}, + ); + } + } + } + } +} diff --git a/clippy_lints/src/attrs/should_panic_without_expect.rs b/clippy_lints/src/attrs/should_panic_without_expect.rs new file mode 100644 index 00000000000..2d45cbbf621 --- /dev/null +++ b/clippy_lints/src/attrs/should_panic_without_expect.rs @@ -0,0 +1,54 @@ +use super::{Attribute, SHOULD_PANIC_WITHOUT_EXPECT}; +use clippy_utils::diagnostics::span_lint_and_sugg; +use rustc_ast::token::{Token, TokenKind}; +use rustc_ast::tokenstream::TokenTree; +use rustc_ast::{AttrArgs, AttrArgsEq, AttrKind}; +use rustc_errors::Applicability; +use rustc_lint::LateContext; +use rustc_span::sym; + +pub(super) fn check(cx: &LateContext<'_>, attr: &Attribute) { + if let AttrKind::Normal(normal_attr) = &attr.kind { + if let AttrArgs::Eq(_, AttrArgsEq::Hir(_)) = &normal_attr.item.args { + // `#[should_panic = ".."]` found, good + return; + } + + if let AttrArgs::Delimited(args) = &normal_attr.item.args + && let mut tt_iter = args.tokens.trees() + && let Some(TokenTree::Token( + Token { + kind: TokenKind::Ident(sym::expected, _), + .. + }, + _, + )) = tt_iter.next() + && let Some(TokenTree::Token( + Token { + kind: TokenKind::Eq, .. + }, + _, + )) = tt_iter.next() + && let Some(TokenTree::Token( + Token { + kind: TokenKind::Literal(_), + .. + }, + _, + )) = tt_iter.next() + { + // `#[should_panic(expected = "..")]` found, good + return; + } + + span_lint_and_sugg( + cx, + SHOULD_PANIC_WITHOUT_EXPECT, + attr.span, + "#[should_panic] attribute without a reason", + "consider specifying the expected panic", + "#[should_panic(expected = /* panic message */)]".into(), + Applicability::HasPlaceholders, + ); + } +} diff --git a/clippy_lints/src/attrs/unnecessary_clippy_cfg.rs b/clippy_lints/src/attrs/unnecessary_clippy_cfg.rs new file mode 100644 index 00000000000..05da69636c6 --- /dev/null +++ b/clippy_lints/src/attrs/unnecessary_clippy_cfg.rs @@ -0,0 +1,70 @@ +use super::{Attribute, UNNECESSARY_CLIPPY_CFG}; +use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_sugg}; +use clippy_utils::source::snippet_opt; +use rustc_ast::AttrStyle; +use rustc_errors::Applicability; +use rustc_lint::{EarlyContext, Level}; +use rustc_span::sym; + +pub(super) fn check( + 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::<Vec<_>>() + .join(","); + span_lint_and_note( + cx, + UNNECESSARY_CLIPPY_CFG, + clippy_lints, + "no need to put clippy lints behind a `clippy` cfg", + None, + &format!( + "write instead: `#{}[{}({})]`", + if attr.style == AttrStyle::Inner { "!" } else { "" }, + ident.name, + snippet + ), + ); + } + } +} diff --git a/clippy_lints/src/attrs/useless_attribute.rs b/clippy_lints/src/attrs/useless_attribute.rs new file mode 100644 index 00000000000..7575f502a7c --- /dev/null +++ b/clippy_lints/src/attrs/useless_attribute.rs @@ -0,0 +1,73 @@ +use super::utils::{extract_clippy_lint, is_lint_level, is_word}; +use super::{Attribute, USELESS_ATTRIBUTE}; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::{first_line_of_span, snippet_opt}; +use rustc_errors::Applicability; +use rustc_hir::{Item, ItemKind}; +use rustc_lint::{LateContext, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_span::sym; + +pub(super) fn check(cx: &LateContext<'_>, item: &Item<'_>, attrs: &[Attribute]) { + let skip_unused_imports = attrs.iter().any(|attr| attr.has_name(sym::macro_use)); + + for attr in attrs { + if in_external_macro(cx.sess(), attr.span) { + return; + } + if let Some(lint_list) = &attr.meta_item_list() { + if attr.ident().map_or(false, |ident| is_lint_level(ident.name, attr.id)) { + for lint in lint_list { + match item.kind { + ItemKind::Use(..) => { + if is_word(lint, sym::unused_imports) + || 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(), + "wildcard_imports" + | "enum_glob_use" + | "redundant_pub_crate" + | "macro_use_imports" + | "unsafe_removed_from_name" + | "module_name_repetitions" + | "single_component_path_imports" + ) + }) + { + return; + } + }, + ItemKind::ExternCrate(..) => { + if is_word(lint, sym::unused_imports) && skip_unused_imports { + return; + } + if is_word(lint, sym!(unused_extern_crates)) { + return; + } + }, + _ => {}, + } + } + let line_span = first_line_of_span(cx, attr.span); + + if let Some(mut sugg) = snippet_opt(cx, line_span) { + if sugg.contains("#[") { + span_lint_and_then(cx, USELESS_ATTRIBUTE, line_span, "useless lint attribute", |diag| { + sugg = sugg.replacen("#[", "#![", 1); + diag.span_suggestion( + line_span, + "if you just forgot a `!`, use", + sugg, + Applicability::MaybeIncorrect, + ); + }); + } + } + } + } + } +} diff --git a/clippy_lints/src/attrs/utils.rs b/clippy_lints/src/attrs/utils.rs new file mode 100644 index 00000000000..9b36cc00444 --- /dev/null +++ b/clippy_lints/src/attrs/utils.rs @@ -0,0 +1,87 @@ +use clippy_utils::macros::{is_panic, macro_backtrace}; +use rustc_ast::{AttrId, NestedMetaItem}; +use rustc_hir::{ + Block, Expr, ExprKind, ImplItem, ImplItemKind, Item, ItemKind, StmtKind, TraitFn, TraitItem, TraitItemKind, +}; +use rustc_lint::{LateContext, Level}; +use rustc_middle::ty; +use rustc_span::sym; +use rustc_span::symbol::Symbol; + +pub(super) fn is_word(nmi: &NestedMetaItem, expected: Symbol) -> bool { + if let NestedMetaItem::MetaItem(mi) = &nmi { + mi.is_word() && mi.has_name(expected) + } else { + false + } +} + +pub(super) fn is_lint_level(symbol: Symbol, attr_id: AttrId) -> bool { + Level::from_symbol(symbol, Some(attr_id)).is_some() +} + +pub(super) fn is_relevant_item(cx: &LateContext<'_>, item: &Item<'_>) -> bool { + if let ItemKind::Fn(_, _, eid) = item.kind { + is_relevant_expr(cx, cx.tcx.typeck_body(eid), cx.tcx.hir().body(eid).value) + } else { + true + } +} + +pub(super) fn is_relevant_impl(cx: &LateContext<'_>, item: &ImplItem<'_>) -> bool { + match item.kind { + ImplItemKind::Fn(_, eid) => is_relevant_expr(cx, cx.tcx.typeck_body(eid), cx.tcx.hir().body(eid).value), + _ => false, + } +} + +pub(super) fn is_relevant_trait(cx: &LateContext<'_>, item: &TraitItem<'_>) -> bool { + match item.kind { + TraitItemKind::Fn(_, TraitFn::Required(_)) => true, + TraitItemKind::Fn(_, TraitFn::Provided(eid)) => { + is_relevant_expr(cx, cx.tcx.typeck_body(eid), cx.tcx.hir().body(eid).value) + }, + _ => false, + } +} + +fn is_relevant_block(cx: &LateContext<'_>, typeck_results: &ty::TypeckResults<'_>, block: &Block<'_>) -> bool { + block.stmts.first().map_or( + block + .expr + .as_ref() + .map_or(false, |e| is_relevant_expr(cx, typeck_results, e)), + |stmt| match &stmt.kind { + StmtKind::Local(_) => true, + StmtKind::Expr(expr) | StmtKind::Semi(expr) => is_relevant_expr(cx, typeck_results, expr), + StmtKind::Item(_) => false, + }, + ) +} + +fn is_relevant_expr(cx: &LateContext<'_>, typeck_results: &ty::TypeckResults<'_>, expr: &Expr<'_>) -> bool { + if macro_backtrace(expr.span).last().map_or(false, |macro_call| { + is_panic(cx, macro_call.def_id) || cx.tcx.item_name(macro_call.def_id) == sym::unreachable + }) { + return false; + } + match &expr.kind { + ExprKind::Block(block, _) => is_relevant_block(cx, typeck_results, block), + ExprKind::Ret(Some(e)) => is_relevant_expr(cx, typeck_results, e), + ExprKind::Ret(None) | ExprKind::Break(_, None) => false, + _ => true, + } +} + +/// Returns the lint name if it is clippy lint. +pub(super) fn extract_clippy_lint(lint: &NestedMetaItem) -> Option<Symbol> { + if let Some(meta_item) = lint.meta_item() + && meta_item.path.segments.len() > 1 + && let tool_name = meta_item.path.segments[0].ident + && tool_name.name == sym::clippy + { + let lint_name = meta_item.path.segments.last().unwrap().ident.name; + return Some(lint_name); + } + None +} diff --git a/clippy_lints/src/booleans.rs b/clippy_lints/src/booleans.rs index 0d66f2d644d..a474356608f 100644 --- a/clippy_lints/src/booleans.rs +++ b/clippy_lints/src/booleans.rs @@ -88,7 +88,6 @@ impl<'tcx> LateLintPass<'tcx> for NonminimalBool { 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: // // ``` @@ -119,27 +118,6 @@ fn bin_op_eq_str(op: BinOpKind) -> Option<&'static str> { } } -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, @@ -148,8 +126,8 @@ fn check_inverted_bool_in_condition( 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()) + || !cx.typeck_results().node_types()[left.hir_id].is_bool() + || !cx.typeck_results().node_types()[right.hir_id].is_bool() { return; } diff --git a/clippy_lints/src/casts/unnecessary_cast.rs b/clippy_lints/src/casts/unnecessary_cast.rs index cc513d46bf4..08341ff32f3 100644 --- a/clippy_lints/src/casts/unnecessary_cast.rs +++ b/clippy_lints/src/casts/unnecessary_cast.rs @@ -151,13 +151,24 @@ pub(super) fn check<'tcx>( return false; } + // If the whole cast expression is a unary expression (`(*x as T)`) or an addressof + // expression (`(&x as T)`), then not surrounding the suggestion into a block risks us + // changing the precedence of operators if the cast expression is followed by an operation + // with higher precedence than the unary operator (`(*x as T).foo()` would become + // `*x.foo()`, which changes what the `*` applies on). + // The same is true if the expression encompassing the cast expression is a unary + // expression or an addressof expression. + let needs_block = matches!(cast_expr.kind, ExprKind::Unary(..) | ExprKind::AddrOf(..)) + || get_parent_expr(cx, expr) + .map_or(false, |e| matches!(e.kind, ExprKind::Unary(..) | ExprKind::AddrOf(..))); + span_lint_and_sugg( cx, UNNECESSARY_CAST, expr.span, &format!("casting to the same type is unnecessary (`{cast_from}` -> `{cast_to}`)"), "try", - if get_parent_expr(cx, expr).map_or(false, |e| matches!(e.kind, ExprKind::AddrOf(..))) { + if needs_block { format!("{{ {cast_str} }}") } else { cast_str diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index fb3ae2457e3..2b324f5f96e 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -47,6 +47,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::asm_syntax::INLINE_ASM_X86_INTEL_SYNTAX_INFO, crate::assertions_on_constants::ASSERTIONS_ON_CONSTANTS_INFO, crate::assertions_on_result_states::ASSERTIONS_ON_RESULT_STATES_INFO, + crate::assigning_clones::ASSIGNING_CLONES_INFO, crate::async_yields_async::ASYNC_YIELDS_ASYNC_INFO, crate::attrs::ALLOW_ATTRIBUTES_WITHOUT_REASON_INFO, crate::attrs::BLANKET_CLIPPY_RESTRICTION_LINTS_INFO, @@ -58,6 +59,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::attrs::INLINE_ALWAYS_INFO, crate::attrs::MAYBE_MISUSED_CFG_INFO, crate::attrs::MISMATCHED_TARGET_OS_INFO, + crate::attrs::MIXED_ATTRIBUTES_STYLE_INFO, crate::attrs::NON_MINIMAL_CFG_INFO, crate::attrs::SHOULD_PANIC_WITHOUT_EXPECT_INFO, crate::attrs::UNNECESSARY_CLIPPY_CFG_INFO, diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index 6144ec7b3ca..f0f2c7d6658 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -451,8 +451,8 @@ fn check_partial_eq_without_eq<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_r && let Some(def_id) = trait_ref.trait_def_id() && cx.tcx.is_diagnostic_item(sym::PartialEq, def_id) && !has_non_exhaustive_attr(cx.tcx, *adt) + && !ty_implements_eq_trait(cx.tcx, ty, eq_trait_def_id) && let param_env = param_env_for_derived_eq(cx.tcx, adt.did(), eq_trait_def_id) - && !implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, None, &[]) // If all of our fields implement `Eq`, we can implement `Eq` too && adt .all_fields() @@ -471,6 +471,10 @@ fn check_partial_eq_without_eq<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_r } } +fn ty_implements_eq_trait<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, eq_trait_id: DefId) -> bool { + tcx.non_blanket_impls_for_ty(eq_trait_id, ty).next().is_some() +} + /// Creates the `ParamEnv` used for the give type's derived `Eq` impl. fn param_env_for_derived_eq(tcx: TyCtxt<'_>, did: DefId, eq_trait_id: DefId) -> ParamEnv<'_> { // Initial map from generic index to param def. diff --git a/clippy_lints/src/doc/markdown.rs b/clippy_lints/src/doc/markdown.rs index a58219c2946..d2f14756591 100644 --- a/clippy_lints/src/doc/markdown.rs +++ b/clippy_lints/src/doc/markdown.rs @@ -8,7 +8,7 @@ use url::Url; use crate::doc::DOC_MARKDOWN; -pub fn check(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, text: &str, span: Span) { +pub fn check(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, text: &str, span: Span, code_level: isize) { for orig_word in text.split(|c: char| c.is_whitespace() || c == '\'') { // Trim punctuation as in `some comment (see foo::bar).` // ^^ @@ -46,11 +46,11 @@ pub fn check(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, text: &str, span.parent(), ); - check_word(cx, word, span); + check_word(cx, word, span, code_level); } } -fn check_word(cx: &LateContext<'_>, word: &str, span: Span) { +fn check_word(cx: &LateContext<'_>, word: &str, span: Span, code_level: isize) { /// Checks if a string is upper-camel-case, i.e., starts with an uppercase and /// contains at least two uppercase letters (`Clippy` is ok) and one lower-case /// letter (`NASA` is ok). @@ -60,7 +60,14 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span) { return false; } - let s = s.strip_suffix('s').unwrap_or(s); + let s = if let Some(prefix) = s.strip_suffix("es") + && prefix.chars().all(|c| c.is_ascii_uppercase()) + && matches!(prefix.chars().last(), Some('S' | 'X')) + { + prefix + } else { + s.strip_suffix('s').unwrap_or(s) + }; s.chars().all(char::is_alphanumeric) && s.chars().filter(|&c| c.is_uppercase()).take(2).count() > 1 @@ -90,7 +97,7 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span) { } // We assume that mixed-case words are not meant to be put inside backticks. (Issue #2343) - if has_underscore(word) && has_hyphen(word) { + if code_level > 0 || (has_underscore(word) && has_hyphen(word)) { return; } diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs index 9af34c3a7bf..003d26b7b89 100644 --- a/clippy_lints/src/doc/mod.rs +++ b/clippy_lints/src/doc/mod.rs @@ -599,10 +599,19 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize let mut ignore = false; let mut edition = None; let mut ticks_unbalanced = false; - let mut text_to_check: Vec<(CowStr<'_>, Range<usize>)> = Vec::new(); + let mut text_to_check: Vec<(CowStr<'_>, Range<usize>, isize)> = Vec::new(); let mut paragraph_range = 0..0; + let mut code_level = 0; + for (event, range) in events { match event { + Html(tag) => { + if tag.starts_with("<code") { + code_level += 1; + } else if tag.starts_with("</code") { + code_level -= 1; + } + }, Start(CodeBlock(ref kind)) => { in_code = true; if let CodeBlockKind::Fenced(lang) = kind { @@ -652,16 +661,15 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize "a backtick may be missing a pair", ); } else { - for (text, range) in text_to_check { + for (text, range, assoc_code_level) in text_to_check { if let Some(span) = fragments.span(cx, range) { - markdown::check(cx, valid_idents, &text, span); + markdown::check(cx, valid_idents, &text, span, assoc_code_level); } } } text_to_check = Vec::new(); }, Start(_tag) | End(_tag) => (), // We don't care about other tags - Html(_html) => (), // HTML is weird, just ignore it SoftBreak | HardBreak | TaskListMarker(_) | Code(_) | Rule => (), FootnoteReference(text) | Text(text) => { paragraph_range.end = range.end; @@ -694,7 +702,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize // Don't check the text associated with external URLs continue; } - text_to_check.push((text, range)); + text_to_check.push((text, range, code_level)); } }, } diff --git a/clippy_lints/src/entry.rs b/clippy_lints/src/entry.rs index 0b4bc375df0..de6073c2723 100644 --- a/clippy_lints/src/entry.rs +++ b/clippy_lints/src/entry.rs @@ -214,12 +214,31 @@ impl MapType { } } +/// Details on an expression checking whether a map contains a key. +/// +/// For instance, with the following: +/// ```ignore +/// !!!self.the_map.contains_key("the_key") +/// ``` +/// +/// - `negated` will be set to `true` (the 3 `!` negate the condition) +/// - `map` will be the `self.the_map` expression +/// - `key` will be the `"the_key"` expression struct ContainsExpr<'tcx> { + /// Whether the check for `contains_key` was negated. negated: bool, + /// The map on which the check is performed. map: &'tcx Expr<'tcx>, + /// The key that is checked to be contained. key: &'tcx Expr<'tcx>, + /// The context of the whole condition expression. call_ctxt: SyntaxContext, } + +/// Inspect the given expression and return details about the `contains_key` check. +/// +/// If the given expression is not a `contains_key` check against a `BTreeMap` or a `HashMap`, +/// return `None`. fn try_parse_contains<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> Option<(MapType, ContainsExpr<'tcx>)> { let mut negated = false; let expr = peel_hir_expr_while(expr, |e| match e.kind { @@ -229,6 +248,7 @@ fn try_parse_contains<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> Optio }, _ => None, }); + match expr.kind { ExprKind::MethodCall( _, @@ -261,11 +281,28 @@ fn try_parse_contains<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> Optio } } +/// Details on an expression inserting a key into a map. +/// +/// For instance, on the following: +/// ```ignore +/// self.the_map.insert("the_key", 3 + 4); +/// ``` +/// +/// - `map` will be the `self.the_map` expression +/// - `key` will be the `"the_key"` expression +/// - `value` will be the `3 + 4` expression struct InsertExpr<'tcx> { + /// The map into which the insertion is performed. map: &'tcx Expr<'tcx>, + /// The key at which to insert. key: &'tcx Expr<'tcx>, + /// The value to insert. value: &'tcx Expr<'tcx>, } + +/// Inspect the given expression and return details about the `insert` call. +/// +/// If the given expression is not an `insert` call into a `BTreeMap` or a `HashMap`, return `None`. fn try_parse_insert<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<InsertExpr<'tcx>> { if let ExprKind::MethodCall(_, map, [key, value], _) = expr.kind { let id = cx.typeck_results().type_dependent_def_id(expr.hir_id)?; @@ -298,7 +335,7 @@ struct Insertion<'tcx> { value: &'tcx Expr<'tcx>, } -/// This visitor needs to do a multiple things: +/// This visitor needs to do multiple things: /// * Find all usages of the map. An insertion can only be made before any other usages of the map. /// * Determine if there's an insertion using the same key. There's no need for the entry api /// otherwise. @@ -346,7 +383,7 @@ impl<'tcx> InsertSearcher<'_, 'tcx> { res } - /// Visits an expression which is not itself in a tail position, but other sibling expressions + /// Visit an expression which is not itself in a tail position, but other sibling expressions /// may be. e.g. if conditions fn visit_non_tail_expr(&mut self, e: &'tcx Expr<'_>) { let in_tail_pos = self.in_tail_pos; @@ -354,6 +391,19 @@ impl<'tcx> InsertSearcher<'_, 'tcx> { self.visit_expr(e); self.in_tail_pos = in_tail_pos; } + + /// Visit the key and value expression of an insert expression. + /// There may not be uses of the map in either of those two either. + fn visit_insert_expr_arguments(&mut self, e: &InsertExpr<'tcx>) { + let in_tail_pos = self.in_tail_pos; + let allow_insert_closure = self.allow_insert_closure; + let is_single_insert = self.is_single_insert; + walk_expr(self, e.key); + walk_expr(self, e.value); + self.in_tail_pos = in_tail_pos; + self.allow_insert_closure = allow_insert_closure; + self.is_single_insert = is_single_insert; + } } impl<'tcx> Visitor<'tcx> for InsertSearcher<'_, 'tcx> { fn visit_stmt(&mut self, stmt: &'tcx Stmt<'_>) { @@ -425,6 +475,7 @@ impl<'tcx> Visitor<'tcx> for InsertSearcher<'_, 'tcx> { match try_parse_insert(self.cx, expr) { Some(insert_expr) if SpanlessEq::new(self.cx).eq_expr(self.map, insert_expr.map) => { + self.visit_insert_expr_arguments(&insert_expr); // Multiple inserts, inserts with a different key, and inserts from a macro can't use the entry api. if self.is_map_used || !SpanlessEq::new(self.cx).eq_expr(self.key, insert_expr.key) diff --git a/clippy_lints/src/let_underscore.rs b/clippy_lints/src/let_underscore.rs index 606c2ed72be..0ea53c39280 100644 --- a/clippy_lints/src/let_underscore.rs +++ b/clippy_lints/src/let_underscore.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::ty::{implements_trait, is_must_use_ty, match_type}; use clippy_utils::{is_from_proc_macro, is_must_use_func_call, paths}; -use rustc_hir::{Local, PatKind}; +use rustc_hir::{Local, LocalSource, PatKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::{GenericArgKind, IsSuggestable}; @@ -139,7 +139,8 @@ const SYNC_GUARD_PATHS: [&[&str]; 3] = [ impl<'tcx> LateLintPass<'tcx> for LetUnderscore { fn check_local(&mut self, cx: &LateContext<'tcx>, local: &Local<'tcx>) { - if !in_external_macro(cx.tcx.sess, local.span) + if matches!(local.source, LocalSource::Normal) + && !in_external_macro(cx.tcx.sess, local.span) && let PatKind::Wild = local.pat.kind && let Some(init) = local.init { diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 76e75968314..b930175c4d8 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -80,6 +80,7 @@ mod as_conversions; mod asm_syntax; mod assertions_on_constants; mod assertions_on_result_states; +mod assigning_clones; mod async_yields_async; mod attrs; mod await_holding_invalid; @@ -1118,6 +1119,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)); + store.register_late_pass(|_| Box::new(assigning_clones::AssigningClones)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/loops/manual_memcpy.rs b/clippy_lints/src/loops/manual_memcpy.rs index 58f713d8187..18f799e875a 100644 --- a/clippy_lints/src/loops/manual_memcpy.rs +++ b/clippy_lints/src/loops/manual_memcpy.rs @@ -3,6 +3,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet; use clippy_utils::sugg::Sugg; use clippy_utils::ty::is_copy; +use clippy_utils::usage::local_used_in; use clippy_utils::{get_enclosing_block, higher, path_to_local, sugg}; use rustc_ast::ast; use rustc_errors::Applicability; @@ -63,8 +64,9 @@ pub(super) fn check<'tcx>( && get_slice_like_element_ty(cx, cx.typeck_results().expr_ty(base_right)).is_some() && let Some((start_left, offset_left)) = get_details_from_idx(cx, idx_left, &starts) && let Some((start_right, offset_right)) = get_details_from_idx(cx, idx_right, &starts) - - // Source and destination must be different + && !local_used_in(cx, canonical_id, base_left) + && !local_used_in(cx, canonical_id, base_right) + // Source and destination must be different && path_to_local(base_left) != path_to_local(base_right) { Some(( diff --git a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs index 69f86836775..b770ad0ddb5 100644 --- a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs +++ b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs @@ -37,12 +37,7 @@ pub(super) fn check<'tcx>( } } -fn set_diagnostic<'tcx>( - diag: &mut Diag<'_, ()>, - cx: &LateContext<'tcx>, - expr: &'tcx Expr<'tcx>, - found: FoundSigDrop, -) { +fn set_diagnostic<'tcx>(diag: &mut Diag<'_, ()>, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, found: FoundSigDrop) { if found.lint_suggestion == LintSuggestion::MoveAndClone { // If our suggestion is to move and clone, then we want to leave it to the user to // decide how to address this lint, since it may be that cloning is inappropriate. diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index adb696ad509..8a24ccea3a1 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3875,6 +3875,7 @@ declare_clippy_lint! { } declare_clippy_lint! { + /// ### What it does /// Checks for usage of `option.map(f).unwrap_or_default()` and `result.map(f).unwrap_or_default()` where f is a function or closure that returns the `bool` type. /// /// ### Why is this bad? @@ -3981,6 +3982,7 @@ declare_clippy_lint! { } declare_clippy_lint! { + /// ### What it does /// Checks for the manual creation of C strings (a string with a `NUL` byte at the end), either /// through one of the `CStr` constructor functions, or more plainly by calling `.as_ptr()` /// on a (byte) string literal with a hardcoded `\0` byte at the end. diff --git a/clippy_lints/src/operators/identity_op.rs b/clippy_lints/src/operators/identity_op.rs index f671517c134..0879dcd9bcd 100644 --- a/clippy_lints/src/operators/identity_op.rs +++ b/clippy_lints/src/operators/identity_op.rs @@ -42,7 +42,7 @@ pub(crate) fn check<'tcx>( match op { BinOpKind::Add | BinOpKind::BitOr | BinOpKind::BitXor => { - check_op( + let _ = check_op( cx, left, 0, @@ -50,8 +50,7 @@ pub(crate) fn check<'tcx>( peeled_right_span, needs_parenthesis(cx, expr, right), right_is_coerced_to_value, - ); - check_op( + ) || check_op( cx, right, 0, @@ -62,7 +61,7 @@ pub(crate) fn check<'tcx>( ); }, BinOpKind::Shl | BinOpKind::Shr | BinOpKind::Sub => { - check_op( + let _ = check_op( cx, right, 0, @@ -73,7 +72,7 @@ pub(crate) fn check<'tcx>( ); }, BinOpKind::Mul => { - check_op( + let _ = check_op( cx, left, 1, @@ -81,8 +80,18 @@ pub(crate) fn check<'tcx>( peeled_right_span, needs_parenthesis(cx, expr, right), right_is_coerced_to_value, + ) || check_op( + cx, + right, + 1, + expr.span, + peeled_left_span, + Parens::Unneeded, + left_is_coerced_to_value, ); - check_op( + }, + BinOpKind::Div => { + let _ = check_op( cx, right, 1, @@ -92,17 +101,8 @@ pub(crate) fn check<'tcx>( left_is_coerced_to_value, ); }, - BinOpKind::Div => check_op( - cx, - right, - 1, - expr.span, - peeled_left_span, - Parens::Unneeded, - left_is_coerced_to_value, - ), BinOpKind::BitAnd => { - check_op( + let _ = check_op( cx, left, -1, @@ -110,8 +110,7 @@ pub(crate) fn check<'tcx>( peeled_right_span, needs_parenthesis(cx, expr, right), right_is_coerced_to_value, - ); - check_op( + ) || check_op( cx, right, -1, @@ -201,12 +200,12 @@ fn check_remainder(cx: &LateContext<'_>, left: &Expr<'_>, right: &Expr<'_>, span } } -fn check_op(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span, parens: Parens, is_erased: bool) { +fn check_op(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span, parens: Parens, is_erased: bool) -> bool { if let Some(Constant::Int(v)) = constant_simple(cx, cx.typeck_results(), e).map(Constant::peel_refs) { let check = match *cx.typeck_results().expr_ty(e).peel_refs().kind() { ty::Int(ity) => unsext(cx.tcx, -1_i128, ity), ty::Uint(uty) => clip(cx.tcx, !0, uty), - _ => return, + _ => return false, }; if match m { 0 => v == 0, @@ -215,8 +214,10 @@ fn check_op(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span, pa _ => unreachable!(), } { span_ineffective_operation(cx, span, arg, parens, is_erased); + return true; } } + false } fn span_ineffective_operation( diff --git a/clippy_lints/src/operators/misrefactored_assign_op.rs b/clippy_lints/src/operators/misrefactored_assign_op.rs index fecc5a8578e..311cbd050a1 100644 --- a/clippy_lints/src/operators/misrefactored_assign_op.rs +++ b/clippy_lints/src/operators/misrefactored_assign_op.rs @@ -21,9 +21,8 @@ pub(super) fn check<'tcx>( // lhs op= l op r if eq_expr_value(cx, lhs, l) { lint_misrefactored_assign_op(cx, expr, op, rhs, lhs, r); - } - // lhs op= l commutative_op r - if is_commutative(op) && eq_expr_value(cx, lhs, r) { + } else if is_commutative(op) && eq_expr_value(cx, lhs, r) { + // lhs op= l commutative_op r lint_misrefactored_assign_op(cx, expr, op, rhs, lhs, l); } } diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs index 1df2d5e4602..cf7f730140c 100644 --- a/clippy_lints/src/question_mark.rs +++ b/clippy_lints/src/question_mark.rs @@ -203,108 +203,106 @@ fn expr_return_none_or_err( } } -impl QuestionMark { - fn inside_try_block(&self) -> bool { - self.try_block_depth_stack.last() > Some(&0) - } - - /// Checks if the given expression on the given context matches the following structure: - /// - /// ```ignore - /// if option.is_none() { - /// return None; - /// } - /// ``` - /// - /// ```ignore - /// if result.is_err() { - /// return result; - /// } - /// ``` - /// - /// If it matches, it will suggest to use the question mark operator instead - fn check_is_none_or_err_and_early_return<'tcx>(&self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { - if !self.inside_try_block() - && let Some(higher::If { cond, then, r#else }) = higher::If::hir(expr) - && !is_else_clause(cx.tcx, expr) - && let ExprKind::MethodCall(segment, caller, ..) = &cond.kind - && let caller_ty = cx.typeck_results().expr_ty(caller) - && let if_block = IfBlockType::IfIs(caller, caller_ty, segment.ident.name, then) - && (is_early_return(sym::Option, cx, &if_block) || is_early_return(sym::Result, cx, &if_block)) - { - let mut applicability = Applicability::MachineApplicable; - let receiver_str = snippet_with_applicability(cx, caller.span, "..", &mut applicability); - let by_ref = !caller_ty.is_copy_modulo_regions(cx.tcx, cx.param_env) - && !matches!(caller.kind, ExprKind::Call(..) | ExprKind::MethodCall(..)); - let sugg = if let Some(else_inner) = r#else { - if eq_expr_value(cx, caller, peel_blocks(else_inner)) { - format!("Some({receiver_str}?)") - } else { - return; - } +/// Checks if the given expression on the given context matches the following structure: +/// +/// ```ignore +/// if option.is_none() { +/// return None; +/// } +/// ``` +/// +/// ```ignore +/// if result.is_err() { +/// return result; +/// } +/// ``` +/// +/// If it matches, it will suggest to use the question mark operator instead +fn check_is_none_or_err_and_early_return<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { + if let Some(higher::If { cond, then, r#else }) = higher::If::hir(expr) + && !is_else_clause(cx.tcx, expr) + && let ExprKind::MethodCall(segment, caller, ..) = &cond.kind + && let caller_ty = cx.typeck_results().expr_ty(caller) + && let if_block = IfBlockType::IfIs(caller, caller_ty, segment.ident.name, then) + && (is_early_return(sym::Option, cx, &if_block) || is_early_return(sym::Result, cx, &if_block)) + { + let mut applicability = Applicability::MachineApplicable; + let receiver_str = snippet_with_applicability(cx, caller.span, "..", &mut applicability); + let by_ref = !caller_ty.is_copy_modulo_regions(cx.tcx, cx.param_env) + && !matches!(caller.kind, ExprKind::Call(..) | ExprKind::MethodCall(..)); + let sugg = if let Some(else_inner) = r#else { + if eq_expr_value(cx, caller, peel_blocks(else_inner)) { + format!("Some({receiver_str}?)") } else { - format!("{receiver_str}{}?;", if by_ref { ".as_ref()" } else { "" }) - }; + return; + } + } else { + format!("{receiver_str}{}?;", if by_ref { ".as_ref()" } else { "" }) + }; - span_lint_and_sugg( - cx, - QUESTION_MARK, - expr.span, - "this block may be rewritten with the `?` operator", - "replace it with", - sugg, - applicability, - ); - } + span_lint_and_sugg( + cx, + QUESTION_MARK, + expr.span, + "this block may be rewritten with the `?` operator", + "replace it with", + sugg, + applicability, + ); } +} - fn check_if_let_some_or_err_and_early_return<'tcx>(&self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { - if !self.inside_try_block() - && let Some(higher::IfLet { - let_pat, - let_expr, - if_then, - if_else, - .. - }) = higher::IfLet::hir(cx, expr) - && !is_else_clause(cx.tcx, expr) - && let PatKind::TupleStruct(ref path1, [field], ddpos) = let_pat.kind - && ddpos.as_opt_usize().is_none() - && let PatKind::Binding(BindingAnnotation(by_ref, _), bind_id, ident, None) = field.kind - && let caller_ty = cx.typeck_results().expr_ty(let_expr) - && let if_block = IfBlockType::IfLet( - cx.qpath_res(path1, let_pat.hir_id), - caller_ty, - ident.name, - let_expr, - if_then, - if_else, - ) - && ((is_early_return(sym::Option, cx, &if_block) && path_to_local_id(peel_blocks(if_then), bind_id)) - || is_early_return(sym::Result, cx, &if_block)) - && if_else - .map(|e| eq_expr_value(cx, let_expr, peel_blocks(e))) - .filter(|e| *e) - .is_none() - { - let mut applicability = Applicability::MachineApplicable; - let receiver_str = snippet_with_applicability(cx, let_expr.span, "..", &mut applicability); - let requires_semi = matches!(cx.tcx.parent_hir_node(expr.hir_id), Node::Stmt(_)); - let sugg = format!( - "{receiver_str}{}?{}", - if by_ref == ByRef::Yes { ".as_ref()" } else { "" }, - if requires_semi { ";" } else { "" } - ); - span_lint_and_sugg( - cx, - QUESTION_MARK, - expr.span, - "this block may be rewritten with the `?` operator", - "replace it with", - sugg, - applicability, - ); - } +fn check_if_let_some_or_err_and_early_return<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { + if let Some(higher::IfLet { + let_pat, + let_expr, + if_then, + if_else, + .. + }) = higher::IfLet::hir(cx, expr) + && !is_else_clause(cx.tcx, expr) + && let PatKind::TupleStruct(ref path1, [field], ddpos) = let_pat.kind + && ddpos.as_opt_usize().is_none() + && let PatKind::Binding(BindingAnnotation(by_ref, _), bind_id, ident, None) = field.kind + && let caller_ty = cx.typeck_results().expr_ty(let_expr) + && let if_block = IfBlockType::IfLet( + cx.qpath_res(path1, let_pat.hir_id), + caller_ty, + ident.name, + let_expr, + if_then, + if_else, + ) + && ((is_early_return(sym::Option, cx, &if_block) && path_to_local_id(peel_blocks(if_then), bind_id)) + || is_early_return(sym::Result, cx, &if_block)) + && if_else + .map(|e| eq_expr_value(cx, let_expr, peel_blocks(e))) + .filter(|e| *e) + .is_none() + { + let mut applicability = Applicability::MachineApplicable; + let receiver_str = snippet_with_applicability(cx, let_expr.span, "..", &mut applicability); + let requires_semi = matches!(cx.tcx.parent_hir_node(expr.hir_id), Node::Stmt(_)); + let sugg = format!( + "{receiver_str}{}?{}", + if by_ref == ByRef::Yes { ".as_ref()" } else { "" }, + if requires_semi { ";" } else { "" } + ); + span_lint_and_sugg( + cx, + QUESTION_MARK, + expr.span, + "this block may be rewritten with the `?` operator", + "replace it with", + sugg, + applicability, + ); + } +} + +impl QuestionMark { + fn inside_try_block(&self) -> bool { + self.try_block_depth_stack.last() > Some(&0) } } @@ -324,15 +322,18 @@ impl<'tcx> LateLintPass<'tcx> for QuestionMark { return; } - if !in_constant(cx, stmt.hir_id) { + if !self.inside_try_block() && !in_constant(cx, stmt.hir_id) { check_let_some_else_return_none(cx, stmt); } self.check_manual_let_else(cx, stmt); } fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if !in_constant(cx, expr.hir_id) && is_lint_allowed(cx, QUESTION_MARK_USED, expr.hir_id) { - self.check_is_none_or_err_and_early_return(cx, expr); - self.check_if_let_some_or_err_and_early_return(cx, expr); + if !self.inside_try_block() + && !in_constant(cx, expr.hir_id) + && is_lint_allowed(cx, QUESTION_MARK_USED, expr.hir_id) + { + check_is_none_or_err_and_early_return(cx, expr); + check_if_let_some_or_err_and_early_return(cx, expr); } } diff --git a/clippy_lints/src/redundant_closure_call.rs b/clippy_lints/src/redundant_closure_call.rs index e3f0ce2a922..f61527cc0a9 100644 --- a/clippy_lints/src/redundant_closure_call.rs +++ b/clippy_lints/src/redundant_closure_call.rs @@ -159,6 +159,15 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClosureCall { // ^^ we only want to lint for this call (but we walk up the calls to consider both calls). // without this check, we'd end up linting twice. && !matches!(recv.kind, hir::ExprKind::Call(..)) + // Check if `recv` comes from a macro expansion. If it does, make sure that it's an expansion that is + // the same as the one the call is in. + // For instance, let's assume `x!()` returns a closure: + // B ---v + // x!()() + // ^- A + // The call happens in the expansion `A`, while the closure originates from the expansion `B`. + // We don't want to suggest replacing `x!()()` with `x!()`. + && recv.span.ctxt().outer_expn() == expr.span.ctxt().outer_expn() && let (full_expr, call_depth) = get_parent_call_exprs(cx, expr) && let Some((body, fn_decl, coroutine_kind, params)) = find_innermost_closure(cx, recv, call_depth) // outside macros we lint properly. Inside macros, we lint only ||() style closures. diff --git a/clippy_lints/src/redundant_field_names.rs b/clippy_lints/src/redundant_field_names.rs index fb000cd7184..17b031d5f01 100644 --- a/clippy_lints/src/redundant_field_names.rs +++ b/clippy_lints/src/redundant_field_names.rs @@ -59,24 +59,22 @@ impl EarlyLintPass for RedundantFieldNames { } if let ExprKind::Struct(ref se) = expr.kind { for field in &se.fields { - if field.is_shorthand { - continue; - } - if let ExprKind::Path(None, path) = &field.expr.kind { - if path.segments.len() == 1 - && path.segments[0].ident == field.ident - && path.segments[0].args.is_none() - { - span_lint_and_sugg( - cx, - REDUNDANT_FIELD_NAMES, - field.span, - "redundant field names in struct initialization", - "replace it with", - field.ident.to_string(), - Applicability::MachineApplicable, - ); - } + if !field.is_shorthand + && let ExprKind::Path(None, path) = &field.expr.kind + && let [segment] = path.segments.as_slice() + && segment.args.is_none() + && segment.ident == field.ident + && field.span.eq_ctxt(field.ident.span) + { + span_lint_and_sugg( + cx, + REDUNDANT_FIELD_NAMES, + field.span, + "redundant field names in struct initialization", + "replace it with", + field.ident.to_string(), + Applicability::MachineApplicable, + ); } } } diff --git a/clippy_lints/src/std_instead_of_core.rs b/clippy_lints/src/std_instead_of_core.rs index 38fd54a0f1e..c0e4f3c368a 100644 --- a/clippy_lints/src/std_instead_of_core.rs +++ b/clippy_lints/src/std_instead_of_core.rs @@ -109,7 +109,6 @@ impl<'tcx> LateLintPass<'tcx> for StdReexports { sym::core => (STD_INSTEAD_OF_CORE, "std", "core"), sym::alloc => (STD_INSTEAD_OF_ALLOC, "std", "alloc"), _ => { - self.prev_span = path.span; return; }, }, @@ -117,13 +116,12 @@ impl<'tcx> LateLintPass<'tcx> for StdReexports { if cx.tcx.crate_name(def_id.krate) == sym::core { (ALLOC_INSTEAD_OF_CORE, "alloc", "core") } else { - self.prev_span = path.span; return; } }, _ => return, }; - if path.span != self.prev_span { + if first_segment.ident.span != self.prev_span { span_lint_and_sugg( cx, lint, @@ -133,7 +131,7 @@ impl<'tcx> LateLintPass<'tcx> for StdReexports { replace_with.to_string(), Applicability::MachineApplicable, ); - self.prev_span = path.span; + self.prev_span = first_segment.ident.span; } } } diff --git a/clippy_lints/src/thread_local_initializer_can_be_made_const.rs b/clippy_lints/src/thread_local_initializer_can_be_made_const.rs index 9fee4c06200..1af3733ebfa 100644 --- a/clippy_lints/src/thread_local_initializer_can_be_made_const.rs +++ b/clippy_lints/src/thread_local_initializer_can_be_made_const.rs @@ -1,12 +1,11 @@ -use clippy_config::msrvs::Msrv; +use clippy_config::msrvs::{self, Msrv}; use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::fn_has_unsatisfiable_preds; use clippy_utils::qualify_min_const_fn::is_min_const_fn; use clippy_utils::source::snippet; +use clippy_utils::{fn_has_unsatisfiable_preds, peel_blocks}; use rustc_errors::Applicability; use rustc_hir::{intravisit, ExprKind}; -use rustc_lint::{LateContext, LateLintPass, LintContext}; -use rustc_middle::lint::in_external_macro; +use rustc_lint::{LateContext, LateLintPass}; use rustc_session::impl_lint_pass; use rustc_span::sym::thread_local_macro; @@ -57,6 +56,31 @@ impl ThreadLocalInitializerCanBeMadeConst { impl_lint_pass!(ThreadLocalInitializerCanBeMadeConst => [THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST]); +#[inline] +fn is_thread_local_initializer( + cx: &LateContext<'_>, + fn_kind: rustc_hir::intravisit::FnKind<'_>, + span: rustc_span::Span, +) -> Option<bool> { + let macro_def_id = span.source_callee()?.macro_def_id?; + Some( + cx.tcx.is_diagnostic_item(thread_local_macro, macro_def_id) + && matches!(fn_kind, intravisit::FnKind::ItemFn(..)), + ) +} + +#[inline] +fn initializer_can_be_made_const(cx: &LateContext<'_>, defid: rustc_span::def_id::DefId, msrv: &Msrv) -> bool { + // Building MIR for `fn`s with unsatisfiable preds results in ICE. + if !fn_has_unsatisfiable_preds(cx, defid) + && let mir = cx.tcx.optimized_mir(defid) + && let Ok(()) = is_min_const_fn(cx.tcx, mir, msrv) + { + return true; + } + false +} + impl<'tcx> LateLintPass<'tcx> for ThreadLocalInitializerCanBeMadeConst { fn check_fn( &mut self, @@ -65,31 +89,32 @@ impl<'tcx> LateLintPass<'tcx> for ThreadLocalInitializerCanBeMadeConst { _: &'tcx rustc_hir::FnDecl<'tcx>, body: &'tcx rustc_hir::Body<'tcx>, span: rustc_span::Span, - defid: rustc_span::def_id::LocalDefId, + local_defid: rustc_span::def_id::LocalDefId, ) { - if in_external_macro(cx.sess(), span) - && let Some(callee) = span.source_callee() - && let Some(macro_def_id) = callee.macro_def_id - && cx.tcx.is_diagnostic_item(thread_local_macro, macro_def_id) - && let intravisit::FnKind::ItemFn(..) = fn_kind - // Building MIR for `fn`s with unsatisfiable preds results in ICE. - && !fn_has_unsatisfiable_preds(cx, defid.to_def_id()) - && let mir = cx.tcx.optimized_mir(defid.to_def_id()) - && let Ok(()) = is_min_const_fn(cx.tcx, mir, &self.msrv) - // this is the `__init` function emitted by the `thread_local!` macro - // when the `const` keyword is not used. We avoid checking the `__init` directly - // as that is not a public API. - // we know that the function is const-qualifiable, so now we need only to get the - // initializer expression to span-lint it. + let defid = local_defid.to_def_id(); + if self.msrv.meets(msrvs::THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST) + && is_thread_local_initializer(cx, fn_kind, span).unwrap_or(false) + // Some implementations of `thread_local!` include an initializer fn. + // In the case of a const initializer, the init fn is also const, + // so we can skip the lint in that case. This occurs only on some + // backends due to conditional compilation: + // https://doc.rust-lang.org/src/std/sys/common/thread_local/mod.rs.html + // for details on this issue, see: + // https://github.com/rust-lang/rust-clippy/pull/12276 + && !cx.tcx.is_const_fn(defid) + && initializer_can_be_made_const(cx, defid, &self.msrv) + // we know that the function is const-qualifiable, so now + // we need only to get the initializer expression to span-lint it. && let ExprKind::Block(block, _) = body.value.kind - && let Some(ret_expr) = block.expr + && let Some(unpeeled) = block.expr + && let ret_expr = peel_blocks(unpeeled) && let initializer_snippet = snippet(cx, ret_expr.span, "thread_local! { ... }") && initializer_snippet != "thread_local! { ... }" { span_lint_and_sugg( cx, THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST, - ret_expr.span, + unpeeled.span, "initializer for `thread_local` value can be made `const`", "replace with", format!("const {{ {initializer_snippet} }}"), diff --git a/clippy_lints/src/transmute/mod.rs b/clippy_lints/src/transmute/mod.rs index 06de7a11031..e47b14bf63b 100644 --- a/clippy_lints/src/transmute/mod.rs +++ b/clippy_lints/src/transmute/mod.rs @@ -592,7 +592,7 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { | (eager_transmute::check(cx, e, arg, from_ty, to_ty)); if !linted { - transmutes_expressible_as_ptr_casts::check(cx, e, from_ty, from_ty_adjusted, to_ty, arg); + transmutes_expressible_as_ptr_casts::check(cx, e, from_ty, from_ty_adjusted, to_ty, arg, const_context); } } } diff --git a/clippy_lints/src/transmute/transmutes_expressible_as_ptr_casts.rs b/clippy_lints/src/transmute/transmutes_expressible_as_ptr_casts.rs index bbecc39a813..043c9c88601 100644 --- a/clippy_lints/src/transmute/transmutes_expressible_as_ptr_casts.rs +++ b/clippy_lints/src/transmute/transmutes_expressible_as_ptr_casts.rs @@ -18,10 +18,12 @@ pub(super) fn check<'tcx>( from_ty_adjusted: bool, to_ty: Ty<'tcx>, arg: &'tcx Expr<'_>, + const_context: bool, ) -> bool { use CastKind::{AddrPtrCast, ArrayPtrCast, FnPtrAddrCast, FnPtrPtrCast, PtrAddrCast, PtrPtrCast}; let mut app = Applicability::MachineApplicable; let mut sugg = match check_cast(cx, e, from_ty, to_ty) { + Some(FnPtrAddrCast | PtrAddrCast) if const_context => return false, Some(PtrPtrCast | AddrPtrCast | ArrayPtrCast | FnPtrPtrCast | FnPtrAddrCast) => { Sugg::hir_with_context(cx, arg, e.span.ctxt(), "..", &mut app) .as_ty(to_ty.to_string()) diff --git a/clippy_lints/src/types/vec_box.rs b/clippy_lints/src/types/vec_box.rs index a285f771f1b..7926738d68f 100644 --- a/clippy_lints/src/types/vec_box.rs +++ b/clippy_lints/src/types/vec_box.rs @@ -1,10 +1,9 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::paths::ALLOCATOR_GLOBAL; +use clippy_utils::last_path_segment; use clippy_utils::source::snippet; -use clippy_utils::{last_path_segment, match_def_path}; use rustc_errors::Applicability; use rustc_hir::def_id::DefId; -use rustc_hir::{self as hir, GenericArg, QPath, TyKind}; +use rustc_hir::{self as hir, GenericArg, LangItem, QPath, TyKind}; use rustc_hir_analysis::hir_ty_to_ty; use rustc_lint::LateContext; use rustc_middle::ty::layout::LayoutOf; @@ -50,7 +49,7 @@ pub(super) fn check<'tcx>( (None, Some(GenericArg::Type(inner))) | (Some(GenericArg::Type(inner)), None) => { if let TyKind::Path(path) = inner.kind && let Some(did) = cx.qpath_res(&path, inner.hir_id).opt_def_id() { - match_def_path(cx, did, &ALLOCATOR_GLOBAL) + cx.tcx.lang_items().get(LangItem::GlobalAlloc) == Some(did) } else { false } |