diff options
| author | Nicholas Nethercote <n.nethercote@gmail.com> | 2025-03-21 09:47:43 +1100 |
|---|---|---|
| committer | Nicholas Nethercote <n.nethercote@gmail.com> | 2025-04-01 14:08:57 +1100 |
| commit | 5101c8e87f6e307c618576f76281e011372a65a4 (patch) | |
| tree | a4d7ac29faf93086a0e133a06845d0db6273c9e0 | |
| parent | 5791b9c4a43da20d846f00cebd6a1007240a7ec7 (diff) | |
| download | rust-5101c8e87f6e307c618576f76281e011372a65a4.tar.gz rust-5101c8e87f6e307c618576f76281e011372a65a4.zip | |
Move `ast::Item::ident` into `ast::ItemKind`.
`ast::Item` has an `ident` field. - It's always non-empty for these item kinds: `ExternCrate`, `Static`, `Const`, `Fn`, `Mod`, `TyAlias`, `Enum`, `Struct`, `Union`, `Trait`, `TraitAlias`, `MacroDef`, `Delegation`. - It's always empty for these item kinds: `Use`, `ForeignMod`, `GlobalAsm`, `Impl`, `MacCall`, `DelegationMac`. There is a similar story for `AssocItemKind` and `ForeignItemKind`. Some sites that handle items check for an empty ident, some don't. This is a very C-like way of doing things, but this is Rust, we have sum types, we can do this properly and never forget to check for the exceptional case and never YOLO possibly empty identifiers (or possibly dummy spans) around and hope that things will work out. The commit is large but it's mostly obvious plumbing work. Some notable things. - `ast::Item` got 8 bytes bigger. This could be avoided by boxing the fields within some of the `ast::ItemKind` variants (specifically: `Struct`, `Union`, `Enum`). I might do that in a follow-up; this commit is big enough already. - For the visitors: `FnKind` no longer needs an `ident` field because the `Fn` within how has one. - In the parser, the `ItemInfo` typedef is no longer needed. It was used in various places to return an `Ident` alongside an `ItemKind`, but now the `Ident` (if present) is within the `ItemKind`. - In a few places I renamed identifier variables called `name` (or `foo_name`) as `ident` (or `foo_ident`), to better match the type, and because `name` is normally used for `Symbol`s. It's confusing to see something like `foo_name.name`.
| -rw-r--r-- | clippy_lints/src/crate_in_macro_def.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/doc/needless_doctest_main.rs | 16 | ||||
| -rw-r--r-- | clippy_lints/src/duplicate_mod.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/empty_line_after.rs | 16 | ||||
| -rw-r--r-- | clippy_lints/src/empty_with_brackets.rs | 5 | ||||
| -rw-r--r-- | clippy_lints/src/field_scoped_visibility_modifiers.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/multiple_bound_locations.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/partial_pub_fields.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/single_component_path_imports.rs | 6 | ||||
| -rw-r--r-- | clippy_utils/src/ast_utils/mod.rs | 79 |
10 files changed, 89 insertions, 43 deletions
diff --git a/clippy_lints/src/crate_in_macro_def.rs b/clippy_lints/src/crate_in_macro_def.rs index 7d86bd3e540..c2aac7ca090 100644 --- a/clippy_lints/src/crate_in_macro_def.rs +++ b/clippy_lints/src/crate_in_macro_def.rs @@ -53,7 +53,7 @@ declare_lint_pass!(CrateInMacroDef => [CRATE_IN_MACRO_DEF]); impl EarlyLintPass for CrateInMacroDef { fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { - if let ItemKind::MacroDef(macro_def) = &item.kind + if let ItemKind::MacroDef(_, macro_def) = &item.kind && item.attrs.iter().any(is_macro_export) && let Some(span) = contains_unhygienic_crate_reference(¯o_def.body.tokens) { diff --git a/clippy_lints/src/doc/needless_doctest_main.rs b/clippy_lints/src/doc/needless_doctest_main.rs index 3008082c232..f6c10da1596 100644 --- a/clippy_lints/src/doc/needless_doctest_main.rs +++ b/clippy_lints/src/doc/needless_doctest_main.rs @@ -13,16 +13,16 @@ use rustc_parse::parser::ForceCollect; use rustc_session::parse::ParseSess; use rustc_span::edition::Edition; use rustc_span::source_map::{FilePathMapping, SourceMap}; -use rustc_span::{FileName, Pos, sym}; +use rustc_span::{FileName, Ident, Pos, sym}; use super::Fragments; -fn get_test_spans(item: &Item, test_attr_spans: &mut Vec<Range<usize>>) { +fn get_test_spans(item: &Item, ident: Ident, test_attr_spans: &mut Vec<Range<usize>>) { test_attr_spans.extend( item.attrs .iter() .find(|attr| attr.has_name(sym::test)) - .map(|attr| attr.span.lo().to_usize()..item.ident.span.hi().to_usize()), + .map(|attr| attr.span.lo().to_usize()..ident.span.hi().to_usize()), ); } @@ -64,10 +64,10 @@ pub fn check( match parser.parse_item(ForceCollect::No) { Ok(Some(item)) => match &item.kind { ItemKind::Fn(box Fn { - sig, body: Some(block), .. - }) if item.ident.name == sym::main => { + ident, sig, body: Some(block), .. + }) if ident.name == sym::main => { if !ignore { - get_test_spans(&item, &mut test_attr_spans); + get_test_spans(&item, *ident, &mut test_attr_spans); } let is_async = matches!(sig.header.coroutine_kind, Some(CoroutineKind::Async { .. })); let returns_nothing = match &sig.decl.output { @@ -85,10 +85,10 @@ pub fn check( } }, // Another function was found; this case is ignored for needless_doctest_main - ItemKind::Fn(box Fn { .. }) => { + ItemKind::Fn(fn_) => { eligible = false; if !ignore { - get_test_spans(&item, &mut test_attr_spans); + get_test_spans(&item, fn_.ident, &mut test_attr_spans); } }, // Tests with one of these items are ignored diff --git a/clippy_lints/src/duplicate_mod.rs b/clippy_lints/src/duplicate_mod.rs index 1dac7b971f9..243c99a19ce 100644 --- a/clippy_lints/src/duplicate_mod.rs +++ b/clippy_lints/src/duplicate_mod.rs @@ -63,7 +63,7 @@ impl_lint_pass!(DuplicateMod => [DUPLICATE_MOD]); impl EarlyLintPass for DuplicateMod { fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { - if let ItemKind::Mod(_, ModKind::Loaded(_, Inline::No, mod_spans, _)) = &item.kind + if let ItemKind::Mod(_, _, ModKind::Loaded(_, Inline::No, mod_spans, _)) = &item.kind && let FileName::Real(real) = cx.sess().source_map().span_to_filename(mod_spans.inner_span) && let Some(local_path) = real.into_local_path() && let Ok(absolute_path) = local_path.canonicalize() diff --git a/clippy_lints/src/empty_line_after.rs b/clippy_lints/src/empty_line_after.rs index 80c2b03c41c..899b5c59526 100644 --- a/clippy_lints/src/empty_line_after.rs +++ b/clippy_lints/src/empty_line_after.rs @@ -9,7 +9,7 @@ use rustc_lexer::TokenKind; use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; use rustc_session::impl_lint_pass; use rustc_span::symbol::kw; -use rustc_span::{BytePos, ExpnKind, Ident, InnerSpan, Span, SpanData, Symbol}; +use rustc_span::{BytePos, ExpnKind, Ident, InnerSpan, Span, SpanData, Symbol, sym}; declare_clippy_lint! { /// ### What it does @@ -375,21 +375,21 @@ impl EmptyLineAfter { &mut self, cx: &EarlyContext<'_>, kind: &ItemKind, - ident: &Ident, + ident: &Option<Ident>, span: Span, attrs: &[Attribute], id: NodeId, ) { self.items.push(ItemInfo { kind: kind.descr(), - name: ident.name, - span: if span.contains(ident.span) { + name: if let Some(ident) = ident { ident.name } else { sym::dummy }, + span: if let Some(ident) = ident { span.with_hi(ident.span.hi()) } else { span.with_hi(span.lo()) }, mod_items: match kind { - ItemKind::Mod(_, ModKind::Loaded(items, _, _, _)) => items + ItemKind::Mod(_, _, ModKind::Loaded(items, _, _, _)) => items .iter() .filter(|i| !matches!(i.span.ctxt().outer_expn_data().kind, ExpnKind::AstPass(_))) .map(|i| i.id) @@ -471,7 +471,7 @@ impl EarlyLintPass for EmptyLineAfter { self.check_item_kind( cx, &item.kind.clone().into(), - &item.ident, + &item.kind.ident(), item.span, &item.attrs, item.id, @@ -482,7 +482,7 @@ impl EarlyLintPass for EmptyLineAfter { self.check_item_kind( cx, &item.kind.clone().into(), - &item.ident, + &item.kind.ident(), item.span, &item.attrs, item.id, @@ -490,6 +490,6 @@ impl EarlyLintPass for EmptyLineAfter { } fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { - self.check_item_kind(cx, &item.kind, &item.ident, item.span, &item.attrs, item.id); + self.check_item_kind(cx, &item.kind, &item.kind.ident(), item.span, &item.attrs, item.id); } } diff --git a/clippy_lints/src/empty_with_brackets.rs b/clippy_lints/src/empty_with_brackets.rs index 743ec5b9ea7..7d87f04fef9 100644 --- a/clippy_lints/src/empty_with_brackets.rs +++ b/clippy_lints/src/empty_with_brackets.rs @@ -74,10 +74,9 @@ declare_lint_pass!(EmptyWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS, EMPTY_ENUM impl EarlyLintPass for EmptyWithBrackets { fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { - let span_after_ident = item.span.with_lo(item.ident.span.hi()); - - if let ItemKind::Struct(var_data, _) = &item.kind + if let ItemKind::Struct(ident, var_data, _) = &item.kind && has_brackets(var_data) + && let span_after_ident = item.span.with_lo(ident.span.hi()) && has_no_fields(cx, var_data, span_after_ident) { span_lint_and_then( diff --git a/clippy_lints/src/field_scoped_visibility_modifiers.rs b/clippy_lints/src/field_scoped_visibility_modifiers.rs index ba2b37fbf11..aae8291905d 100644 --- a/clippy_lints/src/field_scoped_visibility_modifiers.rs +++ b/clippy_lints/src/field_scoped_visibility_modifiers.rs @@ -51,7 +51,7 @@ declare_lint_pass!(FieldScopedVisibilityModifiers => [FIELD_SCOPED_VISIBILITY_MO impl EarlyLintPass for FieldScopedVisibilityModifiers { fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { - let ItemKind::Struct(ref st, _) = item.kind else { + let ItemKind::Struct(_, ref st, _) = item.kind else { return; }; for field in st.fields() { diff --git a/clippy_lints/src/multiple_bound_locations.rs b/clippy_lints/src/multiple_bound_locations.rs index 0e1980a6acb..4b32ba83b32 100644 --- a/clippy_lints/src/multiple_bound_locations.rs +++ b/clippy_lints/src/multiple_bound_locations.rs @@ -39,7 +39,7 @@ declare_lint_pass!(MultipleBoundLocations => [MULTIPLE_BOUND_LOCATIONS]); impl EarlyLintPass for MultipleBoundLocations { fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, _: Span, _: NodeId) { - if let FnKind::Fn(_, _, _, Fn { generics, .. }) = kind + if let FnKind::Fn(_, _, Fn { generics, .. }) = kind && !generics.params.is_empty() && !generics.where_clause.predicates.is_empty() { diff --git a/clippy_lints/src/partial_pub_fields.rs b/clippy_lints/src/partial_pub_fields.rs index 267e2067e10..cda752d003f 100644 --- a/clippy_lints/src/partial_pub_fields.rs +++ b/clippy_lints/src/partial_pub_fields.rs @@ -41,7 +41,7 @@ declare_lint_pass!(PartialPubFields => [PARTIAL_PUB_FIELDS]); impl EarlyLintPass for PartialPubFields { fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { - let ItemKind::Struct(ref st, _) = item.kind else { + let ItemKind::Struct(_, ref st, _) = item.kind else { return; }; diff --git a/clippy_lints/src/single_component_path_imports.rs b/clippy_lints/src/single_component_path_imports.rs index fa082453504..35f80b2acda 100644 --- a/clippy_lints/src/single_component_path_imports.rs +++ b/clippy_lints/src/single_component_path_imports.rs @@ -174,11 +174,11 @@ impl SingleComponentPathImports { } match &item.kind { - ItemKind::Mod(_, ModKind::Loaded(items, ..)) => { + ItemKind::Mod(_, _, ModKind::Loaded(items, ..)) => { self.check_mod(items); }, - ItemKind::MacroDef(MacroDef { macro_rules: true, .. }) => { - macros.push(item.ident.name); + ItemKind::MacroDef(ident, MacroDef { macro_rules: true, .. }) => { + macros.push(ident.name); }, ItemKind::Use(use_tree) => { let segments = &use_tree.prefix.segments; diff --git a/clippy_utils/src/ast_utils/mod.rs b/clippy_utils/src/ast_utils/mod.rs index 6023ae9cc7b..ff63eb505a5 100644 --- a/clippy_utils/src/ast_utils/mod.rs +++ b/clippy_utils/src/ast_utils/mod.rs @@ -321,17 +321,18 @@ pub fn eq_local_kind(l: &LocalKind, r: &LocalKind) -> bool { } pub fn eq_item<K>(l: &Item<K>, r: &Item<K>, mut eq_kind: impl FnMut(&K, &K) -> bool) -> bool { - eq_id(l.ident, r.ident) && over(&l.attrs, &r.attrs, eq_attr) && eq_vis(&l.vis, &r.vis) && eq_kind(&l.kind, &r.kind) + over(&l.attrs, &r.attrs, eq_attr) && eq_vis(&l.vis, &r.vis) && eq_kind(&l.kind, &r.kind) } #[expect(clippy::similar_names, clippy::too_many_lines)] // Just a big match statement pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool { use ItemKind::*; match (l, r) { - (ExternCrate(l), ExternCrate(r)) => l == r, + (ExternCrate(ls, li), ExternCrate(rs, ri)) => ls == rs && eq_id(*li, *ri), (Use(l), Use(r)) => eq_use_tree(l, r), ( Static(box StaticItem { + ident: li, ty: lt, mutability: lm, expr: le, @@ -339,16 +340,22 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool { define_opaque: _, }), Static(box StaticItem { + ident: ri, ty: rt, mutability: rm, expr: re, safety: rs, define_opaque: _, }), - ) => lm == rm && ls == rs && eq_ty(lt, rt) && eq_expr_opt(le.as_ref(), re.as_ref()), + ) => eq_id(*li, *ri) + && lm == rm + && ls == rs + && eq_ty(lt, rt) + && eq_expr_opt(le.as_ref(), re.as_ref()), ( Const(box ConstItem { defaultness: ld, + ident: li, generics: lg, ty: lt, expr: le, @@ -356,16 +363,22 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool { }), Const(box ConstItem { defaultness: rd, + ident: ri, generics: rg, ty: rt, expr: re, define_opaque: _, }), - ) => eq_defaultness(*ld, *rd) && eq_generics(lg, rg) && eq_ty(lt, rt) && eq_expr_opt(le.as_ref(), re.as_ref()), + ) => eq_defaultness(*ld, *rd) + && eq_id(*li, *ri) + && eq_generics(lg, rg) + && eq_ty(lt, rt) + && eq_expr_opt(le.as_ref(), re.as_ref()), ( Fn(box ast::Fn { defaultness: ld, sig: lf, + ident: li, generics: lg, contract: lc, body: lb, @@ -374,6 +387,7 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool { Fn(box ast::Fn { defaultness: rd, sig: rf, + ident: ri, generics: rg, contract: rc, body: rb, @@ -382,12 +396,14 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool { ) => { eq_defaultness(*ld, *rd) && eq_fn_sig(lf, rf) + && eq_id(*li, *ri) && eq_generics(lg, rg) && eq_opt_fn_contract(lc, rc) && both(lb.as_ref(), rb.as_ref(), |l, r| eq_block(l, r)) }, - (Mod(lu, lmk), Mod(ru, rmk)) => { - lu == ru + (Mod(ls, li, lmk), Mod(rs, ri, rmk)) => { + ls == rs + && eq_id(*li, *ri) && match (lmk, rmk) { (ModKind::Loaded(litems, linline, _, _), ModKind::Loaded(ritems, rinline, _, _)) => { linline == rinline && over(litems, ritems, |l, r| eq_item(l, r, eq_item_kind)) @@ -421,33 +437,40 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool { && over(lb, rb, eq_generic_bound) && both(lt.as_ref(), rt.as_ref(), |l, r| eq_ty(l, r)) }, - (Enum(le, lg), Enum(re, rg)) => over(&le.variants, &re.variants, eq_variant) && eq_generics(lg, rg), - (Struct(lv, lg), Struct(rv, rg)) | (Union(lv, lg), Union(rv, rg)) => { - eq_variant_data(lv, rv) && eq_generics(lg, rg) + (Enum(li, le, lg), Enum(ri, re, rg)) => { + eq_id(*li, *ri) && over(&le.variants, &re.variants, eq_variant) && eq_generics(lg, rg) + } + (Struct(li, lv, lg), Struct(ri, rv, rg)) | (Union(li, lv, lg), Union(ri, rv, rg)) => { + eq_id(*li, *ri) && eq_variant_data(lv, rv) && eq_generics(lg, rg) }, ( Trait(box ast::Trait { is_auto: la, safety: lu, + ident: li, generics: lg, bounds: lb, - items: li, + items: lis, }), Trait(box ast::Trait { is_auto: ra, safety: ru, + ident: ri, generics: rg, bounds: rb, - items: ri, + items: ris, }), ) => { la == ra && matches!(lu, Safety::Default) == matches!(ru, Safety::Default) + && eq_id(*li, *ri) && eq_generics(lg, rg) && over(lb, rb, eq_generic_bound) - && over(li, ri, |l, r| eq_item(l, r, eq_assoc_item_kind)) + && over(lis, ris, |l, r| eq_item(l, r, eq_assoc_item_kind)) }, - (TraitAlias(lg, lb), TraitAlias(rg, rb)) => eq_generics(lg, rg) && over(lb, rb, eq_generic_bound), + (TraitAlias(li, lg, lb), TraitAlias(ri, rg, rb)) => { + eq_id(*li, *ri) && eq_generics(lg, rg) && over(lb, rb, eq_generic_bound) + } ( Impl(box ast::Impl { safety: lu, @@ -480,7 +503,9 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool { && over(li, ri, |l, r| eq_item(l, r, eq_assoc_item_kind)) }, (MacCall(l), MacCall(r)) => eq_mac_call(l, r), - (MacroDef(l), MacroDef(r)) => l.macro_rules == r.macro_rules && eq_delim_args(&l.body, &r.body), + (MacroDef(li, ld), MacroDef(ri, rd)) => { + eq_id(*li, *ri) && ld.macro_rules == rd.macro_rules && eq_delim_args(&ld.body, &rd.body) + } _ => false, } } @@ -490,6 +515,7 @@ pub fn eq_foreign_item_kind(l: &ForeignItemKind, r: &ForeignItemKind) -> bool { match (l, r) { ( Static(box StaticItem { + ident: li, ty: lt, mutability: lm, expr: le, @@ -497,17 +523,25 @@ pub fn eq_foreign_item_kind(l: &ForeignItemKind, r: &ForeignItemKind) -> bool { define_opaque: _, }), Static(box StaticItem { + ident: ri, ty: rt, mutability: rm, expr: re, safety: rs, define_opaque: _, }), - ) => lm == rm && eq_ty(lt, rt) && eq_expr_opt(le.as_ref(), re.as_ref()) && ls == rs, + ) => { + eq_id(*li, *ri) + && eq_ty(lt, rt) + && lm == rm + && eq_expr_opt(le.as_ref(), re.as_ref()) + && ls == rs + } ( Fn(box ast::Fn { defaultness: ld, sig: lf, + ident: li, generics: lg, contract: lc, body: lb, @@ -516,6 +550,7 @@ pub fn eq_foreign_item_kind(l: &ForeignItemKind, r: &ForeignItemKind) -> bool { Fn(box ast::Fn { defaultness: rd, sig: rf, + ident: ri, generics: rg, contract: rc, body: rb, @@ -524,6 +559,7 @@ pub fn eq_foreign_item_kind(l: &ForeignItemKind, r: &ForeignItemKind) -> bool { ) => { eq_defaultness(*ld, *rd) && eq_fn_sig(lf, rf) + && eq_id(*li, *ri) && eq_generics(lg, rg) && eq_opt_fn_contract(lc, rc) && both(lb.as_ref(), rb.as_ref(), |l, r| eq_block(l, r)) @@ -560,6 +596,7 @@ pub fn eq_assoc_item_kind(l: &AssocItemKind, r: &AssocItemKind) -> bool { ( Const(box ConstItem { defaultness: ld, + ident: li, generics: lg, ty: lt, expr: le, @@ -567,16 +604,24 @@ pub fn eq_assoc_item_kind(l: &AssocItemKind, r: &AssocItemKind) -> bool { }), Const(box ConstItem { defaultness: rd, + ident: ri, generics: rg, ty: rt, expr: re, define_opaque: _, }), - ) => eq_defaultness(*ld, *rd) && eq_generics(lg, rg) && eq_ty(lt, rt) && eq_expr_opt(le.as_ref(), re.as_ref()), + ) => { + eq_defaultness(*ld, *rd) + && eq_id(*li, *ri) + && eq_generics(lg, rg) + && eq_ty(lt, rt) + && eq_expr_opt(le.as_ref(), re.as_ref()) + } ( Fn(box ast::Fn { defaultness: ld, sig: lf, + ident: li, generics: lg, contract: lc, body: lb, @@ -585,6 +630,7 @@ pub fn eq_assoc_item_kind(l: &AssocItemKind, r: &AssocItemKind) -> bool { Fn(box ast::Fn { defaultness: rd, sig: rf, + ident: ri, generics: rg, contract: rc, body: rb, @@ -593,6 +639,7 @@ pub fn eq_assoc_item_kind(l: &AssocItemKind, r: &AssocItemKind) -> bool { ) => { eq_defaultness(*ld, *rd) && eq_fn_sig(lf, rf) + && eq_id(*li, *ri) && eq_generics(lg, rg) && eq_opt_fn_contract(lc, rc) && both(lb.as_ref(), rb.as_ref(), |l, r| eq_block(l, r)) |
