Skip to content

Commit 9a56153

Browse files
y21y21
committed
[single_call_fn]: merge post-crate visitor into lint pass
1 parent c469cb0 commit 9a56153

File tree

4 files changed

+141
-85
lines changed

4 files changed

+141
-85
lines changed

‎clippy_lints/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1068,7 +1068,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
10681068
store.register_late_pass(move |_| {
10691069
Box::new(single_call_fn::SingleCallFn {
10701070
avoid_breaking_exported_api,
1071-
def_id_to_usage: rustc_data_structures::fx::FxHashMap::default(),
1071+
def_id_to_usage: rustc_data_structures::fx::FxIndexMap::default(),
10721072
})
10731073
});
10741074
store.register_early_pass(move || {

‎clippy_lints/src/single_call_fn.rs

Lines changed: 70 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
use clippy_utils::diagnostics::span_lint_hir_and_then;
22
use clippy_utils::{is_from_proc_macro, is_in_test_function};
3-
use rustc_data_structures::fx::FxHashMap;
3+
use rustc_data_structures::fx::{FxIndexMap, IndexEntry};
4+
use rustc_hir::def::DefKind;
45
use rustc_hir::def_id::LocalDefId;
5-
use rustc_hir::intravisit::{walk_expr, FnKind, Visitor};
6-
use rustc_hir::{Body, Expr, ExprKind, FnDecl};
6+
use rustc_hir::{Expr, ExprKind, HirId, Node};
77
use rustc_lint::{LateContext, LateLintPass, LintContext};
8-
use rustc_middle::hir::nested_filter::OnlyBodies;
98
use rustc_middle::lint::in_external_macro;
109
use rustc_session::impl_lint_pass;
1110
use rustc_span::Span;
@@ -54,82 +53,93 @@ declare_clippy_lint! {
5453
}
5554
impl_lint_pass!(SingleCallFn => [SINGLE_CALL_FN]);
5655

56+
#[derive(Debug, Clone)]
57+
pub enum CallState {
58+
Once { call_site: Span },
59+
Multiple,
60+
}
61+
5762
#[derive(Clone)]
5863
pub struct SingleCallFn {
5964
pub avoid_breaking_exported_api: bool,
60-
pub def_id_to_usage: FxHashMap<LocalDefId, (Span, Vec<Span>)>,
65+
pub def_id_to_usage: FxIndexMap<LocalDefId, CallState>,
66+
}
67+
68+
impl SingleCallFn {
69+
fn is_function_allowed(
70+
&self,
71+
cx: &LateContext<'_>,
72+
fn_def_id: LocalDefId,
73+
fn_hir_id: HirId,
74+
fn_span: Span,
75+
) -> bool {
76+
(self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(fn_def_id))
77+
|| in_external_macro(cx.sess(), fn_span)
78+
|| cx
79+
.tcx
80+
.hir()
81+
.maybe_body_owned_by(fn_def_id)
82+
.map(|body| cx.tcx.hir().body(body))
83+
.map_or(true, |body| is_in_test_function(cx.tcx, body.value.hir_id))
84+
|| match cx.tcx.hir_node(fn_hir_id) {
85+
Node::Item(item) => is_from_proc_macro(cx, item),
86+
Node::ImplItem(item) => is_from_proc_macro(cx, item),
87+
Node::TraitItem(item) => is_from_proc_macro(cx, item),
88+
_ => true,
89+
}
90+
}
91+
}
92+
93+
/// Whether a called function is a kind of item that the lint cares about.
94+
/// For example, calling an `extern "C" { fn fun(); }` only once is totally fine and does not
95+
/// to be considered.
96+
fn is_valid_item_kind(cx: &LateContext<'_>, def_id: LocalDefId) -> bool {
97+
matches!(
98+
cx.tcx.hir_node(cx.tcx.local_def_id_to_hir_id(def_id)),
99+
Node::Item(_) | Node::ImplItem(_) | Node::TraitItem(_)
100+
)
61101
}
62102

63103
impl<'tcx> LateLintPass<'tcx> for SingleCallFn {
64-
fn check_fn(
65-
&mut self,
66-
cx: &LateContext<'tcx>,
67-
kind: FnKind<'tcx>,
68-
_: &'tcx FnDecl<'_>,
69-
body: &'tcx Body<'_>,
70-
span: Span,
71-
def_id: LocalDefId,
72-
) {
73-
if self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(def_id)
74-
|| in_external_macro(cx.sess(), span)
75-
|| is_in_test_function(cx.tcx, body.value.hir_id)
76-
|| is_from_proc_macro(cx, &(&kind, body, cx.tcx.local_def_id_to_hir_id(def_id), span))
104+
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) {
105+
if let ExprKind::Path(qpath) = expr.kind
106+
&& let res = cx.qpath_res(&qpath, expr.hir_id)
107+
&& let Some(call_def_id) = res.opt_def_id()
108+
&& let Some(def_id) = call_def_id.as_local()
109+
&& let DefKind::Fn | DefKind::AssocFn = cx.tcx.def_kind(def_id)
110+
&& is_valid_item_kind(cx, def_id)
77111
{
78-
return;
112+
match self.def_id_to_usage.entry(def_id) {
113+
IndexEntry::Occupied(mut entry) => {
114+
if let CallState::Once { .. } = entry.get() {
115+
entry.insert(CallState::Multiple);
116+
}
117+
},
118+
IndexEntry::Vacant(entry) => {
119+
entry.insert(CallState::Once { call_site: expr.span });
120+
},
121+
}
79122
}
80-
81-
self.def_id_to_usage.insert(def_id, (span, vec![]));
82123
}
83124

84125
fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
85-
let mut v = FnUsageVisitor {
86-
cx,
87-
def_id_to_usage: &mut self.def_id_to_usage,
88-
};
89-
cx.tcx.hir().visit_all_item_likes_in_crate(&mut v);
90-
91126
for (&def_id, usage) in &self.def_id_to_usage {
92-
let single_call_fn_span = usage.0;
93-
if let [caller_span] = *usage.1 {
127+
if let CallState::Once { call_site } = *usage
128+
&& let fn_hir_id = cx.tcx.local_def_id_to_hir_id(def_id)
129+
&& let fn_span = cx.tcx.hir().span_with_body(fn_hir_id)
130+
&& !self.is_function_allowed(cx, def_id, fn_hir_id, fn_span)
131+
{
94132
span_lint_hir_and_then(
95133
cx,
96134
SINGLE_CALL_FN,
97-
cx.tcx.local_def_id_to_hir_id(def_id),
98-
single_call_fn_span,
135+
fn_hir_id,
136+
fn_span,
99137
"this function is only used once",
100138
|diag| {
101-
diag.span_help(caller_span, "used here");
139+
diag.span_note(call_site, "used here");
102140
},
103141
);
104142
}
105143
}
106144
}
107145
}
108-
109-
struct FnUsageVisitor<'a, 'tcx> {
110-
cx: &'a LateContext<'tcx>,
111-
def_id_to_usage: &'a mut FxHashMap<LocalDefId, (Span, Vec<Span>)>,
112-
}
113-
114-
impl<'a, 'tcx> Visitor<'tcx> for FnUsageVisitor<'a, 'tcx> {
115-
type NestedFilter = OnlyBodies;
116-
117-
fn nested_visit_map(&mut self) -> Self::Map {
118-
self.cx.tcx.hir()
119-
}
120-
121-
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
122-
let Self { cx, .. } = *self;
123-
124-
if let ExprKind::Path(qpath) = expr.kind
125-
&& let res = cx.qpath_res(&qpath, expr.hir_id)
126-
&& let Some(call_def_id) = res.opt_def_id()
127-
&& let Some(def_id) = call_def_id.as_local()
128-
&& let Some(usage) = self.def_id_to_usage.get_mut(&def_id)
129-
{
130-
usage.1.push(expr.span);
131-
}
132-
133-
walk_expr(self, expr);
134-
}
135-
}

