crates/oxc/src/main.rs (3)

380-383: field_to_id returns None – limits query expressiveness

Language::field_to_id is currently unimplemented, which means $foo.bar style field queries in patterns cannot work. Even a partial mapping for the common fields ("body", "argument", …) would greatly improve usability.

Consider wiring oxc_ast::ast::FIELD_MAP (or similar) here, or transparently fall back to a slow string comparison.

---

205-376: Gigantic match table can be generated – keep the source maintainable

Hard-coding 170-plus match arms is error-prone and hard to maintain.
Instead, generate the mapping once:

lazy_static::lazy_static! {
  static ref KIND_TO_ID: HashMap<&'static str, u16> =
    IDS.iter().enumerate().map(|(i, k)| (*k, i as u16)).collect();
}

fn kind_to_id(&self, kind: &str) -> u16 {
  *KIND_TO_ID.get(kind).unwrap_or_else(|| panic!("Unknown kind: {kind}"))
}

This avoids duplicating the data, shrinks the binary a bit, and removes the need to update two places when Oxc adds a new node kind.

---

548-550: do_edit not implemented – AstGrep::replace & incremental features will break

The trait requires an implementation; leaving it as todo!() prevents consumers from performing edits and makes the doc non-compliant with Doc expectations. Even a pessimistic “full re-parse” implementation is preferable.

Would you like assistance in adding a minimal, safe implementation?

crates/oxc/test.ts (1)

1-3: Remove leftover commented-out console log

The commented line adds noise to the sample; if it’s not needed, delete it.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

crates/oxc/src/main.rs:9

• [nitpick] The variable name pat is ambiguous; consider renaming to pattern_str or pattern_arg for clarity.

let pat = args.get(2).expect("Must provide a pattern");

crates/oxc/src/binding.rs:1

• Add unit tests for OxcDoc::try_new, parse error handling, and pattern-building to ensure the Oxc integration is covered.

use ast_grep_core::matcher::{Pattern, PatternBuilder, PatternError};

crates/oxc/src/binding.rs:183

• The panic formatting is incorrect and will fail to compile; use named or positional parameters, e.g., panic!("Unknown kind: {}", kind).

_ => panic!("Unknown kind: {kind}"),

crates/oxc/src/binding.rs (2)

269-274: ⚠️ Potential issue

Critical: Implement start_pos and end_pos methods to prevent runtime panics.

These methods are part of the public SgNode trait contract. Leaving them unimplemented will cause a panic when any code tries to get position information (e.g., for error reporting or code actions).

   fn start_pos(&self) -> ast_grep_core::Position {
-    todo!("not implemented")
+    // Calculate position from span start
+    let pos = self.inner.span.start as usize;
+    // TODO: Implement proper line/column calculation from byte offset
+    ast_grep_core::Position {
+      line: 0,  // Placeholder - needs proper implementation
+      column: pos,
+    }
   }
   fn end_pos(&self) -> ast_grep_core::Position {
-    todo!("not implemented")
+    // Calculate position from span end
+    let pos = self.inner.span.end as usize;
+    // TODO: Implement proper line/column calculation from byte offset
+    ast_grep_core::Position {
+      line: 0,  // Placeholder - needs proper implementation
+      column: pos,
+    }
   }

Alternatively, you could store line/column information during parsing or use a source map to convert byte offsets to positions.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 272-273: crates/oxc/src/binding.rs#L272-L273
Added lines #L272 - L273 were not covered by tests

---

203-227: ⚠️ Potential issue

Fix parent-child relationship ordering issue in convert function.

The current implementation only updates parent's children list if the parent has already been inserted into the pool. This creates incomplete parent-child relationships when nodes are processed in certain orders.

Use a two-pass approach to ensure all parent-child relationships are properly established:

 fn convert(nodes: &AstNodes) -> HashMap<NodeId, InnerOxc> {
-  let mut pool = HashMap::default();
-  for node in nodes.iter() {
-    let id = node.id();
-    let span = node.span();
-    let parent_id = nodes.parent_id(node.id());
-    let kind_id = node.kind().ty() as u16;
-    pool.insert(
-      id,
-      InnerOxc {
-        id,
-        kind_id,
-        parent_id,
-        span,
-        children: vec![],
-      },
-    );
-    if let Some(parent_id) = parent_id {
-      if let Some(parent) = pool.get_mut(&parent_id) {
-        parent.children.push(id);
-      }
-    }
-  }
+  // First pass: Create all nodes
+  let mut pool: HashMap<NodeId, InnerOxc> = nodes
+    .iter()
+    .map(|node| {
+      let id = node.id();
+      (
+        id,
+        InnerOxc {
+          id,
+          kind_id: node.kind().ty() as u16,
+          parent_id: nodes.parent_id(id),
+          span: node.span(),
+          children: Vec::new(),
+        },
+      )
+    })
+    .collect();
+
+  // Second pass: Populate children
+  let node_ids: Vec<NodeId> = pool.keys().copied().collect();
+  for node_id in node_ids {
+    if let Some(parent_id) = pool[&node_id].parent_id {
+      if let Some(parent) = pool.get_mut(&parent_id) {
+        parent.children.push(node_id);
+      }
+    }
+  }
   pool
 }

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 203-224: crates/oxc/src/binding.rs#L203-L224
Added lines #L203 - L224 were not covered by tests

---

[warning] 226-227: crates/oxc/src/binding.rs#L226-L227
Added lines #L226 - L227 were not covered by tests

crates/oxc/src/binding.rs (2)

186-188: Implement field_to_id or document why it returns None.

The method always returns None, which might limit field-based pattern matching capabilities. If this is intentional, please add a comment explaining why fields are not supported.

Would you like me to help implement field mapping for Oxc AST nodes if field-based matching is needed?

---

357-359: Implement the do_edit method or document its limitations.

The method is marked as todo!() which will panic if called. This limits the usefulness of the Oxc integration for code modification scenarios.

Would you like me to help implement the editing functionality? This would involve:

1. Re-parsing the modified source
2. Updating the AST node pool
3. Maintaining node ID stability where possible

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: HerringtonDarkholme
PR: ast-grep/ast-grep#1497
File: crates/cli/src/utils/mod.rs:174-175
Timestamp: 2024-09-29T21:59:50.675Z
Learning: ast-grep requires Rust 1.65 or newer. It's acceptable to use syntax and features introduced in Rust 1.65 or newer in ast-grep code.