|
1 | 1 | //! lint on enum variants that are prefixed or suffixed by the same characters
|
2 | 2 |
|
3 | 3 | use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_hir};
|
| 4 | +use clippy_utils::macros::span_is_local; |
4 | 5 | use clippy_utils::source::is_present_in_source;
|
5 |
| -use clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start}; |
6 |
| -use rustc_hir::{EnumDef, Item, ItemKind, OwnerId, Variant}; |
| 6 | +use clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start, to_camel_case, to_snake_case}; |
| 7 | +use rustc_hir::{EnumDef, FieldDef, Item, ItemKind, OwnerId, Variant, VariantData}; |
7 | 8 | use rustc_lint::{LateContext, LateLintPass};
|
8 | 9 | use rustc_session::{declare_tool_lint, impl_lint_pass};
|
9 | 10 | use rustc_span::source_map::Span;
|
@@ -103,32 +104,184 @@ declare_clippy_lint! {
|
103 | 104 | style,
|
104 | 105 | "modules that have the same name as their parent module"
|
105 | 106 | }
|
| 107 | +declare_clippy_lint! { |
| 108 | + /// ### What it does |
| 109 | + /// Detects struct fields that are prefixed or suffixed |
| 110 | + /// by the same characters or the name of the struct itself. |
| 111 | + /// |
| 112 | + /// ### Why is this bad? |
| 113 | + /// Information common to all struct fields is better represented in the struct name. |
| 114 | + /// |
| 115 | + /// ### Limitations |
| 116 | + /// Characters with no casing will be considered when comparing prefixes/suffixes |
| 117 | + /// This applies to numbers and non-ascii characters without casing |
| 118 | + /// e.g. `foo1` and `foo2` is considered to have different prefixes |
| 119 | + /// (the prefixes are `foo1` and `foo2` respectively), as also `bar螃`, `bar蟹` |
| 120 | + /// |
| 121 | + /// ### Example |
| 122 | + /// ```rust |
| 123 | + /// struct Cake { |
| 124 | + /// cake_sugar: u8, |
| 125 | + /// cake_flour: u8, |
| 126 | + /// cake_eggs: u8 |
| 127 | + /// } |
| 128 | + /// ``` |
| 129 | + /// Use instead: |
| 130 | + /// ```rust |
| 131 | + /// struct Cake { |
| 132 | + /// sugar: u8, |
| 133 | + /// flour: u8, |
| 134 | + /// eggs: u8 |
| 135 | + /// } |
| 136 | + /// ``` |
| 137 | + #[clippy::version = "1.75.0"] |
| 138 | + pub STRUCT_FIELD_NAMES, |
| 139 | + pedantic, |
| 140 | + "structs where all fields share a prefix/postfix or contain the name of the struct" |
| 141 | +} |
106 | 142 |
|
107 |
| -pub struct EnumVariantNames { |
| 143 | +pub struct ItemNameRepetitions { |
108 | 144 | modules: Vec<(Symbol, String, OwnerId)>,
|
109 |
| - threshold: u64, |
| 145 | + enum_threshold: u64, |
| 146 | + struct_threshold: u64, |
110 | 147 | avoid_breaking_exported_api: bool,
|
111 | 148 | allow_private_module_inception: bool,
|
112 | 149 | }
|
113 | 150 |
|
114 |
| -impl EnumVariantNames { |
| 151 | +impl ItemNameRepetitions { |
115 | 152 | #[must_use]
|
116 |
| - pub fn new(threshold: u64, avoid_breaking_exported_api: bool, allow_private_module_inception: bool) -> Self { |
| 153 | + pub fn new( |
| 154 | + enum_threshold: u64, |
| 155 | + struct_threshold: u64, |
| 156 | + avoid_breaking_exported_api: bool, |
| 157 | + allow_private_module_inception: bool, |
| 158 | + ) -> Self { |
117 | 159 | Self {
|
118 | 160 | modules: Vec::new(),
|
119 |
| - threshold, |
| 161 | + enum_threshold, |
| 162 | + struct_threshold, |
120 | 163 | avoid_breaking_exported_api,
|
121 | 164 | allow_private_module_inception,
|
122 | 165 | }
|
123 | 166 | }
|
124 | 167 | }
|
125 | 168 |
|
126 |
| -impl_lint_pass!(EnumVariantNames => [ |
| 169 | +impl_lint_pass!(ItemNameRepetitions => [ |
127 | 170 | ENUM_VARIANT_NAMES,
|
| 171 | + STRUCT_FIELD_NAMES, |
128 | 172 | MODULE_NAME_REPETITIONS,
|
129 | 173 | MODULE_INCEPTION
|
130 | 174 | ]);
|
131 | 175 |
|
| 176 | +#[must_use] |
| 177 | +fn have_no_extra_prefix(prefixes: &[&str]) -> bool { |
| 178 | + prefixes.iter().all(|p| p == &"" || p == &"_") |
| 179 | +} |
| 180 | + |
| 181 | +fn check_fields(cx: &LateContext<'_>, threshold: u64, item: &Item<'_>, fields: &[FieldDef<'_>]) { |
| 182 | + if (fields.len() as u64) < threshold { |
| 183 | + return; |
| 184 | + } |
| 185 | + |
| 186 | + check_struct_name_repetition(cx, item, fields); |
| 187 | + |
| 188 | + // if the SyntaxContext of the identifiers of the fields and struct differ dont lint them. |
| 189 | + // this prevents linting in macros in which the location of the field identifier names differ |
| 190 | + if !fields.iter().all(|field| item.ident.span.eq_ctxt(field.ident.span)) { |
| 191 | + return; |
| 192 | + } |
| 193 | + |
| 194 | + let mut pre: Vec<&str> = match fields.first() { |
| 195 | + Some(first_field) => first_field.ident.name.as_str().split('_').collect(), |
| 196 | + None => return, |
| 197 | + }; |
| 198 | + let mut post = pre.clone(); |
| 199 | + post.reverse(); |
| 200 | + for field in fields { |
| 201 | + let field_split: Vec<&str> = field.ident.name.as_str().split('_').collect(); |
| 202 | + if field_split.len() == 1 { |
| 203 | + return; |
| 204 | + } |
| 205 | + |
| 206 | + pre = pre |
| 207 | + .into_iter() |
| 208 | + .zip(field_split.iter()) |
| 209 | + .take_while(|(a, b)| &a == b) |
| 210 | + .map(|e| e.0) |
| 211 | + .collect(); |
| 212 | + post = post |
| 213 | + .into_iter() |
| 214 | + .zip(field_split.iter().rev()) |
| 215 | + .take_while(|(a, b)| &a == b) |
| 216 | + .map(|e| e.0) |
| 217 | + .collect(); |
| 218 | + } |
| 219 | + let prefix = pre.join("_"); |
| 220 | + post.reverse(); |
| 221 | + let postfix = match post.last() { |
| 222 | + Some(&"") => post.join("_") + "_", |
| 223 | + Some(_) | None => post.join("_"), |
| 224 | + }; |
| 225 | + if fields.len() > 1 { |
| 226 | + let (what, value) = match ( |
| 227 | + prefix.is_empty() || prefix.chars().all(|c| c == '_'), |
| 228 | + postfix.is_empty(), |
| 229 | + ) { |
| 230 | + (true, true) => return, |
| 231 | + (false, _) => ("pre", prefix), |
| 232 | + (true, false) => ("post", postfix), |
| 233 | + }; |
| 234 | + span_lint_and_help( |
| 235 | + cx, |
| 236 | + STRUCT_FIELD_NAMES, |
| 237 | + item.span, |
| 238 | + &format!("all fields have the same {what}fix: `{value}`"), |
| 239 | + None, |
| 240 | + &format!("remove the {what}fixes"), |
| 241 | + ); |
| 242 | + } |
| 243 | +} |
| 244 | + |
| 245 | +fn check_struct_name_repetition(cx: &LateContext<'_>, item: &Item<'_>, fields: &[FieldDef<'_>]) { |
| 246 | + let snake_name = to_snake_case(item.ident.name.as_str()); |
| 247 | + let item_name_words: Vec<&str> = snake_name.split('_').collect(); |
| 248 | + for field in fields { |
| 249 | + if field.ident.span.eq_ctxt(item.ident.span) { |
| 250 | + //consider linting only if the field identifier has the same SyntaxContext as the item(struct) |
| 251 | + let field_words: Vec<&str> = field.ident.name.as_str().split('_').collect(); |
| 252 | + if field_words.len() >= item_name_words.len() { |
| 253 | + // if the field name is shorter than the struct name it cannot contain it |
| 254 | + if field_words.iter().zip(item_name_words.iter()).all(|(a, b)| a == b) { |
| 255 | + span_lint_hir( |
| 256 | + cx, |
| 257 | + STRUCT_FIELD_NAMES, |
| 258 | + field.hir_id, |
| 259 | + field.span, |
| 260 | + "field name starts with the struct's name", |
| 261 | + ); |
| 262 | + } |
| 263 | + if field_words.len() > item_name_words.len() { |
| 264 | + // lint only if the end is not covered by the start |
| 265 | + if field_words |
| 266 | + .iter() |
| 267 | + .rev() |
| 268 | + .zip(item_name_words.iter().rev()) |
| 269 | + .all(|(a, b)| a == b) |
| 270 | + { |
| 271 | + span_lint_hir( |
| 272 | + cx, |
| 273 | + STRUCT_FIELD_NAMES, |
| 274 | + field.hir_id, |
| 275 | + field.span, |
| 276 | + "field name ends with the struct's name", |
| 277 | + ); |
| 278 | + } |
| 279 | + } |
| 280 | + } |
| 281 | + } |
| 282 | + } |
| 283 | +} |
| 284 | + |
132 | 285 | fn check_enum_start(cx: &LateContext<'_>, item_name: &str, variant: &Variant<'_>) {
|
133 | 286 | let name = variant.ident.name.as_str();
|
134 | 287 | let item_name_chars = item_name.chars().count();
|
@@ -218,35 +371,7 @@ fn check_variant(cx: &LateContext<'_>, threshold: u64, def: &EnumDef<'_>, item_n
|
218 | 371 | );
|
219 | 372 | }
|
220 | 373 |
|
221 |
| -#[must_use] |
222 |
| -fn have_no_extra_prefix(prefixes: &[&str]) -> bool { |
223 |
| - prefixes.iter().all(|p| p == &"" || p == &"_") |
224 |
| -} |
225 |
| - |
226 |
| -#[must_use] |
227 |
| -fn to_camel_case(item_name: &str) -> String { |
228 |
| - let mut s = String::new(); |
229 |
| - let mut up = true; |
230 |
| - for c in item_name.chars() { |
231 |
| - if c.is_uppercase() { |
232 |
| - // we only turn snake case text into CamelCase |
233 |
| - return item_name.to_string(); |
234 |
| - } |
235 |
| - if c == '_' { |
236 |
| - up = true; |
237 |
| - continue; |
238 |
| - } |
239 |
| - if up { |
240 |
| - up = false; |
241 |
| - s.extend(c.to_uppercase()); |
242 |
| - } else { |
243 |
| - s.push(c); |
244 |
| - } |
245 |
| - } |
246 |
| - s |
247 |
| -} |
248 |
| - |
249 |
| -impl LateLintPass<'_> for EnumVariantNames { |
| 374 | +impl LateLintPass<'_> for ItemNameRepetitions { |
250 | 375 | fn check_item_post(&mut self, _cx: &LateContext<'_>, _item: &Item<'_>) {
|
251 | 376 | let last = self.modules.pop();
|
252 | 377 | assert!(last.is_some());
|
@@ -303,9 +428,15 @@ impl LateLintPass<'_> for EnumVariantNames {
|
303 | 428 | }
|
304 | 429 | }
|
305 | 430 | }
|
306 |
| - if let ItemKind::Enum(ref def, _) = item.kind { |
307 |
| - if !(self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(item.owner_id.def_id)) { |
308 |
| - check_variant(cx, self.threshold, def, item_name, item.span); |
| 431 | + if !(self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(item.owner_id.def_id)) |
| 432 | + && span_is_local(item.span) |
| 433 | + { |
| 434 | + match item.kind { |
| 435 | + ItemKind::Enum(def, _) => check_variant(cx, self.enum_threshold, &def, item_name, item.span), |
| 436 | + ItemKind::Struct(VariantData::Struct(fields, _), _) => { |
| 437 | + check_fields(cx, self.struct_threshold, item, fields); |
| 438 | + }, |
| 439 | + _ => (), |
309 | 440 | }
|
310 | 441 | }
|
311 | 442 | self.modules.push((item.ident.name, item_camel, item.owner_id));
|
|
0 commit comments