Skip to content

Commit

Permalink
Auto merge of #10951 - Centri3:single_call_fn, r=giraffate
Browse files Browse the repository at this point in the history
new lint [`single_call_fn`]

Closes #10861

changelog: New lint [`single_call_fn`]
  • Loading branch information
bors committed Jun 19, 2023
2 parents 8c8ff5f + 2cd4a91 commit 06b444b
Show file tree
Hide file tree
Showing 8 changed files with 274 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}

Expand Down
133 changes: 133 additions & 0 deletions clippy_lints/src/single_call_fn.rs
Original file line number Diff line number Diff line change
@@ -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);
}
}
2 changes: 1 addition & 1 deletion clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
75 changes: 75 additions & 0 deletions tests/ui/single_call_fn.rs
Original file line number Diff line number Diff line change
@@ -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();
}
55 changes: 55 additions & 0 deletions tests/ui/single_call_fn.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 06b444b

Please sign in to comment.