feat(runtime): support custom parser construction in parse drivers - #350
Merged
Conversation
Generated parse entry points always called Parser::new, which prevented grammars with typed semantic hooks from using the shared lexer-to-parser driver. Add text and stream constructor-aware entry points that retain the concrete hooked parser in GeneratedParseOutput. Make generated ParseOutput aliases generic over the hook type with a backward-compatible default, and add an into_parsed_file conversion alongside validation. Cover all parsed, validated, and retained-parser result modes with a hook-lowered superClass fixture, update API snapshots, and document the JavaScript and TypeScript hook paths. The generated-code API revision stays unchanged because the emitted macro invocation and generated-source/runtime contract are unchanged. Closes #349
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Copy/Paste DetectionFound 5 duplication(s) across 6 changed non-generated Rust file(s) (threshold: 100 tokens). Show duplicationsFound a 66 line (387 tokens) duplication in the following files:
use javascript_parser_base::JavaScriptParserBase;
fn dump_tree<S: AsRef<str>>(
out: &mut dyn Write,
tree: Node<'_>,
rule_names: &[S],
depth: usize,
) -> io::Result<()> {
let pad = " ".repeat(depth);
match tree.kind() {
NodeKind::Rule => {
let rule = tree.as_rule().expect("rule node kind checked");
let name = rule_names
.get(rule.rule_index())
.map_or("<?>", AsRef::as_ref);
writeln!(
out,
"{pad}Rule({name}, children={})",
rule.child_count()
)?;
for child in rule.children() {
dump_tree(out, child, rule_names, depth + 1)?;
}
}
NodeKind::Terminal => writeln!(
out,
"{pad}Term({:?})",
tree.as_terminal().expect("terminal node kind checked").text()
)?,
NodeKind::Error => writeln!(
out,
"{pad}Err({:?})",
tree.as_error().expect("error node kind checked").text()
)?,
}
Ok(())
}
fn main() -> ExitCode {
let mut args = env::args().skip(1);
let mut input: Option<PathBuf> = None;
let mut tokens_only = false;
while let Some(arg) = args.next() {
match arg.as_str() {
"--input" => input = args.next().map(PathBuf::from),
"--tokens" => tokens_only = true,
other => {
eprintln!("unknown argument: {other}");
return ExitCode::from(2);
}
}
}
let Some(input) = input else {
eprintln!("missing --input <path>");
return ExitCode::from(2);
};
let source = match fs::read_to_string(&input) {
Ok(source) => source,
Err(error) => {
eprintln!("failed to read {}: {error}", input.display());
return ExitCode::FAILURE;
}
};
if tokens_only {
let lexer = JavaScriptLexer::with_typed_hooks(
```rust
---
Found a 25 line (126 tokens) duplication in the following files:
* Starting at line 93 of tests/javascript-parity/dumper/src/main.rs
* Starting at line 93 of tests/typescript-parity/dumper/src/main.rs
```rust
JavaScriptLexerBase::with_strict_default(false),
);
let mut stream = CommonTokenStream::new(lexer);
stream.fill();
let errors = stream.drain_source_errors();
if !errors.is_empty() {
for error in errors {
eprintln!("line {}:{} {}", error.line, error.column, error.message);
}
return ExitCode::FAILURE;
}
for token in stream.tokens() {
if token.token_type() != TOKEN_EOF {
println!(
"{}\t{}\t{:?}",
token.token_type(),
token.channel(),
token.text_or_empty()
);
}
}
return ExitCode::SUCCESS;
}
let output = match java_script_parser::parse_with_parser_constructor(Found a 26 line (125 tokens) duplication in the following files:
$input: $crate::char_stream::CharStream,
$hooks: $crate::parser::SemanticHooks,
{
pub fn metadata() -> &'static $crate::generated::GrammarMetadata {
$metadata()
}
/// Adds a listener for lexer diagnostics.
pub fn add_error_listener<T>(&mut self, listener: T)
where
T: for<'a> $crate::errors::ErrorListener<dyn $crate::recognizer::Recognizer + 'a>
+ ::core::marker::Send
+ 'static,
{
$crate::recognizer::Recognizer::add_error_listener(&mut self.$base, listener);
}
/// Removes every lexer error listener, including the default console listener.
pub fn remove_error_listeners(&mut self) {
$crate::recognizer::Recognizer::remove_error_listeners(&mut self.$base);
}
/// Routes every token through ATN interpretation instead of the compiled
/// lexer DFA, so the learned-DFA trace (`lexer_dfa_string`) observes each
/// match.
pub fn set_force_interpreted(&mut self, force_interpreted: bool) {
```rust
---
Found a 17 line (105 tokens) duplication in the following files:
* Starting at line 183 of crates/antlr-rust-runtime/src/generated.rs
* Starting at line 280 of crates/antlr-rust-runtime/src/generated.rs
```rust
fn __from_node_with_invocation_states(
node: $crate::RuleNodeView<'a>,
invocation_states: Option<Vec<isize>>,
) -> Self {
$(
let __default = <$attrs>::default();
let __attrs = node.generated_attrs::<$attrs>().unwrap_or(&__default);
)?
Self {
__node: __GeneratedRuleContext::Stored(node),
__invocation_states: invocation_states,
__state: std::marker::PhantomData,
$(
$($field: __attrs.$field.clone(),)+
)?
}
}Found a 20 line (101 tokens) duplication in the following files:
JavaScriptParser::program,
) {
Ok(output) => output,
Err(error) => {
eprintln!("parse failed: {error}");
return ExitCode::FAILURE;
}
};
let tree = output.result;
let parser = output.parser;
if parser.number_of_syntax_errors() != 0 {
eprintln!(
"parse produced {} syntax error(s)",
parser.number_of_syntax_errors()
);
return ExitCode::FAILURE;
}
if let Err(error) = dump_tree(
&mut io::stdout().lock(),
parser.node(tree),
```rust |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (5)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe runtime adds typed parser-constructor entry points for text and stream parsing. Parse outputs retain semantic-hook types and support conversion into ChangesTyped parser-constructor support
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🟡 Moderate · up to This PR changes generated parsing APIs, and formatting, workspace tests, and clippy still need confirmed results before merge. No concrete defect or failed check is identified, but the PR is not fully merge-ready until that validation is complete. Sequence Diagram(s)sequenceDiagram
participant Caller
participant ParserEntryPoint
participant Lexer
participant CommonTokenStream
participant TypedParser
Caller->>ParserEntryPoint: provide input, lexer, parser constructor, and entry rule
ParserEntryPoint->>Lexer: lex input
Lexer->>CommonTokenStream: provide tokens
ParserEntryPoint->>TypedParser: construct parser with typed hooks
ParserEntryPoint->>TypedParser: invoke entry rule
TypedParser-->>Caller: return GeneratedParseOutput
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Code review — PR #350
|
Contributor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📊 Source Code Metrics (this PR vs
|
| File | Cyclomatic | Cognitive | Functions | LLOC | MI |
|---|---|---|---|---|---|
| crates/antlr-rust-runtime/src/generated.rs | 129 (main: 126) 🔴 | 33 ⚪ | 61 (main: 60) 🔴 | 152 (main: 151) 🔴 | 0 ⚪ |
| crates/antlr-rust-toml-parser/src/lib.rs | 16 (main: 15) 🔴 | 1 ⚪ | 14 (main: 13) 🔴 | 39 (main: 33) 🔴 | 21.55 (main: 24.14) 🔴 |
Generated by mehen v1.8.1 — the code quality watcher.
Address Claude's non-blocking review notes by routing Kotlin text access through ParsedFile or the parser facade, showing the current defaulted hook parameter in the migration alias, and linking the README hook example to the full JavaScript setup guide.
Contributor
Author
|
Addressed all three optional documentation follow-ups in
The PR description is updated, and the exact-head checks/reviews are running |
Exercise the constructor-aware text and stream entry points through the checked-in TOML recognizer so runtime coverage observes the macro paths directly. Route the JavaScript and TypeScript parity dumpers through the documented typed-hook driver, clarify diagnostics and conversion behavior, and keep the migration history distinct from the current widened alias.
Contributor
Author
|
Addressed the five exact-head suggestions in
I also restored the historical revision-11 alias spelling and moved the current The explicit six-case generated-project fixture remains deliberate because it Local verification includes the full workspace suite, exact CI clippy, both |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #349.
Summary
parse_with_parser_constructorandparse_stream_with_parser_constructorto the runtime-owned generated entrypoint surface
ParseOutputaliases,with
NoSemanticHooksas the backward-compatible default.into_parsed_file(), or cross the validated-tree boundary with.validate()rustdoc and the JavaScript/TypeScript integration guides
ParsedFileor thegenerated parser facade
recognizer and route the JavaScript/TypeScript parity dumpers through the
documented helper
The generated-code API remains revision 14. This is an additive extension to
the existing runtime macro expansion: generated source and its macro invocation
are unchanged, and revision 12-14 recognizers gain the helpers when linked
against the updated runtime.
Tests
cargo test --locked --workspace --all-featurescargo clippy --locked --workspace --all-targets --all-features -- -D warningscargo fmt --all -- --checkRUSTDOCFLAGS='-D warnings -A rustdoc::private-intra-doc-links' cargo doc --locked --workspace --all-features --no-depsuv tool run rumdl==0.2.34 check README.md docs/*.mdcargo check --manifest-path tests/javascript-parity/dumper/Cargo.tomlcargo check --manifest-path tests/typescript-parity/dumper/Cargo.tomltests/javascript-parity/run.sh ...(6/6 snippets match)tests/typescript-parity/run.sh ...(5/5 snippets match)The typed-hook generated-project fixture exercises text and stream inputs for
all three result modes: retained parser, owned
ParsedFile, and validated tree.The TOML test covers both new entry points in the in-repo coverage build.
Reviewer Notes
parse_stream_with_parsernow delegates to the constructor-aware core withParser::newprivate link to
GENERATED_RULE_STACK_CHECK_INTERVALinparser.rsrelated documentation, coverage, and parity suggestions are included in the
second and third commits
Summary by CodeRabbit
New Features
Documentation
Tests