Skip to content

Commit 931cac8

Browse files
committed
add lint for struct field names
1 parent 33f084e commit 931cac8

32 files changed

+622
-160
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5370,6 +5370,7 @@ Released 2018-09-13
53705370
[`string_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_to_string
53715371
[`strlen_on_c_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#strlen_on_c_strings
53725372
[`struct_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#struct_excessive_bools
5373+
[`struct_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#struct_field_names
53735374
[`stutter`]: https://rust-lang.github.io/rust-clippy/master/index.html#stutter
53745375
[`suboptimal_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#suboptimal_flops
53755376
[`suspicious_arithmetic_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_arithmetic_impl

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
619619
crate::strings::STR_TO_STRING_INFO,
620620
crate::strings::TRIM_SPLIT_WHITESPACE_INFO,
621621
crate::strlen_on_c_strings::STRLEN_ON_C_STRINGS_INFO,
622+
crate::struct_fields::STRUCT_FIELD_NAMES_INFO,
622623
crate::suspicious_doc_comments::SUSPICIOUS_DOC_COMMENTS_INFO,
623624
crate::suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS_INFO,
624625
crate::suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL_INFO,

clippy_lints/src/functions/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,7 @@ declare_clippy_lint! {
360360
}
361361

362362
#[derive(Copy, Clone)]
363+
#[allow(clippy::struct_field_names)]
363364
pub struct Functions {
364365
too_many_arguments_threshold: u64,
365366
too_many_lines_threshold: u64,

clippy_lints/src/lib.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,7 @@ mod slow_vector_initialization;
308308
mod std_instead_of_core;
309309
mod strings;
310310
mod strlen_on_c_strings;
311+
mod struct_fields;
311312
mod suspicious_doc_comments;
312313
mod suspicious_operation_groupings;
313314
mod suspicious_trait_impl;
@@ -1117,6 +1118,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
11171118
msrv(),
11181119
))
11191120
});
1121+
let struct_fields_name_threshold = conf.struct_fields_name_threshold;
1122+
store.register_late_pass(move |_| {
1123+
Box::new(struct_fields::StructFields::new(
1124+
struct_fields_name_threshold,
1125+
avoid_breaking_exported_api,
1126+
))
1127+
});
11201128
// add lints here, do not remove this comment, it's used in `new_lint`
11211129
}
11221130

clippy_lints/src/only_used_in_recursion.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ impl Usage {
134134
/// The parameters being checked by the lint, indexed by both the parameter's `HirId` and the
135135
/// `DefId` of the function paired with the parameter's index.
136136
#[derive(Default)]
137+
#[allow(clippy::struct_field_names)]
137138
struct Params {
138139
params: Vec<Param>,
139140
by_id: HirIdMap<usize>,

clippy_lints/src/struct_fields.rs

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_hir};
2+
use clippy_utils::str_utils::to_snake_case;
3+
use rustc_hir::{FieldDef, ItemKind, VariantData};
4+
use rustc_lint::{LateContext, LateLintPass};
5+
use rustc_session::{declare_tool_lint, impl_lint_pass};
6+
use rustc_span::Span;
7+
8+
declare_clippy_lint! {
9+
/// ### What it does
10+
/// Detects struct fields that are prefixed or suffixed
11+
/// by the same characters or the name of the struct itself.
12+
///
13+
/// ### Why is this bad?
14+
/// Information common to all struct fields is better represented in the struct name.
15+
///
16+
/// ### Limitations
17+
/// Characters with no casing will be considered when comparing prefixes/suffixes
18+
/// This applies to numbers and non-ascii characters without casing
19+
/// e.g. `Foo1` and `foo2` is considered to have different prefixes
20+
/// (the prefixes are `foo1` and `foo2` respectively), as also `bar螃`, `bar蟹`
21+
///
22+
/// ### Example
23+
/// ```rust
24+
/// struct Cake {
25+
/// cake_sugar: u8,
26+
/// cake_flour: u8,
27+
/// cake_eggs: u8
28+
/// }
29+
/// ```
30+
/// Use instead:
31+
/// ```rust
32+
/// struct Cake {
33+
/// sugar: u8,
34+
/// flour: u8,
35+
/// eggs: u8
36+
/// }
37+
/// ```
38+
#[clippy::version = "1.74.0"]
39+
pub STRUCT_FIELD_NAMES,
40+
style,
41+
"structs where all fields share a prefix/postfix or contain the name of the struct"
42+
}
43+
44+
pub struct StructFields {
45+
threshold: u64,
46+
avoid_breaking_exported_api: bool,
47+
}
48+
49+
impl StructFields {
50+
pub fn new(threshold: u64, avoid_breaking_exported_api: bool) -> StructFields {
51+
StructFields {
52+
threshold,
53+
avoid_breaking_exported_api,
54+
}
55+
}
56+
}
57+
58+
impl_lint_pass!(StructFields => [STRUCT_FIELD_NAMES]);
59+
60+
fn is_snake_case(name: &str) -> bool {
61+
name.chars().all(|c| !c.is_uppercase() || c == '_')
62+
}
63+
64+
fn check_fields(cx: &LateContext<'_>, threshold: u64, fields: &[FieldDef<'_>], item_name: &str, item_span: Span) {
65+
if (fields.len() as u64) < threshold {
66+
return;
67+
}
68+
69+
for field in fields {
70+
if is_snake_case(field.ident.name.as_str()) {
71+
check_struct_start(cx, item_name, field);
72+
check_struct_end(cx, item_name, field);
73+
}
74+
}
75+
76+
let first_field = match fields.get(0) {
77+
Some(field) => field.ident.name.as_str(),
78+
None => return,
79+
};
80+
let mut pre: Vec<&str> = first_field.split('_').collect();
81+
let mut post = pre.clone();
82+
post.reverse();
83+
for field in fields {
84+
let field_split: Vec<&str> = field.ident.name.as_str().split('_').collect();
85+
if field_split.len() == 1 {
86+
return;
87+
}
88+
89+
pre = pre
90+
.iter()
91+
.zip(field_split.iter())
92+
.take_while(|(a, b)| a == b)
93+
.map(|e| *e.0)
94+
.collect();
95+
post = post
96+
.iter()
97+
.zip(field_split.iter().rev())
98+
.take_while(|(a, b)| a == b)
99+
.map(|e| *e.0)
100+
.collect();
101+
}
102+
let prefix = pre.join("_");
103+
post.reverse();
104+
let postfix = post.join("_");
105+
if fields.len() > 1 {
106+
let (what, value) = match (prefix.is_empty(), postfix.is_empty()) {
107+
(true, true) => return,
108+
(false, _) => ("pre", prefix),
109+
(true, false) => ("post", postfix),
110+
};
111+
span_lint_and_help(
112+
cx,
113+
STRUCT_FIELD_NAMES,
114+
item_span,
115+
&format!("all fields have the same {what}fix: `{value}`"),
116+
None,
117+
&format!("remove the {what}fixes"),
118+
);
119+
}
120+
}
121+
122+
fn check_struct_start(cx: &LateContext<'_>, item_name: &str, field: &FieldDef<'_>) {
123+
let item_name_snake = to_snake_case(item_name);
124+
let name = field.ident.name.as_str();
125+
126+
if name.starts_with(&item_name_snake) {
127+
span_lint_hir(
128+
cx,
129+
STRUCT_FIELD_NAMES,
130+
field.hir_id,
131+
field.span,
132+
"field name starts with the struct's name",
133+
);
134+
}
135+
}
136+
137+
fn check_struct_end(cx: &LateContext<'_>, item_name: &str, field: &FieldDef<'_>) {
138+
let item_name_snake = to_snake_case(item_name);
139+
let name = field.ident.name.as_str();
140+
141+
if name.ends_with(&item_name_snake) {
142+
span_lint_hir(
143+
cx,
144+
STRUCT_FIELD_NAMES,
145+
field.hir_id,
146+
field.span,
147+
"field name ends with the struct's name",
148+
);
149+
}
150+
}
151+
152+
impl LateLintPass<'_> for StructFields {
153+
fn check_item(&mut self, cx: &LateContext<'_>, item: &'_ rustc_hir::Item<'_>) {
154+
if self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(item.owner_id.def_id) {
155+
return;
156+
}
157+
158+
let item_name = item.ident.name.as_str();
159+
if let ItemKind::Struct(variant_data, _) = item.kind
160+
&& let VariantData::Struct(fields, _) = variant_data {
161+
check_fields(cx, self.threshold, fields, item_name, item.span);
162+
}
163+
}
164+
}