‎tests/ui/single_call_fn.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,25 @@ mod issue12182 {
8484
fn l() {
8585
k();
8686
}
87+
88+
trait Trait {
89+
fn default() {}
90+
fn foo(&self);
91+
}
92+
extern "C" {
93+
// test some kind of foreign item
94+
fn rand() -> std::ffi::c_int;
95+
}
96+
fn m<T: Trait>(v: T) {
97+
const NOT_A_FUNCTION: i32 = 1;
98+
let _ = NOT_A_FUNCTION;
99+
100+
struct S;
101+
impl S {
102+
fn foo() {}
103+
}
104+
T::default();
105+
S::foo();
106+
v.foo();
107+
unsafe { rand() };
108+
}

‎tests/ui/single_call_fn.stderr

Lines changed: 48 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,29 @@
1+
error: this function is only used once
2+
--> tests/ui/single_call_fn.rs:13:1
3+
|
4+
LL | fn i() {}
5+
| ^^^^^^^^^
6+
|
7+
note: used here
8+
--> tests/ui/single_call_fn.rs:18:13
9+
|
10+
LL | let a = i;
11+
| ^
12+
= note: `-D clippy::single-call-fn` implied by `-D warnings`
13+
= help: to override `-D warnings` add `#[allow(clippy::single_call_fn)]`
14+
15+
error: this function is only used once
16+
--> tests/ui/single_call_fn.rs:14:1
17+
|
18+
LL | fn j() {}
19+
| ^^^^^^^^^
20+
|
21+
note: used here
22+
--> tests/ui/single_call_fn.rs:25:9
23+
|
24+
LL | j();
25+
| ^
26+
127
error: this function is only used once
228
--> tests/ui/single_call_fn.rs:34:1
329
|
@@ -8,49 +34,47 @@ LL | | println!("function...");
834
LL | | }
935
| |_^
1036
|
11-
help: used here
37+
note: used here
1238
--> tests/ui/single_call_fn.rs:41:5
1339
|
1440
LL | c();
1541
| ^
16-
= note: `-D clippy::single-call-fn` implied by `-D warnings`
17-
= help: to override `-D warnings` add `#[allow(clippy::single_call_fn)]`
18-
19-
error: this function is only used once
20-
--> tests/ui/single_call_fn.rs:13:1
21-
|
22-
LL | fn i() {}
23-
| ^^^^^^^^^
24-
|
25-
help: used here
26-
--> tests/ui/single_call_fn.rs:18:13
27-
|
28-
LL | let a = i;
29-
| ^
3042

