Skip to content

add lint for struct field names #11496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5463,6 +5463,7 @@ Released 2018-09-13
[`string_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_to_string
[`strlen_on_c_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#strlen_on_c_strings
[`struct_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#struct_excessive_bools
[`struct_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#struct_field_names
[`stutter`]: https://rust-lang.github.io/rust-clippy/master/index.html#stutter
[`suboptimal_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#suboptimal_flops
[`suspicious_arithmetic_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_arithmetic_impl
Expand Down Expand Up @@ -5625,6 +5626,7 @@ Released 2018-09-13
[`single-char-binding-names-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#single-char-binding-names-threshold
[`too-large-for-stack`]: https://doc.rust-lang.org/clippy/lint_configuration.html#too-large-for-stack
[`enum-variant-name-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enum-variant-name-threshold
[`struct-field-name-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#struct-field-name-threshold
[`enum-variant-size-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enum-variant-size-threshold
[`verbose-bit-mask-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#verbose-bit-mask-threshold
[`literal-representation-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#literal-representation-threshold
Expand Down
10 changes: 10 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,16 @@ The minimum number of enum variants for the lints about variant names to trigger
* [`enum_variant_names`](https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names)


## `struct-field-name-threshold`
The minimum number of struct fields for the lints about field names to trigger

**Default Value:** `3` (`u64`)

---
**Affected lints:**
* [`struct_variant_names`](https://rust-lang.github.io/rust-clippy/master/index.html#struct_variant_names)


## `enum-variant-size-threshold`
The maximum size of an enum's variant to avoid box suggestion

Expand Down
7 changes: 4 additions & 3 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::endian_bytes::LITTLE_ENDIAN_BYTES_INFO,
crate::entry::MAP_ENTRY_INFO,
crate::enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT_INFO,
crate::enum_variants::ENUM_VARIANT_NAMES_INFO,
crate::enum_variants::MODULE_INCEPTION_INFO,
crate::enum_variants::MODULE_NAME_REPETITIONS_INFO,
crate::equatable_if_let::EQUATABLE_IF_LET_INFO,
crate::error_impl_error::ERROR_IMPL_ERROR_INFO,
crate::escape::BOXED_LOCAL_INFO,
Expand Down Expand Up @@ -226,6 +223,10 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::instant_subtraction::UNCHECKED_DURATION_SUBTRACTION_INFO,
crate::int_plus_one::INT_PLUS_ONE_INFO,
crate::invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS_INFO,
crate::item_name_repetitions::ENUM_VARIANT_NAMES_INFO,
crate::item_name_repetitions::MODULE_INCEPTION_INFO,
crate::item_name_repetitions::MODULE_NAME_REPETITIONS_INFO,
crate::item_name_repetitions::STRUCT_FIELD_NAMES_INFO,
crate::items_after_statements::ITEMS_AFTER_STATEMENTS_INFO,
crate::items_after_test_module::ITEMS_AFTER_TEST_MODULE_INFO,
crate::iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR_INFO,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/functions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ declare_clippy_lint! {
}

#[derive(Copy, Clone)]
#[allow(clippy::struct_field_names)]
pub struct Functions {
too_many_arguments_threshold: u64,
too_many_lines_threshold: u64,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
//! lint on enum variants that are prefixed or suffixed by the same characters

use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_hir};
use clippy_utils::macros::span_is_local;
use clippy_utils::source::is_present_in_source;
use clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start};
use rustc_hir::{EnumDef, Item, ItemKind, OwnerId, Variant};
use clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start, to_camel_case, to_snake_case};
use rustc_hir::{EnumDef, FieldDef, Item, ItemKind, OwnerId, Variant, VariantData};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::Span;
Expand Down Expand Up @@ -103,32 +104,184 @@ declare_clippy_lint! {
style,
"modules that have the same name as their parent module"
}
declare_clippy_lint! {
/// ### What it does
/// Detects struct fields that are prefixed or suffixed
/// by the same characters or the name of the struct itself.
///
/// ### Why is this bad?
/// Information common to all struct fields is better represented in the struct name.
///
/// ### Limitations
/// Characters with no casing will be considered when comparing prefixes/suffixes
/// This applies to numbers and non-ascii characters without casing
/// e.g. `foo1` and `foo2` is considered to have different prefixes
/// (the prefixes are `foo1` and `foo2` respectively), as also `bar螃`, `bar蟹`
///
/// ### Example
/// ```rust
/// struct Cake {
/// cake_sugar: u8,
/// cake_flour: u8,
/// cake_eggs: u8
/// }
/// ```
/// Use instead:
/// ```rust
/// struct Cake {
/// sugar: u8,
/// flour: u8,
/// eggs: u8
/// }
/// ```
#[clippy::version = "1.75.0"]
pub STRUCT_FIELD_NAMES,
pedantic,
"structs where all fields share a prefix/postfix or contain the name of the struct"
}

pub struct EnumVariantNames {
pub struct ItemNameRepetitions {
modules: Vec<(Symbol, String, OwnerId)>,
threshold: u64,
enum_threshold: u64,
struct_threshold: u64,
avoid_breaking_exported_api: bool,
allow_private_module_inception: bool,
}

impl EnumVariantNames {
impl ItemNameRepetitions {
#[must_use]
pub fn new(threshold: u64, avoid_breaking_exported_api: bool, allow_private_module_inception: bool) -> Self {
pub fn new(
enum_threshold: u64,
struct_threshold: u64,
avoid_breaking_exported_api: bool,
allow_private_module_inception: bool,
) -> Self {
Self {
modules: Vec::new(),
threshold,
enum_threshold,
struct_threshold,
avoid_breaking_exported_api,
allow_private_module_inception,
}
}
}

impl_lint_pass!(EnumVariantNames => [
impl_lint_pass!(ItemNameRepetitions => [
ENUM_VARIANT_NAMES,
STRUCT_FIELD_NAMES,
MODULE_NAME_REPETITIONS,
MODULE_INCEPTION
]);

#[must_use]
fn have_no_extra_prefix(prefixes: &[&str]) -> bool {
prefixes.iter().all(|p| p == &"" || p == &"_")
}

fn check_fields(cx: &LateContext<'_>, threshold: u64, item: &Item<'_>, fields: &[FieldDef<'_>]) {
if (fields.len() as u64) < threshold {
return;
}

check_struct_name_repetition(cx, item, fields);

// if the SyntaxContext of the identifiers of the fields and struct differ dont lint them.
// this prevents linting in macros in which the location of the field identifier names differ
if !fields.iter().all(|field| item.ident.span.eq_ctxt(field.ident.span)) {
return;
}

let mut pre: Vec<&str> = match fields.first() {
Some(first_field) => first_field.ident.name.as_str().split('_').collect(),
None => return,
};
let mut post = pre.clone();
post.reverse();
for field in fields {
let field_split: Vec<&str> = field.ident.name.as_str().split('_').collect();
if field_split.len() == 1 {
return;
}

pre = pre
.into_iter()
.zip(field_split.iter())
.take_while(|(a, b)| &a == b)
.map(|e| e.0)
.collect();
post = post
.into_iter()
.zip(field_split.iter().rev())
.take_while(|(a, b)| &a == b)
.map(|e| e.0)
.collect();
}
let prefix = pre.join("_");
post.reverse();
let postfix = match post.last() {
Some(&"") => post.join("_") + "_",
Some(_) | None => post.join("_"),
};
if fields.len() > 1 {
let (what, value) = match (
prefix.is_empty() || prefix.chars().all(|c| c == '_'),
postfix.is_empty(),
) {
(true, true) => return,
(false, _) => ("pre", prefix),
(true, false) => ("post", postfix),
};
span_lint_and_help(
cx,
STRUCT_FIELD_NAMES,
item.span,
&format!("all fields have the same {what}fix: `{value}`"),
None,
&format!("remove the {what}fixes"),
);
}
}

fn check_struct_name_repetition(cx: &LateContext<'_>, item: &Item<'_>, fields: &[FieldDef<'_>]) {
let snake_name = to_snake_case(item.ident.name.as_str());
let item_name_words: Vec<&str> = snake_name.split('_').collect();
for field in fields {
if field.ident.span.eq_ctxt(item.ident.span) {
//consider linting only if the field identifier has the same SyntaxContext as the item(struct)
let field_words: Vec<&str> = field.ident.name.as_str().split('_').collect();
if field_words.len() >= item_name_words.len() {
// if the field name is shorter than the struct name it cannot contain it
if field_words.iter().zip(item_name_words.iter()).all(|(a, b)| a == b) {
span_lint_hir(
cx,
STRUCT_FIELD_NAMES,
field.hir_id,
field.span,
"field name starts with the struct's name",
);
}
if field_words.len() > item_name_words.len() {
// lint only if the end is not covered by the start
if field_words
.iter()
.rev()
.zip(item_name_words.iter().rev())
.all(|(a, b)| a == b)
{
span_lint_hir(
cx,
STRUCT_FIELD_NAMES,
field.hir_id,
field.span,
"field name ends with the struct's name",
);
}
}
}
}
}
}

fn check_enum_start(cx: &LateContext<'_>, item_name: &str, variant: &Variant<'_>) {
let name = variant.ident.name.as_str();
let item_name_chars = item_name.chars().count();
Expand Down Expand Up @@ -218,35 +371,7 @@ fn check_variant(cx: &LateContext<'_>, threshold: u64, def: &EnumDef<'_>, item_n
);
}

#[must_use]
fn have_no_extra_prefix(prefixes: &[&str]) -> bool {
prefixes.iter().all(|p| p == &"" || p == &"_")
}

#[must_use]
fn to_camel_case(item_name: &str) -> String {
let mut s = String::new();
let mut up = true;
for c in item_name.chars() {
if c.is_uppercase() {
// we only turn snake case text into CamelCase
return item_name.to_string();
}
if c == '_' {
up = true;
continue;
}
if up {
up = false;
s.extend(c.to_uppercase());
} else {
s.push(c);
}
}
s
}

impl LateLintPass<'_> for EnumVariantNames {
impl LateLintPass<'_> for ItemNameRepetitions {
fn check_item_post(&mut self, _cx: &LateContext<'_>, _item: &Item<'_>) {
let last = self.modules.pop();
assert!(last.is_some());
Expand Down Expand Up @@ -303,9 +428,15 @@ impl LateLintPass<'_> for EnumVariantNames {
}
}
}
if let ItemKind::Enum(ref def, _) = item.kind {
if !(self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(item.owner_id.def_id)) {
check_variant(cx, self.threshold, def, item_name, item.span);
if !(self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(item.owner_id.def_id))
&& span_is_local(item.span)
{
match item.kind {
ItemKind::Enum(def, _) => check_variant(cx, self.enum_threshold, &def, item_name, item.span),
ItemKind::Struct(VariantData::Struct(fields, _), _) => {
check_fields(cx, self.struct_threshold, item, fields);
},
_ => (),
}
}
self.modules.push((item.ident.name, item_camel, item.owner_id));
Expand Down
6 changes: 4 additions & 2 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ mod empty_structs_with_brackets;
mod endian_bytes;
mod entry;
mod enum_clike;
mod enum_variants;
mod equatable_if_let;
mod error_impl_error;
mod escape;
Expand Down Expand Up @@ -166,6 +165,7 @@ mod inline_fn_without_body;
mod instant_subtraction;
mod int_plus_one;
mod invalid_upcast_comparisons;
mod item_name_repetitions;
mod items_after_statements;
mod items_after_test_module;
mod iter_not_returning_iterator;
Expand Down Expand Up @@ -852,10 +852,12 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
))
});
let enum_variant_name_threshold = conf.enum_variant_name_threshold;
let struct_field_name_threshold = conf.struct_field_name_threshold;
let allow_private_module_inception = conf.allow_private_module_inception;
store.register_late_pass(move |_| {
Box::new(enum_variants::EnumVariantNames::new(
Box::new(item_name_repetitions::ItemNameRepetitions::new(
enum_variant_name_threshold,
struct_field_name_threshold,
avoid_breaking_exported_api,
allow_private_module_inception,
))
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/only_used_in_recursion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ impl Usage {
/// The parameters being checked by the lint, indexed by both the parameter's `HirId` and the
/// `DefId` of the function paired with the parameter's index.
#[derive(Default)]
#[allow(clippy::struct_field_names)]
struct Params {
params: Vec<Param>,
by_id: HirIdMap<usize>,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ impl Types {
}
}

#[allow(clippy::struct_excessive_bools)]
#[allow(clippy::struct_excessive_bools, clippy::struct_field_names)]
#[derive(Clone, Copy, Default)]
struct CheckTyContext {
is_in_trait_impl: bool,
Expand Down
4 changes: 4 additions & 0 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,10 @@ define_Conf! {
///
/// The minimum number of enum variants for the lints about variant names to trigger
(enum_variant_name_threshold: u64 = 3),
/// Lint: STRUCT_VARIANT_NAMES.
///
/// The minimum number of struct fields for the lints about field names to trigger
(struct_field_name_threshold: u64 = 3),
/// Lint: LARGE_ENUM_VARIANT.
///
/// The maximum size of an enum's variant to avoid box suggestion
Expand Down
Loading