Auto merge of rust-lang#10951 - Centri3:single_call_fn, r=giraffate

new lint [`single_call_fn`]

Closes rust-lang#10861

changelog: New lint [`single_call_fn`]

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

book/src/lint_configuration.md

@@ -94,6 +94,7 @@ Suppress lints whenever the suggested change would cause breakage for other crat
 * [`linkedlist`](https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist)
 * [`rc_mutex`](https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex)
 * [`unnecessary_box_returns`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_box_returns)
+* [`single_call_fn`](https://rust-lang.github.io/rust-clippy/master/index.html#single_call_fn)
 
 
 ## `msrv`

clippy_lints/src/declared_lints.rs

@@ -570,6 +570,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     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,

clippy_lints/src/lib.rs

@@ -287,6 +287,7 @@ mod semicolon_if_nothing_returned;
 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`
 }
 

clippy_lints/src/single_call_fn.rs

@@ -0,0 +1,133 @@
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::{is_from_proc_macro, is_in_test_function};
+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. Does not lint tests.
+    ///
+    /// ### 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: &T)
+    /// where
+    ///     T: AsRef<str>,
+    /// {
+    ///     a_inner(t.as_ref())
+    /// }
+    ///
+    /// fn a_inner(t: &str) {
+    ///     /* snip */
+    /// }
+    ///
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// pub fn a<T>(t: &T)
+    /// where
+    ///     T: AsRef<str>,
+    /// {
+    ///     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<LocalDefId, (Span, Vec<Span>)>,
+}
+
+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))
+            || is_in_test_function(cx.tcx, body.value.hir_id)
+        {
+            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 single_call_fn_span = usage.0;
+            if let [caller_span] = *usage.1 {
+                span_lint_and_help(
+                    cx,
+                    SINGLE_CALL_FN,
+                    single_call_fn_span,
+                    "this function is only used once",
+                    Some(caller_span),
+                    "used here",
+                );
+            }
+        }
+    }
+}
+
+struct FnUsageVisitor<'a, 'tcx> {
+    cx: &'a LateContext<'tcx>,
+    def_id_to_usage: &'a mut FxHashMap<LocalDefId, (Span, Vec<Span>)>,
+}
+
+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);
+    }
+}

clippy_lints/src/utils/conf.rs

@@ -289,7 +289,7 @@ define_Conf! {
     /// arithmetic-side-effects-allowed-unary = ["SomeType", "AnotherType"]
     /// ```
     (arithmetic_side_effects_allowed_unary: rustc_data_structures::fx::FxHashSet<String> = <_>::default()),
-    /// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UNUSED_SELF, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION, BOX_COLLECTION, REDUNDANT_ALLOCATION, RC_BUFFER, VEC_BOX, OPTION_OPTION, LINKEDLIST, RC_MUTEX, UNNECESSARY_BOX_RETURNS.
+    /// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UNUSED_SELF, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION, BOX_COLLECTION, REDUNDANT_ALLOCATION, RC_BUFFER, VEC_BOX, OPTION_OPTION, LINKEDLIST, RC_MUTEX, UNNECESSARY_BOX_RETURNS, SINGLE_CALL_FN.
     ///
     /// Suppress lints whenever the suggested change would cause breakage for other crates.
     (avoid_breaking_exported_api: bool = true),

tests/ui/single_call_fn.rs

@@ -0,0 +1,75 @@
+//@aux-build:proc_macros.rs
+//@compile-flags: --test
+#![allow(clippy::redundant_closure_call, 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 i() {}
+fn j() {}
+
+fn h() {
+    // Linted
+    let a = i;
+    // Do not lint closures
+    let a = (|| {
+        // Not linted
+        a();
+        // Imo, it's reasonable to lint this as the function is still only being used once. Just in
+        // a closure.
+        j();
+    });
+    a();
+}
+
+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();
+}
+
+#[test]
+fn k() {}
+
+#[test]
+fn l() {
+    k();
+}

tests/ui/single_call_fn.stderr

@@ -0,0 +1,55 @@
+error: this function is only used once
+  --> $DIR/single_call_fn.rs:34:1
+   |
+LL | / fn c() {
+LL | |     println!("really");
+LL | |     println!("long");
+LL | |     println!("function...");
+LL | | }
+   | |_^
+   |
+help: used here
+  --> $DIR/single_call_fn.rs:41:5
+   |
+LL |     c();
+   |     ^
+   = note: `-D clippy::single-call-fn` implied by `-D warnings`
+
+error: this function is only used once
+  --> $DIR/single_call_fn.rs:13:1
+   |
+LL | fn i() {}
+   | ^^^^^^^^^
+   |
+help: used here
+  --> $DIR/single_call_fn.rs:18:13
+   |
+LL |     let a = i;
+   |             ^
+
+error: this function is only used once
+  --> $DIR/single_call_fn.rs:44:1
+   |
+LL | fn a() {}
+   | ^^^^^^^^^
+   |
+help: used here
+  --> $DIR/single_call_fn.rs:47:5
+   |
+LL |     a();
+   |     ^
+
+error: this function is only used once
+  --> $DIR/single_call_fn.rs:14:1
+   |
+LL | fn j() {}
+   | ^^^^^^^^^
+   |
+help: used here
+  --> $DIR/single_call_fn.rs:25:9
+   |
+LL |         j();
+   |         ^
+
+error: aborting due to 4 previous errors
+