Auto merge of #12930 - DaniPopes:missing-const-for-fn-suggestion, r=Jarcho

[`missing_const_for_fn`]: add machine-applicable suggestion

Add a machine-applicable suggestion to the `missing_const_for_fn` lint.

changelog: [`missing_const_for_fn`]: add machine-applicable suggestion

clippy_lints/src/missing_const_for_fn.rs

@@ -1,11 +1,11 @@
 use clippy_config::msrvs::{self, Msrv};
-use clippy_utils::diagnostics::span_lint;
+use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::qualify_min_const_fn::is_min_const_fn;
 use clippy_utils::{fn_has_unsatisfiable_preds, is_entrypoint_fn, is_from_proc_macro, trait_ref_of_method};
-use rustc_hir as hir;
+use rustc_errors::Applicability;
 use rustc_hir::def_id::CRATE_DEF_ID;
 use rustc_hir::intravisit::FnKind;
-use rustc_hir::{Body, Constness, FnDecl, GenericParamKind};
+use rustc_hir::{self as hir, Body, Constness, FnDecl, GenericParamKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::lint::in_external_macro;
 use rustc_session::impl_lint_pass;
@@ -120,7 +120,7 @@ fn check_fn(
                 }
             },
             FnKind::Method(_, sig, ..) => {
-                if trait_ref_of_method(cx, def_id).is_some() || already_const(sig.header) {
+                if already_const(sig.header) || trait_ref_of_method(cx, def_id).is_some() {
                     return;
                 }
             },
@@ -147,10 +147,22 @@ fn check_fn(
 
         let mir = cx.tcx.optimized_mir(def_id);
 
-        if let Ok(()) = is_min_const_fn(cx.tcx, mir, &self.msrv) {
+        if let Ok(()) = is_min_const_fn(cx.tcx, mir, &self.msrv)
-            span_lint(cx, MISSING_CONST_FOR_FN, span, "this could be a `const fn`");
+            && let hir::Node::Item(hir::Item { vis_span, .. }) | hir::Node::ImplItem(hir::ImplItem { vis_span, .. }) =
+                cx.tcx.hir_node_by_def_id(def_id)
+        {
+            let suggestion = if vis_span.is_empty() { "const " } else { " const" };
+            span_lint_and_then(cx, MISSING_CONST_FOR_FN, span, "this could be a `const fn`", |diag| {
+                diag.span_suggestion_verbose(
+                    vis_span.shrink_to_hi(),
+                    "make the function `const`",
+                    suggestion,
+                    Applicability::MachineApplicable,
+                );
+            });
         }
     }
+
     extract_msrv_attr!(LateContext);
 }
 

tests/ui/missing_const_for_fn/could_be_const.fixed

@@ -0,0 +1,173 @@
+#![warn(clippy::missing_const_for_fn)]
+#![allow(incomplete_features, clippy::let_and_return, clippy::missing_transmute_annotations)]
+#![feature(const_mut_refs)]
+#![feature(const_trait_impl)]
+
+use std::mem::transmute;
+
+struct Game {
+    guess: i32,
+}
+
+impl Game {
+    // Could be const
+    pub const fn new() -> Self {
+        //~^ ERROR: this could be a `const fn`
+        //~| NOTE: `-D clippy::missing-const-for-fn` implied by `-D warnings`
+        Self { guess: 42 }
+    }
+
+    const fn const_generic_params<'a, T, const N: usize>(&self, b: &'a [T; N]) -> &'a [T; N] {
+        //~^ ERROR: this could be a `const fn`
+        b
+    }
+}
+
+// Could be const
+const fn one() -> i32 {
+    //~^ ERROR: this could be a `const fn`
+    1
+}
+
+// Could also be const
+const fn two() -> i32 {
+    //~^ ERROR: this could be a `const fn`
+    let abc = 2;
+    abc
+}
+
+// Could be const (since Rust 1.39)
+const fn string() -> String {
+    //~^ ERROR: this could be a `const fn`
+    String::new()
+}
+
+// Could be const
+const unsafe fn four() -> i32 {
+    //~^ ERROR: this could be a `const fn`
+    4
+}
+
+// Could also be const
+const fn generic<T>(t: T) -> T {
+    //~^ ERROR: this could be a `const fn`
+    t
+}
+
+fn sub(x: u32) -> usize {
+    unsafe { transmute(&x) }
+}
+
+const fn generic_arr<T: Copy>(t: [T; 1]) -> T {
+    //~^ ERROR: this could be a `const fn`
+    t[0]
+}
+
+mod with_drop {
+    pub struct A;
+    pub struct B;
+    impl Drop for A {
+        fn drop(&mut self) {}
+    }
+
+    impl B {
+        // This can be const, because `a` is passed by reference
+        pub const fn b(self, a: &A) -> B {
+            //~^ ERROR: this could be a `const fn`
+            B
+        }
+    }
+}
+
+#[clippy::msrv = "1.47.0"]
+mod const_fn_stabilized_before_msrv {
+    // This could be const because `u8::is_ascii_digit` is a stable const function in 1.47.
+    const fn const_fn_stabilized_before_msrv(byte: u8) {
+        //~^ ERROR: this could be a `const fn`
+        byte.is_ascii_digit();
+    }
+}
+
+#[clippy::msrv = "1.45"]
+fn msrv_1_45() -> i32 {
+    45
+}
+
+#[clippy::msrv = "1.46"]
+const fn msrv_1_46() -> i32 {
+    //~^ ERROR: this could be a `const fn`
+    46
+}
+
+// Should not be const
+fn main() {}
+
+struct D;
+
+impl const Drop for D {
+    fn drop(&mut self) {
+        todo!();
+    }
+}
+
+// Lint this, since it can be dropped in const contexts
+// FIXME(effects)
+fn d(this: D) {}
+
+mod msrv {
+    struct Foo(*const u8, &'static u8);
+
+    impl Foo {
+        #[clippy::msrv = "1.58"]
+        const fn deref_ptr_can_be_const(self) -> usize {
+            //~^ ERROR: this could be a `const fn`
+            unsafe { *self.0 as usize }
+        }
+
+        const fn deref_copied_val(self) -> usize {
+            //~^ ERROR: this could be a `const fn`
+            *self.1 as usize
+        }
+    }
+
+    union Bar {
+        val: u8,
+    }
+
+    #[clippy::msrv = "1.56"]
+    const fn union_access_can_be_const() {
+        //~^ ERROR: this could be a `const fn`
+        let bar = Bar { val: 1 };
+        let _ = unsafe { bar.val };
+    }
+}
+
+mod issue12677 {
+    pub struct Wrapper {
+        pub strings: Vec<String>,
+    }
+
+    impl Wrapper {
+        #[must_use]
+        pub const fn new(strings: Vec<String>) -> Self {
+            Self { strings }
+        }
+
+        #[must_use]
+        pub const fn empty() -> Self {
+            Self { strings: Vec::new() }
+        }
+    }
+
+    pub struct Other {
+        pub text: String,
+        pub vec: Vec<String>,
+    }
+
+    impl Other {
+        pub const fn new(text: String) -> Self {
+            let vec = Vec::new();
+            Self { text, vec }
+        }
+    }
+}

tests/ui/missing_const_for_fn/could_be_const.stderr

@@ -10,6 +10,10 @@ LL | |     }
    |
    = note: `-D clippy::missing-const-for-fn` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::missing_const_for_fn)]`
+help: make the function `const`
+   |
+LL |     pub const fn new() -> Self {
+   |         +++++
 
 error: this could be a `const fn`
   --> tests/ui/missing_const_for_fn/could_be_const.rs:20:5
@@ -19,6 +23,11 @@ LL | |
 LL | |         b
 LL | |     }
    | |_____^
+   |
+help: make the function `const`
+   |
+LL |     const fn const_generic_params<'a, T, const N: usize>(&self, b: &'a [T; N]) -> &'a [T; N] {
+   |     +++++
 
 error: this could be a `const fn`
   --> tests/ui/missing_const_for_fn/could_be_const.rs:27:1
@@ -28,6 +37,11 @@ LL | |
 LL | |     1
 LL | | }
    | |_^
+   |
+help: make the function `const`
+   |
+LL | const fn one() -> i32 {
+   | +++++
 
 error: this could be a `const fn`
   --> tests/ui/missing_const_for_fn/could_be_const.rs:33:1
@@ -38,6 +52,11 @@ LL | |     let abc = 2;
 LL | |     abc
 LL | | }
    | |_^
+   |
+help: make the function `const`
+   |
+LL | const fn two() -> i32 {
+   | +++++
 
 error: this could be a `const fn`
   --> tests/ui/missing_const_for_fn/could_be_const.rs:40:1
@@ -47,6 +66,11 @@ LL | |
 LL | |     String::new()
 LL | | }
    | |_^
+   |
+help: make the function `const`
+   |
+LL | const fn string() -> String {
+   | +++++
 
 error: this could be a `const fn`
   --> tests/ui/missing_const_for_fn/could_be_const.rs:46:1
@@ -56,6 +80,11 @@ LL | |
 LL | |     4
 LL | | }
    | |_^
+   |
+help: make the function `const`
+   |
+LL | const unsafe fn four() -> i32 {
+   | +++++
 
 error: this could be a `const fn`
   --> tests/ui/missing_const_for_fn/could_be_const.rs:52:1
@@ -65,6 +94,11 @@ LL | |
 LL | |     t
 LL | | }
    | |_^
+   |
+help: make the function `const`
+   |
+LL | const fn generic<T>(t: T) -> T {
+   | +++++
 
 error: this could be a `const fn`
   --> tests/ui/missing_const_for_fn/could_be_const.rs:61:1
@@ -74,6 +108,11 @@ LL | |
 LL | |     t[0]
 LL | | }
    | |_^
+   |
+help: make the function `const`
+   |
+LL | const fn generic_arr<T: Copy>(t: [T; 1]) -> T {
+   | +++++
 
 error: this could be a `const fn`
   --> tests/ui/missing_const_for_fn/could_be_const.rs:75:9
@@ -83,6 +122,11 @@ LL | |
 LL | |             B
 LL | |         }
    | |_________^
+   |
+help: make the function `const`
+   |
+LL |         pub const fn b(self, a: &A) -> B {
+   |             +++++
 
 error: this could be a `const fn`
   --> tests/ui/missing_const_for_fn/could_be_const.rs:85:5
@@ -92,6 +136,11 @@ LL | |
 LL | |         byte.is_ascii_digit();
 LL | |     }
    | |_____^
+   |
+help: make the function `const`
+   |
+LL |     const fn const_fn_stabilized_before_msrv(byte: u8) {
+   |     +++++
 
 error: this could be a `const fn`
   --> tests/ui/missing_const_for_fn/could_be_const.rs:97:1
@@ -101,6 +150,11 @@ LL | |
 LL | |     46
 LL | | }
    | |_^
+   |
+help: make the function `const`
+   |
+LL | const fn msrv_1_46() -> i32 {
+   | +++++
 
 error: this could be a `const fn`
   --> tests/ui/missing_const_for_fn/could_be_const.rs:122:9
@@ -110,6 +164,11 @@ LL | |
 LL | |             unsafe { *self.0 as usize }
 LL | |         }
    | |_________^
+   |
+help: make the function `const`
+   |
+LL |         const fn deref_ptr_can_be_const(self) -> usize {
+   |         +++++
 
 error: this could be a `const fn`
   --> tests/ui/missing_const_for_fn/could_be_const.rs:127:9
@@ -119,6 +178,11 @@ LL | |
 LL | |             *self.1 as usize
 LL | |         }
    | |_________^
+   |
+help: make the function `const`
+   |
+LL |         const fn deref_copied_val(self) -> usize {
+   |         +++++
 
 error: this could be a `const fn`
   --> tests/ui/missing_const_for_fn/could_be_const.rs:138:5
@@ -129,6 +193,11 @@ LL | |         let bar = Bar { val: 1 };
 LL | |         let _ = unsafe { bar.val };
 LL | |     }
    | |_____^
+   |
+help: make the function `const`
+   |
+LL |     const fn union_access_can_be_const() {
+   |     +++++
 
 error: this could be a `const fn`
   --> tests/ui/missing_const_for_fn/could_be_const.rs:152:9
@@ -137,6 +206,11 @@ LL | /         pub fn new(strings: Vec<String>) -> Self {
 LL | |             Self { strings }
 LL | |         }
    | |_________^
+   |
+help: make the function `const`
+   |
+LL |         pub const fn new(strings: Vec<String>) -> Self {
+   |             +++++
 
 error: this could be a `const fn`
   --> tests/ui/missing_const_for_fn/could_be_const.rs:157:9
@@ -145,6 +219,11 @@ LL | /         pub fn empty() -> Self {
 LL | |             Self { strings: Vec::new() }
 LL | |         }
    | |_________^
+   |
+help: make the function `const`
+   |
+LL |         pub const fn empty() -> Self {
+   |             +++++
 
 error: this could be a `const fn`
   --> tests/ui/missing_const_for_fn/could_be_const.rs:168:9
@@ -154,6 +233,11 @@ LL | |             let vec = Vec::new();
 LL | |             Self { text, vec }
 LL | |         }
    | |_________^
+   |
+help: make the function `const`
+   |
+LL |         pub const fn new(text: String) -> Self {
+   |             +++++
 
 error: aborting due to 17 previous errors