about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSamuel Tardieu <sam@rfc1149.net>2025-09-08 08:29:52 +0000
committerGitHub <noreply@github.com>2025-09-08 08:29:52 +0000
commit368b2355797b13511776f0a35ec4864468b87ece (patch)
tree8a7b85c36600c2cddfbd5ca1fb40f3ad3e066df2
parent847ca731dff5fb7ee00621a1b04c8efb203585bc (diff)
parent2c27b586183779d2437ac758055bb21011c6551c (diff)
downloadrust-368b2355797b13511776f0a35ec4864468b87ece.tar.gz
rust-368b2355797b13511776f0a35ec4864468b87ece.zip
move `toplevel_ref_args` out of `misc` (#15627)
For https://github.com/rust-lang/rust-clippy/issues/6680

I wanted to move the lint into `functions` because it has a `check_fn`
part, but it also has a `check_stmt`, which I wouldn't know what to do
with.

changelog: none
-rw-r--r--clippy_lints/src/declared_lints.rs2
-rw-r--r--clippy_lints/src/lib.rs2
-rw-r--r--clippy_lints/src/misc.rs118
-rw-r--r--clippy_lints/src/toplevel_ref_arg.rs119
4 files changed, 125 insertions, 116 deletions
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index d0c7443a4a4..e610e655f15 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -503,7 +503,6 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::min_ident_chars::MIN_IDENT_CHARS_INFO,
     crate::minmax::MIN_MAX_INFO,
     crate::misc::SHORT_CIRCUIT_STATEMENT_INFO,
-    crate::misc::TOPLEVEL_REF_ARG_INFO,
     crate::misc::USED_UNDERSCORE_BINDING_INFO,
     crate::misc::USED_UNDERSCORE_ITEMS_INFO,
     crate::misc_early::BUILTIN_TYPE_SHADOW_INFO,
@@ -706,6 +705,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::tests_outside_test_module::TESTS_OUTSIDE_TEST_MODULE_INFO,
     crate::to_digit_is_some::TO_DIGIT_IS_SOME_INFO,
     crate::to_string_trait_impl::TO_STRING_TRAIT_IMPL_INFO,
+    crate::toplevel_ref_arg::TOPLEVEL_REF_ARG_INFO,
     crate::trailing_empty_array::TRAILING_EMPTY_ARRAY_INFO,
     crate::trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS_INFO,
     crate::trait_bounds::TYPE_REPETITION_IN_BOUNDS_INFO,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 397dae58e5c..01e296ea33c 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -360,6 +360,7 @@ mod temporary_assignment;
 mod tests_outside_test_module;
 mod to_digit_is_some;
 mod to_string_trait_impl;
+mod toplevel_ref_arg;
 mod trailing_empty_array;
 mod trait_bounds;
 mod transmute;
@@ -831,5 +832,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
     store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf)));
     store.register_late_pass(|_| Box::new(infallible_try_from::InfallibleTryFrom));
     store.register_late_pass(|_| Box::new(coerce_container_to_any::CoerceContainerToAny));
+    store.register_late_pass(|_| Box::new(toplevel_ref_arg::ToplevelRefArg));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs
index 09ee6f7037c..19e9910dfe9 100644
--- a/clippy_lints/src/misc.rs
+++ b/clippy_lints/src/misc.rs
@@ -1,57 +1,11 @@
-use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir, span_lint_hir_and_then};
-use clippy_utils::source::{snippet, snippet_with_context};
+use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
 use clippy_utils::sugg::Sugg;