clippy_lints/src/types/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ impl Types {
578578
}
579579
}
580580

581-
#[allow(clippy::struct_excessive_bools)]
581+
#[allow(clippy::struct_excessive_bools, clippy::struct_field_names)]
582582
#[derive(Clone, Copy, Default)]
583583
struct CheckTyContext {
584584
is_in_trait_impl: bool,

clippy_lints/src/utils/conf.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,10 @@ define_Conf! {
360360
///
361361
/// The minimum number of enum variants for the lints about variant names to trigger
362362
(enum_variant_name_threshold: u64 = 3),
363+
/// Lint: STRUCT_FIELD_NAMES.
364+
///
365+
/// The minimum number of struct fields for the lints about field names to trigger
366+
(struct_fields_name_threshold: u64 = 3),
363367
/// Lint: LARGE_ENUM_VARIANT.
364368
///
365369
/// The maximum size of an enum's variant to avoid box suggestion

clippy_utils/src/str_utils.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,30 @@ pub fn count_match_end(str1: &str, str2: &str) -> StrCount {
236236
})
237237
}
238238

239+
/// Returns a `snake_case` version of the input
240+
/// ```
241+
/// use clippy_utils::str_utils::to_snake_case;
242+
/// assert_eq!(to_snake_case("AbcDef"), "abc_def");
243+
/// assert_eq!(to_snake_case("ABCD"), "a_b_c_d");
244+
/// assert_eq!(to_snake_case("AbcDD"), "abc_d_d");
245+
/// assert_eq!(to_snake_case("Abc1DD"), "abc1_d_d");
246+
/// ```
247+
pub fn to_snake_case(name: &str) -> String {
248+
let mut s = String::new();
249+
for (i, c) in name.chars().enumerate() {
250+
if c.is_uppercase() {
251+
// characters without capitalization are considered lowercase
252+
if i != 0 {
253+
s.push('_');
254+
}
255+
s.extend(c.to_lowercase());
256+
} else {
257+
s.push(c);
258+
}
259+
}
260+
s
261+
}
262+
239263
#[cfg(test)]
240264
mod test {
241265
use super::*;

tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//! this is crate
22
#![allow(missing_docs)]
3+
#![allow(clippy::struct_field_names)]
34
#![warn(clippy::missing_docs_in_private_items)]
45

56
/// this is mod

0 commit comments

Comments
 (0)