feat(adapters): add JSON, YAML, TOML, PowerShell language support - #28
feat(adapters): add JSON, YAML, TOML, PowerShell language support#28aspnmy wants to merge 2 commits into
Conversation
- JSON: SupportLang::Json + tree-sitter-json - YAML: SupportLang::Yaml + tree-sitter-yaml - TOML: toml_edit-based parser - PowerShell: regex-based function/class/enum detection 4 new adapters, 2 dispatch files modified.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughAdds four new language adapters — JSON ( ChangesNew Language Adapters and Dispatch Wiring
Minor Iterator and Sort Refactors
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces new parsing adapters for JSON, YAML, TOML, and PowerShell files, integrating them into the main helper utilities. The review feedback highlights several important improvements: correcting hardcoded line numbers and byte offsets in the TOML adapter using the toml_edit .span() API, recursively traversing container nodes in the YAML adapter to prevent empty declarations, and optimizing the PowerShell adapter by statically compiling regular expressions with OnceLock to avoid recompilation on every invocation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| pub fn parse_toml(path: &Path, source: &[u8]) -> ParseResult { | ||
| let line_count = source.iter().filter(|&&b| b == b'\n').count() + 1; | ||
| let mut decls = Vec::new(); | ||
|
|
||
| if let Ok(text) = std::str::from_utf8(source) { | ||
| if let Ok(doc) = text.parse::<toml_edit::ImDocument<String>>() { | ||
| // Walk top-level keys | ||
| for (key, value) in doc.iter() { | ||
| let key_str = key.to_string(); | ||
| let sig = _value_summary(value); | ||
| let decl = Declaration { | ||
| kind: DeclarationKind::Field, | ||
| name: key_str, | ||
| signature: sig, | ||
| bases: Vec::new(), | ||
| attrs: Vec::new(), | ||
| docs: Vec::new(), | ||
| docs_inside: false, | ||
| visibility: String::new(), | ||
| start_line: 1, | ||
| end_line: line_count, | ||
| start_byte: 0, | ||
| end_byte: source.len(), | ||
| doc_start_byte: 0, | ||
| native_kind: None, | ||
| modifiers: Vec::new(), | ||
| deprecated: false, | ||
| children: _table_children(value), | ||
| calls: Vec::new(), | ||
| }; | ||
| decls.push(decl); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| ParseResult { | ||
| path: path.to_path_buf(), | ||
| language: "toml", | ||
| source: source.to_vec(), | ||
| line_count, | ||
| declarations: decls, | ||
| error_count: 0, | ||
| imports: Vec::new(), | ||
| } | ||
| } |
There was a problem hiding this comment.
Hardcoding the start line to 1 and end line to line_count for all parsed keys is incorrect. We can use toml_edit's .span() API to retrieve the actual byte offsets and map them to correct line numbers.
fn byte_to_line(source: &[u8], byte_offset: usize) -> usize {
let safe_offset = std::cmp::min(byte_offset, source.len());
source[..safe_offset].iter().filter(|&&b| b == b'\n').count() + 1
}
pub fn parse_toml(path: &Path, source: &[u8]) -> ParseResult {
let line_count = source.iter().filter(|&&b| b == b'\n').count() + 1;
let mut decls = Vec::new();
if let Ok(text) = std::str::from_utf8(source) {
if let Ok(doc) = text.parse::<toml_edit::ImDocument<String>>() {
// Walk top-level keys
for (key, value) in doc.iter() {
let key_str = key.to_string();
let sig = _value_summary(value);
let key_span = key.span().unwrap_or(0..0);
let val_span = value.span().unwrap_or(0..0);
let start_byte = key_span.start;
let end_byte = if val_span.end > 0 { val_span.end } else { key_span.end };
let start_line = byte_to_line(source, start_byte);
let end_line = byte_to_line(source, end_byte);
let decl = Declaration {
kind: DeclarationKind::Field,
name: key_str,
signature: sig,
bases: Vec::new(),
attrs: Vec::new(),
docs: Vec::new(),
docs_inside: false,
visibility: String::new(),
start_line,
end_line,
start_byte,
end_byte,
doc_start_byte: start_byte,
native_kind: None,
modifiers: Vec::new(),
deprecated: false,
children: _table_children(value, source),
calls: Vec::new(),
};
decls.push(decl);
}
}
}
ParseResult {
path: path.to_path_buf(),
language: "toml",
source: source.to_vec(),
line_count,
declarations: decls,
error_count: 0,
imports: Vec::new(),
}References
- In this repository, line numbers in parsed symbol structures are 1-indexed, matching standard text editor line numbers, rather than 0-indexed.
| fn _table_children(value: &toml_edit::Item) -> Vec<Declaration> { | ||
| match value { | ||
| toml_edit::Item::Table(table) => { | ||
| let mut children = Vec::new(); | ||
| for (key, val) in table.iter() { | ||
| children.push(Declaration { | ||
| kind: DeclarationKind::Field, | ||
| name: key.to_string(), | ||
| signature: _value_summary(val), | ||
| bases: Vec::new(), | ||
| attrs: Vec::new(), | ||
| docs: Vec::new(), | ||
| docs_inside: false, | ||
| visibility: String::new(), | ||
| start_line: 1, | ||
| end_line: 1, | ||
| start_byte: 0, | ||
| end_byte: 0, | ||
| doc_start_byte: 0, | ||
| native_kind: None, | ||
| modifiers: Vec::new(), | ||
| deprecated: false, | ||
| children: Vec::new(), | ||
| calls: Vec::new(), | ||
| }); | ||
| } | ||
| children | ||
| } | ||
| _ => Vec::new(), | ||
| } | ||
| } |
There was a problem hiding this comment.
Hardcoding the start line and end line to 1 for table children is incorrect. We can pass the source slice and use .span() to compute the correct lines and byte offsets.
fn _table_children(value: &toml_edit::Item, source: &[u8]) -> Vec<Declaration> {
match value {
toml_edit::Item::Table(table) => {
let mut children = Vec::new();
for (key, val) in table.iter() {
let key_span = key.span().unwrap_or(0..0);
let val_span = val.span().unwrap_or(0..0);
let start_byte = key_span.start;
let end_byte = if val_span.end > 0 { val_span.end } else { key_span.end };
let start_line = byte_to_line(source, start_byte);
let end_line = byte_to_line(source, end_byte);
children.push(Declaration {
kind: DeclarationKind::Field,
name: key.to_string(),
signature: _value_summary(val),
bases: Vec::new(),
attrs: Vec::new(),
docs: Vec::new(),
docs_inside: false,
visibility: String::new(),
start_line,
end_line,
start_byte,
end_byte,
doc_start_byte: start_byte,
native_kind: None,
modifiers: Vec::new(),
deprecated: false,
children: Vec::new(),
calls: Vec::new(),
});
}
children
}
_ => Vec::new(),
}References
- In this repository, line numbers in parsed symbol structures are 1-indexed, matching standard text editor line numbers, rather than 0-indexed.
| fn _walk_yaml<'a, D: Doc>(node: &Node<'a, D>, src: &[u8], out: &mut Vec<Declaration>) { | ||
| let kind = node.kind(); | ||
| let kind: &str = kind.as_ref(); | ||
| match kind { | ||
| "block_mapping" | "flow_mapping" | "stream" | "document" => { | ||
| for child in node.children() { | ||
| let ck = child.kind(); | ||
| let ck: &str = ck.as_ref(); | ||
| if ck == "block_mapping_pair" || ck == "flow_pair" { | ||
| if let Some(decl) = _pair_to_decl(&child, src) { | ||
| out.push(decl); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The YAML adapter currently returns empty declarations because it expects mapping pairs to be direct children of stream or document nodes. In tree-sitter-yaml, these are nested inside container nodes like block_node or flow_node. We should recursively traverse these container nodes to find the mapping pairs.
fn _walk_yaml<'a, D: Doc>(node: &Node<'a, D>, src: &[u8], out: &mut Vec<Declaration>) {
let kind = node.kind();
let kind: &str = kind.as_ref();
match kind {
"stream" | "document" | "block_node" | "flow_node" => {
for child in node.children() {
_walk_yaml(&child, src, out);
}
}
"block_mapping" | "flow_mapping" => {
for child in node.children() {
let ck = child.kind();
let ck: &str = ck.as_ref();
if ck == "block_mapping_pair" || ck == "flow_pair" {
if let Some(decl) = _pair_to_decl(&child, src) {
out.push(decl);
}
}
}
}| fn _recurse_yaml<'a, D: Doc>(node: &Node<'a, D>, src: &[u8], out: &mut Vec<Declaration>) { | ||
| let kind = node.kind(); | ||
| let kind: &str = kind.as_ref(); | ||
| match kind { | ||
| "block_mapping" | "flow_mapping" | "block_sequence" | "flow_sequence" | "stream" | "document" => { | ||
| _walk_yaml(node, src, out); | ||
| } | ||
| _ => {} | ||
| } | ||
| } |
There was a problem hiding this comment.
Update _recurse_yaml to also recurse into block_node and flow_node container types so that nested mappings and sequences are correctly walked.
| fn _recurse_yaml<'a, D: Doc>(node: &Node<'a, D>, src: &[u8], out: &mut Vec<Declaration>) { | |
| let kind = node.kind(); | |
| let kind: &str = kind.as_ref(); | |
| match kind { | |
| "block_mapping" | "flow_mapping" | "block_sequence" | "flow_sequence" | "stream" | "document" => { | |
| _walk_yaml(node, src, out); | |
| } | |
| _ => {} | |
| } | |
| } | |
| fn _recurse_yaml<'a, D: Doc>(node: &Node<'a, D>, src: &[u8], out: &mut Vec<Declaration>) { | |
| let kind = node.kind(); | |
| let kind: &str = kind.as_ref(); | |
| match kind { | |
| "block_mapping" | "flow_mapping" | "block_sequence" | "flow_sequence" | "stream" | "document" | "block_node" | "flow_node" => { | |
| _walk_yaml(node, src, out); | |
| } | |
| _ => {} | |
| } | |
| } |
| use crate::core::{Declaration, DeclarationKind, ParseResult}; | ||
| use regex::Regex; | ||
| use std::path::Path; | ||
|
|
||
| pub fn parse_powershell(path: &Path, source: &[u8]) -> ParseResult { | ||
| let text = String::from_utf8_lossy(source); | ||
| let line_count = source.iter().filter(|&&b| b == b'\n').count() + 1; | ||
|
|
||
| let mut decls = Vec::new(); | ||
|
|
||
| // Regex for function/filter/workflow definitions | ||
| let fn_re = Regex::new( | ||
| r"(?im)^\s*(function|filter|workflow)\s+(\w[\w-]*)\s*" | ||
| ).unwrap(); | ||
|
|
||
| // Regex for class definitions | ||
| let class_re = Regex::new( | ||
| r"(?im)^\s*class\s+(\w[\w-]*)\s*" | ||
| ).unwrap(); | ||
|
|
||
| // Regex for enum definitions | ||
| let enum_re = Regex::new( | ||
| r"(?im)^\s*enum\s+(\w[\w-]*)\s*" | ||
| ).unwrap(); |
There was a problem hiding this comment.
Compiling the regular expressions inside parse_powershell on every invocation is inefficient. We can use std::sync::OnceLock to compile them once statically.
use crate::core::{Declaration, DeclarationKind, ParseResult};
use regex::Regex;
use std::path::Path;
use std::sync::OnceLock;
static FN_RE: OnceLock<Regex> = OnceLock::new();
static CLASS_RE: OnceLock<Regex> = OnceLock::new();
static ENUM_RE: OnceLock<Regex> = OnceLock::new();
pub fn parse_powershell(path: &Path, source: &[u8]) -> ParseResult {
let text = String::from_utf8_lossy(source);
let line_count = source.iter().filter(|&&b| b == b'\n').count() + 1;
let mut decls = Vec::new();
// Regex for function/filter/workflow definitions
let fn_re = FN_RE.get_or_init(|| {
Regex::new(r"(?im)^\s*(function|filter|workflow)\s+(\w[\w-]*)\s*").unwrap()
});
// Regex for class definitions
let class_re = CLASS_RE.get_or_init(|| {
Regex::new(r"(?im)^\s*class\s+(\w[\w-]*)\s*").unwrap()
});
// Regex for enum definitions
let enum_re = ENUM_RE.get_or_init(|| {
Regex::new(r"(?im)^\s*enum\s+(\w[\w-]*)\s*").unwrap()
});… yaml recursive traversal, OnceLock regex - toml: use Item::span() instead of hardcoded line 1/line_count - toml: _table_children now uses span() for correct byte offsets - yaml: _walk_yaml recurses into block_node/flow_node containers - yaml: _recurse_yaml adds block_node/flow_node to container types - powershell: compile regexes via OnceLock for static reuse - tests: update unsupported file test to use Cargo.lock (now supported) - clippy: fix unnecessary_sort_by and useless_conversion warnings
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (11)
src/adapters/json.rs (3)
123-133: ⚡ Quick winConsider returning &'static str instead of String for type names.
All branches return static string literals wrapped in
.to_string(), which allocates unnecessarily. Returning&'static strwould avoid allocations and better reflect that these are constant type names.Proposed optimization
-fn _value_type_name(kind: &str) -> String { +fn _value_type_name(kind: &str) -> &'static str { match kind { - "string" => "string".to_string(), - "number" => "number".to_string(), - "true" | "false" => "bool".to_string(), - "null" => "null".to_string(), - "object" => "object".to_string(), - "array" => "array".to_string(), - other => other.to_string(), + "string" => "string", + "number" => "number", + "true" | "false" => "bool", + "null" => "null", + "object" => "object", + "array" => "array", + _ => "unknown", } }Note: The
other => other.to_string()branch would need to change to a default since we can't return the inputkinddirectly as&'static str. Using"unknown"is a reasonable fallback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/json.rs` around lines 123 - 133, The _value_type_name function unnecessarily allocates String objects for constant type names. Change the return type from String to &'static str and remove the .to_string() calls from all match arms that return static string literals. For the `other => other.to_string()` branch, replace it with a default case that returns a static string literal like "unknown" instead of attempting to return the input parameter as &'static str.
34-35: 💤 Low valueConsider extracting kind once per node.
The pattern of calling
.kind()then immediately converting with.as_ref()is repeated throughout. This two-step conversion is correct but verbose.Example consolidation
-let kind = node.kind(); -let kind: &str = kind.as_ref(); +let kind: &str = node.kind().as_ref();Also applies to: 39-40, 51-52
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/json.rs` around lines 34 - 35, The code contains a verbose two-step pattern for converting the node kind to a string reference: first assigning the result of node.kind() to a variable, then immediately converting it with as_ref(). This pattern appears at three locations in the file. Consolidate each occurrence by combining the two steps into a single assignment statement that calls node.kind().as_ref() directly and assigns it to the kind variable with the &str type annotation, eliminating the redundant intermediate variable assignment.
27-27: Refactoringcount_parse_errorsto accept a reference requires redesigning the algorithm, not just the signature.The function's tree traversal implementation (
vec![root]+ stack popping) requires ownership of nodes to push them into a vector. Changing the parameter to&Node<D>would introduce complex lifetime management. However, the clone is called in 12+ adapter files (json, yaml, typescript, scala, rust, php, python, kotlin, java, go, csharp, cpp, ruby), making this a systemic optimization opportunity if the API is redesigned to avoid it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/json.rs` at line 27, The `count_parse_errors` function currently requires ownership of the Node due to its tree traversal implementation using a vector-based stack approach. Refactor the algorithm in the `count_parse_errors` function to accept a reference parameter `&Node<D>` instead of owned values, redesigning the traversal logic to work with borrowed references rather than requiring nodes to be moved into vectors. This will eliminate the need for the `.clone()` call at the call site and should be applied consistently across all adapter files where this function is invoked (json, yaml, typescript, scala, rust, php, python, kotlin, java, go, csharp, cpp, ruby).src/adapters/yaml.rs (2)
139-140: ⚡ Quick winString truncation length should be consistent across adapters.
The YAML adapter truncates strings at 40 characters. The TOML adapter (in toml.rs:115) also uses 40 characters. Consider defining a shared constant if this is a design decision, or document why each adapter may choose different truncation lengths.
Suggested constant
// In a shared location, e.g., src/adapters/base.rs or src/core.rs pub const MAX_SIGNATURE_LENGTH: usize = 40;Then use it in both adapters:
-if trimmed.len() > 40 { - format!("{}...", &trimmed[..40]) +if trimmed.len() > MAX_SIGNATURE_LENGTH { + format!("{}...", &trimmed[..MAX_SIGNATURE_LENGTH])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/yaml.rs` around lines 139 - 140, Define a shared constant for the string truncation length (currently hardcoded to 40) in a central location such as src/adapters/base.rs or src/core.rs, naming it something like MAX_SIGNATURE_LENGTH. Then replace the magic number 40 in the truncation logic in the YAML adapter (around the format call at line 139-140) with this constant, and also update the TOML adapter at toml.rs:115 to use the same constant for consistency across all adapters.
22-22: ⚡ Quick winConsider avoiding the clone if count_parse_errors can accept a reference.
Cloning the entire AST root node may be expensive for large YAML files. This is the same pattern as in json.rs. If
count_parse_errorscan be refactored to accept&Nodeinstead ofNode, this clone could be eliminated.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/yaml.rs` at line 22, The call to count_parse_errors is unnecessarily cloning the root node, which can be expensive for large YAML files. Refactor the count_parse_errors function signature to accept a reference parameter &Node instead of taking ownership of Node, and update the call site in the error_count assignment to pass &root instead of root.clone(). Ensure the function implementation works correctly with the borrowed reference.src/adapters/toml.rs (3)
115-116: 💤 Low valueString truncation format differs from YAML adapter.
TOML shows
"...first40chars..."while YAML shows"first40chars...". Consider aligning the format for consistency, or document why they differ.YAML (line 140 in yaml.rs):
format!("{}...", &trimmed[..40])TOML (line 116 here):
format!("\"...{}...\"", &v[..40])The TOML version includes leading
"...while YAML only has trailing....🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/toml.rs` around lines 115 - 116, The string truncation format in the truncate_value function or similar context in toml.rs uses format!("\"...{}...\"", &v[..40]) which includes leading ellipsis and quotes, while the YAML adapter uses format!("{}...", &trimmed[..40]) with only trailing ellipsis. Modify the TOML format string to match the YAML format for consistency by removing the leading "..." and quotes, keeping only the trailing ellipsis pattern. This ensures both adapters display truncated strings in the same way.
63-100: ⚖️ Poor tradeoffTable children are extracted only one level deep.
_table_childrenrecursively extracts children only forItem::Tableand setschildren: Vec::new()at line 92, meaning nested tables within tables won't be extracted. If TOML files commonly have deeply nested tables (e.g.,[a.b.c]style or inline tables within tables), consider deeper recursion.This may be intentional for simplicity, but document the limitation if users expect full nested structure visibility.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/toml.rs` around lines 63 - 100, The `_table_children` function currently sets `children: Vec::new()` for all declarations, which means nested tables are not recursively extracted. To fix this, modify the Declaration construction within the loop to check if each value item `val` is itself a Table, and if so, recursively call `_table_children` on that nested table to populate the `children` field; otherwise, keep it as an empty Vec. This will enable extraction of deeply nested table structures like `[a.b.c]` style hierarchies instead of stopping at one level.
58-58: Extract and report TOML parse errors using TomlError diagnostics.The
error_countis hardcoded to0, silently discarding parse failures fromtext.parse::<toml_edit::ImDocument<String>>(). Thetoml_editcrate providesTomlError(returned in theErrvariant of the parseResult) which includes diagnostic information viamessage(),span(), andDisplayformatting. Instead of usingif let Ok(doc) = ..., capture theErrvariant to extract and count parse errors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/toml.rs` at line 58, The error_count field is hardcoded to 0, which silently discards parse failures from the text.parse() call for toml_edit::ImDocument. Instead of using only the Ok branch with if let Ok(doc), also handle the Err variant to capture the TomlError returned by the parse operation. Extract diagnostic information from the TomlError using its message(), span(), and Display formatting methods to properly report the error details, and increment error_count for each parse failure encountered rather than leaving it hardcoded to 0.src/adapters/powershell.rs (2)
81-81: ⚡ Quick winInconsistent native_kind format across declaration types.
Functions, filters, and workflows store
native_kind: Some(kind_str.to_string())(line 54), which preserves the exact keyword ("function", "filter", "workflow"). However, classes use"class_definition"(line 81) and enums use"enum_definition"(line 108) instead of"class"and"enum".Consider using consistent naming: either use the exact keyword everywhere or use
_definitionsuffix everywhere.Proposed fix for consistency
Option 1 (use exact keywords):
-native_kind: Some("class_definition".to_string()), +native_kind: Some("class".to_string()),-native_kind: Some("enum_definition".to_string()), +native_kind: Some("enum".to_string()),Option 2 (use
_definitionsuffix everywhere):-native_kind: Some(kind_str.to_string()), +native_kind: Some(format!("{}_definition", kind_str)),Also applies to: 108-108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/powershell.rs` at line 81, The native_kind values are inconsistent across declaration types: functions, filters, and workflows use exact keywords (like "function", "filter", "workflow"), but classes and enums use suffixed names ("class_definition", "enum_definition"). Standardize by changing the native_kind assignments for classes (currently "class_definition") and enums (currently "enum_definition") to use just the exact keywords "class" and "enum" respectively, to match the pattern already used for the function, filter, and workflow declaration types.
50-50: 🏗️ Heavy liftAll declarations report single-line ranges.
Setting
end_line: start_linemeans all declarations span only their opening line. This is a known limitation of regex-based parsing (can't find block endings), but it may limit utility for large functions or classes.The PR objectives mention this limitation ("Users need function signatures, parameter blocks, and precise line numbers"). If line ranges are critical, consider using a tree-sitter-powershell grammar in the future, though regex-based detection is reasonable for initial support.
Also applies to: 77-77, 104-104
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/powershell.rs` at line 50, The PowerShell adapter's declaration parsing sets end_line to start_line across multiple locations (the assignments at lines 50, 77, and 104), which causes all declarations to report as single-line ranges regardless of their actual block size. This is a known limitation of regex-based parsing which cannot reliably detect block endings. Either add explanatory comments near these end_line assignments documenting this as a known limitation of the current regex-based approach, or if enhanced line range detection is prioritized, consider implementing more sophisticated block boundary detection or plan for future adoption of tree-sitter-powershell for more accurate parsing.src/main_helpers.rs (1)
119-186: ⚖️ Poor tradeoffConsider reducing repeated source.clone() calls if possible.
Each match arm calls
source.clone()to pass toast_grep(). Ifast_grepcan accept a reference instead of taking ownership, or if clones can be avoided via refactoring, this would reduce allocations for large source files.This is likely constrained by the ast-grep API design, so may not be actionable without upstream changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main_helpers.rs` around lines 119 - 186, The match statement in the result variable assignment contains repeated source.clone() calls in each match arm where lang.ast_grep(source.clone()).root() is invoked. To reduce unnecessary allocations, move the ast_grep root computation outside the match statement by storing the result of lang.ast_grep(source.clone()).root() in a variable before the match, then reference that computed value in each arm instead of calling source.clone() repeatedly. Alternatively, check if the ast_grep method signature can be modified to accept a reference to source rather than requiring ownership, which would eliminate the need for cloning entirely.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/adapters/json.rs`:
- Around line 123-133: The _value_type_name function unnecessarily allocates
String objects for constant type names. Change the return type from String to
&'static str and remove the .to_string() calls from all match arms that return
static string literals. For the `other => other.to_string()` branch, replace it
with a default case that returns a static string literal like "unknown" instead
of attempting to return the input parameter as &'static str.
- Around line 34-35: The code contains a verbose two-step pattern for converting
the node kind to a string reference: first assigning the result of node.kind()
to a variable, then immediately converting it with as_ref(). This pattern
appears at three locations in the file. Consolidate each occurrence by combining
the two steps into a single assignment statement that calls node.kind().as_ref()
directly and assigns it to the kind variable with the &str type annotation,
eliminating the redundant intermediate variable assignment.
- Line 27: The `count_parse_errors` function currently requires ownership of the
Node due to its tree traversal implementation using a vector-based stack
approach. Refactor the algorithm in the `count_parse_errors` function to accept
a reference parameter `&Node<D>` instead of owned values, redesigning the
traversal logic to work with borrowed references rather than requiring nodes to
be moved into vectors. This will eliminate the need for the `.clone()` call at
the call site and should be applied consistently across all adapter files where
this function is invoked (json, yaml, typescript, scala, rust, php, python,
kotlin, java, go, csharp, cpp, ruby).
In `@src/adapters/powershell.rs`:
- Line 81: The native_kind values are inconsistent across declaration types:
functions, filters, and workflows use exact keywords (like "function", "filter",
"workflow"), but classes and enums use suffixed names ("class_definition",
"enum_definition"). Standardize by changing the native_kind assignments for
classes (currently "class_definition") and enums (currently "enum_definition")
to use just the exact keywords "class" and "enum" respectively, to match the
pattern already used for the function, filter, and workflow declaration types.
- Line 50: The PowerShell adapter's declaration parsing sets end_line to
start_line across multiple locations (the assignments at lines 50, 77, and 104),
which causes all declarations to report as single-line ranges regardless of
their actual block size. This is a known limitation of regex-based parsing which
cannot reliably detect block endings. Either add explanatory comments near these
end_line assignments documenting this as a known limitation of the current
regex-based approach, or if enhanced line range detection is prioritized,
consider implementing more sophisticated block boundary detection or plan for
future adoption of tree-sitter-powershell for more accurate parsing.
In `@src/adapters/toml.rs`:
- Around line 115-116: The string truncation format in the truncate_value
function or similar context in toml.rs uses format!("\"...{}...\"", &v[..40])
which includes leading ellipsis and quotes, while the YAML adapter uses
format!("{}...", &trimmed[..40]) with only trailing ellipsis. Modify the TOML
format string to match the YAML format for consistency by removing the leading
"..." and quotes, keeping only the trailing ellipsis pattern. This ensures both
adapters display truncated strings in the same way.
- Around line 63-100: The `_table_children` function currently sets `children:
Vec::new()` for all declarations, which means nested tables are not recursively
extracted. To fix this, modify the Declaration construction within the loop to
check if each value item `val` is itself a Table, and if so, recursively call
`_table_children` on that nested table to populate the `children` field;
otherwise, keep it as an empty Vec. This will enable extraction of deeply nested
table structures like `[a.b.c]` style hierarchies instead of stopping at one
level.
- Line 58: The error_count field is hardcoded to 0, which silently discards
parse failures from the text.parse() call for toml_edit::ImDocument. Instead of
using only the Ok branch with if let Ok(doc), also handle the Err variant to
capture the TomlError returned by the parse operation. Extract diagnostic
information from the TomlError using its message(), span(), and Display
formatting methods to properly report the error details, and increment
error_count for each parse failure encountered rather than leaving it hardcoded
to 0.
In `@src/adapters/yaml.rs`:
- Around line 139-140: Define a shared constant for the string truncation length
(currently hardcoded to 40) in a central location such as src/adapters/base.rs
or src/core.rs, naming it something like MAX_SIGNATURE_LENGTH. Then replace the
magic number 40 in the truncation logic in the YAML adapter (around the format
call at line 139-140) with this constant, and also update the TOML adapter at
toml.rs:115 to use the same constant for consistency across all adapters.
- Line 22: The call to count_parse_errors is unnecessarily cloning the root
node, which can be expensive for large YAML files. Refactor the
count_parse_errors function signature to accept a reference parameter &Node
instead of taking ownership of Node, and update the call site in the error_count
assignment to pass &root instead of root.clone(). Ensure the function
implementation works correctly with the borrowed reference.
In `@src/main_helpers.rs`:
- Around line 119-186: The match statement in the result variable assignment
contains repeated source.clone() calls in each match arm where
lang.ast_grep(source.clone()).root() is invoked. To reduce unnecessary
allocations, move the ast_grep root computation outside the match statement by
storing the result of lang.ast_grep(source.clone()).root() in a variable before
the match, then reference that computed value in each arm instead of calling
source.clone() repeatedly. Alternatively, check if the ast_grep method signature
can be modified to accept a reference to source rather than requiring ownership,
which would eliminate the need for cloning entirely.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c18020d4-4880-426b-89b0-b596e46a728c
📒 Files selected for processing (10)
src/adapters/json.rssrc/adapters/mod.rssrc/adapters/powershell.rssrc/adapters/toml.rssrc/adapters/yaml.rssrc/deps/manifest.rssrc/deps/scc.rssrc/main_helpers.rssrc/search/index.rstests/cli_ergonomics.rs
3 participants
What this PR does
Adds 4 new language adapters to ast-bro: JSON, YAML, TOML, and PowerShell. These cover the most common non-code file types (config files and scripts) that are currently blind spots in ast-bro's language coverage.
SupportLang::Json+ tree-sitter-json — key-value pairs, nested objects, arraysSupportLang::Yaml+ tree-sitter-yaml — block/flow mappings and sequencestoml_edit-based parser — tables, dependencies, inline values (no external tree-sitter dep needed)Why it's needed
Current ast-bro supports 14 programming languages but cannot parse
.json,.yaml/.yml,.toml, or.ps1files. These file types dominate config/script-heavy projects (CI/CD pipelines, launchers, build tooling). Without them,sb map/searchsilently fails, forcing fallback to rawcat+grep— exactly the pattern ast-bro was built to eliminate.Reviewer Test Plan
How to verify
Evidence (Before & After)
Before:
sb map config.json→ silent empty output (file ignored as unknown language)After:
sb map config.json→ 8 fields parsed, nested objects/arrays with type annotationsFull feature verification report: https://github.com/aspnmy/ast-bro/blob/main/.qwen/insights/ast-bro-v3.1.0-feature-verification.md
Tested on
Environment
Risk & Scope
src/adapters/, dispatch additions inmod.rsandmain_helpers.rs). No existing adapter is modified.Linked Issues
Fixes aspnmy#1
中文说明
变更摘要
新增 4 个语言适配器：
仅含业务代码（6 文件，+549/-3 行），不含 Cargo.toml 或 CI 变更。分支基于 upstream/main 干净创建。
测试结果
mapsearch--jsonAI Assistance Disclosure
I used Qwen Code to review the changes and sanity-check the implementation.
Summary by CodeRabbit