-use clippy_utils::{
-    SpanlessEq, fulfill_or_allowed, get_parent_expr, in_automatically_derived, is_lint_allowed, iter_input_pats,
-    last_path_segment,
-};
+use clippy_utils::{SpanlessEq, fulfill_or_allowed, get_parent_expr, in_automatically_derived, last_path_segment};
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
-use rustc_hir::intravisit::FnKind;
-use rustc_hir::{
-    BinOpKind, BindingMode, Body, ByRef, Expr, ExprKind, FnDecl, Mutability, PatKind, QPath, Stmt, StmtKind,
-};
+use rustc_hir::{BinOpKind, Expr, ExprKind, QPath, Stmt, StmtKind};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_session::declare_lint_pass;
-use rustc_span::Span;
-use rustc_span::def_id::LocalDefId;
-
-use crate::ref_patterns::REF_PATTERNS;
-
-declare_clippy_lint! {
-    /// ### What it does
-    /// Checks for function arguments and let bindings denoted as
-    /// `ref`.
-    ///
-    /// ### Why is this bad?
-    /// The `ref` declaration makes the function take an owned
-    /// value, but turns the argument into a reference (which means that the value
-    /// is destroyed when exiting the function). This adds not much value: either
-    /// take a reference type, or take an owned value and create references in the
-    /// body.
-    ///
-    /// For let bindings, `let x = &foo;` is preferred over `let ref x = foo`. The
-    /// type of `x` is more obvious with the former.
-    ///
-    /// ### Known problems
-    /// If the argument is dereferenced within the function,
-    /// removing the `ref` will lead to errors. This can be fixed by removing the
-    /// dereferences, e.g., changing `*x` to `x` within the function.
-    ///
-    /// ### Example
-    /// ```no_run
-    /// fn foo(ref _x: u8) {}
-    /// ```
-    ///
-    /// Use instead:
-    /// ```no_run
-    /// fn foo(_x: &u8) {}
-    /// ```
-    #[clippy::version = "pre 1.29.0"]
-    pub TOPLEVEL_REF_ARG,
-    style,
-    "an entire binding declared as `ref`, in a function argument or a `let` statement"
-}
 
 declare_clippy_lint! {
     /// ### What it does
@@ -140,79 +94,13 @@ declare_clippy_lint! {
 }
 
 declare_lint_pass!(LintPass => [
-    TOPLEVEL_REF_ARG,
     USED_UNDERSCORE_BINDING,
     USED_UNDERSCORE_ITEMS,
     SHORT_CIRCUIT_STATEMENT,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for LintPass {
-    fn check_fn(
-        &mut self,
-        cx: &LateContext<'tcx>,
-        k: FnKind<'tcx>,
-        decl: &'tcx FnDecl<'_>,
-        body: &'tcx Body<'_>,
-        _: Span,
-        _: LocalDefId,
-    ) {
-        if !matches!(k, FnKind::Closure) {
-            for arg in iter_input_pats(decl, body) {
-                if let PatKind::Binding(BindingMode(ByRef::Yes(_), _), ..) = arg.pat.kind
-                    && is_lint_allowed(cx, REF_PATTERNS, arg.pat.hir_id)
-                    && !arg.span.in_external_macro(cx.tcx.sess.source_map())
-                {
-                    span_lint_hir(
-                        cx,
-                        TOPLEVEL_REF_ARG,
-                        arg.hir_id,
-                        arg.pat.span,
-                        "`ref` directly on a function parameter does not prevent taking ownership of the passed argument. \
-                        Consider using a reference type instead",
-                    );
-                }
-            }
-        }
-    }
-
     fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
-        if let StmtKind::Let(local) = stmt.kind
-            && let PatKind::Binding(BindingMode(ByRef::Yes(mutabl), _), .., name, None) = local.pat.kind
-            && let Some(init) = local.init
-            // Do not emit if clippy::ref_patterns is not allowed to avoid having two lints for the same issue.
-            && is_lint_allowed(cx, REF_PATTERNS, local.pat.hir_id)
-            && !stmt.span.in_external_macro(cx.tcx.sess.source_map())
-        {
-            let ctxt = local.span.ctxt();
-            let mut app = Applicability::MachineApplicable;
-            let sugg_init = Sugg::hir_with_context(cx, init, ctxt, "..", &mut app);
-            let (mutopt, initref) = if mutabl == Mutability::Mut {
-                ("mut ", sugg_init.mut_addr())
-            } else {
-                ("", sugg_init.addr())
-            };
-            let tyopt = if let Some(ty) = local.ty {
-                let ty_snip = snippet_with_context(cx, ty.span, ctxt, "_", &mut app).0;
-                format!(": &{mutopt}{ty_snip}")
-            } else {
-                String::new()
-            };
-            span_lint_hir_and_then(
-                cx,
-                TOPLEVEL_REF_ARG,
-                init.hir_id,
-                local.pat.span,
-                "`ref` on an entire `let` pattern is discouraged, take a reference with `&` instead",
-                |diag| {
-                    diag.span_suggestion(
-                        stmt.span,
-                        "try",
-                        format!("let {name}{tyopt} = {initref};", name = snippet(cx, name.span, ".."),),
-                        app,
-                    );
-                },
-            );
-        }
         if let StmtKind::Semi(expr) = stmt.kind
             && let ExprKind::Binary(binop, a, b) = &expr.kind
             && matches!(binop.node, BinOpKind::And | BinOpKind::Or)
diff --git a/clippy_lints/src/toplevel_ref_arg.rs b/clippy_lints/src/toplevel_ref_arg.rs
new file mode 100644
index 00000000000..074b79263d3
--- /dev/null
+++ b/clippy_lints/src/toplevel_ref_arg.rs
@@ -0,0 +1,119 @@
+use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then};
+use clippy_utils::source::{snippet, snippet_with_context};
+use clippy_utils::sugg::Sugg;
+use clippy_utils::{is_lint_allowed, iter_input_pats};
+use rustc_errors::Applicability;
+use rustc_hir::intravisit::FnKind;
+use rustc_hir::{BindingMode, Body, ByRef, FnDecl, Mutability, PatKind, Stmt, StmtKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::declare_lint_pass;
+use rustc_span::Span;
+use rustc_span::def_id::LocalDefId;
+
+use crate::ref_patterns::REF_PATTERNS;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for function arguments and let bindings denoted as
+    /// `ref`.
+    ///
+    /// ### Why is this bad?
+    /// The `ref` declaration makes the function take an owned
+    /// value, but turns the argument into a reference (which means that the value
+    /// is destroyed when exiting the function). This adds not much value: either
+    /// take a reference type, or take an owned value and create references in the
+    /// body.
+    ///
+    /// For let bindings, `let x = &foo;` is preferred over `let ref x = foo`. The
+    /// type of `x` is more obvious with the former.
+    ///
+    /// ### Known problems
+    /// If the argument is dereferenced within the function,
+    /// removing the `ref` will lead to errors. This can be fixed by removing the
+    /// dereferences, e.g., changing `*x` to `x` within the function.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// fn foo(ref _x: u8) {}
+    /// ```
+    ///
+    /// Use instead:
+    /// ```no_run
+    /// fn foo(_x: &u8) {}
+    /// ```
+    #[clippy::version = "pre 1.29.0"]
+    pub TOPLEVEL_REF_ARG,
+    style,
+    "an entire binding declared as `ref`, in a function argument or a `let` statement"
+}
+
+declare_lint_pass!(ToplevelRefArg => [TOPLEVEL_REF_ARG]);
+
+impl<'tcx> LateLintPass<'tcx> for ToplevelRefArg {
+    fn check_fn(
+        &mut self,
+        cx: &LateContext<'tcx>,
+        k: FnKind<'tcx>,
+        decl: &'tcx FnDecl<'_>,
+        body: &'tcx Body<'_>,
+        _: Span,
+        _: LocalDefId,
+    ) {
+        if !matches!(k, FnKind::Closure) {
+            for arg in iter_input_pats(decl, body) {
+                if let PatKind::Binding(BindingMode(ByRef::Yes(_), _), ..) = arg.pat.kind
+                    && is_lint_allowed(cx, REF_PATTERNS, arg.pat.hir_id)
+                    && !arg.span.in_external_macro(cx.tcx.sess.source_map())
+                {
+                    span_lint_hir(
+                        cx,
+                        TOPLEVEL_REF_ARG,
+                        arg.hir_id,
+                        arg.pat.span,
+                        "`ref` directly on a function parameter does not prevent taking ownership of the passed argument. \
+                            Consider using a reference type instead",
+                    );
+                }
+            }
+        }
+    }
+
+    fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
+        if let StmtKind::Let(local) = stmt.kind
+            && let PatKind::Binding(BindingMode(ByRef::Yes(mutabl), _), .., name, None) = local.pat.kind
+            && let Some(init) = local.init
+            // Do not emit if clippy::ref_patterns is not allowed to avoid having two lints for the same issue.
+            && is_lint_allowed(cx, REF_PATTERNS, local.pat.hir_id)
+            && !stmt.span.in_external_macro(cx.tcx.sess.source_map())
+        {
+            let ctxt = local.span.ctxt();
+            let mut app = Applicability::MachineApplicable;
+            let sugg_init = Sugg::hir_with_context(cx, init, ctxt, "..", &mut app);
+            let (mutopt, initref) = match mutabl {
+                Mutability::Mut => ("mut ", sugg_init.mut_addr()),
+                Mutability::Not => ("", sugg_init.addr()),
+            };
+            let tyopt = if let Some(ty) = local.ty {
+                let ty_snip = snippet_with_context(cx, ty.span, ctxt, "_", &mut app).0;
+                format!(": &{mutopt}{ty_snip}")
+            } else {
+                String::new()
+            };
+            span_lint_hir_and_then(
+                cx,
+                TOPLEVEL_REF_ARG,
+                init.hir_id,
+                local.pat.span,
+                "`ref` on an entire `let` pattern is discouraged, take a reference with `&` instead",
+                |diag| {
+                    diag.span_suggestion(
+                        stmt.span,
+                        "try",
+                        format!("let {name}{tyopt} = {initref};", name = snippet(cx, name.span, ".."),),
+                        app,
+                    );
+                },
+            );
+        }
+    }
+}