From 72c7cd3869d614dab9bd525f36934cf34951501d Mon Sep 17 00:00:00 2001
From: Prajwal S N <prajwalnadig21@gmail.com>
Date: Thu, 3 Apr 2025 19:56:14 +0530
Subject: [PATCH] chore: clean up some FIXMEs

Signed-off-by: Prajwal S N <prajwalnadig21@gmail.com>
---
 crates/mbe/src/expander/transcriber.rs | 11 +++---
 crates/syntax/src/syntax_editor.rs     | 12 +-----
 xtask/src/codegen/grammar.rs           |  5 +--
 xtask/src/tidy.rs                      | 55 ++++++++++++--------------
 4 files changed, 36 insertions(+), 47 deletions(-)

diff --git a/crates/mbe/src/expander/transcriber.rs b/crates/mbe/src/expander/transcriber.rs
index b1f542eac7..1459243b89 100644
--- a/crates/mbe/src/expander/transcriber.rs
+++ b/crates/mbe/src/expander/transcriber.rs
@@ -210,8 +210,11 @@ fn expand_subtree(
             }
             Op::Ignore { name, id } => {
                 // Expand the variable, but ignore the result. This registers the repetition count.
-                // FIXME: Any emitted errors are dropped.
-                let _ = ctx.bindings.get_fragment(name, *id, &mut ctx.nesting, marker);
+                let e = ctx.bindings.get_fragment(name, *id, &mut ctx.nesting, marker).err();
+                // FIXME: The error gets dropped if there were any previous errors.
+                // This should be reworked in a way where the errors can be combined
+                // and reported rather than storing the first error encountered.
+                err = err.or(e);
             }
             Op::Index { depth } => {
                 let index =
@@ -239,9 +242,7 @@ fn expand_subtree(
                 let mut binding = match ctx.bindings.get(name, ctx.call_site) {
                     Ok(b) => b,
                     Err(e) => {
-                        if err.is_none() {
-                            err = Some(e);
-                        }
+                        err = err.or(Some(e));
                         continue;
                     }
                 };
diff --git a/crates/syntax/src/syntax_editor.rs b/crates/syntax/src/syntax_editor.rs
index 15515dd1fe..1f722720d4 100644
--- a/crates/syntax/src/syntax_editor.rs
+++ b/crates/syntax/src/syntax_editor.rs
@@ -385,7 +385,7 @@ mod tests {
     use expect_test::expect;
 
     use crate::{
-        AstNode, SyntaxKind,
+        AstNode,
         ast::{self, make, syntax_factory::SyntaxFactory},
     };
 
@@ -631,20 +631,12 @@ mod tests {
         }
 
         if let Some(tail) = parent_fn.body().unwrap().tail_expr() {
-            // FIXME: We do this because `xtask tidy` will not allow us to have trailing whitespace in the expect string.
-            if let Some(SyntaxElement::Token(token)) = tail.syntax().prev_sibling_or_token() {
-                if let SyntaxKind::WHITESPACE = token.kind() {
-                    editor.delete(token);
-                }
-            }
             editor.delete(tail.syntax().clone());
         }
 
         let edit = editor.finish();
 
-        let expect = expect![[r#"
-fn it() {
-}"#]];
+        let expect = expect![["fn it() {\n    \n}"]];
         expect.assert_eq(&edit.new_root.to_string());
     }
 }
diff --git a/xtask/src/codegen/grammar.rs b/xtask/src/codegen/grammar.rs
index bdba1b348d..82df78c1a8 100644
--- a/xtask/src/codegen/grammar.rs
+++ b/xtask/src/codegen/grammar.rs
@@ -414,9 +414,8 @@ fn generate_nodes(kinds: KindsSrc, grammar: &AstSrc) -> String {
         .map(|kind| to_pascal_case(kind))
         .filter(|name| !defined_nodes.iter().any(|&it| it == name))
     {
-        drop(node)
-        // FIXME: restore this
-        // eprintln!("Warning: node {} not defined in ast source", node);
+        eprintln!("Warning: node {} not defined in AST source", node);
+        drop(node);
     }
 
     let ast = quote! {
diff --git a/xtask/src/tidy.rs b/xtask/src/tidy.rs
index 343f76f647..c11d1c4247 100644
--- a/xtask/src/tidy.rs
+++ b/xtask/src/tidy.rs
@@ -126,31 +126,28 @@ fn check_cargo_toml(path: &Path, text: String) {
 }
 
 fn check_licenses(sh: &Shell) {
-    let expected = "
-(MIT OR Apache-2.0) AND Unicode-3.0
-0BSD OR MIT OR Apache-2.0
-Apache-2.0
-Apache-2.0 OR BSL-1.0
-Apache-2.0 OR MIT
-Apache-2.0 WITH LLVM-exception
-Apache-2.0 WITH LLVM-exception OR Apache-2.0 OR MIT
-Apache-2.0/MIT
-CC0-1.0
-ISC
-MIT
-MIT / Apache-2.0
-MIT OR Apache-2.0
-MIT OR Zlib OR Apache-2.0
-MIT/Apache-2.0
-MPL-2.0
-Unicode-3.0
-Unlicense OR MIT
-Unlicense/MIT
-Zlib
-"
-    .lines()
-    .filter(|it| !it.is_empty())
-    .collect::<Vec<_>>();
+    const EXPECTED: [&str; 20] = [
+        "(MIT OR Apache-2.0) AND Unicode-3.0",
+        "0BSD OR MIT OR Apache-2.0",
+        "Apache-2.0",
+        "Apache-2.0 OR BSL-1.0",
+        "Apache-2.0 OR MIT",
+        "Apache-2.0 WITH LLVM-exception",
+        "Apache-2.0 WITH LLVM-exception OR Apache-2.0 OR MIT",
+        "Apache-2.0/MIT",
+        "CC0-1.0",
+        "ISC",
+        "MIT",
+        "MIT / Apache-2.0",
+        "MIT OR Apache-2.0",
+        "MIT OR Zlib OR Apache-2.0",
+        "MIT/Apache-2.0",
+        "MPL-2.0",
+        "Unicode-3.0",
+        "Unlicense OR MIT",
+        "Unlicense/MIT",
+        "Zlib",
+    ];
 
     let meta = cmd!(sh, "cargo metadata --format-version 1").read().unwrap();
     let mut licenses = meta
@@ -161,18 +158,18 @@ Zlib
         .collect::<Vec<_>>();
     licenses.sort_unstable();
     licenses.dedup();
-    if licenses != expected {
+    if licenses != EXPECTED {
         let mut diff = String::new();
 
         diff.push_str("New Licenses:\n");
         for &l in licenses.iter() {
-            if !expected.contains(&l) {
+            if !EXPECTED.contains(&l) {
                 diff += &format!("  {l}\n")
             }
         }
 
         diff.push_str("\nMissing Licenses:\n");
-        for &l in expected.iter() {
+        for l in EXPECTED {
             if !licenses.contains(&l) {
                 diff += &format!("  {l}\n")
             }
@@ -180,7 +177,7 @@ Zlib
 
         panic!("different set of licenses!\n{diff}");
     }
-    assert_eq!(licenses, expected);
+    assert_eq!(licenses, EXPECTED);
 }
 
 fn check_test_attrs(path: &Path, text: &str) {