Skip to content

Commit 7064751

Browse files
authored
Add new disallowed_fields lint (rust-lang#16218)
Fixes rust-lang#9278. This is something that we need for `cg_gcc` (cc @antoyo). It's almost the same as rust-lang#6674 (which I used as base). changelog: Add new `disallowed_fields` lint r? @samueltardieu
2 parents d197fc8 + cb68b21 commit 7064751

File tree

12 files changed

+368
-0
lines changed

12 files changed

+368
-0
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6433,6 +6433,7 @@ Released 2018-09-13
64336433
[`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord
64346434
[`derive_partial_eq_without_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_partial_eq_without_eq
64356435
[`derived_hash_with_manual_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derived_hash_with_manual_eq
6436+
[`disallowed_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_fields
64366437
[`disallowed_macros`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_macros
64376438
[`disallowed_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method
64386439
[`disallowed_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods
@@ -7252,6 +7253,7 @@ Released 2018-09-13
72527253
[`check-private-items`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-private-items
72537254
[`cognitive-complexity-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#cognitive-complexity-threshold
72547255
[`const-literal-digits-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#const-literal-digits-threshold
7256+
[`disallowed-fields`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-fields
72557257
[`disallowed-macros`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-macros
72567258
[`disallowed-methods`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-methods
72577259
[`disallowed-names`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-names

book/src/lint_configuration.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,23 @@ The minimum digits a const float literal must have to supress the `excessive_pre
505505
* [`excessive_precision`](https://rust-lang.github.io/rust-clippy/master/index.html#excessive_precision)
506506

507507

508+
## `disallowed-fields`
509+
The list of disallowed fields, written as fully qualified paths.
510+
511+
**Fields:**
512+
- `path` (required): the fully qualified path to the field that should be disallowed
513+
- `reason` (optional): explanation why this field is disallowed
514+
- `replacement` (optional): suggested alternative method
515+
- `allow-invalid` (optional, `false` by default): when set to `true`, it will ignore this entry
516+
if the path doesn't exist, instead of emitting an error
517+
518+
**Default Value:** `[]`
519+
520+
---
521+
**Affected lints:**
522+
* [`disallowed_fields`](https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_fields)
523+
524+
508525
## `disallowed-macros`
509526
The list of disallowed macros, written as fully qualified paths.
510527

clippy_config/src/conf.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,17 @@ define_Conf! {
581581
/// Use the Cognitive Complexity lint instead.
582582
#[conf_deprecated("Please use `cognitive-complexity-threshold` instead", cognitive_complexity_threshold)]
583583
cyclomatic_complexity_threshold: u64 = 25,
584+
/// The list of disallowed fields, written as fully qualified paths.
585+
///
586+
/// **Fields:**
587+
/// - `path` (required): the fully qualified path to the field that should be disallowed
588+
/// - `reason` (optional): explanation why this field is disallowed
589+
/// - `replacement` (optional): suggested alternative method
590+
/// - `allow-invalid` (optional, `false` by default): when set to `true`, it will ignore this entry
591+
/// if the path doesn't exist, instead of emitting an error
592+
#[disallowed_paths_allow_replacements = true]
593+
#[lints(disallowed_fields)]
594+
disallowed_fields: Vec<DisallowedPath> = Vec::new(),
584595
/// The list of disallowed macros, written as fully qualified paths.
585596
///
586597
/// **Fields:**

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
105105
crate::derive::DERIVE_PARTIAL_EQ_WITHOUT_EQ_INFO,
106106
crate::derive::EXPL_IMPL_CLONE_ON_COPY_INFO,
107107
crate::derive::UNSAFE_DERIVE_DESERIALIZE_INFO,
108+
crate::disallowed_fields::DISALLOWED_FIELDS_INFO,
108109
crate::disallowed_macros::DISALLOWED_MACROS_INFO,
109110
crate::disallowed_methods::DISALLOWED_METHODS_INFO,
110111
crate::disallowed_names::DISALLOWED_NAMES_INFO,
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
use clippy_config::Conf;
2+
use clippy_config::types::{DisallowedPath, create_disallowed_map};
3+
use clippy_utils::diagnostics::span_lint_and_then;
4+
use clippy_utils::paths::PathNS;
5+
use clippy_utils::ty::get_field_def_id_by_name;
6+
use rustc_hir::def::{DefKind, Res};
7+
use rustc_hir::def_id::DefIdMap;
8+
use rustc_hir::{Expr, ExprKind, Pat, PatKind};
9+
use rustc_lint::{LateContext, LateLintPass};
10+
use rustc_middle::ty::TyCtxt;
11+
use rustc_session::impl_lint_pass;
12+
13+
declare_clippy_lint! {
14+
/// ### What it does
15+
/// Denies the configured fields in clippy.toml
16+
///
17+
/// Note: Even though this lint is warn-by-default, it will only trigger if
18+
/// fields are defined in the clippy.toml file.
19+
///
20+
/// ### Why is this bad?
21+
/// Some fields are undesirable in certain contexts, and it's beneficial to
22+
/// lint for them as needed.
23+
///
24+
/// ### Example
25+
/// An example clippy.toml configuration:
26+
/// ```toml
27+
/// # clippy.toml
28+
/// disallowed-fields = [
29+
/// # Can use a string as the path of the disallowed field.
30+
/// "std::ops::Range::start",
31+
/// # Can also use an inline table with a `path` key.
32+
/// { path = "std::ops::Range::start" },
33+
/// # When using an inline table, can add a `reason` for why the field
34+
/// # is disallowed.
35+
/// { path = "std::ops::Range::start", reason = "The start of the range is not used" },
36+
/// ]
37+
/// ```
38+
///
39+
/// ```rust
40+
/// use std::ops::Range;
41+
///
42+
/// let range = Range { start: 0, end: 1 };
43+
/// println!("{}", range.start); // `start` is disallowed in the config.
44+
/// ```
45+
///
46+
/// Use instead:
47+
/// ```rust
48+
/// use std::ops::Range;
49+
///
50+
/// let range = Range { start: 0, end: 1 };
51+
/// println!("{}", range.end); // `end` is _not_ disallowed in the config.
52+
/// ```
53+
#[clippy::version = "1.93.0"]
54+
pub DISALLOWED_FIELDS,
55+
style,
56+
"declaration of a disallowed field use"
57+
}
58+
59+
pub struct DisallowedFields {
60+
disallowed: DefIdMap<(&'static str, &'static DisallowedPath)>,
61+
}
62+
63+
impl DisallowedFields {
64+
pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self {
65+
let (disallowed, _) = create_disallowed_map(
66+
tcx,
67+
&conf.disallowed_fields,
68+
PathNS::Field,
69+
|def_kind| matches!(def_kind, DefKind::Field),
70+
"field",
71+
false,
72+
);
73+
Self { disallowed }
74+
}
75+
}
76+
77+
impl_lint_pass!(DisallowedFields => [DISALLOWED_FIELDS]);
78+
79+
impl<'tcx> LateLintPass<'tcx> for DisallowedFields {
80+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
81+
let (id, span) = match &expr.kind {
82+
ExprKind::Path(path) if let Res::Def(_, id) = cx.qpath_res(path, expr.hir_id) => (id, expr.span),
83+
ExprKind::Field(e, ident) => {
84+
// Very round-about way to get the field `DefId` from the expr: first we get its
85+
// parent `Ty`. Then we go through all its fields to find the one with the expected
86+
// name and get the `DefId` from it.
87+
if let Some(parent_ty) = cx.typeck_results().expr_ty_adjusted_opt(e)
88+
&& let Some(field_def_id) = get_field_def_id_by_name(parent_ty, ident.name)
89+
{
90+
(field_def_id, ident.span)
91+
} else {
92+
return;
93+
}
94+
},
95+
_ => return,
96+
};
97+
if let Some(&(path, disallowed_path)) = self.disallowed.get(&id) {
98+
span_lint_and_then(
99+
cx,
100+
DISALLOWED_FIELDS,
101+
span,
102+
format!("use of a disallowed field `{path}`"),
103+
disallowed_path.diag_amendment(span),
104+
);
105+
}
106+
}
107+
108+
fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
109+
let PatKind::Struct(struct_path, pat_fields, _) = pat.kind else {
110+
return;
111+
};
112+
match cx.typeck_results().qpath_res(&struct_path, pat.hir_id) {
113+
Res::Def(DefKind::Struct, struct_def_id) => {
114+
let adt_def = cx.tcx.adt_def(struct_def_id);
115+
for field in pat_fields {
116+
if let Some(def_id) = adt_def.all_fields().find_map(|adt_field| {
117+
if field.ident.name == adt_field.name {
118+
Some(adt_field.did)
119+
} else {
120+
None
121+
}
122+
}) && let Some(&(path, disallowed_path)) = self.disallowed.get(&def_id)
123+
{
124+
span_lint_and_then(
125+
cx,
126+
DISALLOWED_FIELDS,
127+
field.span,
128+
format!("use of a disallowed field `{path}`"),
129+
disallowed_path.diag_amendment(field.span),
130+
);
131+
}
132+
}
133+
},
134+
Res::Def(DefKind::Variant, variant_def_id) => {
135+
let enum_def_id = cx.tcx.parent(variant_def_id);
136+
let variant = cx.tcx.adt_def(enum_def_id).variant_with_id(variant_def_id);
137+
138+
for field in pat_fields {
139+
if let Some(def_id) = variant.fields.iter().find_map(|adt_field| {
140+
if field.ident.name == adt_field.name {
141+
Some(adt_field.did)
142+
} else {
143+
None
144+
}
145+
}) && let Some(&(path, disallowed_path)) = self.disallowed.get(&def_id)
146+
{
147+
span_lint_and_then(
148+
cx,
149+
DISALLOWED_FIELDS,
150+
field.span,
151+
format!("use of a disallowed field `{path}`"),
152+
disallowed_path.diag_amendment(field.span),
153+
);
154+
}
155+
}
156+
},
157+
_ => {},
158+
}
159+
}
160+
}

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ mod default_union_representation;
103103
mod dereference;
104104
mod derivable_impls;
105105
mod derive;
106+
mod disallowed_fields;
106107
mod disallowed_macros;
107108
mod disallowed_methods;
108109
mod disallowed_names;
@@ -857,6 +858,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
857858
Box::new(|_| Box::new(toplevel_ref_arg::ToplevelRefArg)),
858859
Box::new(|_| Box::new(volatile_composites::VolatileComposites)),
859860
Box::new(|_| Box::<replace_box::ReplaceBox>::default()),
861+
Box::new(move |tcx| Box::new(disallowed_fields::DisallowedFields::new(tcx, conf))),
860862
Box::new(move |_| Box::new(manual_ilog2::ManualIlog2::new(conf))),
861863
Box::new(|_| Box::new(same_length_and_capacity::SameLengthAndCapacity)),
862864
Box::new(move |tcx| Box::new(duration_suboptimal_units::DurationSuboptimalUnits::new(tcx, conf))),

clippy_utils/src/paths.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ pub enum PathNS {
2626
Type,
2727
Value,
2828
Macro,
29+
Field,
2930

3031
/// Resolves to the name in the first available namespace, e.g. for `std::vec` this would return
3132
/// either the macro or the module but **not** both
@@ -41,6 +42,7 @@ impl PathNS {
4142
PathNS::Type => TypeNS,
4243
PathNS::Value => ValueNS,
4344
PathNS::Macro => MacroNS,
45+
PathNS::Field => return false,
4446
PathNS::Arbitrary => return true,
4547
};
4648

@@ -286,6 +288,20 @@ fn local_item_child_by_name(tcx: TyCtxt<'_>, local_id: LocalDefId, ns: PathNS, n
286288
&root_mod
287289
},
288290
Node::Item(item) => &item.kind,
291+
Node::Variant(variant) if ns == PathNS::Field => {
292+
return if let rustc_hir::VariantData::Struct { fields, .. } = variant.data
293+
&& let Some(field_def_id) = fields.iter().find_map(|field| {
294+
if field.ident.name == name {
295+
Some(field.def_id.to_def_id())
296+
} else {
297+
None
298+
}
299+
}) {
300+
Some(field_def_id)
301+
} else {
302+
None
303+
};
304+
},
289305
_ => return None,
290306
};
291307

@@ -299,6 +315,7 @@ fn local_item_child_by_name(tcx: TyCtxt<'_>, local_id: LocalDefId, ns: PathNS, n
299315
PathNS::Type => opt_def_id(path.res.type_ns),
300316
PathNS::Value => opt_def_id(path.res.value_ns),
301317
PathNS::Macro => opt_def_id(path.res.macro_ns),
318+
PathNS::Field => None,
302319
PathNS::Arbitrary => unreachable!(),
303320
}
304321
} else {
@@ -318,6 +335,24 @@ fn local_item_child_by_name(tcx: TyCtxt<'_>, local_id: LocalDefId, ns: PathNS, n
318335
.filter_by_name_unhygienic(name)
319336
.find(|assoc_item| ns.matches(Some(assoc_item.namespace())))
320337
.map(|assoc_item| assoc_item.def_id),
338+
ItemKind::Struct(_, _, rustc_hir::VariantData::Struct { fields, .. }) if ns == PathNS::Field => {
339+
fields.iter().find_map(|field| {
340+
if field.ident.name == name {
341+
Some(field.def_id.to_def_id())
342+
} else {
343+
None
344+
}
345+
})
346+
},
347+
ItemKind::Enum(_, _, rustc_hir::EnumDef { variants }) if ns == PathNS::Type => {
348+
variants.iter().find_map(|variant| {
349+
if variant.ident.name == name {
350+
Some(variant.def_id.to_def_id())
351+
} else {
352+
None
353+
}
354+
})
355+
},
321356
_ => None,
322357
}
323358
}
@@ -336,6 +371,11 @@ fn non_local_item_child_by_name(tcx: TyCtxt<'_>, def_id: DefId, ns: PathNS, name
336371
.iter()
337372
.copied()
338373
.find(|assoc_def_id| tcx.item_name(*assoc_def_id) == name && ns.matches(tcx.def_kind(assoc_def_id).ns())),
374+
DefKind::Struct => tcx
375+
.associated_item_def_ids(def_id)
376+
.iter()
377+
.copied()
378+
.find(|assoc_def_id| tcx.item_name(*assoc_def_id) == name),
339379
_ => None,
340380
}
341381
}

clippy_utils/src/ty/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,6 +1265,13 @@ pub fn get_field_by_name<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, name: Symbol) ->
12651265
}
12661266
}
12671267

1268+
pub fn get_field_def_id_by_name(ty: Ty<'_>, name: Symbol) -> Option<DefId> {
1269+
let ty::Adt(adt_def, ..) = ty.kind() else { return None };
1270+
adt_def
1271+
.all_fields()
1272+
.find_map(|field| if field.name == name { Some(field.did) } else { None })
1273+
}
1274+
12681275
/// Check if `ty` is an `Option` and return its argument type if it is.
12691276
pub fn option_arg_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
12701277
match *ty.kind() {
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
disallowed-fields = [
2+
# just a string is shorthand for path only
3+
"std::ops::Range::start",
4+
# can give path and reason with an inline table
5+
{ path = "std::ops::Range::end", reason = "no end allowed" },
6+
# can use an inline table but omit reason
7+
{ path = "std::ops::RangeTo::end" },
8+
# local paths
9+
"conf_disallowed_fields::X::y",
10+
# re-exports
11+
"conf_disallowed_fields::Y::y",
12+
# field of a variant
13+
"conf_disallowed_fields::Z::B::x",
14+
]

0 commit comments

Comments
 (0)