1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
use std::collections::BTreeSet;
use clippy_utils::diagnostics::span_lint_and_help;
use rustc_hir as hir;
use rustc_lint::{LateContext, LateLintPass, LintStore};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::Symbol;
use crate::ascii_letter::AsciiLetter;
use crate::common::{
DefaultState, binding_ident, hir_in_external_macro, is_single_ascii_letter, resolve_symbol_set,
resolve_symbol_set_from_chars, resolved_state,
};
mod triviality;
use triviality::{is_trivial_wrapper, parent_call_callee_name, single_expression_body};
declare_tool_lint! {
/// ### What it does
///
/// Flags closure parameters whose identifier is one ASCII
/// letter, unless the closure is a trivial single-expression
/// callback or the identifier is in the conventional-name
/// exempt set (`n` for an unsigned count, `f` for a
/// `fmt::Formatter`, `i` / `j` / `k` for indices).
/// "Single-expression" is a shared precondition for the
/// trivial-callback exception: the body must be a bare
/// expression or a block whose only content is a trailing
/// expression — a body with any `let` binding or other
/// statement before the trailing expression disqualifies the
/// closure regardless of which branch below would otherwise
/// apply. Given that, one of two further shapes must hold:
/// - the closure is the immediate argument of a call whose
/// callee name is in the trivial-callback method set
/// (`sort_by`, `sort_by_key`, `min_by`, `max_by`,
/// `binary_search_by`, `cmp_by`, `partial_cmp_by`,
/// `fold`, `try_fold`, ...). The set also covers the
/// matching adaptors from `itertools` (`sorted_by`,
/// `k_smallest_by`, `minmax_by_key`, ...) and `into-sorted`
/// (`into_sorted_by`, `into_sorted_by_key`, ...);
/// - the body is a trivial wrapper around the parameter —
/// a field access (`|x| x.field`), a method call
/// (`|x| x.foo()`), a one-argument call where the
/// parameter is the sole argument (`|x| f(x)`), a
/// reference (`|x| &x`), or a macro call
/// (`|x| vec![x]`, `|x| dbg!(x)`,
/// `|x| format!("{x}")`). Surrounding `*` / `&`
/// operators around the parameter inside any of the
/// non-macro shapes are peeled before the match, so
/// `|s| (*s).foo()` qualifies.
///
/// The conventional-name exempt set matches the one used by
/// `perfectionist::single_letter_function_param`: `|i| ...`
/// is the canonical index closure, just as `fn step(i: usize)`
/// is the canonical index parameter. Bodies that use the
/// index for slicing or arithmetic (`|i| &hex[i..i + 2]`)
/// are not structurally trivial, so the exempt set is what
/// keeps them out of the diagnostic.
///
/// ### Why restrict this?
///
/// This is a stylistic preference, not a correctness issue.
/// A multi-line closure body whose parameter is a single
/// letter forces the reader to scroll back to the closure
/// header for context on every reference. The
/// trivial-callback exception covers `sort_by(|a, b| ...)` and
/// `.map(|x| x.field)` shapes that are short enough that the
/// parameter's role is unambiguous from the call site.
///
/// ### Example
///
/// **Avoid:**
///
/// ```rust,ignore
/// .map(|t| {
/// let columns = build_columns(t);
/// format_row(&columns)
/// })
/// ```
///
/// **Prefer:**
///
/// ```rust,ignore
/// .map(|tree_row| {
/// let columns = build_columns(tree_row);
/// format_row(&columns)
/// })
/// ```
pub perfectionist::SINGLE_LETTER_CLOSURE_PARAM,
Warn,
"closure parameter has a single-letter name",
report_in_external_macro: false
}
const CONFIG_KEY: &str = "perfectionist::single_letter_closure_param";
/// Default exempt identifiers for closure parameter names,
/// mirroring the `single_letter_function_param` defaults: `n` for
/// an unsigned count, `f` for a `fmt::Formatter`, and `i` / `j`
/// / `k` for indices. Closures use the same conventional
/// one-letter names as function and method signatures; an
/// `|i| &hex[i..i + 2]` indexing closure is just as canonical as
/// a `fn step(i: usize)` signature, and is not covered by the
/// structural trivial-wrapper shapes.
const DEFAULT_CLOSURE_PARAM_EXEMPTIONS: &[char] = &['n', 'f', 'i', 'j', 'k'];
/// Default trivial-callback method names whose closure argument
/// may use single-letter parameters when the body is a single
/// expression. Both source documents agree on this set.
const DEFAULT_TRIVIAL_CALLBACK_METHODS: &[&str] = &[
"sort_by",
"sort_unstable_by",
"sort_by_key",
"sort_unstable_by_key",
"min_by",
"max_by",
"min_by_key",
"max_by_key",
"binary_search_by",
"binary_search_by_key",
// `Iterator::cmp_by` / `partial_cmp_by` / `eq_by` take a
// closure of two parameters. The bare `cmp` / `partial_cmp`
// trait methods are not closure-accepting, so they are not
// listed here.
"cmp_by",
"partial_cmp_by",
"eq_by",
"fold",
"try_fold",
"rfold",
"reduce",
// `itertools::Itertools` adaptors that take a comparator or
// key closure and follow the same `|a, b| ...` / `|x| ...`
// convention as their std counterparts.
"sorted_by",
"sorted_by_key",
"sorted_by_cached_key",
"sorted_unstable_by",
"sorted_unstable_by_key",
"k_smallest_by",
"k_smallest_by_key",
"k_smallest_relaxed_by",
"k_smallest_relaxed_by_key",
"k_largest_by",
"k_largest_by_key",
"k_largest_relaxed_by",
"k_largest_relaxed_by_key",
"min_set_by",
"min_set_by_key",
"max_set_by",
"max_set_by_key",
"minmax_by",
"minmax_by_key",
"position_min_by",
"position_min_by_key",
"position_max_by",
"position_max_by_key",
"position_minmax_by",
"position_minmax_by_key",
// `into_sorted::{IntoSorted, IntoSortedUnstable}` adaptors
// that return an owned sorted array, mirroring the std
// `sort_by` / `sort_by_key` shapes.
"into_sorted_by",
"into_sorted_by_key",
"into_sorted_by_cached_key",
"into_sorted_unstable_by",
"into_sorted_unstable_by_key",
];
#[derive(Debug, Default, serde::Deserialize)]
#[serde(default, deny_unknown_fields, rename_all = "snake_case")]
struct Config {
/// Additional method / function names whose closure argument
/// may carry single-letter parameters when the body is a
/// single expression. Merged with the built-in defaults (the
/// curated `core` / `std` callbacks plus selected `itertools`
/// and `into-sorted` adaptors); empty by default. List
/// project-specific DSL helpers (`when`, `iter_by`, third-party
/// callbacks such as `into_sorted_by`, ...) here without having
/// to re-state the standard ones.
extra_trivial_callback_methods: Vec<String>,
/// Method / function names to drop from the trivial-callback
/// set, even if they appear in the built-in defaults or in
/// `extra_trivial_callback_methods`. Empty by default; checked
/// after the merge with the built-ins, so this knob always
/// wins. Useful for opting back into linting on a default
/// entry the project does not consider trivial.
ignore_trivial_callback_methods: Vec<String>,
/// Additional identifiers to allow as closure parameter names.
/// Merged with the built-in defaults
/// (`["n", "f", "i", "j", "k"]`); empty by default. Use this
/// to whitelist project-specific conventional names without
/// having to re-state the standard ones. Each entry is a
/// single ASCII letter (`a`-`z`, `A`-`Z`); any other
/// character is rejected at config-parse time.
extra_allowed_idents: Vec<AsciiLetter>,
/// Identifiers to deny (always flag), removing them from the
/// exempt set even if they appear in the built-in defaults or
/// in `extra_allowed_idents`. Empty by default; checked after
/// the merge with the built-ins, so this knob always wins.
/// Each entry is a single ASCII letter (`a`-`z`, `A`-`Z`);
/// any other character is rejected at config-parse time.
extra_denied_idents: Vec<AsciiLetter>,
}
pub struct SingleLetterClosureParam {
trivial_callback_methods: BTreeSet<Symbol>,
allowed_idents: BTreeSet<Symbol>,
}
impl SingleLetterClosureParam {
fn new() -> Self {
let config: Config = dylint_linting::config_or_default(CONFIG_KEY);
let trivial_callback_methods = resolve_symbol_set(
DEFAULT_TRIVIAL_CALLBACK_METHODS,
config.extra_trivial_callback_methods,
config.ignore_trivial_callback_methods,
);
let allowed_idents = resolve_symbol_set_from_chars(
DEFAULT_CLOSURE_PARAM_EXEMPTIONS,
config.extra_allowed_idents,
config.extra_denied_idents,
);
Self {
trivial_callback_methods,
allowed_idents,
}
}
}
impl_lint_pass!(SingleLetterClosureParam => [SINGLE_LETTER_CLOSURE_PARAM]);
pub fn register_lint(lint_store: &mut LintStore) {
lint_store.register_lints(&[SINGLE_LETTER_CLOSURE_PARAM]);
}
pub fn register_pass(lint_store: &mut LintStore) {
if let DefaultState::Inactive =
resolved_state("single_letter_closure_param", DefaultState::Active)
{
return;
}
lint_store.register_late_pass(|_| Box::new(SingleLetterClosureParam::new()));
}
impl<'tcx> LateLintPass<'tcx> for SingleLetterClosureParam {
fn check_expr(&mut self, lint_context: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
let hir::ExprKind::Closure(closure) = expr.kind else {
return;
};
if hir_in_external_macro(lint_context, expr.hir_id, expr.span) {
return;
}
let body = lint_context.tcx.hir_body(closure.body);
let single_letter_params: Vec<rustc_span::Ident> = body
.params
.iter()
.filter_map(|param| {
let ident = binding_ident(param.pat)?;
is_single_ascii_letter(ident.name.as_str()).then_some(ident)
})
.filter(|ident| !self.allowed_idents.contains(&ident.name))
.collect();
if single_letter_params.is_empty() {
return;
}
if self.closure_is_trivial(lint_context, expr, body) {
return;
}
for ident in single_letter_params {
span_lint_and_help(
lint_context,
SINGLE_LETTER_CLOSURE_PARAM,
ident.span,
format!(
"closure parameter `{}` has a single-letter name",
ident.name,
),
None,
"rename to a descriptive identifier, or rewrite the closure as \
a trivial single-expression callback",
);
}
}
}
impl SingleLetterClosureParam {
fn closure_is_trivial<'tcx>(
&self,
lint_context: &LateContext<'tcx>,
closure_expr: &'tcx hir::Expr<'tcx>,
body: &'tcx hir::Body<'tcx>,
) -> bool {
let Some(body_expr) = single_expression_body(body) else {
return false;
};
if self.parent_call_is_trivial_callback(lint_context, closure_expr) {
return true;
}
if is_trivial_wrapper(body_expr, body.params) {
return true;
}
false
}
fn parent_call_is_trivial_callback<'tcx>(
&self,
lint_context: &LateContext<'tcx>,
closure_expr: &'tcx hir::Expr<'tcx>,
) -> bool {
let Some(name) = parent_call_callee_name(lint_context, closure_expr) else {
return false;
};
self.trivial_callback_methods.contains(&name)
}
}