Merge branch 'master' into new-lint-rwlock_atomic

Author: Tunglies

clippy_dev/src/new_lint.rs

@@ -157,18 +157,19 @@ fn add_lint(lint: &LintData<'_>, enable_msrv: bool) -> io::Result<()> {
     let path = "clippy_lints/src/lib.rs";
     let mut lib_rs = fs::read_to_string(path).context("reading")?;
 
-    let comment_start = lib_rs.find("// add lints here,").expect("Couldn't find comment");
-    let ctor_arg = if lint.pass == Pass::Late { "_" } else { "" };
-    let lint_pass = lint.pass;
+    let (comment, ctor_arg) = if lint.pass == Pass::Late {
+        ("// add late passes here", "_")
+    } else {
+        ("// add early passes here", "")
+    };
+    let comment_start = lib_rs.find(comment).expect("Couldn't find comment");
     let module_name = lint.name;
     let camel_name = to_camel_case(lint.name);
 
     let new_lint = if enable_msrv {
-        format!(
-            "store.register_{lint_pass}_pass(move |{ctor_arg}| Box::new({module_name}::{camel_name}::new(conf)));\n    ",
-        )
+        format!("Box::new(move |{ctor_arg}| Box::new({module_name}::{camel_name}::new(conf))),\n        ",)
     } else {
-        format!("store.register_{lint_pass}_pass(|{ctor_arg}| Box::new({module_name}::{camel_name}));\n    ",)
+        format!("Box::new(|{ctor_arg}| Box::new({module_name}::{camel_name})),\n        ",)
     };
 
     lib_rs.insert_str(comment_start, &new_lint);

clippy_lints/src/incompatible_msrv.rs

@@ -1,14 +1,14 @@
 use clippy_config::Conf;
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::msrvs::Msrv;
-use clippy_utils::{is_in_const_context, is_in_test};
+use clippy_utils::{is_in_const_context, is_in_test, sym};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::{self as hir, AmbigArg, Expr, ExprKind, HirId, RustcVersion, StabilityLevel, StableSince};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_session::impl_lint_pass;
 use rustc_span::def_id::{CrateNum, DefId};
-use rustc_span::{ExpnKind, Span, sym};
+use rustc_span::{ExpnKind, Span};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -77,11 +77,36 @@ enum Availability {
     Since(RustcVersion),
 }
 
+/// All known std crates containing a stability attribute.
+struct StdCrates([Option<CrateNum>; 6]);
+impl StdCrates {
+    fn new(tcx: TyCtxt<'_>) -> Self {
+        let mut res = Self([None; _]);
+        for &krate in tcx.crates(()) {
+            // FIXME(@Jarcho): We should have an internal lint to detect when this list is out of date.
+            match tcx.crate_name(krate) {
+                sym::alloc => res.0[0] = Some(krate),
+                sym::core => res.0[1] = Some(krate),
+                sym::core_arch => res.0[2] = Some(krate),
+                sym::proc_macro => res.0[3] = Some(krate),
+                sym::std => res.0[4] = Some(krate),
+                sym::std_detect => res.0[5] = Some(krate),
+                _ => {},
+            }
+        }
+        res
+    }
+
+    fn contains(&self, krate: CrateNum) -> bool {
+        self.0.contains(&Some(krate))
+    }
+}
+
 pub struct IncompatibleMsrv {
     msrv: Msrv,
     availability_cache: FxHashMap<(DefId, bool), Availability>,
     check_in_tests: bool,
-    core_crate: Option<CrateNum>,
+    std_crates: StdCrates,
 
     // The most recently called path. Used to skip checking the path after it's
     // been checked when visiting the call expression.
@@ -96,11 +121,7 @@ impl IncompatibleMsrv {
             msrv: conf.msrv,
             availability_cache: FxHashMap::default(),
             check_in_tests: conf.check_incompatible_msrv_in_tests,
-            core_crate: tcx
-                .crates(())
-                .iter()
-                .find(|krate| tcx.crate_name(**krate) == sym::core)
-                .copied(),
+            std_crates: StdCrates::new(tcx),
             called_path: None,
         }
     }
@@ -152,21 +173,24 @@ impl IncompatibleMsrv {
         node: HirId,
         span: Span,
     ) {
-        if def_id.is_local() {
-            // We don't check local items since their MSRV is supposed to always be valid.
+        if !self.std_crates.contains(def_id.krate) {
+            // No stability attributes to lookup for these items.
             return;
         }
-        let expn_data = span.ctxt().outer_expn_data();
-        if let ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) = expn_data.kind {
-            // Desugared expressions get to cheat and stability is ignored.
-            // Intentionally not using `.from_expansion()`, since we do still care about macro expansions
-            return;
-        }
-        // Functions coming from `core` while expanding a macro such as `assert*!()` get to cheat too: the
-        // macros may have existed prior to the checked MSRV, but their expansion with a recent compiler
-        // might use recent functions or methods. Compiling with an older compiler would not use those.
-        if Some(def_id.krate) == self.core_crate && expn_data.macro_def_id.map(|did| did.krate) == self.core_crate {
-            return;
+        // Use `from_expansion` to fast-path the common case.
+        if span.from_expansion() {
+            let expn = span.ctxt().outer_expn_data();
+            match expn.kind {
+                // FIXME(@Jarcho): Check that the actual desugaring or std macro is supported by the
+                // current MSRV. Note that nested expansions need to be handled as well.
+                ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) => return,
+                ExpnKind::Macro(..) if expn.macro_def_id.is_some_and(|did| self.std_crates.contains(did.krate)) => {
+                    return;
+                },
+                // All other expansions share the target's MSRV.
+                // FIXME(@Jarcho): What should we do about version dependant macros from external crates?
+                _ => {},
+            }
         }
 
         if (self.check_in_tests || !is_in_test(cx.tcx, node))

clippy_lints/src/let_if_seq.rs

@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_hir_and_then;
 use clippy_utils::res::MaybeResPath;
-use clippy_utils::source::snippet;
+use clippy_utils::source::snippet_with_context;
 use clippy_utils::visitors::is_local_used;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
@@ -59,80 +59,88 @@ declare_lint_pass!(LetIfSeq => [USELESS_LET_IF_SEQ]);
 impl<'tcx> LateLintPass<'tcx> for LetIfSeq {
     fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) {
         for [stmt, next] in block.stmts.array_windows::<2>() {
-            if let hir::StmtKind::Let(local) = stmt.kind
-                && let hir::PatKind::Binding(mode, canonical_id, ident, None) = local.pat.kind
-                && let hir::StmtKind::Expr(if_) = next.kind
-                && let hir::ExprKind::If(cond, then, else_) = if_.kind
-                && !is_local_used(cx, cond, canonical_id)
-                && let hir::ExprKind::Block(then, _) = then.kind
-                && let Some(value) = check_assign(cx, canonical_id, then)
-                && !is_local_used(cx, value, canonical_id)
-            {
-                let span = stmt.span.to(if_.span);
+            if let hir::StmtKind::Expr(if_) = next.kind {
+                check_block_inner(cx, stmt, if_);
+            }
+        }
 
-                let has_interior_mutability = !cx
-                    .typeck_results()
-                    .node_type(canonical_id)
-                    .is_freeze(cx.tcx, cx.typing_env());
-                if has_interior_mutability {
-                    return;
-                }
+        if let Some(expr) = block.expr
+            && let Some(stmt) = block.stmts.last()
+        {
+            check_block_inner(cx, stmt, expr);
+        }
+    }
+}
+
+fn check_block_inner<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx hir::Stmt<'tcx>, if_: &'tcx hir::Expr<'tcx>) {
+    if let hir::StmtKind::Let(local) = stmt.kind
+        && let hir::PatKind::Binding(mode, canonical_id, ident, None) = local.pat.kind
+        && let hir::ExprKind::If(cond, then, else_) = if_.kind
+        && !is_local_used(cx, cond, canonical_id)
+        && let hir::ExprKind::Block(then, _) = then.kind
+        && let Some(value) = check_assign(cx, canonical_id, then)
+        && !is_local_used(cx, value, canonical_id)
+    {
+        let span = stmt.span.to(if_.span);
 
-                let (default_multi_stmts, default) = if let Some(else_) = else_ {
-                    if let hir::ExprKind::Block(else_, _) = else_.kind {
-                        if let Some(default) = check_assign(cx, canonical_id, else_) {
-                            (else_.stmts.len() > 1, default)
-                        } else if let Some(default) = local.init {
-                            (true, default)
-                        } else {
-                            continue;
-                        }
-                    } else {
-                        continue;
-                    }
+        let has_interior_mutability = !cx
+            .typeck_results()
+            .node_type(canonical_id)
+            .is_freeze(cx.tcx, cx.typing_env());
+        if has_interior_mutability {
+            return;
+        }
+
+        let (default_multi_stmts, default) = if let Some(else_) = else_ {
+            if let hir::ExprKind::Block(else_, _) = else_.kind {
+                if let Some(default) = check_assign(cx, canonical_id, else_) {
+                    (else_.stmts.len() > 1, default)
                 } else if let Some(default) = local.init {
-                    (false, default)
+                    (true, default)
                 } else {
-                    continue;
-                };
+                    return;
+                }
+            } else {
+                return;
+            }
+        } else if let Some(default) = local.init {
+            (false, default)
+        } else {
+            return;
+        };
 
-                let mutability = match mode {
-                    BindingMode(_, Mutability::Mut) => "<mut> ",
-                    _ => "",
-                };
+        let mutability = match mode {
+            BindingMode(_, Mutability::Mut) => "<mut> ",
+            _ => "",
+        };
 
-                // FIXME: this should not suggest `mut` if we can detect that the variable is not
-                // use mutably after the `if`
+        // FIXME: this should not suggest `mut` if we can detect that the variable is not
+        // use mutably after the `if`
 
-                let sug = format!(
-                    "let {mutability}{name} = if {cond} {{{then} {value} }} else {{{else} {default} }};",
-                    name=ident.name,
-                    cond=snippet(cx, cond.span, "_"),
-                    then=if then.stmts.len() > 1 { " ..;" } else { "" },
-                    else=if default_multi_stmts { " ..;" } else { "" },
-                    value=snippet(cx, value.span, "<value>"),
-                    default=snippet(cx, default.span, "<default>"),
-                );
-                span_lint_hir_and_then(
-                    cx,
-                    USELESS_LET_IF_SEQ,
-                    local.hir_id,
-                    span,
-                    "`if _ { .. } else { .. }` is an expression",
-                    |diag| {
-                        diag.span_suggestion(
-                            span,
-                            "it is more idiomatic to write",
-                            sug,
-                            Applicability::HasPlaceholders,
-                        );
-                        if !mutability.is_empty() {
-                            diag.note("you might not need `mut` at all");
-                        }
-                    },
-                );
-            }
-        }
+        let mut applicability = Applicability::HasPlaceholders;
+        let (cond_snip, _) = snippet_with_context(cx, cond.span, if_.span.ctxt(), "_", &mut applicability);
+        let (value_snip, _) = snippet_with_context(cx, value.span, if_.span.ctxt(), "<value>", &mut applicability);
+        let (default_snip, _) =
+            snippet_with_context(cx, default.span, if_.span.ctxt(), "<default>", &mut applicability);
+        let sug = format!(
+            "let {mutability}{name} = if {cond_snip} {{{then} {value_snip} }} else {{{else} {default_snip} }};",
+            name=ident.name,
+            then=if then.stmts.len() > 1 { " ..;" } else { "" },
+            else=if default_multi_stmts { " ..;" } else { "" },
+        );
+        span_lint_hir_and_then(
+            cx,
+            USELESS_LET_IF_SEQ,
+            local.hir_id,
+            span,
+            "`if _ { .. } else { .. }` is an expression",
+            |diag| {
+                diag.span_suggestion(span, "it is more idiomatic to write", sug, applicability);
+                if !mutability.is_empty() {
+                    diag.note("you might not need `mut` at all");
+                }
+            },
+        );
     }
 }