From 07dbcbda12cacb924c2d95719ad598ee9de8738d Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Wed, 14 Jun 2023 06:48:34 -0500 Subject: [PATCH] new lint `single_call_fn` --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 7 ++ clippy_lints/src/single_call_fn.rs | 132 +++++++++++++++++++++++++++++ tests/ui/single_call_fn.rs | 49 +++++++++++ tests/ui/single_call_fn.stderr | 31 +++++++ 6 files changed, 221 insertions(+) create mode 100644 clippy_lints/src/single_call_fn.rs create mode 100644 tests/ui/single_call_fn.rs create mode 100644 tests/ui/single_call_fn.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 3eaa0d199fd..6fb7ac740b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5158,6 +5158,7 @@ Released 2018-09-13 [`significant_drop_in_scrutinee`]: https://rust-lang.github.io/rust-clippy/master/index.html#significant_drop_in_scrutinee [`significant_drop_tightening`]: https://rust-lang.github.io/rust-clippy/master/index.html#significant_drop_tightening [`similar_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#similar_names +[`single_call_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_call_fn [`single_char_add_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_add_str [`single_char_lifetime_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_lifetime_names [`single_char_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 7690e8f7247..9c1e1f6702d 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -570,6 +570,7 @@ crate::shadow::SHADOW_SAME_INFO, crate::shadow::SHADOW_UNRELATED_INFO, crate::significant_drop_tightening::SIGNIFICANT_DROP_TIGHTENING_INFO, + crate::single_call_fn::SINGLE_CALL_FN_INFO, crate::single_char_lifetime_names::SINGLE_CHAR_LIFETIME_NAMES_INFO, crate::single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS_INFO, crate::single_range_in_vec_init::SINGLE_RANGE_IN_VEC_INIT_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 6ea329a9542..939b1136b46 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -287,6 +287,7 @@ mod serde_api; mod shadow; mod significant_drop_tightening; +mod single_call_fn; mod single_char_lifetime_names; mod single_component_path_imports; mod single_range_in_vec_init; @@ -1055,6 +1056,12 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move |_| Box::new(large_stack_frames::LargeStackFrames::new(stack_size_threshold))); store.register_late_pass(|_| Box::new(single_range_in_vec_init::SingleRangeInVecInit)); store.register_late_pass(|_| Box::new(incorrect_impls::IncorrectImpls)); + store.register_late_pass(move |_| { + Box::new(single_call_fn::SingleCallFn { + avoid_breaking_exported_api, + def_id_to_usage: rustc_data_structures::fx::FxHashMap::default(), + }) + }); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/single_call_fn.rs b/clippy_lints/src/single_call_fn.rs new file mode 100644 index 00000000000..d0fbedee832 --- /dev/null +++ b/clippy_lints/src/single_call_fn.rs @@ -0,0 +1,132 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::is_from_proc_macro; +use rustc_data_structures::fx::FxHashMap; +use rustc_hir::def_id::LocalDefId; +use rustc_hir::intravisit::{walk_expr, Visitor}; +use rustc_hir::{intravisit::FnKind, Body, Expr, ExprKind, FnDecl}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::hir::nested_filter::OnlyBodies; +use rustc_middle::lint::in_external_macro; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::Span; + +declare_clippy_lint! { + /// ### What it does + /// Checks for functions that are only used once. + /// + /// ### Why is this bad? + /// It's usually not, splitting a function into multiple parts often improves readability and in + /// the case of generics, can prevent the compiler from duplicating the function dozens of + /// time; instead, only duplicating a thunk. But this can prevent segmentation across a + /// codebase, where many small functions are used only once. + /// + /// Note: If this lint is used, prepare to allow this a lot. + /// + /// ### Example + /// ```rust + /// pub fn a(t: &T) + /// where + /// T: AsRef, + /// { + /// a_inner(t.as_ref()) + /// } + /// + /// fn a_inner(t: &str) { + /// /* snip */ + /// } + /// + /// ``` + /// Use instead: + /// ```rust + /// pub fn a(t: &T) + /// where + /// T: AsRef, + /// { + /// let t = t.as_ref(); + /// /* snip */ + /// } + /// + /// ``` + #[clippy::version = "1.72.0"] + pub SINGLE_CALL_FN, + restriction, + "checks for functions that are only used once" +} +impl_lint_pass!(SingleCallFn => [SINGLE_CALL_FN]); + +#[derive(Clone)] +pub struct SingleCallFn { + pub avoid_breaking_exported_api: bool, + pub def_id_to_usage: FxHashMap)>, +} + +impl<'tcx> LateLintPass<'tcx> for SingleCallFn { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + kind: FnKind<'tcx>, + _: &'tcx FnDecl<'_>, + body: &'tcx Body<'_>, + span: Span, + def_id: LocalDefId, + ) { + if self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(def_id) + || in_external_macro(cx.sess(), span) + || is_from_proc_macro(cx, &(&kind, body, cx.tcx.local_def_id_to_hir_id(def_id), span)) + { + return; + } + + self.def_id_to_usage.insert(def_id, (span, vec![])); + } + + fn check_crate_post(&mut self, cx: &LateContext<'tcx>) { + let mut v = FnUsageVisitor { + cx, + def_id_to_usage: &mut self.def_id_to_usage, + }; + cx.tcx.hir().visit_all_item_likes_in_crate(&mut v); + + for usage in self.def_id_to_usage.values() { + let fn_span = usage.0; + if let [usage] = *usage.1 { + span_lint_and_help( + cx, + SINGLE_CALL_FN, + fn_span, + "this function is only used once", + Some(usage), + "used here", + ); + } + } + } +} + +struct FnUsageVisitor<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + def_id_to_usage: &'a mut FxHashMap)>, +} + +impl<'a, 'tcx> Visitor<'tcx> for FnUsageVisitor<'a, 'tcx> { + type NestedFilter = OnlyBodies; + + fn nested_visit_map(&mut self) -> Self::Map { + self.cx.tcx.hir() + } + + fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { + let Self { cx, .. } = *self; + + if let ExprKind::Path(qpath) = expr.kind + && let res = cx.qpath_res(&qpath, expr.hir_id) + && let Some(call_def_id) = res.opt_def_id() + && let Some(def_id) = call_def_id.as_local() + && let Some(usage) = self.def_id_to_usage.get_mut(&def_id) + { + usage.1.push(expr.span); + } + + walk_expr(self, expr); + } +} diff --git a/tests/ui/single_call_fn.rs b/tests/ui/single_call_fn.rs new file mode 100644 index 00000000000..9806eaf3c42 --- /dev/null +++ b/tests/ui/single_call_fn.rs @@ -0,0 +1,49 @@ +//@aux-build:proc_macros.rs +#![allow(unused)] +#![warn(clippy::single_call_fn)] +#![no_main] + +#[macro_use] +extern crate proc_macros; + +// Do not lint since it's public +pub fn f() {} + +fn g() { + f(); +} + +fn c() { + println!("really"); + println!("long"); + println!("function..."); +} + +fn d() { + c(); +} + +fn a() {} + +fn b() { + a(); + + external! { + fn lol() { + lol_inner(); + } + fn lol_inner() {} + } + with_span! { + span + fn lol2() { + lol2_inner(); + } + fn lol2_inner() {} + } +} + +fn e() { + b(); + b(); +} diff --git a/tests/ui/single_call_fn.stderr b/tests/ui/single_call_fn.stderr new file mode 100644 index 00000000000..decf561eed4 --- /dev/null +++ b/tests/ui/single_call_fn.stderr @@ -0,0 +1,31 @@ +error: this function is only used once + --> $DIR/single_call_fn.rs:26:1 + | +LL | fn a() {} + | ^^^^^^^^^ + | +help: used here + --> $DIR/single_call_fn.rs:29:5 + | +LL | a(); + | ^ + = note: `-D clippy::single-call-fn` implied by `-D warnings` + +error: this function is only used once + --> $DIR/single_call_fn.rs:16:1 + | +LL | / fn c() { +LL | | println!("really"); +LL | | println!("long"); +LL | | println!("function..."); +LL | | } + | |_^ + | +help: used here + --> $DIR/single_call_fn.rs:23:5 + | +LL | c(); + | ^ + +error: aborting due to 2 previous errors +