From 82f0dc95a00bfc1adb395f30f8d9c242fc53db37 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 14 Jun 2024 14:37:11 +0200 Subject: [PATCH] [`missing_const_for_fn`]: add machine-applicable suggestion --- clippy_lints/src/missing_const_for_fn.rs | 24 ++- .../missing_const_for_fn/could_be_const.fixed | 173 ++++++++++++++++++ .../could_be_const.stderr | 84 +++++++++ 3 files changed, 275 insertions(+), 6 deletions(-) create mode 100644 tests/ui/missing_const_for_fn/could_be_const.fixed diff --git a/clippy_lints/src/missing_const_for_fn.rs b/clippy_lints/src/missing_const_for_fn.rs index 4592324809fa..bb0d714a31fd 100644 --- a/clippy_lints/src/missing_const_for_fn.rs +++ b/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 @@ impl<'tcx> LateLintPass<'tcx> for MissingConstForFn { } }, 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 @@ impl<'tcx> LateLintPass<'tcx> for MissingConstForFn { let mir = cx.tcx.optimized_mir(def_id); - 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`"); + if let Ok(()) = is_min_const_fn(cx.tcx, mir, &self.msrv) + && 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); } diff --git a/tests/ui/missing_const_for_fn/could_be_const.fixed b/tests/ui/missing_const_for_fn/could_be_const.fixed new file mode 100644 index 000000000000..921dcf0b1626 --- /dev/null +++ b/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 { + //~^ ERROR: this could be a `const fn` + t +} + +fn sub(x: u32) -> usize { + unsafe { transmute(&x) } +} + +const fn generic_arr(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, + } + + impl Wrapper { + #[must_use] + pub const fn new(strings: Vec) -> Self { + Self { strings } + } + + #[must_use] + pub const fn empty() -> Self { + Self { strings: Vec::new() } + } + } + + pub struct Other { + pub text: String, + pub vec: Vec, + } + + impl Other { + pub const fn new(text: String) -> Self { + let vec = Vec::new(); + Self { text, vec } + } + } +} diff --git a/tests/ui/missing_const_for_fn/could_be_const.stderr b/tests/ui/missing_const_for_fn/could_be_const.stderr index 1c61c3e87132..8999af761e31 100644 --- a/tests/ui/missing_const_for_fn/could_be_const.stderr +++ b/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 { + | +++++ 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: [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) -> Self { LL | | Self { strings } LL | | } | |_________^ + | +help: make the function `const` + | +LL | pub const fn new(strings: Vec) -> 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