]> git.lizzy.rs Git - rust.git/commitdiff
Split `MacArgs` in two.
authorNicholas Nethercote <n.nethercote@gmail.com>
Fri, 18 Nov 2022 00:24:21 +0000 (11:24 +1100)
committerNicholas Nethercote <n.nethercote@gmail.com>
Mon, 21 Nov 2022 22:04:15 +0000 (09:04 +1100)
`MacArgs` is an enum with three variants: `Empty`, `Delimited`, and `Eq`. It's
used in two ways:
- For representing attribute macro arguments (e.g. in `AttrItem`), where all
  three variants are used.
- For representing function-like macros (e.g. in `MacCall` and `MacroDef`),
  where only the `Delimited` variant is used.

In other words, `MacArgs` is used in two quite different places due to them
having partial overlap. I find this makes the code hard to read. It also leads
to various unreachable code paths, and allows invalid values (such as
accidentally using `MacArgs::Empty` in a `MacCall`).

This commit splits `MacArgs` in two:
- `DelimArgs` is a new struct just for the "delimited arguments" case. It is
  now used in `MacCall` and `MacroDef`.
- `AttrArgs` is a renaming of the old `MacArgs` enum for the attribute macro
  case. Its `Delimited` variant now contains a `DelimArgs`.

Various other related things are renamed as well.

These changes make the code clearer, avoids several unreachable paths, and
disallows the invalid values.

clippy_lints/src/crate_in_macro_def.rs
clippy_utils/src/ast_utils.rs

index 20cc330e035f8990563c10bdfd313dedcfc15fa5..b2fe0386f945dd82f8531f7643a6e5c06869f75f 100644 (file)
@@ -55,7 +55,7 @@ fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
         if_chain! {
             if item.attrs.iter().any(is_macro_export);
             if let ItemKind::MacroDef(macro_def) = &item.kind;
-            let tts = macro_def.body.inner_tokens();
+            let tts = macro_def.body.tokens.clone();
             if let Some(span) = contains_unhygienic_crate_reference(&tts);
             then {
                 span_lint_and_sugg(
index 23aed4b5ba2f40f2c675009c338736431a5fdd1e..87b378bfd1982517ad2d9e53b600c71ff1e5ff14 100644 (file)
@@ -388,7 +388,7 @@ 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_mac_args(&l.body, &r.body),
+        (MacroDef(l), MacroDef(r)) => l.macro_rules == r.macro_rules && eq_delim_args(&l.body, &r.body),
         _ => false,
     }
 }
@@ -709,7 +709,7 @@ pub fn eq_assoc_constraint(l: &AssocConstraint, r: &AssocConstraint) -> bool {
 }
 
 pub fn eq_mac_call(l: &MacCall, r: &MacCall) -> bool {
-    eq_path(&l.path, &r.path) && eq_mac_args(&l.args, &r.args)
+    eq_path(&l.path, &r.path) && eq_delim_args(&l.args, &r.args)
 }
 
 pub fn eq_attr(l: &Attribute, r: &Attribute) -> bool {
@@ -717,18 +717,22 @@ pub fn eq_attr(l: &Attribute, r: &Attribute) -> bool {
     l.style == r.style
         && match (&l.kind, &r.kind) {
             (DocComment(l1, l2), DocComment(r1, r2)) => l1 == r1 && l2 == r2,
-            (Normal(l), Normal(r)) => eq_path(&l.item.path, &r.item.path) && eq_mac_args(&l.item.args, &r.item.args),
+            (Normal(l), Normal(r)) => eq_path(&l.item.path, &r.item.path) && eq_attr_args(&l.item.args, &r.item.args),
             _ => false,
         }
 }
 
-pub fn eq_mac_args(l: &MacArgs, r: &MacArgs) -> bool {
-    use MacArgs::*;
+pub fn eq_attr_args(l: &AttrArgs, r: &AttrArgs) -> bool {
+    use AttrArgs::*;
     match (l, r) {
         (Empty, Empty) => true,
-        (Delimited(_, ld, lts), Delimited(_, rd, rts)) => ld == rd && lts.eq_unspanned(rts),
-        (Eq(_, MacArgsEq::Ast(le)), Eq(_, MacArgsEq::Ast(re))) => eq_expr(le, re),
-        (Eq(_, MacArgsEq::Hir(ll)), Eq(_, MacArgsEq::Hir(rl))) => ll.kind == rl.kind,
+        (Delimited(la), Delimited(ra)) => eq_delim_args(la, ra),
+        (Eq(_, AttrArgsEq::Ast(le)), Eq(_, AttrArgsEq::Ast(re))) => eq_expr(le, re),
+        (Eq(_, AttrArgsEq::Hir(ll)), Eq(_, AttrArgsEq::Hir(rl))) => ll.kind == rl.kind,
         _ => false,
     }
 }
+
+pub fn eq_delim_args(l: &DelimArgs, r: &DelimArgs) -> bool {
+    l.delim == r.delim && l.tokens.eq_unspanned(&r.tokens)
+}