3143
error: this function is only used once
3244
--> tests/ui/single_call_fn.rs:44:1
3345
|
3446
LL | fn a() {}
3547
| ^^^^^^^^^
3648
|
37-
help: used here
49+
note: used here
3850
--> tests/ui/single_call_fn.rs:47:5
3951
|
4052
LL | a();
4153
| ^
4254

4355
error: this function is only used once
44-
--> tests/ui/single_call_fn.rs:14:1
56+
--> tests/ui/single_call_fn.rs:89:5
4557
|
46-
LL | fn j() {}
47-
| ^^^^^^^^^
58+
LL | fn default() {}
59+
| ^^^^^^^^^^^^^^^
4860
|
49-
help: used here
50-
--> tests/ui/single_call_fn.rs:25:9
61+
note: used here
62+
--> tests/ui/single_call_fn.rs:104:5
5163
|
52-
LL | j();
53-
| ^
64+
LL | T::default();
65+
| ^^^^^^^^^^
66+
67+
error: this function is only used once
68+
--> tests/ui/single_call_fn.rs:102:9
69+
|
70+
LL | fn foo() {}
71+
| ^^^^^^^^^^^
72+
|
73+
note: used here
74+
--> tests/ui/single_call_fn.rs:105:5
75+
|
76+
LL | S::foo();
77+
| ^^^^^^
5478

55-
error: aborting due to 4 previous errors
79+
error: aborting due to 6 previous errors
5680

0 commit comments

Comments
 (0)