Skip to content

Commit eee8ef8

Browse files
authored
New internal lint: unusual_names (rust-lang#15794)
This lint aims at detecting unusual names used in Clippy source code, such as `appl` or `application` for a `rustc_errors::Applicability` variable, instead of `app` and `applicability` which are commonly used throughout Clippy. This helps maintaining the consistency of the Clippy source code. It is currently implemented for: - `Applicability`: `app` or `applicability` - `EarlyContext`/`LateContext`: `cx` - `TyCtxt`: `tcx` changelog: none
2 parents c6f2557 + 5de7da8 commit eee8ef8

File tree

11 files changed

+135
-18
lines changed

11 files changed

+135
-18
lines changed

clippy_lints/src/loops/utils.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -256,15 +256,15 @@ fn is_conditional(expr: &Expr<'_>) -> bool {
256256

257257
/// If `arg` was the argument to a `for` loop, return the "cleanest" way of writing the
258258
/// actual `Iterator` that the loop uses.
259-
pub(super) fn make_iterator_snippet(cx: &LateContext<'_>, arg: &Expr<'_>, applic_ref: &mut Applicability) -> String {
259+
pub(super) fn make_iterator_snippet(cx: &LateContext<'_>, arg: &Expr<'_>, applicability: &mut Applicability) -> String {
260260
let impls_iterator = cx
261261
.tcx
262262
.get_diagnostic_item(sym::Iterator)
263263
.is_some_and(|id| implements_trait(cx, cx.typeck_results().expr_ty(arg), id, &[]));
264264
if impls_iterator {
265265
format!(
266266
"{}",
267-
sugg::Sugg::hir_with_applicability(cx, arg, "_", applic_ref).maybe_paren()
267+
sugg::Sugg::hir_with_applicability(cx, arg, "_", applicability).maybe_paren()
268268
)
269269
} else {
270270
// (&x).into_iter() ==> x.iter()
@@ -282,12 +282,12 @@ pub(super) fn make_iterator_snippet(cx: &LateContext<'_>, arg: &Expr<'_>, applic
282282
};
283283
format!(
284284
"{}.{method_name}()",
285-
sugg::Sugg::hir_with_applicability(cx, caller, "_", applic_ref).maybe_paren(),
285+
sugg::Sugg::hir_with_applicability(cx, caller, "_", applicability).maybe_paren(),
286286
)
287287
},
288288
_ => format!(
289289
"{}.into_iter()",
290-
sugg::Sugg::hir_with_applicability(cx, arg, "_", applic_ref).maybe_paren()
290+
sugg::Sugg::hir_with_applicability(cx, arg, "_", applicability).maybe_paren()
291291
),
292292
}
293293
}

clippy_lints/src/methods/iter_skip_next.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use super::ITER_SKIP_NEXT;
1212
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, arg: &hir::Expr<'_>) {
1313
// lint if caller of skip is an Iterator
1414
if cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::Iterator) {
15-
let mut application = Applicability::MachineApplicable;
15+
let mut applicability = Applicability::MachineApplicable;
1616
span_lint_and_then(
1717
cx,
1818
ITER_SKIP_NEXT,
@@ -24,7 +24,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr
2424
&& let PatKind::Binding(ann, _, _, _) = pat.kind
2525
&& ann != BindingMode::MUT
2626
{
27-
application = Applicability::Unspecified;
27+
applicability = Applicability::Unspecified;
2828
diag.span_help(
2929
pat.span,
3030
format!("for this change `{}` has to be mutable", snippet(cx, pat.span, "..")),
@@ -35,7 +35,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr
3535
expr.span.trim_start(recv.span).unwrap(),
3636
"use `nth` instead",
3737
format!(".nth({})", snippet(cx, arg.span, "..")),
38-
application,
38+
applicability,
3939
);
4040
},
4141
);

clippy_lints/src/non_std_lazy_statics.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ impl LazyInfo {
220220
fn lint(&self, cx: &LateContext<'_>, sugg_map: &FxIndexMap<DefId, Option<String>>) {
221221
// Applicability might get adjusted to `Unspecified` later if any calls
222222
// in `calls_span_and_id` are not replaceable judging by the `sugg_map`.
223-
let mut appl = Applicability::MachineApplicable;
223+
let mut app = Applicability::MachineApplicable;
224224
let mut suggs = vec![(self.ty_span_no_args, "std::sync::LazyLock".to_string())];
225225

226226
for (span, def_id) in &self.calls_span_and_id {
@@ -229,7 +229,7 @@ impl LazyInfo {
229229
suggs.push((*span, sugg));
230230
} else {
231231
// If NO suggested replacement, not machine applicable
232-
appl = Applicability::Unspecified;
232+
app = Applicability::Unspecified;
233233
}
234234
}
235235

@@ -240,7 +240,7 @@ impl LazyInfo {
240240
self.ty_span_no_args,
241241
"this type has been superseded by `LazyLock` in the standard library",
242242
|diag| {
243-
diag.multipart_suggestion("use `std::sync::LazyLock` instead", suggs, appl);
243+
diag.multipart_suggestion("use `std::sync::LazyLock` instead", suggs, app);
244244
},
245245
);
246246
}

clippy_lints/src/significant_drop_tightening.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ impl<'tcx> Visitor<'tcx> for StmtsChecker<'_, '_, '_, '_, 'tcx> {
320320
}
321321
},
322322
hir::StmtKind::Semi(semi_expr) => {
323-
if has_drop(semi_expr, apa.first_bind_ident, self.cx) {
323+
if has_drop(self.cx, semi_expr, apa.first_bind_ident) {
324324
apa.has_expensive_expr_after_last_attr = false;
325325
apa.last_stmt_span = DUMMY_SP;
326326
return;
@@ -417,11 +417,11 @@ fn dummy_stmt_expr<'any>(expr: &'any hir::Expr<'any>) -> hir::Stmt<'any> {
417417
}
418418
}
419419

420-
fn has_drop(expr: &hir::Expr<'_>, first_bind_ident: Option<Ident>, lcx: &LateContext<'_>) -> bool {
420+
fn has_drop(cx: &LateContext<'_>, expr: &hir::Expr<'_>, first_bind_ident: Option<Ident>) -> bool {
421421
if let hir::ExprKind::Call(fun, [first_arg]) = expr.kind
422422
&& let hir::ExprKind::Path(hir::QPath::Resolved(_, fun_path)) = &fun.kind
423423
&& let Res::Def(DefKind::Fn, did) = fun_path.res
424-
&& lcx.tcx.is_diagnostic_item(sym::mem_drop, did)
424+
&& cx.tcx.is_diagnostic_item(sym::mem_drop, did)
425425
{
426426
let has_ident = |local_expr: &hir::Expr<'_>| {
427427
if let hir::ExprKind::Path(hir::QPath::Resolved(_, arg_path)) = &local_expr.kind

clippy_lints/src/single_char_lifetime_names.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ declare_clippy_lint! {
4040
declare_lint_pass!(SingleCharLifetimeNames => [SINGLE_CHAR_LIFETIME_NAMES]);
4141

4242
impl EarlyLintPass for SingleCharLifetimeNames {
43-
fn check_generic_param(&mut self, ctx: &EarlyContext<'_>, param: &GenericParam) {
44-
if param.ident.span.in_external_macro(ctx.sess().source_map()) {
43+
fn check_generic_param(&mut self, cx: &EarlyContext<'_>, param: &GenericParam) {
44+
if param.ident.span.in_external_macro(cx.sess().source_map()) {
4545
return;
4646
}
4747

@@ -51,7 +51,7 @@ impl EarlyLintPass for SingleCharLifetimeNames {
5151
{
5252
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
5353
span_lint_and_then(
54-
ctx,
54+
cx,
5555
SINGLE_CHAR_LIFETIME_NAMES,
5656
param.ident.span,
5757
"single-character lifetime names are likely uninformative",

clippy_lints_internal/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ edition = "2024"
66
[dependencies]
77
clippy_config = { path = "../clippy_config" }
88
clippy_utils = { path = "../clippy_utils" }
9+
itertools = "0.12"
910
regex = { version = "1.5" }
1011
rustc-semver = "1.1"
1112

clippy_lints_internal/src/internal_paths.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,18 @@ use clippy_utils::paths::{PathLookup, PathNS};
22
use clippy_utils::{sym, type_path, value_path};
33

44
// Paths inside rustc
5+
pub static APPLICABILITY: PathLookup = type_path!(rustc_errors::Applicability);
6+
pub static EARLY_CONTEXT: PathLookup = type_path!(rustc_lint::EarlyContext);
57
pub static EARLY_LINT_PASS: PathLookup = type_path!(rustc_lint::passes::EarlyLintPass);
68
pub static KW_MODULE: PathLookup = type_path!(rustc_span::symbol::kw);
9+
pub static LATE_CONTEXT: PathLookup = type_path!(rustc_lint::LateContext);
710
pub static LINT: PathLookup = type_path!(rustc_lint_defs::Lint);
811
pub static SYMBOL: PathLookup = type_path!(rustc_span::symbol::Symbol);
912
pub static SYMBOL_AS_STR: PathLookup = value_path!(rustc_span::symbol::Symbol::as_str);
1013
pub static SYM_MODULE: PathLookup = type_path!(rustc_span::symbol::sym);
1114
pub static SYNTAX_CONTEXT: PathLookup = type_path!(rustc_span::hygiene::SyntaxContext);
15+
#[expect(clippy::unnecessary_def_path, reason = "for uniform checking in internal lint")]
16+
pub static TY_CTXT: PathLookup = type_path!(rustc_middle::ty::TyCtxt);
1217

1318
// Paths in clippy itself
1419
pub static CLIPPY_SYM_MODULE: PathLookup = type_path!(clippy_utils::sym);

clippy_lints_internal/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ mod produce_ice;
4141
mod symbols;
4242
mod unnecessary_def_path;
4343
mod unsorted_clippy_utils_paths;
44+
mod unusual_names;
4445

4546
use rustc_lint::{Lint, LintStore};
4647

@@ -59,6 +60,7 @@ static LINTS: &[&Lint] = &[
5960
symbols::SYMBOL_AS_STR,
6061
unnecessary_def_path::UNNECESSARY_DEF_PATH,
6162
unsorted_clippy_utils_paths::UNSORTED_CLIPPY_UTILS_PATHS,
63+
unusual_names::UNUSUAL_NAMES,
6264
];
6365

6466
pub fn register_lints(store: &mut LintStore) {
@@ -74,4 +76,5 @@ pub fn register_lints(store: &mut LintStore) {
7476
store.register_late_pass(|_| Box::new(outer_expn_data_pass::OuterExpnDataPass));
7577
store.register_late_pass(|_| Box::new(msrv_attr_impl::MsrvAttrImpl));
7678
store.register_late_pass(|_| Box::new(almost_standard_lint_formulation::AlmostStandardFormulation::new()));
79+
store.register_late_pass(|_| Box::new(unusual_names::UnusualNames));
7780
}

clippy_lints_internal/src/produce_ice.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ declare_tool_lint! {
2525
declare_lint_pass!(ProduceIce => [PRODUCE_ICE]);
2626

2727
impl EarlyLintPass for ProduceIce {
28-
fn check_fn(&mut self, ctx: &EarlyContext<'_>, fn_kind: FnKind<'_>, span: Span, _: NodeId) {
28+
fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, span: Span, _: NodeId) {
2929
if is_trigger_fn(fn_kind) {
30-
ctx.sess()
30+
cx.sess()
3131
.dcx()
3232
.span_delayed_bug(span, "Would you like some help with that?");
3333
}
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
use clippy_utils::diagnostics::span_lint_and_help;
2+
use clippy_utils::paths::PathLookup;
3+
use clippy_utils::sym;
4+
use itertools::Itertools;
5+
use rustc_hir::def_id::LocalDefId;
6+
use rustc_hir::intravisit::FnKind;
7+
use rustc_hir::{Body, FnDecl, Pat, PatKind, Stmt, StmtKind};
8+
use rustc_lint::{LateContext, LateLintPass};
9+
use rustc_middle::ty::Ty;
10+
use rustc_session::{declare_lint_pass, declare_tool_lint};
11+
use rustc_span::symbol::kw;
12+
use rustc_span::{Span, Symbol};
13+
14+
use crate::internal_paths::{APPLICABILITY, EARLY_CONTEXT, LATE_CONTEXT, TY_CTXT};
15+
16+
declare_tool_lint! {
17+
/// ### What it does
18+
/// Checks if variables of some types use the usual name.
19+
///
20+
/// ### Why is this bad?
21+
/// Restricting the identifiers used for common things in
22+
/// Clippy sources increases consistency.
23+
///
24+
/// ### Example
25+
/// Check that an `rustc_errors::Applicability` variable is
26+
/// named either `app` or `applicability`, and not
27+
/// `a` or `appl`.
28+
pub clippy::UNUSUAL_NAMES,
29+
Warn,
30+
"commonly used concepts should use usual same variable name.",
31+
report_in_external_macro: true
32+
}
33+
34+
declare_lint_pass!(UnusualNames => [UNUSUAL_NAMES]);
35+
36+
const USUAL_NAMES: [(&PathLookup, &str, &[Symbol]); 4] = [
37+
(
38+
&APPLICABILITY,
39+
"rustc_errors::Applicability",
40+
&[sym::app, sym::applicability],
41+
),
42+
(&EARLY_CONTEXT, "rustc_lint::EarlyContext", &[sym::cx]),
43+
(&LATE_CONTEXT, "rustc_lint::LateContext", &[sym::cx]),
44+
(&TY_CTXT, "rustc_middle::ty::TyCtxt", &[sym::tcx]),
45+
];
46+
47+
impl<'tcx> LateLintPass<'tcx> for UnusualNames {
48+
fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
49+
if let StmtKind::Let(let_stmt) = stmt.kind
50+
&& let Some(init_expr) = let_stmt.init
51+
{
52+
check_pat_name_for_ty(cx, let_stmt.pat, cx.typeck_results().expr_ty(init_expr), "variable");
53+
}
54+
}
55+
56+
fn check_fn(
57+
&mut self,
58+
cx: &LateContext<'tcx>,
59+
kind: FnKind<'tcx>,
60+
_decl: &'tcx FnDecl<'_>,
61+
body: &'tcx Body<'_>,
62+
_span: Span,
63+
def_id: LocalDefId,
64+
) {
65+
if matches!(kind, FnKind::Closure) {
66+
return;
67+
}
68+
for (param, ty) in body
69+
.params
70+
.iter()
71+
.zip(cx.tcx.fn_sig(def_id).instantiate_identity().skip_binder().inputs())
72+
{
73+
check_pat_name_for_ty(cx, param.pat, *ty, "parameter");
74+
}
75+
}
76+
}
77+
78+
fn check_pat_name_for_ty(cx: &LateContext<'_>, pat: &Pat<'_>, ty: Ty<'_>, kind: &str) {
79+
if let PatKind::Binding(_, _, ident, _) = pat.kind {
80+
let ty = ty.peel_refs();
81+
for (usual_ty, ty_str, usual_names) in USUAL_NAMES {
82+
if usual_ty.matches_ty(cx, ty)
83+
&& !usual_names.contains(&ident.name)
84+
&& ident.name != kw::SelfLower
85+
&& !ident.name.as_str().starts_with('_')
86+
{
87+
let usual_names = usual_names.iter().map(|name| format!("`{name}`")).join(" or ");
88+
span_lint_and_help(
89+
cx,
90+
UNUSUAL_NAMES,
91+
ident.span,
92+
format!("unusual name for a {kind} of type `{ty_str}`"),
93+
None,
94+
format!("prefer using {usual_names}"),
95+
);
96+
}
97+
}
98+
}
99+
}

0 commit comments

Comments
 (0)