refactor: use a virtual Cargo workspace root - #291
Conversation
Move antlr-rust-runtime from the repository root into crates/antlr-rust-runtime so every published package has the same ownership boundary. Relocate the upstream conformance harness from tools/ into tests/ because it is integration-test infrastructure, while keeping it an opt-in standalone package. Update temporary consumer manifests, parity and benchmark runners, CI and coverage paths, snapshots, port-evidence ledgers, package preflight scripts, and documentation. Adapt Release Please to the virtual manifest with a checked VERSION adapter and targeted workspace lockfile updates. This changes repository paths only; the generated-source/runtime contract and codegen API revision remain unchanged.
Copy/Paste DetectionFound 34 duplication(s) across 31 changed non-generated Rust file(s) (threshold: 100 tokens). Show duplicationsFound a 54 line (320 tokens) duplication in the following files:
atn: &LexerAtn,
hooks: &mut H,
mut generated_action: A,
mut generated_predicate: P,
unknown_policy: UnknownSemanticPolicy,
mut accept_adjuster: E,
) -> Result<TokenId, TokenStoreError>
where
I: CharStream,
H: SemanticHooks,
A: FnMut(&mut BaseLexer<I>, LexerCustomAction) -> bool,
P: FnMut(&BaseLexer<I>, LexerPredicate) -> Option<bool>,
E: FnMut(&mut BaseLexer<I>, i32, usize),
{
let hooks = RefCell::new(hooks);
let token = next_token_with_hooks_impl(
lexer,
sink,
atn,
&mut |lexer, action| {
if !generated_action(lexer, action)
&& !dispatch_lexer_action_hook(&hooks, lexer, action)
&& unknown_policy == UnknownSemanticPolicy::Error
&& let (Ok(rule), Ok(index)) = (
usize::try_from(action.rule_index()),
usize::try_from(action.action_index()),
)
{
lexer.record_semantic_error(true, rule, index);
}
},
&mut |lexer, predicate| {
generated_predicate(lexer, predicate)
.or_else(|| dispatch_lexer_predicate_hook(&hooks, lexer, predicate))
.unwrap_or_else(|| match unknown_policy {
UnknownSemanticPolicy::AssumeTrue => true,
UnknownSemanticPolicy::AssumeFalse => false,
UnknownSemanticPolicy::Error => {
lexer.record_semantic_error(
false,
predicate.rule_index(),
predicate.pred_index(),
);
false
}
})
},
&mut |lexer| dispatch_lexer_before_token_hook(&hooks, lexer),
&mut accept_adjuster,
&mut |lexer, accept_position| {
dispatch_lexer_after_accept_hook(&hooks, lexer, accept_position);
},
LexerMatchStrategy {
compiled: None,
```rust
---
Found a 21 line (226 tokens) duplication in the following files:
* Starting at line 14965 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 16370 of crates/antlr-rust-runtime/src/parser.rs
```rust
(9, AtnStateKind::RuleStop),
] {
assert_eq!(atn.add_state(kind, Some(0)).expect("state").index(), state);
}
atn.set_left_recursive_rule(0)
.expect("left-recursive rule start");
atn.set_precedence_rule_decision(2)
.expect("precedence decision");
atn.set_loop_back_state(8, 7).expect("loop-back state");
atn.set_rule_to_start_state(vec![0])
.expect("rule start states");
atn.set_rule_to_stop_state(vec![9])
.expect("rule stop states");
for state in [1, 2, 3] {
atn.add_decision_state(state).expect("decision state");
}
for (source, target) in [(0, 1), (2, 3), (2, 8), (7, 2), (8, 9)] {
atn.add_transition(source, ParserTransitionSpec::Epsilon { target })
.expect("epsilon transition");
}
for (source, target, label) in [(1, 2, 1), (1, 2, 2), (4, 6, 4), (5, 6, 3), (6, 7, 1)] {Found a 25 line (193 tokens) duplication in the following files:
let mut atn = ParserAtnBuilder::new(1);
for (state_number, kind) in [
(0, AtnStateKind::RuleStart),
(1, AtnStateKind::StarLoopEntry),
(2, AtnStateKind::Basic),
(3, AtnStateKind::Basic),
(4, AtnStateKind::StarLoopBack),
(5, AtnStateKind::LoopEnd),
(6, AtnStateKind::RuleStop),
] {
assert_eq!(
atn.add_state(kind, Some(0)).expect("state").index(),
state_number
);
}
atn.set_rule_to_start_state(vec![0])
.expect("rule start states");
atn.set_rule_to_stop_state(vec![6])
.expect("rule stop states");
atn.add_decision_state(1).expect("decision state");
atn.set_loop_back_state(5, 4).expect("loop back state");
atn.add_transition(0, ParserTransitionSpec::Epsilon { target: 1 })
.expect("entry transition");
atn.add_transition(1, ParserTransitionSpec::Epsilon { target: 2 })
.expect("loop body");
```rust
---
Found a 26 line (153 tokens) duplication in the following files:
* Starting at line 317 of crates/antlr-rust-runtime/src/atn/lexer.rs
* Starting at line 742 of crates/antlr-rust-runtime/src/atn/lexer.rs
```rust
pub fn next_token_with_hooks<I, A, P, E>(
lexer: &mut BaseLexer<I>,
sink: &mut TokenSink<'_>,
atn: &LexerAtn,
mut custom_action: A,
mut semantic_predicate: P,
mut accept_adjuster: E,
) -> Result<TokenId, TokenStoreError>
where
I: CharStream,
A: FnMut(&mut BaseLexer<I>, LexerCustomAction),
P: FnMut(&BaseLexer<I>, LexerPredicate) -> bool,
E: FnMut(&mut BaseLexer<I>, i32, usize),
{
next_token_with_hooks_impl(
lexer,
sink,
atn,
&mut custom_action,
&mut semantic_predicate,
&mut |_| {},
&mut accept_adjuster,
&mut |_, _| {},
LexerMatchStrategy {
compiled: None,
use_cache: false,Found a 27 line (145 tokens) duplication in the following files:
fn generated_match_token_recovers_missing_token_from_context_follow() {
let atn = generated_match_recovery_atn();
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new(
[None, Some("'X'"), Some("'Y'")],
[None, Some("X"), Some("Y")],
[None::<&str>, None, None],
),
);
let mut parser = BaseParser::new(
CommonTokenStream::new(Source {
tokens: vec![TestToken::eof("parser-test", 3, 1, 3)],
index: 0,
}),
data,
);
parser.rule_context_stack = vec![
RuleContextFrame {
rule_index: 0,
invoking_state: 0,
},
RuleContextFrame {
rule_index: 1,
invoking_state: 1,
},
];
```rust
---
Found a 25 line (142 tokens) duplication in the following files:
* Starting at line 450 of crates/antlr-rust-runtime/src/atn/lexer.rs
* Starting at line 559 of crates/antlr-rust-runtime/src/atn/lexer.rs
```rust
atn: &LexerAtn,
hooks: &mut H,
) -> Result<TokenId, TokenStoreError>
where
I: CharStream,
H: SemanticHooks,
{
let hooks = RefCell::new(hooks);
let token = next_token_with_hooks_impl(
lexer,
sink,
atn,
&mut |lexer, action| {
let _ = dispatch_lexer_action_hook(&hooks, lexer, action);
},
&mut |lexer, predicate| {
dispatch_lexer_predicate_hook(&hooks, lexer, predicate).unwrap_or(true)
},
&mut |lexer| dispatch_lexer_before_token_hook(&hooks, lexer),
&mut |_, _, _| {},
&mut |lexer, accept_position| {
dispatch_lexer_after_accept_hook(&hooks, lexer, accept_position);
},
LexerMatchStrategy {
compiled: None,Found a 24 line (134 tokens) duplication in the following files:
let atn = AtnDeserializer::new(&SerializedAtn::from_i32(&[
4, 0, 2, // version, lexer, max token type
9, // states
6, -1, // 0 token start
2, 0, // 1 rule 0 start
1, 0, // 2
1, 0, // 3
7, 0, // 4 rule 0 stop
2, 1, // 5 rule 1 start
1, 1, // 6
1, 1, // 7
7, 1, // 8 rule 1 stop
0, // non-greedy
0, // precedence
2, // rules
1, 1, // rule 0 starts at 1, token type 1
5, 2, // rule 1 starts at 5, token type 2
1, // modes
0, // default mode starts at 0
0, // sets
8, // edges
0, 1, 1, 0, 0, 0, // start -> rule 0
0, 5, 1, 0, 0, 0, // start -> rule 1
1, 2, 5, 'a' as i32, 0, 0, 2, 3, 5, 'b' as i32, 0, 0, 3, 4, 1, 0, 0, 0, 5, 6, 5,
```rust
---
Found a 23 line (133 tokens) duplication in the following files:
* Starting at line 1056 of crates/antlr-rust-runtime/src/atn/lexer.rs
* Starting at line 1215 of crates/antlr-rust-runtime/src/atn/lexer.rs
```rust
let source_has_semantic_context = dfa_state_has_semantic_context;
for config in active {
let Some(state) = atn.state(config.state) else {
continue;
};
for transition in &state.transitions {
if !transition.matches(symbol, MIN_CHAR_VALUE, MAX_CHAR_VALUE) {
continue;
}
let mut advanced = config.clone();
set_config_state(atn, &mut advanced, transition.target());
if symbol == EOF {
advanced.consumed_eof = true;
} else {
advanced.position += 1;
}
next.push(advanced);
}
}
let closure = epsilon_closure_with_lexer(lexer, atn, next, semantic_predicate);
let target_has_semantic_context = closure.has_semantic_context;
let suppress_edge = source_has_semantic_context || target_has_semantic_context;Found a 18 line (128 tokens) duplication in the following files:
fn epsilon_cycle_atn() -> Atn {
let mut atn = ParserAtnBuilder::new(1);
for (state_number, kind) in [
(0, AtnStateKind::RuleStart),
(1, AtnStateKind::Basic),
(2, AtnStateKind::RuleStop),
] {
assert_eq!(
atn.add_state(kind, Some(0)).expect("state").index(),
state_number
);
}
atn.set_rule_to_start_state(vec![0])
.expect("rule start states");
atn.set_rule_to_stop_state(vec![2])
.expect("rule stop states");
atn.add_transition(0, ParserTransitionSpec::Epsilon { target: 1 })
.expect("transition");
```rust
---
Found a 27 line (127 tokens) duplication in the following files:
* Starting at line 16420 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 16492 of crates/antlr-rust-runtime/src/parser.rs
```rust
let mut atn = ParserAtnBuilder::new(2);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(0))
.expect("state")
.index(),
0
);
assert_eq!(
atn.add_state(AtnStateKind::BlockStart, Some(0))
.expect("state")
.index(),
1
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
2
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
3
);
assert_eq!(
atn.add_state(AtnStateKind::BlockEnd, Some(0))Found a 22 line (125 tokens) duplication in the following files:
atn.add_state(AtnStateKind::BlockEnd, Some(0))
.expect("state")
.index(),
4
);
assert_eq!(
atn.add_state(AtnStateKind::RuleStop, Some(0))
.expect("state")
.index(),
5
);
atn.set_rule_to_start_state(vec![0])
.expect("rule start states");
atn.set_rule_to_stop_state(vec![5])
.expect("rule stop states");
atn.add_decision_state(1).expect("decision state");
atn.add_transition(0, ParserTransitionSpec::Epsilon { target: 1 })
.expect("transition");
atn.add_transition(
1,
ParserTransitionSpec::Atom {
target: 2,
```rust
---
Found a 18 line (122 tokens) duplication in the following files:
* Starting at line 3644 of crates/antlr-rust-runtime/src/atn/parser.rs
* Starting at line 3788 of crates/antlr-rust-runtime/src/atn/parser.rs
```rust
atn.set_rule_to_stop_state(vec![7])
.expect("rule stop states");
atn.add_decision_state(1).expect("decision state");
atn.add_transition(0, ParserTransitionSpec::Epsilon { target: 1 })
.expect("transition");
atn.add_transition(1, ParserTransitionSpec::Epsilon { target: 2 })
.expect("transition");
atn.add_transition(1, ParserTransitionSpec::Epsilon { target: 4 })
.expect("transition");
atn.add_transition(
2,
ParserTransitionSpec::Atom {
target: 3,
label: 1,
},
)
.expect("transition");
atn.add_transition(3, ParserTransitionSpec::Epsilon { target: 6 })Found a 20 line (120 tokens) duplication in the following files:
Self::Range { start, stop, .. } => (*start..=*stop).contains(&symbol),
Self::Set { set, .. } => set.contains(symbol),
Self::NotSet { set, .. } => {
(min_vocabulary..=max_vocabulary).contains(&symbol) && !set.contains(symbol)
}
Self::Wildcard { .. } => (min_vocabulary..=max_vocabulary).contains(&symbol),
Self::Epsilon { .. }
| Self::Rule { .. }
| Self::Predicate { .. }
| Self::Action { .. }
| Self::Precedence { .. } => false,
}
}
}
/// Ordered set of integer intervals used by set and negated-set transitions.
///
/// Unicode grammars can contain very large ranges, so this stores normalized
/// intervals rather than expanding every code point into a flat set.
#[derive(Clone, Debug, Default, Eq, PartialEq)]
```rust
---
Found a 34 line (119 tokens) duplication in the following files:
* Starting at line 10550 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 10625 of crates/antlr-rust-runtime/src/parser.rs
```rust
outcomes.extend(
self.recognize_state(
atn,
RecognizeRequest {
state_number: *target,
stop_state,
index,
rule_start_index,
decision_start_index: next_decision_start_index,
init_action_rules,
predicates,
semantics,
rule_args,
member_actions,
return_actions,
local_int_arg,
member_values: member_values.clone(),
return_values: return_values.clone(),
rule_alt_number: next_alt_number,
track_alt_numbers,
consumed_eof,
committed_decision: transition_committed,
precedence,
depth: depth + 1,
recovery_symbols: epsilon_recovery_symbols.clone(),
recovery_state: epsilon_recovery_state,
},
visiting,
memo,
expected,
)
.into_iter()
.map(|mut outcome| {
prepend_decision(&mut outcome, decision);Found a 22 line (117 tokens) duplication in the following files:
atn: &LexerAtn,
mut custom_action: A,
mut semantic_predicate: P,
mut accept_adjuster: E,
) -> Result<TokenId, TokenStoreError>
where
I: CharStream,
A: FnMut(&mut BaseLexer<I>, LexerCustomAction),
P: FnMut(&BaseLexer<I>, LexerPredicate) -> bool,
E: FnMut(&mut BaseLexer<I>, i32, usize),
{
next_token_with_hooks_impl(
lexer,
sink,
atn,
&mut custom_action,
&mut semantic_predicate,
&mut |_| {},
&mut accept_adjuster,
&mut |_, _| {},
LexerMatchStrategy {
compiled: None,
```rust
---
Found a 25 line (116 tokens) duplication in the following files:
* Starting at line 123 of crates/antlr-rust-runtime/src/byte_stream.rs
* Starting at line 202 of crates/antlr-rust-runtime/src/char_stream.rs
```rust
impl<B: AsRef<[u8]>> IntStream for ByteStream<B> {
fn consume(&mut self) {
if !self.is_eof() {
self.cursor += 1;
}
}
fn la(&mut self, offset: isize) -> i32 {
if offset == 0 {
return 0;
}
// Mirror `InputStream::la`: `+1` is the symbol under the cursor, and
// negative offsets look behind. `checked_*` keeps `isize::MIN` and
// out-of-range lookahead on the EOF path instead of panicking.
let absolute = if offset > 0 {
self.cursor.checked_add((offset - 1).cast_unsigned())
} else {
offset
.checked_neg()
.and_then(|distance| usize::try_from(distance).ok())
.and_then(|distance| self.cursor.checked_sub(distance))
};
absolute.map_or(EOF, |index| self.symbol_at(index).unwrap_or(EOF))Found a 24 line (113 tokens) duplication in the following files:
impl IntStream for LookaheadIntStream {
fn consume(&mut self) {
if self.la(1) != TOKEN_EOF {
self.index += 1;
}
}
fn la(&mut self, offset: isize) -> i32 {
if offset <= 0 {
return 0;
}
let offset = offset.cast_unsigned() - 1;
self.symbols
.get(self.index + offset)
.copied()
.unwrap_or(TOKEN_EOF)
}
fn index(&self) -> usize {
self.index
}
fn seek(&mut self, index: usize) {
self.index = index.min(self.symbols.len());
```rust
---
Found a 15 line (113 tokens) duplication in the following files:
* Starting at line 17846 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 18124 of crates/antlr-rust-runtime/src/parser.rs
```rust
fn generated_match_token_counts_single_token_deletion_recovery() {
let atn = generated_match_recovery_atn();
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new(
[None, Some("'X'"), Some("'Y'"), Some("'Z'")],
[None, Some("X"), Some("Y"), Some("Z")],
[None::<&str>, None, None, None],
),
);
let mut parser = BaseParser::new(
CommonTokenStream::new(Source {
tokens: vec![
TestToken::new(3).with_text("z"),
TestToken::new(2).with_text("y"),Found a 18 line (112 tokens) duplication in the following files:
(4, AtnStateKind::Basic, 0),
(5, AtnStateKind::RuleStop, 0),
(6, AtnStateKind::RuleStart, 1),
(7, AtnStateKind::Basic, 1),
(8, AtnStateKind::RuleStop, 1),
] {
assert_eq!(
atn.add_state(kind, Some(rule_index))
.expect("state")
.index(),
state_number
);
}
atn.set_rule_to_start_state(vec![0, 6])
.expect("rule start states");
atn.set_rule_to_stop_state(vec![5, 8])
.expect("rule stop states");
atn.add_decision_state(2).expect("decision state");
```rust
---
Found a 12 line (112 tokens) duplication in the following files:
* Starting at line 15018 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 15101 of crates/antlr-rust-runtime/src/parser.rs
```rust
let mut atn = ParserAtnBuilder::new(1);
for (state, kind, rule) in [
(0, AtnStateKind::RuleStart, 0),
(1, AtnStateKind::StarLoopEntry, 0),
(2, AtnStateKind::Basic, 0), // ops hub
(3, AtnStateKind::Basic, 0), // shift prec
(4, AtnStateKind::Basic, 0), // shift first >
(5, AtnStateKind::Basic, 0), // shift second >
(6, AtnStateKind::Basic, 0), // rel prec
(7, AtnStateKind::Basic, 0), // rel >
(8, AtnStateKind::LoopEnd, 0),
(9, AtnStateKind::RuleStop, 0),Found a 22 line (112 tokens) duplication in the following files:
fn predicate_after_token_atn() -> Atn {
let mut atn = ParserAtnBuilder::new(2);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(0))
.expect("state")
.index(),
0
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
1
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
2
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
```rust
---
Found a 22 line (111 tokens) duplication in the following files:
* Starting at line 14840 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 16966 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 17383 of crates/antlr-rust-runtime/src/parser.rs
```rust
fn left_recursive_loop_with_caller_follow_atn(caller_symbol: i32) -> Atn {
let mut atn = ParserAtnBuilder::new(2);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(0))
.expect("state")
.index(),
0
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
1
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
2
);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(1))Found a 14 line (110 tokens) duplication in the following files:
fn parser_matches_token_and_reports_mismatch() {
let source = Source {
tokens: vec![
TestToken::new(1).with_text("x"),
TestToken::eof("parser-test", 1, 1, 1),
],
index: 0,
};
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new([None, Some("'x'")], [None, Some("X")], [None::<&str>, None]),
);
let mut parser = BaseParser::new(CommonTokenStream::new(source), data);
let matched = parser.match_token(1).expect("token 1 should match");
```rust
---
Found a 13 line (109 tokens) duplication in the following files:
* Starting at line 2061 of crates/antlr-rust-runtime/src/lexer.rs
* Starting at line 2087 of crates/antlr-rust-runtime/src/lexer.rs
```rust
let mut lexer = BaseLexer::new(InputStream::new("β"), data);
lexer.consume_char();
let mut store = TokenStore::new(lexer.source_text(), lexer.source_name());
let mut sink = TokenSink::new(&mut store);
let id = lexer.eof_token(&mut sink).expect("test token should fit");
let token = sink.view(id).expect("emitted token should exist");
// byte_span is the field this test exists to pin and is absent from TokenView's Debug, so
// snapshot the explicit (start, stop, text, byte_span) record rather than the token.
insta::assert_compact_debug_snapshot!(
(token.start(), token.stop(), token.text(), token.byte_span()),
@r#"(1, 0, Some("<EOF>"), Some(2..2))"#Found a 13 line (109 tokens) duplication in the following files:
fn parser_matches_token_and_reports_mismatch() {
let source = Source {
tokens: vec![
TestToken::new(1).with_text("x"),
TestToken::eof("parser-test", 1, 1, 1),
],
index: 0,
};
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new([None, Some("'x'")], [None, Some("X")], [None::<&str>, None]),
);
let mut parser = BaseParser::new(CommonTokenStream::new(source), data);
```rust
---
Found a 22 line (108 tokens) duplication in the following files:
* Starting at line 8340 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 8731 of crates/antlr-rust-runtime/src/parser.rs
```rust
) -> Option<RecognizeOutcome> {
let (error_index, message) = self.expected_error_message(rule_index, start_index, expected);
let diagnostic = diagnostic_for_token(self.token_at(error_index), message);
let mut next_index = error_index;
loop {
let symbol = self.token_type_at(next_index);
if sync_symbols.contains(&symbol) {
if next_index == error_index {
return None;
}
break;
}
if symbol == TOKEN_EOF {
break;
}
let after = self.consume_index(next_index, symbol);
if after == next_index {
break;
}
next_index = after;
}
let mut nodes = NodeSeqId::EMPTY;Found a 15 line (108 tokens) duplication in the following files:
fn outcome_ties_keep_later_non_recursive_alternative() {
let arena = RecognitionArena::default();
let first = RecognizeOutcome {
index: 1,
consumed_eof: false,
alt_number: 0,
member_values: MemberEnv::new(),
return_values: BTreeMap::new(),
diagnostics: DiagnosticSeqId::EMPTY,
decisions: Vec::new(),
actions: vec![ParserAction::new(1, 0, 0, None)],
nodes: NodeSeqId::EMPTY,
};
let second = RecognizeOutcome {
actions: vec![ParserAction::new(2, 0, 0, None)],
```rust
---
Found a 13 line (107 tokens) duplication in the following files:
* Starting at line 3021 of crates/antlr-rust-runtime/src/atn/lexer.rs
* Starting at line 3278 of crates/antlr-rust-runtime/src/atn/lexer.rs
```rust
let mut hooks = LifecycleRecordingHooks::default();
let mut store = TokenStore::new(lexer.source_text(), lexer.source_name());
let mut sink = TokenSink::new(&mut store);
let mut ids = Vec::new();
for _ in 0..3 {
let id = if compiled {
next_token_compiled_with_semantic_hooks(
&mut lexer, &mut sink, &atn, &dfa, &mut hooks,
)
} else {
next_token_with_semantic_hooks(&mut lexer, &mut sink, &atn, &mut hooks)
}
.expect("lifecycle token should fit");Found a 17 line (107 tokens) duplication in the following files:
let report_unrecovered_error = self.is_top_level_entry();
let start_state = atn.rule_to_start_state().get(rule_index).ok_or_else(|| {
AntlrError::Unsupported(format!("rule {rule_index} has no start state"))
})?;
let stop_state = atn
.rule_to_stop_state()
.get(rule_index)
.filter(|state| *state != usize::MAX)
.ok_or_else(|| {
AntlrError::Unsupported(format!("rule {rule_index} has no stop state"))
})?;
let start_index = self.current_visible_index();
self.clear_prediction_diagnostics();
self.reset_per_parse_caches();
self.reset_recognition_arena();
let caller_follow_state = self.pending_invoking_follow_state(atn);
```rust
---
Found a 15 line (106 tokens) duplication in the following files:
* Starting at line 2595 of crates/antlr-rust-runtime/src/atn/lexer.rs
* Starting at line 2643 of crates/antlr-rust-runtime/src/atn/lexer.rs
```rust
(4, AtnStateKind::Basic, Some(1)),
(5, AtnStateKind::RuleStop, Some(1)),
] {
let mut state = LexerAtnState::new(state_number, kind);
if let Some(rule_index) = rule_index {
state = state.with_rule_index(rule_index);
}
atn.add_state(state);
}
atn.state_mut(0)
.expect("token start")
.add_transition(LexerTransition::Epsilon { target: 1 });
atn.state_mut(0)
.expect("token start")
.add_transition(LexerTransition::Epsilon { target: 3 });Found a 15 line (104 tokens) duplication in the following files:
impl ParserTransitionData<'_> {
pub const fn target(self) -> usize {
match self {
Self::Epsilon { target }
| Self::Atom { target, .. }
| Self::Range { target, .. }
| Self::Set { target, .. }
| Self::NotSet { target, .. }
| Self::Wildcard { target }
| Self::Rule { target, .. }
| Self::Predicate { target, .. }
| Self::Action { target, .. }
| Self::Precedence { target, .. } => target,
}
}
```rust
---
Found a 13 line (104 tokens) duplication in the following files:
* Starting at line 14953 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 16357 of crates/antlr-rust-runtime/src/parser.rs
```rust
fn labeled_left_recursive_operator_atn() -> Atn {
let mut atn = ParserAtnBuilder::new(4);
for (state, kind) in [
(0, AtnStateKind::RuleStart),
(1, AtnStateKind::BlockStart),
(2, AtnStateKind::StarLoopEntry),
(3, AtnStateKind::StarBlockStart),
(4, AtnStateKind::Basic),
(5, AtnStateKind::Basic),
(6, AtnStateKind::Basic),
(7, AtnStateKind::StarLoopBack),
(8, AtnStateKind::LoopEnd),
(9, AtnStateKind::RuleStop),Found a 14 line (102 tokens) duplication in the following files:
let id = next_token_compiled(lexer, &mut sink, atn, dfa).expect("test token should fit");
let token = sink.view(id).expect("emitted token should exist");
TokenSnapshot {
token_type: token.token_type(),
text: token.text_or_empty().to_owned(),
channel: token.channel(),
start: token.start(),
stop: token.stop(),
start_byte: token.start_byte(),
stop_byte: token.stop_byte(),
line: token.line(),
column: token.column(),
}
}
```rust
---
Found a 13 line (100 tokens) duplication in the following files:
* Starting at line 7050 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 7074 of crates/antlr-rust-runtime/src/parser.rs
```rust
let mut expected = BTreeSet::new();
for index in (1..self.rule_context_stack.len()).rev() {
let invoking_state = self.rule_context_stack[index].invoking_state;
let Ok(state_number) = usize::try_from(invoking_state) else {
continue;
};
let Some(Transition::Rule { follow_state, .. }) = atn
.state(state_number)
.and_then(|state| state.transitions().first())
.map(ParserTransition::data)
else {
continue;
}; |
|
Claude Code is working… I'll analyze this and get back to you. |
|
Warning Review limit reached
Next review available in: 39 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe pull request moves the runtime into ChangesRuntime workspace integration
Estimated code review effort: 5 (Critical) | ~120 minutes 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2aa2fa9619
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| license.workspace = true | ||
| keywords = ["antlr", "antlr4", "parser", "lexer", "runtime"] | ||
| categories = ["parser-implementations", "development-tools", "parsing"] | ||
| include = ["/Cargo.toml", "/README.md", "/src/**"] |
There was a problem hiding this comment.
When antlr-rust-runtime is published from its new subdirectory, this include list cannot capture the repository-root LICENSE; a cargo package reproduction contains the README and sources but no license file. The previous root manifest explicitly included /LICENSE, so this silently removes the BSD license text from future runtime archives; copy or otherwise package the license within the crate boundary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f0cf8f9. The workspace now declares license-file = "LICENSE", and every publishable member inherits it alongside the SPDX license field. Cargo 1.95 packages the root file as LICENSE in each crate archive. I also made verify-package-contents.sh require both LICENSE and README.md; the full four-crate package preflight passes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
🤖 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.
Inline comments:
In `@crates/antlr-rust-runtime/src/atn/bypass.rs`:
- Around line 199-210: In the rule-end-state collection loop, add a debug
assertion before assigning bypass_stop_for_end[end_state] to verify that the
entry for end_state is still None. Anchor the check in the rule_end_state and
bypass_stop_for_end handling, preserving the existing retarget map construction
while catching duplicate rule end states during tests.
In `@crates/antlr-rust-runtime/src/atn/lexer.rs`:
- Around line 3052-3072: Replace the complete token-tuple and lifecycle-event
assertions in crates/antlr-rust-runtime/src/atn/lexer.rs lines 3052-3072 with
appropriate insta debug snapshot assertions, while retaining explicit
interpreted-versus-compiled equivalence assertions; add
#[allow(clippy::disallowed_methods)] to its tests module. Also replace the
complete error-message list assertion in
crates/antlr-rust-runtime/src/atn/lexer_dfa.rs lines 2204-2207 with an insta
snapshot assertion and add the same allowance to that tests module.
- Around line 1086-1090: Collapse the nested conditions in the lexer DFA
edge-recording logic so the operation remains guarded by both !suppress_edge and
symbol != EOF. Update the surrounding branch in the lexer method containing
record_lexer_dfa_edge to satisfy clippy without changing behavior.
In `@crates/antlr-rust-runtime/src/atn/parser_atn.rs`:
- Around line 1350-1367: Update transitions_from to retrieve transitions through
the existing transitions_by_source index instead of scanning self.transitions
and filtering by source. Preserve its DoubleEndedIterator return type and
insertion-order semantics, including next() and next_back(), so
mark_precedence_decisions and add_parser_rule_return_edges retain their current
behavior.
In `@crates/antlr-rust-runtime/src/byte_stream.rs`:
- Around line 249-251: Update the #[allow(clippy::disallowed_methods)] attribute
on the tests module to match actual usage: remove it if clippy does not report a
violation, otherwise revise its comment to explain the module’s expect call
rather than referencing nonexistent insta macros.
In `@crates/antlr-rust-runtime/src/char_stream.rs`:
- Around line 351-382: Add assertions to
optional_fast_paths_preserve_scalar_indexes_and_positions for the Unicode
InputStream covering byte_interval and text_source_interval on "aβ\nγ",
including an interval ending at the final scalar so the source.len() fallback
and byte_offsets multi-byte boundary calculations are verified.
- Around line 318-328: Remove the custom byte_interval override and rely on the
trait default that derives byte bounds from text_source_interval. Keep the
existing text_source_interval implementation as the single source for
empty-interval handling and index clamping, ensuring token text and byte spans
remain consistent.
In `@crates/antlr-rust-runtime/src/dfa.rs`:
- Around line 364-376: Propagate the existing change result from
EdgeTable::update_sparse through EdgeTable::add instead of discarding it. Update
add_edge to use the returned bool directly for bump_learning_revision, removing
both pre- and post-add edge lookups while preserving false for unchanged targets
or rejected symbols.
In `@crates/antlr-rust-runtime/src/errors.rs`:
- Around line 36-57: add a public constructor or builder on SyntaxErrorEvent
that accepts all intended diagnostic fields, including offending, line, column,
span, message, and error. Document its usage for external TokenSource
implementations and recognizers invoking the public notify_error_listeners API,
while preserving the #[non_exhaustive] struct design.
In `@crates/antlr-rust-runtime/src/lib.rs`:
- Around line 72-76: Update the crate-root token re-export list in lib.rs to
include TokenSourceError alongside TokenSource and the other public token
symbols, so external users can name the type exposed by TokenSource and
SyntaxErrorEvent APIs.
In `@crates/antlr-rust-runtime/src/prediction.rs`:
- Around line 2374-2429: Replace the hand-written per-case expected DOT
constants and comparison in pinned_upstream_test_graph_nodes_matches_dot with
insta::assert_snapshot! keyed by case.logical_id, while preserving the
inventory, uniqueness, logical-id, and selector assertions. Remove only the
obsolete expected-string storage and mismatch aggregation, and add
#[allow(clippy::disallowed_methods)] to the upstream_graph_nodes module.
In `@crates/antlr-rust-runtime/src/recognizer.rs`:
- Around line 62-68: Document on RecognizerData or its error_listeners field
that cloning creates separate listener lists sharing the same listener instances
via Arc. Add a test covering a registered listener before cloning, then trigger
recording through both recognizers and assert both writes reach the same shared
buffer.
- Around line 18-23: Update the lock acquisition in the error-listener
forwarding method syntax_error to recover the mutex’s inner listener value when
the lock is poisoned instead of calling expect and panicking. Preserve the
existing delegation to self.0’s syntax_error with the recovered guard for both
healthy and poisoned locks.
In `@crates/antlr-rust-runtime/src/token.rs`:
- Around line 384-394: Add TokenStoreErrorKind::MissingSource and
TokenStoreErrorKind::InvalidByteSpan { start, stop, source_len }, including
matching Display formatting. In the source-backed validation flow, replace the
overflow error for absent self.source with MissingSource, and distinguish
start_byte > stop_byte by returning InvalidByteSpan with the span values; retain
the existing overflow error for stop_byte exceeding source.len().
- Around line 868-894: Convert the complete rendered-string assertions in
token_view_display_matches_antlr_shape and
synthetic_token_display_uses_antlr_negative_index to insta snapshots, using
assert_snapshot! for display output and assert_debug_snapshot! for the complete
ordered token collection around the collected-list test. Preserve the existing
property assertions for synthetic status and add
#[allow(clippy::disallowed_methods)] to the test module.
In `@crates/antlr-rust-runtime/src/tree_pattern.rs`:
- Around line 1365-1378: Update the state-range comment above the table in the
fixture ATN builder to match the actual assignments: stat states 0..=4 and expr
states 5..=10, consistent with set_rule_to_start_state and
set_rule_to_stop_state.
- Around line 677-682: Update rekey_tags_by_token_id to return a
Result<BTreeMap<TokenId, TagInfo>, ParseTreePatternError> instead of dropping
failed TokenId conversions through filter_map. Convert each index with error
propagation, preserving all successful entries, and update interpret to
propagate the returned ParseTreePatternError through the compile path.
In `@crates/antlr-rust-runtime/src/tree.rs`:
- Around line 1543-1546: Replace the inline to_string_tree_with_names output
assertions in stores_rule_children_in_one_pooled_range with insta snapshots,
covering the dumps currently checked at the referenced assertions. Keep explicit
assertions for properties, bounds, round-trip invariants, and ordering checks,
including the ordering assertions near the end of the test.
In `@crates/antlr-rust-runtime/src/xpath.rs`:
- Around line 284-298: Update compile_element so only LexemeKind::Identifier
falls through to resolve_rule; explicitly return XPathError::UnknownPathElement
for delimiter lexemes such as Anywhere, Root, and Bang encountered in a word
position. Preserve the existing wildcard, string, uppercase-identifier, and
valid rule-name handling, then regenerate the upstream_invalid_paths snapshot.
In `@tests/antlr-rust-runtime-testsuite/templates/Rust.test.stg`:
- Around line 178-193: Update LeafListener to store an injected output sink
instead of calling self.output(), since listener callbacks cannot access the
recognizer. Replace the affected visit_terminal and exit_* callback output calls
to use that sink, and update WalkListener to construct LeafListener with the
sink. Apply this consistently to every generated listener implementation in the
template.
- Around line 232-237: Update the token output in the relevant writeln! call
within the Rust test template to invoke text_or_empty() on the result of
ctx.id_token().expect("ID alternative").symbol(), matching the neighboring
token-printing logic and emitting token text rather than the token value.
In `@tests/antlr-rust-runtime-testsuite/templates/Rust.test.stg.design-notes.md`:
- Around line 212-272: Fix the markdownlint issues in the “Runtime API surface
this assumes” section: add blank lines after the headings identified by MD022,
and preserve the intentional global numbering for the capability checklist while
resolving MD029 by configuring ordered-list numbering for this file or making
the checklist one continuous list under a single heading.
- Around line 125-210: Update the conflicting per-template notes in the design
document to match the corrected renderings in the reference corrections section
and Rust.test.stg, including Assert, Append, member initializers,
RuleInvocationStack, ParserPropertyMember, TreeNodeWithAltNumField,
WalkListener, and Cast. Alternatively, clearly label the entire notes section as
a pre-correction record, consistent with rust-test-stg-honest-reference-gap.md.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e3e3f702-6e50-44f4-92b7-12462823231a
⛔ Files ignored due to path filters (40)
crates/antlr-rust-runtime/src/atn/snapshots/antlr4_runtime__atn__parser__tests__adaptive_predict_marks_sll_conflict_for_full_context.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/atn/snapshots/antlr4_runtime__atn__parser__tests__adaptive_predict_stream_retries_full_context_conflict.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/atn/snapshots/antlr4_runtime__atn__parser__tests__context_prediction_reports_context_sensitivity_for_dfa_conflict.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/atn/snapshots/antlr4_runtime__atn__serialized__tests__reads_small_parser_atn.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__parser__tests__committed_bail_error_notifies_error_listener.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__parser__tests__committed_left_recursive_depth_cap_keeps_listener_events_balanced.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__parser__tests__committed_predicate_star_loop_uses_single_token_deletion.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__parser__tests__committed_walker_dispatches_recovery_diagnostics.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__parser__tests__deferred_alternatives_preserve_left_recursive_contexts.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__parser__tests__failed_interpreted_parse_notifies_error_listener.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__parser__tests__fast_recognizer_preserves_labeled_left_recursive_operator_context.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__parser__tests__folds_left_recursive_boundary_into_rule_node.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__parser__tests__generated_prediction_diagnostics_use_adaptive_context.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__parser__tests__parsed_file_exposes_all_buffered_tokens.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__parser__tests__parser_dispatches_recovery_diagnostics_through_registered_listeners.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__parser__tests__private_context_alt_tracking_keeps_fast_predicate_recognition.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__parser__tests__recovery_diagnostics_expose_the_offending_token_to_listeners.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__recognizer__tests__recognizers_replace_the_default_console_error_listener.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__tree__tests__terminal_children_raw_vs_labeled.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__tree_pattern__tests__split_custom_delimiters.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__tree_pattern__tests__split_interleaves_text_and_tags.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__tree_pattern__tests__split_no_tags.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__tree_pattern__tests__split_parses_labeled_tags.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__tree_pattern__tests__split_rejects_malformed.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__tree_pattern__tests__split_strips_escapes.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__vocabulary__tests__upstream_vocabulary__vocabulary_from_token_names_matches_java.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__xpath__tests__lexer_and_parser_edge_cases.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__xpath__tests__named_anywhere_inversion_matches_java_4_13_2.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__xpath__tests__upstream_invalid_paths.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__xpath__tests__upstream_valid_paths.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__xpath__tests__wildcard_anywhere_virtual_root.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/xpath/generated/semantics.jsonis excluded by!**/generated/**crates/antlr-rust-runtime/src/xpath/generated/x_path_lexer.rsis excluded by!**/generated/**docs/antlr4-rust-gen-refactoring-plan.mdis excluded by!**/docs/**docs/honest-action-transpiler-plan-concerns.mdis excluded by!**/docs/**docs/honest-action-transpiler-plan.mdis excluded by!**/docs/**docs/issue-141-direct-g4-codegen-plan.mdis excluded by!**/docs/**docs/issue-9-semantic-predicates-actions-design.mdis excluded by!**/docs/**docs/perf-issue-15-csharp-gap-plan.mdis excluded by!**/docs/**docs/runtime-testsuite.mdis excluded by!**/docs/**
📒 Files selected for processing (95)
.github/workflows/ci.yml.typos.tomlAGENTS.mdCLAUDE.mdCargo.tomlREADME.mdVERSIONcodecov.ymlcrates/antlr-rust-codegen/tests/antlr4_rust_gen_cli/multi_recognizer.rscrates/antlr-rust-codegen/tests/antlr4_rust_gen_cli/support.rscrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-9ea85e6b69/revisions/testgraphnodes-test-9ea85e6b69-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-a-a-429589e373/revisions/testgraphnodes-test-a-a-429589e373-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-a-a-fullctx-b023f64b6c/revisions/testgraphnodes-test-a-a-fullctx-b023f64b6c-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-a-ax-fd976a340d/revisions/testgraphnodes-test-a-ax-fd976a340d-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-a-ax-fullctx-502155fcf9/revisions/testgraphnodes-test-a-ax-fullctx-502155fcf9-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-a-b-080058428f/revisions/testgraphnodes-test-a-b-080058428f-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-a-bx-b15f7b876f/revisions/testgraphnodes-test-a-bx-b15f7b876f-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-a-bx-fullctx-a35242b6cf/revisions/testgraphnodes-test-a-bx-fullctx-a35242b6cf-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-aa-a-fullctx-8e728ea773/revisions/testgraphnodes-test-aa-a-fullctx-8e728ea773-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-aa-aa-0a175c83db/revisions/testgraphnodes-test-aa-aa-0a175c83db-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-aa-abc-db12d99894/revisions/testgraphnodes-test-aa-abc-db12d99894-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-aab-aa-d90d8d54f0/revisions/testgraphnodes-test-aab-aa-d90d8d54f0-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-aab-ab-e2d46352b4/revisions/testgraphnodes-test-aab-ab-e2d46352b4-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-aab-ac-139c5b709d/revisions/testgraphnodes-test-aab-ac-139c5b709d-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-aac-ab-ef785e17e7/revisions/testgraphnodes-test-aac-ab-ef785e17e7-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-aaubu-acudu-7cb798b616/revisions/testgraphnodes-test-aaubu-acudu-7cb798b616-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-aaubv-abvdu-ecc8850384/revisions/testgraphnodes-test-aaubv-abvdu-ecc8850384-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-aaubv-abvdx-01eb5714fe/revisions/testgraphnodes-test-aaubv-abvdx-01eb5714fe-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-aaubv-abwdx-7953c9b489/revisions/testgraphnodes-test-aaubv-abwdx-7953c9b489-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-aaubv-acwdx-f479c849df/revisions/testgraphnodes-test-aaubv-acwdx-f479c849df-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-aax-aay-c0f9b80842/revisions/testgraphnodes-test-aax-aay-c0f9b80842-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-aax-aby-cccf935759/revisions/testgraphnodes-test-aax-aby-cccf935759-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-aaxc-aayd-a73533f64d/revisions/testgraphnodes-test-aaxc-aayd-a73533f64d-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-abx-abx-77366e32e9/revisions/testgraphnodes-test-abx-abx-77366e32e9-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-abx-acx-a3af7f90fa/revisions/testgraphnodes-test-abx-acx-a3af7f90fa-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-aex-bfx-07ad9de126/revisions/testgraphnodes-test-aex-bfx-07ad9de126-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-ax-a-62a48f251b/revisions/testgraphnodes-test-ax-a-62a48f251b-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-ax-a-fullctx-7ef9c1d6b2/revisions/testgraphnodes-test-ax-a-fullctx-7ef9c1d6b2-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-ax-ax-48f57578fa/revisions/testgraphnodes-test-ax-ax-48f57578fa-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-ax-ax-same-1504dc3dd3/revisions/testgraphnodes-test-ax-ax-same-1504dc3dd3-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-ax-bx-1ea2df9a04/revisions/testgraphnodes-test-ax-bx-1ea2df9a04-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-ax-bx-same-d0506bf7a9/revisions/testgraphnodes-test-ax-bx-same-d0506bf7a9-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-ax-by-47815d59d2/revisions/testgraphnodes-test-ax-by-47815d59d2-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-fullctx-3a6b2d8201/revisions/testgraphnodes-test-fullctx-3a6b2d8201-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-x-546922b23c/revisions/testgraphnodes-test-x-546922b23c-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testgraphnodes-test-x-fullctx-7fdaaf473e/revisions/testgraphnodes-test-x-fullctx-7fdaaf473e-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testvocabulary-testemptyvocabulary-66d31ad014/revisions/testvocabulary-testemptyvocabulary-66d31ad014-r1/manifest.jsoncrates/antlr-rust-codegen/tests/codegen-direct/port-evidence/testvocabulary-testvocabularyfromtokennames-d047506a84/revisions/testvocabulary-testvocabularyfromtokennames-d047506a84-r1/manifest.jsoncrates/antlr-rust-runtime/Cargo.tomlcrates/antlr-rust-runtime/src/atn/ascii_range.rscrates/antlr-rust-runtime/src/atn/bypass.rscrates/antlr-rust-runtime/src/atn/lexer.rscrates/antlr-rust-runtime/src/atn/lexer_dfa.rscrates/antlr-rust-runtime/src/atn/mod.rscrates/antlr-rust-runtime/src/atn/parser.rscrates/antlr-rust-runtime/src/atn/parser_atn.rscrates/antlr-rust-runtime/src/atn/serialized.rscrates/antlr-rust-runtime/src/byte_stream.rscrates/antlr-rust-runtime/src/char_stream.rscrates/antlr-rust-runtime/src/dfa.rscrates/antlr-rust-runtime/src/errors.rscrates/antlr-rust-runtime/src/generated.rscrates/antlr-rust-runtime/src/int_stream.rscrates/antlr-rust-runtime/src/lexer.rscrates/antlr-rust-runtime/src/lib.rscrates/antlr-rust-runtime/src/parser.rscrates/antlr-rust-runtime/src/perf.rscrates/antlr-rust-runtime/src/prediction.rscrates/antlr-rust-runtime/src/recognizer.rscrates/antlr-rust-runtime/src/semir.rscrates/antlr-rust-runtime/src/token.rscrates/antlr-rust-runtime/src/token_stream.rscrates/antlr-rust-runtime/src/tree.rscrates/antlr-rust-runtime/src/tree_pattern.rscrates/antlr-rust-runtime/src/vocabulary.rscrates/antlr-rust-runtime/src/xpath.rscrates/antlr-rust-runtime/src/xpath/XPathLexer.g4release-please-config.jsontests/antlr-rust-runtime-testsuite/Cargo.tomltests/antlr-rust-runtime-testsuite/java/RenderGrammar.javatests/antlr-rust-runtime-testsuite/src/main.rstests/antlr-rust-runtime-testsuite/src/rust_names.rstests/antlr-rust-runtime-testsuite/templates/Rust.test.stgtests/antlr-rust-runtime-testsuite/templates/Rust.test.stg.design-notes.mdtests/antlr-rust-runtime-testsuite/templates/rust-test-stg-honest-reference-gap.mdtests/javascript-parity/dumper/Cargo.tomltests/kotlin-parity/dumper/Cargo.tomltests/typescript-parity/dumper/Cargo.tomltool/resources/org/antlr/v4/tool/templates/codegen/Rust/Rust.stgtools/fixed-lookahead-bench/run.shtools/grammar-frontend/generate-port-evidence.mjstools/parse-bench/run.pytools/parse-bench/test_run.pytools/release/check-workspace-version.shtools/release/preflight-package-archives.sh
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (23)
crates/antlr-rust-runtime/src/atn/bypass.rs (1)
199-210: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value
Assert that each rule has a distinct end state before building the retarget map.
bypass_stop_for_end[end_state] = Some(bypass_stop)overwrites the entry when two rules resolve to the same end state. The retarget pass at Line 227 then routes the first rule's incoming edges through the second rule's bypass stop, which produces a silently wrong parse tree rather than an error.Deserialized ATNs derive one
RuleStopper rule, so the assumption holds there.ParserAtnBuilder::set_rule_to_stop_stateaccepts duplicates, andleft_recursive_loop_entry_atnincrates/antlr-rust-runtime/src/atn/parser.rs(Line 3863) already builds an ATN withvec![7, 7]. Add a debug assertion so the assumption is stated and caught in tests.🛡️ Proposed defensive assertion
let (end_state, exclude) = self.rule_end_state(rule)?; end_states.push(end_state); + debug_assert!( + bypass_stop_for_end[end_state].is_none(), + "rule {rule} shares end state {end_state} with an earlier rule; \ + bypass retargeting requires one end state per rule" + ); bypass_stop_for_end[end_state] = Some(bypass_stop);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.let mut end_states: Vec<usize> = Vec::with_capacity(rule_count); let mut bypass_stop_for_end: Vec<Option<usize>> = vec![None; self.kinds.len()]; let mut excluded: BTreeSet<(usize, usize)> = BTreeSet::new(); for rule in 0..rule_count { let bypass_stop = new_state_base + rule * 3 + 1; let (end_state, exclude) = self.rule_end_state(rule)?; end_states.push(end_state); debug_assert!( bypass_stop_for_end[end_state].is_none(), "rule {rule} shares end state {end_state} with an earlier rule; \ bypass retargeting requires one end state per rule" ); bypass_stop_for_end[end_state] = Some(bypass_stop); if let Some(exclude) = exclude { excluded.insert(exclude); } }🤖 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 `@crates/antlr-rust-runtime/src/atn/bypass.rs` around lines 199 - 210, In the rule-end-state collection loop, add a debug assertion before assigning bypass_stop_for_end[end_state] to verify that the entry for end_state is still None. Anchor the check in the rule_end_state and bypass_stop_for_end handling, preserving the existing retarget map construction while catching duplicate rule end states during tests.crates/antlr-rust-runtime/src/atn/lexer.rs (2)
1086-1090: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Confirm no other nested-if sites in the touched lexer files and check clippy config. fd -H -t f 'clippy.toml|Cargo.toml' -d 2 --exec sh -c 'echo "== $1"; grep -n -A5 "lints\|clippy" "$1" | head -40' sh {} rg -nP -U '(?s)if\s+!?[a-z_]+\s*\{\s*\n\s*if\s' crates/antlr-rust-runtime/src/atn/lexer.rs crates/antlr-rust-runtime/src/atn/lexer_dfa.rsRepository: ophi-dev/antlr-rust-runtime
Length of output: 2164
🏁 Script executed:
#!/bin/bash set -euo pipefail printf 'pwd: '; pwd printf '\nTop-level files: '; git ls-files | sed -n '1,120p' printf '\nTarget lexer outline/size: ' wc -l crates/antlr-rust-runtime/src/atn/lexer.rs crates/antlr-rust-runtime/src/atn/lexer_dfa.rs 2>/dev/null || true printf '\nRelevant code slices: ' sed -n '1076,1096p' crates/antlr-rust-runtime/src/atn/lexer.rs 2>/dev/null || true sed -n '1258,1278p' crates/antlr-rust-runtime/src/atn/lexer.rs 2>/dev/null || true printf '\nClippy/toml lints references: ' fd -H -t f 'clippy.toml|Cargo.toml' -d 2 --exec sh -c 'echo "== $1"; grep -n -A7 -B2 -i "lints\|clippy" "$1" | head -80' sh {} 2>/dev/null printf '\nNested if search lexical approximation: ' python3 - <<'PY' from pathlib import Path for p in ['crates/antlr-rust-runtime/src/atn/lexer.rs','crates/antlr-rust-runtime/src/atn/lexer_dfa.rs']: path=Path(p) if not path.exists(): continue lines=path.read_text().splitlines() for i,l in enumerate(lines): if l.lstrip().startswith('if ') and '{' in l: start=i+1 j=k=0 for nxt in range(start, min(len(lines), start+10)): line=lines[nxt].split('///')[0] # ignore trailing comments for brace count j+=line.count('{')-line.count('}') if '{' in line and line.strip()[:4]=='if ': print(f'{p}:{start}:{nxt+1}: outer at line {start}, inner at line {nxt+1}') break PYRepository: ophi-dev/antlr-rust-runtime
Length of output: 2135
Collapse the nested
ifbefore pushing.
clippy::collapsible_ifis warn by default, and CI treats clippy warnings as errors for Rust. Combine the condition socrates/antlr-rust-runtime/src/atn/lexer.rsdoes not fail clippy.🤖 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 `@crates/antlr-rust-runtime/src/atn/lexer.rs` around lines 1086 - 1090, Collapse the nested conditions in the lexer DFA edge-recording logic so the operation remains guarded by both !suppress_edge and symbol != EOF. Update the surrounding branch in the lexer method containing record_lexer_dfa_edge to satisfy clippy without changing behavior.Source: Coding guidelines
3052-3072: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Apply the
instasnapshot guideline to tests that pin complete collections. Both test modules pin whole ordered collections with hand-written assertions. The shared root cause is that the repository snapshot guideline is not applied to these value pins. Keep the interpreted-versus-compiled equivalence assertions as explicit assertions, because they check an invariant.
crates/antlr-rust-runtime/src/atn/lexer.rs#L3052-L3072: replace the full lifecycle-event and token-tuple pins withassert_debug_snapshot!orassert_compact_debug_snapshot!, and add#[allow(clippy::disallowed_methods)]on thetestsmodule.crates/antlr-rust-runtime/src/atn/lexer_dfa.rs#L2204-L2207: replace the full error-message list pin withassert_debug_snapshot!orassert_compact_debug_snapshot!, and add#[allow(clippy::disallowed_methods)]on thetestsmodule.As per coding guidelines: "Use
instasnapshots instead of hand-written assertions when tests pin complete values, collections, diagnostics, generated code, or token/tree/ATN/DFA dumps" and "Every test module or bare test function invoking aninstamacro must add#[allow(clippy::disallowed_methods)]".📍 Affects 2 files
crates/antlr-rust-runtime/src/atn/lexer.rs#L3052-L3072(this comment)crates/antlr-rust-runtime/src/atn/lexer_dfa.rs#L2204-L2207🤖 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 `@crates/antlr-rust-runtime/src/atn/lexer.rs` around lines 3052 - 3072, Replace the complete token-tuple and lifecycle-event assertions in crates/antlr-rust-runtime/src/atn/lexer.rs lines 3052-3072 with appropriate insta debug snapshot assertions, while retaining explicit interpreted-versus-compiled equivalence assertions; add #[allow(clippy::disallowed_methods)] to its tests module. Also replace the complete error-message list assertion in crates/antlr-rust-runtime/src/atn/lexer_dfa.rs lines 2204-2207 with an insta snapshot assertion and add the same allowance to that tests module.Source: Coding guidelines
crates/antlr-rust-runtime/src/atn/parser_atn.rs (1)
1350-1367: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
transitions_fromscans the whole transition pool, making ATN construction quadratic.
transitions_fromfiltersself.transitionsin full for onesource. Two build-time callers invoke it per state:
mark_precedence_decisions(Line 1522) callsis_precedence_decisionfor every state, and that helper callstransitions_fromtwice (Lines 1545 and 1552).add_parser_rule_return_edgesincrates/antlr-rust-runtime/src/atn/serialized.rs(Line 735) calls it for every state.Total cost is O(state_count × transition_count) on every parser ATN build and on every ANTLR-metadata deserialization. The comment at Lines 1192-1194 states that duplicate-edge detection was made non-quadratic with
transitions_by_source, but this accessor bypasses that index. Reuse the index so lookup is proportional to one state's out-degree.⚡ Proposed fix to index the lookup by source
pub fn transitions_from( &self, source: usize, ) -> impl DoubleEndedIterator<Item = ParserTransitionSpec> + '_ { - self.transitions - .iter() - .filter(move |transition| transition.source.index() == source) - .map(TransitionBuild::spec) + AtnStateId::try_from(source) + .ok() + .and_then(|id| self.transitions_by_source.get(&id)) + .map_or(&[][..], Vec::as_slice) + .iter() + .map(|&index| self.transitions[index].spec()) }Note:
transitions_by_sourcerecords insertion order per source, sonext()andnext_back()keep the same first/last edge semantics thatis_precedence_decisionrelies on. Call this accessor beforefinish()sortsself.transitions, which is already the case today.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.pub fn transitions_from( &self, source: usize, ) -> impl DoubleEndedIterator<Item = ParserTransitionSpec> + '_ { AtnStateId::try_from(source) .ok() .and_then(|id| self.transitions_by_source.get(&id)) .map_or(&[][..], Vec::as_slice) .iter() .map(|&index| self.transitions[index].spec()) } pub fn finish(mut self) -> Result<ParserAtn, ParserAtnError> { self.mark_precedence_decisions(); self.transitions.sort_by_key(|transition| transition.source); let transition_ranges = self.transition_ranges()?; self.precompute_state_flags(&transition_ranges); let words = self.encode(&transition_ranges)?; ParserAtn::from_owned(words) }🤖 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 `@crates/antlr-rust-runtime/src/atn/parser_atn.rs` around lines 1350 - 1367, Update transitions_from to retrieve transitions through the existing transitions_by_source index instead of scanning self.transitions and filtering by source. Preserve its DoubleEndedIterator return type and insertion-order semantics, including next() and next_back(), so mark_precedence_decisions and add_parser_rule_return_edges retain their current behavior.crates/antlr-rust-runtime/src/byte_stream.rs (1)
249-251: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Correct or remove the
disallowed_methodsallow attribute.The comment states that
instaassertion macros need this allow, but this test module calls noinstamacro. The module does callexpectat line 352. Either remove the attribute if the lint does not fire, or restate the reason to match the actual disallowed call.♻️ Proposed change if the lint does not fire
#[cfg(test)] -#[allow(clippy::disallowed_methods)] // insta assertion macros unwrap internal I/O. mod tests {As per coding guidelines: "Every test module or bare test function invoking an
instamacro must add#[allow(clippy::disallowed_methods)]because the macros internally unwrap I/O."🤖 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 `@crates/antlr-rust-runtime/src/byte_stream.rs` around lines 249 - 251, Update the #[allow(clippy::disallowed_methods)] attribute on the tests module to match actual usage: remove it if clippy does not report a violation, otherwise revise its comment to explain the module’s expect call rather than referencing nonexistent insta macros.Source: Coding guidelines
crates/antlr-rust-runtime/src/char_stream.rs (2)
318-328: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Remove the duplicated clamping in
byte_interval.
byte_intervalrepeats the empty-check and clamping already performed bytext_source_interval, and both callbyte_bounds. The two code paths must stay in sync, because token text and reported byte spans must agree. The trait default already derivesbyte_intervalfromtext_source_interval, so the override adds no behavior.♻️ Proposed refactor
- fn byte_interval(&self, interval: TextInterval) -> Option<(usize, usize)> { - let len = self.data.len(&self.source); - if interval.is_empty() || len == 0 { - return None; - } - let start = interval.start.min(len); - let stop = interval.stop.min(len.saturating_sub(1)); - (start <= stop) - .then(|| self.data.byte_bounds(&self.source, start, stop)) - .flatten() - }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.🤖 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 `@crates/antlr-rust-runtime/src/char_stream.rs` around lines 318 - 328, Remove the custom byte_interval override and rely on the trait default that derives byte bounds from text_source_interval. Keep the existing text_source_interval implementation as the single source for empty-interval handling and index clamping, ensuring token text and byte spans remain consistent.
351-382: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add coverage for Unicode byte bounds.
The tests exercise
contiguous_ascii,symbol_at, andposition_summary, but no test pinsbyte_intervalortext_source_intervalfor theInputData::Unicodevariant. That path computes byte offsets frombyte_offsetsand falls back tosource.len()for the final scalar. A test over"aβ\nγ"would pin the multi-byte boundary arithmetic.🤖 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 `@crates/antlr-rust-runtime/src/char_stream.rs` around lines 351 - 382, Add assertions to optional_fast_paths_preserve_scalar_indexes_and_positions for the Unicode InputStream covering byte_interval and text_source_interval on "aβ\nγ", including an interval ending at the final scalar so the source.len() fallback and byte_offsets multi-byte boundary calculations are verified.crates/antlr-rust-runtime/src/dfa.rs (1)
364-376: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Use the change flag that
update_sparsealready returns instead of two extra edge lookups.
add_edgecallsself.edge(source, symbol)before and afterself.hot.edges.add(...)to detect whether the edge changed.update_sparse(Line 839) already computes exactly that and returns it asOption<bool>, butadd(Line 793) discards the payload withis_some(). Every learned edge therefore pays two extra row walks, and a sparse row walk is O(row length).Return the change flag from
EdgeTable::addand use it directly.♻️ Proposed refactor to propagate the change flag
pub(crate) fn add_edge(&mut self, source: DfaStateId, symbol: i32, target: DfaStateId) { self.assert_valid_state(source); self.assert_valid_state(target); - let previous = self.edge(source, symbol); - self.hot.edges.add(source, symbol, target); - if self.edge(source, symbol) != previous { + if self.hot.edges.add(source, symbol, target) { self.bump_learning_revision(); } }
EdgeTable::addthen returnsbool:truefor a new slot or a replaced target,falsewhen the stored target already equalstargetor whenslot()rejects the symbol.🤖 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 `@crates/antlr-rust-runtime/src/dfa.rs` around lines 364 - 376, Propagate the existing change result from EdgeTable::update_sparse through EdgeTable::add instead of discarding it. Update add_edge to use the returned bool directly for bump_learning_revision, removing both pre- and post-add edge lookups while preserving false for unchanged targets or rejected symbols.crates/antlr-rust-runtime/src/errors.rs (1)
36-57: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Find constructors and construction sites for SyntaxErrorEvent. set -euo pipefail ast-grep run --pattern 'SyntaxErrorEvent { $$$ }' --lang rust crates tests rg -nP --type=rust -C3 'impl(<[^>]*>)?\s+SyntaxErrorEvent' crates rg -nP --type=rust -C3 'notify_error_listeners\s*\(' crates testsRepository: ophi-dev/antlr-rust-runtime
Length of output: 2164
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "PWD: $(pwd)" echo "Top-level files:" git ls-files | sed -n '1,80p' echo "Cargo files:" git ls-files '*Cargo.toml' echo "Target file existence:" git ls-files | grep 'crates/antlr-rust-runtime/src/errors.rs' || trueRepository: ophi-dev/antlr-rust-runtime
Length of output: 2135
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "PWD: $(pwd)" echo "Top-level entries:" ls -la echo "Git status:" git status --short echo "Known Rust files mentioning SyntaxErrorEvent:" rg -n --glob '*.rs' 'SyntaxErrorEvent|notify_error_listeners|TokenSource|Recognizer' . || trueRepository: ophi-dev/antlr-rust-runtime
Length of output: 2135
Provide a public constructor for
SyntaxErrorEvent.
SyntaxErrorEventis#[non_exhaustive]and has no publicimpl, so externalTokenSourceimplementors or recognizers cannot construct the value required by the publicnotify_error_listenersAPI. Add a constructor or builder with the intended diagnostics payload and document how external callers should emit syntax 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 `@crates/antlr-rust-runtime/src/errors.rs` around lines 36 - 57, add a public constructor or builder on SyntaxErrorEvent that accepts all intended diagnostic fields, including offending, line, column, span, message, and error. Document its usage for external TokenSource implementations and recognizers invoking the public notify_error_listeners API, while preserving the #[non_exhaustive] struct design.crates/antlr-rust-runtime/src/lib.rs (1)
72-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Re-export
TokenSourceErrorat the crate root.
TokenSourceErrorappears in the publicTokenSource::drain_errorsandTokenSource::report_errorsignatures, and in the publicFrom<&TokenSourceError> for SyntaxErrorEventconversion. ExternalTokenSourceimplementors must name the type, but the root re-export list omits it while includingTokenStoreError,TokenSink, andTokenSource. Add it for a consistent root surface.♻️ Proposed change
pub use token::{ DEFAULT_CHANNEL, HIDDEN_CHANNEL, INVALID_TOKEN_TYPE, MAX_TOKEN_OFFSET, TOKEN_EOF, Token, TokenChannel, TokenId, TokenIter, TokenSink, TokenSource, TokenSpec, TokenStore, - TokenStoreError, TokenView, + TokenSourceError, TokenStoreError, TokenView, };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.pub use token::{ DEFAULT_CHANNEL, HIDDEN_CHANNEL, INVALID_TOKEN_TYPE, MAX_TOKEN_OFFSET, TOKEN_EOF, Token, TokenChannel, TokenId, TokenIter, TokenSink, TokenSource, TokenSpec, TokenStore, TokenSourceError, TokenStoreError, TokenView, };🤖 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 `@crates/antlr-rust-runtime/src/lib.rs` around lines 72 - 76, Update the crate-root token re-export list in lib.rs to include TokenSourceError alongside TokenSource and the other public token symbols, so external users can name the type exposed by TokenSource and SyntaxErrorEvent APIs.crates/antlr-rust-runtime/src/prediction.rs (1)
2374-2429: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
Pin the upstream DOT dumps with
instasnapshots instead of hand-written constants.
pinned_upstream_test_graph_nodes_matches_dotcompares generated prediction-context DOT dumps against 38 hand-writtenconst &strvalues (Lines 1635-1900). The repository guideline requiresinstasnapshots when a test pins complete generated dumps of this kind. Hand-written constants also make an intentional rendering change expensive to update, because each expected string must be edited by hand.Keep the explicit assertions that guard the inventory and the selector:
CASES.len() == 38, the unique-source-test count, the logical-id count, and the non-emptyANTLR_GRAPH_NODE_CASEselection. Those are inventory and bounds checks, which the guideline retains. Replace only the per-case expected string withinsta::assert_snapshot!keyed bycase.logical_id, and add#[allow(clippy::disallowed_methods)]to theupstream_graph_nodesmodule, because theinstamacros unwrap internal I/O.
render_dotassigns node ids through the deterministicwork_listwalk and never emitsHashMapiteration order, so the rendered string is stable and safe to snapshot.Based on learnings from the coding guidelines: "Use
instasnapshots instead of hand-written assertions when tests pin complete values, collections, diagnostics, generated code, or token/tree/ATN/DFA dumps" and "Every test module or bare test function invoking aninstamacro must add#[allow(clippy::disallowed_methods)]".🤖 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 `@crates/antlr-rust-runtime/src/prediction.rs` around lines 2374 - 2429, Replace the hand-written per-case expected DOT constants and comparison in pinned_upstream_test_graph_nodes_matches_dot with insta::assert_snapshot! keyed by case.logical_id, while preserving the inventory, uniqueness, logical-id, and selector assertions. Remove only the obsolete expected-string storage and mismatch aggregation, and add #[allow(clippy::disallowed_methods)] to the upstream_graph_nodes module.Source: Coding guidelines
crates/antlr-rust-runtime/src/recognizer.rs (2)
18-23: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not panic on a poisoned error-listener lock.
syntax_errorcallsexpecton the lock. If any registered listener panics once, the mutex stays poisoned, and every later diagnostic panics. Diagnostics run on the error path, so this converts a recoverable syntax error into a panic and hides the original failure. Recover the inner value instead.🐛 Proposed fix
fn syntax_error(&self, recognizer: &(dyn Recognizer + '_), event: &SyntaxErrorEvent<'_>) { self.0 .lock() - .expect("error listener lock poisoned") + .unwrap_or_else(std::sync::PoisonError::into_inner) .syntax_error(recognizer, event); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.fn syntax_error(&self, recognizer: &(dyn Recognizer + '_), event: &SyntaxErrorEvent<'_>) { self.0 .lock() .unwrap_or_else(std::sync::PoisonError::into_inner) .syntax_error(recognizer, event); }🤖 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 `@crates/antlr-rust-runtime/src/recognizer.rs` around lines 18 - 23, Update the lock acquisition in the error-listener forwarding method syntax_error to recover the mutex’s inner listener value when the lock is poisoned instead of calling expect and panicking. Preserve the existing delegation to self.0’s syntax_error with the recovered guard for both healthy and poisoned locks.
62-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Document that cloned recognizers share listener instances.
ErrorListenerSlotclones anArc, so cloningRecognizerDataproduces an independent listener list whose entries point at the same listener objects. The test at lines 369-380 only covers the empty-list case, so this sharing is neither documented nor pinned. Add a doc comment on theerror_listenersfield or onRecognizerData, and add a test that registers a listener before cloning and asserts that both recognizers record into the same buffer.🤖 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 `@crates/antlr-rust-runtime/src/recognizer.rs` around lines 62 - 68, Document on RecognizerData or its error_listeners field that cloning creates separate listener lists sharing the same listener instances via Arc. Add a test covering a registered listener before cloning, then trigger recording through both recognizers and assert both writes reach the same shared buffer.crates/antlr-rust-runtime/src/token.rs (2)
384-394: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add dedicated error kinds for a missing source and an inverted byte span.
Line 386 reports a missing source buffer as
overflow("source text", 1, 0). The rendered message is "token source text 1 exceeds the supported limit 0", which does not describe the condition. Line 389 reports both an inverted span (start_byte > stop_byte) and an out-of-range span through the same overflow variant, so the inverted case reports an unrelated value and limit pair. A token-source integrator reading these messages receives a wrong explanation.Add
TokenStoreErrorKind::MissingSourceandTokenStoreErrorKind::InvalidByteSpan { start, stop, source_len }with matchingDisplayarms, and use them here.🤖 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 `@crates/antlr-rust-runtime/src/token.rs` around lines 384 - 394, Add TokenStoreErrorKind::MissingSource and TokenStoreErrorKind::InvalidByteSpan { start, stop, source_len }, including matching Display formatting. In the source-backed validation flow, replace the overflow error for absent self.source with MissingSource, and distinguish start_byte > stop_byte by returning InvalidByteSpan with the span values; retain the existing overflow error for stop_byte exceeding source.len().
868-894: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Pin the complete token display output with
instasnapshots.Lines 876-879 and 890-893 assert complete rendered token strings. The collected token list at lines 995-1005 pins a complete ordered collection. The guidelines require
instasnapshots for tests that pin complete values, collections, or token dumps, and keep explicit assertions for properties, bounds, and ordering invariants. Convert these assertions toassert_snapshot!andassert_debug_snapshot!, and add#[allow(clippy::disallowed_methods)]to the test module.♻️ Proposed change
#[cfg(test)] +#[allow(clippy::disallowed_methods)] // `insta` assertion macros unwrap internal I/O. mod tests {assert!(!store.view(TokenId(0)).expect("token").is_synthetic()); - assert_eq!( - store.view(TokenId(0)).expect("token").to_string(), - "[`@0`,2:4='abc',<7>,3:9]" - ); + insta::assert_snapshot!( + "token_view_display_matches_antlr_shape", + store.view(TokenId(0)).expect("token").to_string() + );As per coding guidelines: "Use
instasnapshots instead of hand-written assertions when tests pin complete values, collections, diagnostics, generated code, or token/tree/ATN/DFA dumps".🤖 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 `@crates/antlr-rust-runtime/src/token.rs` around lines 868 - 894, Convert the complete rendered-string assertions in token_view_display_matches_antlr_shape and synthetic_token_display_uses_antlr_negative_index to insta snapshots, using assert_snapshot! for display output and assert_debug_snapshot! for the complete ordered token collection around the collected-list test. Preserve the existing property assertions for synthetic status and add #[allow(clippy::disallowed_methods)] to the test module.Source: Coding guidelines
crates/antlr-rust-runtime/src/tree_pattern.rs (2)
677-682: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not discard tags when
TokenId::try_fromfails; surface the failure.
filter_mapwithTokenId::try_from(index).ok()?drops a tag entry when the conversion fails. A dropped entry has a silent, non-local effect:pattern_token_tagandrule_tag_ofno longer recognize that leaf, somatch_terminalsfalls back to literal text comparison against the tag's display text, for example<ID>. That text never equals real subject input, socompilereturnsOkand the compiled pattern matches nothing, with no diagnostic.Return a
Resultand propagate aParseTreePatternErrorinstead, so a compile that cannot preserve every tag fails loudly. This matches the module's stated policy of failing loudly rather than silently truncating, whichtokenizealready applies to the EOF suffix.🐛 Proposed fix
-fn rekey_tags_by_token_id(tags_by_index: &BTreeMap<usize, TagInfo>) -> BTreeMap<TokenId, TagInfo> { - tags_by_index - .iter() - .filter_map(|(&index, tag)| Some((TokenId::try_from(index).ok()?, tag.clone()))) - .collect() -} +fn rekey_tags_by_token_id( + tags_by_index: &BTreeMap<usize, TagInfo>, +) -> Result<BTreeMap<TokenId, TagInfo>, ParseTreePatternError> { + tags_by_index + .iter() + .map(|(&index, tag)| { + let id = TokenId::try_from(index).map_err(|_| ParseTreePatternError::Tokenization { + message: format!("pattern token index {index} exceeds the token-ID range"), + })?; + Ok((id, tag.clone())) + }) + .collect() +}Update the call site in
interpret:- let tags = rekey_tags_by_token_id(tags_by_index); + let tags = rekey_tags_by_token_id(tags_by_index)?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.fn rekey_tags_by_token_id( tags_by_index: &BTreeMap<usize, TagInfo>, ) -> Result<BTreeMap<TokenId, TagInfo>, ParseTreePatternError> { tags_by_index .iter() .map(|(&index, tag)| { let id = TokenId::try_from(index).map_err(|_| ParseTreePatternError::Tokenization { message: format!("pattern token index {index} exceeds the token-ID range"), })?; Ok((id, tag.clone())) }) .collect() }🤖 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 `@crates/antlr-rust-runtime/src/tree_pattern.rs` around lines 677 - 682, Update rekey_tags_by_token_id to return a Result<BTreeMap<TokenId, TagInfo>, ParseTreePatternError> instead of dropping failed TokenId conversions through filter_map. Convert each index with error propagation, preserving all successful entries, and update interpret to propagate the returned ParseTreePatternError through the compile path.
1365-1378: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the stale state-range comment.
The comment says
stat = 0..=6, expr = 7..=12. The table below assigns stat states 0..=4 and expr states 5..=10, whichset_rule_to_start_state(vec![0, 5])andset_rule_to_stop_state(vec![4, 10])confirm. The wrong ranges will mislead the next person who edits this fixture ATN.📝 Proposed fix
- // States: stat = 0..=6, expr = 7..=12. + // States: stat = 0..=4, expr = 5..=10.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// States: stat = 0..=4, expr = 5..=10. for (number, kind, rule) in [ (0, AtnStateKind::RuleStart, 0), // stat start (1, AtnStateKind::Basic, 0), // after ID (2, AtnStateKind::Basic, 0), // after '=' (3, AtnStateKind::Basic, 0), // after expr (4, AtnStateKind::RuleStop, 0), // stat stop (5, AtnStateKind::RuleStart, 1), // expr start (6, AtnStateKind::BlockStart, 1), // expr decision (7, AtnStateKind::Basic, 1), // INT alt (8, AtnStateKind::Basic, 1), // ID alt (9, AtnStateKind::BlockEnd, 1), // expr block end (10, AtnStateKind::RuleStop, 1), // expr stop ] {🤖 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 `@crates/antlr-rust-runtime/src/tree_pattern.rs` around lines 1365 - 1378, Update the state-range comment above the table in the fixture ATN builder to match the actual assignments: stat states 0..=4 and expr states 5..=10, consistent with set_rule_to_start_state and set_rule_to_stop_state.crates/antlr-rust-runtime/src/tree.rs (1)
1543-1546: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Snapshot the rendered tree dumps instead of asserting them inline.
Lines 1543-1546, 1589-1592, and 1599 pin complete
to_string_tree_with_namesoutput withassert_eq!. The repository guidelines requireinstasnapshots for tests that pin tree dumps, and retain explicit assertions only for properties, bounds, round-trip invariants, and ordering checks. The ordering assertions at Lines 1639 and 1662 stay as they are.♻️ Proposed change for `stores_rule_children_in_one_pooled_range`
assert_eq!(parsed.tree().text(), "ab"); assert_eq!(parsed.tree().children().count(), 2); assert_eq!(parsed.storage().stats().edges, 2); assert_eq!(parsed.storage().stats().scratch_links, 0); - assert_eq!( - parsed.tree().to_string_tree_with_names(&["root"]), - "(root a b)" - ); + insta::assert_snapshot!( + "pooled_children_tree", + parsed.tree().to_string_tree_with_names(&["root"]) + );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.insta::assert_snapshot!( "pooled_children_tree", parsed.tree().to_string_tree_with_names(&["root"]) );🤖 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 `@crates/antlr-rust-runtime/src/tree.rs` around lines 1543 - 1546, Replace the inline to_string_tree_with_names output assertions in stores_rule_children_in_one_pooled_range with insta snapshots, covering the dumps currently checked at the referenced assertions. Keep explicit assertions for properties, bounds, round-trip invariants, and ordering checks, including the ordering assertions near the end of the test.Source: Coding guidelines
crates/antlr-rust-runtime/src/xpath.rs (1)
284-298: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Report a delimiter lexeme as
UnknownPathElement, notInvalidRuleName.The
_arm treats every lexeme that is not a wildcard, a string, or an upper-case identifier as a rule reference.Anywhere,Root, andBangreach that arm when they appear in a word position. For the path"///",compile_elementspasses theRootlexeme here, and the caller receivesInvalidRuleName { name: "/", index: 2 }. Upstream ANTLR reports an unknown path element for that input, andXPathError::UnknownPathElementalready exists for exactly this case at Lines 273-278.Add an explicit guard so only an
Identifierresolves as a rule name. Regenerate theupstream_invalid_pathssnapshot after the change.🐛 Proposed fix
let node_test = match token.kind { LexemeKind::Wildcard => NodeTest::Wildcard, LexemeKind::String => NodeTest::Token(resolve_token(token, vocabulary)?), LexemeKind::Identifier if token.text.starts_with(char::is_uppercase) => { NodeTest::Token(resolve_token(token, vocabulary)?) } - _ => NodeTest::Rule(resolve_rule(token, rule_names)?), + LexemeKind::Identifier => NodeTest::Rule(resolve_rule(token, rule_names)?), + LexemeKind::Anywhere | LexemeKind::Root | LexemeKind::Bang | LexemeKind::Eof => { + return Err(XPathError::UnknownPathElement { + element: token.text.clone(), + index: token.index, + }); + } };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.fn compile_element( token: &Lexeme, axis: Axis, invert: bool, rule_names: &[String], vocabulary: &Vocabulary, ) -> Result<PathElement, XPathError> { let node_test = match token.kind { LexemeKind::Wildcard => NodeTest::Wildcard, LexemeKind::String => NodeTest::Token(resolve_token(token, vocabulary)?), LexemeKind::Identifier if token.text.starts_with(char::is_uppercase) => { NodeTest::Token(resolve_token(token, vocabulary)?) } LexemeKind::Identifier => NodeTest::Rule(resolve_rule(token, rule_names)?), LexemeKind::Anywhere | LexemeKind::Root | LexemeKind::Bang | LexemeKind::Eof => { return Err(XPathError::UnknownPathElement { element: token.text.clone(), index: token.index, }); } };🤖 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 `@crates/antlr-rust-runtime/src/xpath.rs` around lines 284 - 298, Update compile_element so only LexemeKind::Identifier falls through to resolve_rule; explicitly return XPathError::UnknownPathElement for delimiter lexemes such as Anywhere, Root, and Bang encountered in a word position. Preserve the existing wildcard, string, uppercase-identifier, and valid rule-name handling, then regenerate the upstream_invalid_paths snapshot.tests/antlr-rust-runtime-testsuite/templates/Rust.test.stg (2)
178-193: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
self.output()is not reachable inside the listener impls.
LeafListeneris a unit struct. Insidevisit_terminaland everyexit_*callback,selfis the listener, not the recognizer, soself.output()has no definition. The design notes state this same risk atRust.test.stg.design-notes.mdlines 296-302, andrust-test-stg-honest-reference-gap.mdlines 124-127 repeat it. The gap document also states that the render-then-compile pipeline is now the harness's only pipeline, so these templates must compile.The same defect exists at lines 225, 233, 256, 264, 285, 293, 315, and 327. Inject the sink into the listener, as Java and C# thread a
TextWriterintoLeafListener.🔧 Sketch of an injected sink
-#[derive(Default)] -struct LeafListener; +struct LeafListener<'a, W: std::io::Write> { + out: &'a mut W, +} impl TListener for LeafListener { fn visit_terminal( &mut self, node: &TerminalNode, ) -> Result\<(), std::convert::Infallible> { - writeln!(self.output(), "{}", node.symbol().text_or_empty()); + writeln!(self.out, "{}", node.symbol().text_or_empty()); Ok(()) } }
WalkListenerat lines 195-202 must then construct the listener with the sink.🤖 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 `@tests/antlr-rust-runtime-testsuite/templates/Rust.test.stg` around lines 178 - 193, Update LeafListener to store an injected output sink instead of calling self.output(), since listener callbacks cannot access the recognizer. Replace the affected visit_terminal and exit_* callback output calls to use that sink, and update WalkListener to construct LeafListener with the sink. Apply this consistently to every generated listener implementation in the template.
232-237: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Print the token text, not the token.
Line 227 and line 228 use
.symbol().text_or_empty(). Line 235 stops at.symbol(), so the{}placeholder formats the token value instead of its text. The upstream Java descriptor prints the token text. Add.text_or_empty().🐛 Proposed fix
writeln!( self.output(), "{}", - ctx.id_token().expect("ID alternative").symbol() + ctx.id_token().expect("ID alternative").symbol().text_or_empty() );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.writeln!( self.output(), "{}", ctx.id_token().expect("ID alternative").symbol().text_or_empty() ); }🤖 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 `@tests/antlr-rust-runtime-testsuite/templates/Rust.test.stg` around lines 232 - 237, Update the token output in the relevant writeln! call within the Rust test template to invoke text_or_empty() on the result of ctx.id_token().expect("ID alternative").symbol(), matching the neighboring token-printing logic and emitting token text rather than the token value.tests/antlr-rust-runtime-testsuite/templates/Rust.test.stg.design-notes.md (2)
125-210: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
The per-template notes contradict the corrections section and the template file.
Section "Reference corrections (post-validation)" at lines 3-49 records the final renderings, but the per-template notes still describe the pre-correction ones. Examples:
- Line 131:
Assertis documented asassert!(<s>). Correction 3 andRust.test.stgline 56 render it empty.- Line 137:
Appendis documented as<a> + &(<b>).to_string(). Correction 4 and template line 71 renderformat!.- Line 143: the member initializers are documented as
let mut <n>: T = <v>;. Correction 2 and template lines 86-88 render<n>: i32 = <v>;.- Line 166:
RuleInvocationStackis documented asformat!("{:?}", …). Correction 6 and template line 150 calljava_style_list.- Line 182:
ParserPropertyMemberis documented asfn property(&self). Correction 5 and template line 163 definefn Property.- Line 200:
TreeNodeWithAltNumFieldis documented as aMyRuleNodestruct. Template line 210 renders empty.- Line 204:
WalkListeneris documented asParseTreeWalker::walk. Template line 197 callswalk_with_invocation_states.- Line 293:
Castis documented asdowncast_ref. Template line 66 uses__active_context_view.Either update these entries, or add one sentence that marks the per-template notes as the pre-correction record, as
rust-test-stg-honest-reference-gap.mdlines 22-23 do for its own analysis.🤖 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 `@tests/antlr-rust-runtime-testsuite/templates/Rust.test.stg.design-notes.md` around lines 125 - 210, Update the conflicting per-template notes in the design document to match the corrected renderings in the reference corrections section and Rust.test.stg, including Assert, Append, member initializers, RuleInvocationStack, ParserPropertyMember, TreeNodeWithAltNumField, WalkListener, and Cast. Alternatively, clearly label the entire notes section as a pre-correction record, consistent with rust-test-stg-honest-reference-gap.md.
212-272: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Fix the markdownlint warnings in this section.
markdownlint-cli2reports MD022 for the headings at lines 216, 224, 236, 253, 258, and 268, which need a blank line below them. It also reports MD029 for the ordered list items from line 225 onward, because each subsection restarts its own list while continuing the global numbering. The numbering is intentional, since the gap document refers to a "21-capability" checklist. Keep the numbers and either configure MD029 asorderedfor this file, or convert the checklist to a single list under one heading.🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 216-216: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
[warning] 224-224: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
[warning] 225-225: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/2/3(MD029, ol-prefix)
[warning] 227-227: Ordered list item prefix
Expected: 2; Actual: 4; Style: 1/2/3(MD029, ol-prefix)
[warning] 229-229: Ordered list item prefix
Expected: 3; Actual: 5; Style: 1/2/3(MD029, ol-prefix)
[warning] 233-233: Ordered list item prefix
Expected: 4; Actual: 6; Style: 1/2/3(MD029, ol-prefix)
[warning] 236-236: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
[warning] 237-237: Ordered list item prefix
Expected: 1; Actual: 7; Style: 1/2/3(MD029, ol-prefix)
[warning] 239-239: Ordered list item prefix
Expected: 2; Actual: 8; Style: 1/2/3(MD029, ol-prefix)
[warning] 243-243: Ordered list item prefix
Expected: 3; Actual: 9; Style: 1/2/3(MD029, ol-prefix)
[warning] 245-245: Ordered list item prefix
Expected: 4; Actual: 10; Style: 1/2/3(MD029, ol-prefix)
[warning] 247-247: Ordered list item prefix
Expected: 5; Actual: 11; Style: 1/2/3(MD029, ol-prefix)
[warning] 250-250: Ordered list item prefix
Expected: 6; Actual: 12; Style: 1/2/3(MD029, ol-prefix)
[warning] 253-253: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
[warning] 254-254: Ordered list item prefix
Expected: 1; Actual: 13; Style: 1/2/3(MD029, ol-prefix)
[warning] 256-256: Ordered list item prefix
Expected: 2; Actual: 14; Style: 1/2/3(MD029, ol-prefix)
[warning] 258-258: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
[warning] 259-259: Ordered list item prefix
Expected: 1; Actual: 15; Style: 1/2/3(MD029, ol-prefix)
[warning] 261-261: Ordered list item prefix
Expected: 2; Actual: 16; Style: 1/2/3(MD029, ol-prefix)
[warning] 263-263: Ordered list item prefix
Expected: 3; Actual: 17; Style: 1/2/3(MD029, ol-prefix)
[warning] 266-266: Ordered list item prefix
Expected: 4; Actual: 18; Style: 1/2/3(MD029, ol-prefix)
[warning] 268-268: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
[warning] 269-269: Ordered list item prefix
Expected: 1; Actual: 19; Style: 1/2/3(MD029, ol-prefix)
[warning] 270-270: Ordered list item prefix
Expected: 2; Actual: 20; Style: 1/2/3(MD029, ol-prefix)
[warning] 271-271: Ordered list item prefix
Expected: 3; Actual: 21; Style: 1/2/3(MD029, ol-prefix)
🤖 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 `@tests/antlr-rust-runtime-testsuite/templates/Rust.test.stg.design-notes.md` around lines 212 - 272, Fix the markdownlint issues in the “Runtime API surface this assumes” section: add blank lines after the headings identified by MD022, and preserve the intentional global numbering for the capability checklist while resolving MD029 by configuring ordered-list numbering for this file or making the checklist one continuous list under a single heading.Source: Linters/SAST tools
The CodegenLiteralAccessor probe in interp_test.rs sits inside the phase-C locked oracle section, so switching its expectations from literal `pub fn` accessor signatures to `__antlr4_rust_context_accessors!` declaration lines changed the section hash that every phase-C evidence ledger pins. Record the change the sanctioned way: approve the rewrite as a normalization in refactorSectionsEquivalent (mirroring the CodegenData precedent) and regenerate the ledgers with generate-port-evidence --update, which re-verifies the pinned test and implementation commits modulo the approved normalizations. Regeneration itself was blocked by a pre-existing break: pinned commits that predate the virtual-workspace-root move (#291) store runtime sources at src/, but gitShowOptional only consulted historicalPath, which has no antlr-rust-runtime remap, so the empty-vocabulary check aborted the run even on main (CI only executes the validator, which tolerates missing history, so this went unnoticed). Resolve pinned sources by trying the current path, the historical remap, and the pre-move runtime location in order.
…#326) * feat(codegen): emit context accessors from one declaration per method Every generated context's child accessors were emitted twice: once for the recovery-oriented states (generic over `__RecoveryContextState`, returning `Result<_, MissingChildError>` for required children) and once for the validated state (`ValidatedTreeContext`, returning required children directly behind an `unreachable!` guard). The pairs differed only in required-child unwrapping and the validated-child constructor, making up roughly 23% of generated parser source and leaving room for variant-drift bugs. Extend the #276 technique to the accessor surface: a new runtime-owned `__antlr4_rust_context_accessors!` macro expands one declarative record per accessor into both state-variant impls. Generated source now declares each accessor once, e.g. antlr4_runtime::__antlr4_rust_context_accessors! { KeyValueContext { rule key: required(KeyContext[4], "key"), token equals_token: required(8, "EQUALS"), } } with `rule`/`token`/`label_rule`/`label_token` kinds covering every existing shape: required/optional/many cardinalities, `nth`/`last_after` label selectors, `skip` for list labels, and single-type vs token-set labeled-token sources (`__labeled_token_children` vs `_matching`). All grammar data (names, indices, token types) appears only in the generated invocation, keeping the macro grammar-agnostic. Signatures, behavior, `MissingChildError` diagnostics, and the validated `unreachable!` message text are preserved; the antlr4rust-compat wrapper impls and `validate_tree_structure` arms remain generator-rendered since they have no state-variant duplication. Newly generated source requires the new macro, so the generated-code API revision increments to 8; revisions 1-7 stay accepted because the runtime still provides everything their generated source needs. All checked-in recognizers are regenerated and the compatibility test, snapshots, and docs/migration.md (previously stale at revision 6) are updated. Generated parser source shrinks accordingly (lines/bytes): toml_parser.rs 3884 -> 3123 198169 -> 163702 (-19.6%/-17.4%) antlr_v4_parser.rs 9858 -> 7595 573366 -> 466022 (-23.0%/-18.7%) rust_parser.rs 37087 -> 30975 2609384 -> 2291910 (-16.5%/-12.2%) Verified: workspace clippy/tests clean, upstream conformance sweep 357/357, grammar-frontend Stage 0/1/2 fixed point, and rustdoc for a generated crate still documents the macro-expanded public accessors. Closes #323 * fix(codegen): restore accessor-surface assertions and add macro catch-alls Address review feedback on the declarative accessor macro: - The antlr4rust_unrelated_context_surface snapshot filtered for `pub fn` lines, which the declaration format no longer produces, leaving an empty snapshot that could not catch a dropped accessor. Widen the filter to also keep rule/token/label_rule/label_token declaration lines so the snapshot again pins UnrelatedContext's full surface. - The compat surface test lost its `self__token` assertion when the paired impls collapsed into declarations; assert its declaration alongside r#type and self_. - Give __antlr4_rust_context_accessors! catch-all arms that emit a compile_error! naming the context and the offending declaration, selector, or token set, following the __antlr4_rust_require_codegen_api! precedent, instead of failing with a bare "no rules expected this token" deep inside a #[rustfmt::skip] generated module. - Document the __from_child_node/__from_validated_child_node constructors and the __node/__invocation_states fields in the macro's generator-contract doc comment. * test(codegen): re-lock phase-C evidence for declaration-based accessors The CodegenLiteralAccessor probe in interp_test.rs sits inside the phase-C locked oracle section, so switching its expectations from literal `pub fn` accessor signatures to `__antlr4_rust_context_accessors!` declaration lines changed the section hash that every phase-C evidence ledger pins. Record the change the sanctioned way: approve the rewrite as a normalization in refactorSectionsEquivalent (mirroring the CodegenData precedent) and regenerate the ledgers with generate-port-evidence --update, which re-verifies the pinned test and implementation commits modulo the approved normalizations. Regeneration itself was blocked by a pre-existing break: pinned commits that predate the virtual-workspace-root move (#291) store runtime sources at src/, but gitShowOptional only consulted historicalPath, which has no antlr-rust-runtime remap, so the empty-vocabulary check aborted the run even on main (CI only executes the validator, which tolerates missing history, so this went unnoticed). Resolve pinned sources by trying the current path, the historical remap, and the pre-move runtime location in order. * test(codegen): pin native/compat coexistence and compile every macro arm Address the second review pass: - The antlr4rust_compat_generated_surface snapshot needles matched only the compat wrapper methods after accessors became declarations; add the rule/token declaration needles so the snapshot again pins that the native fallible surface coexists with the compat getters, matching the unrelated-context snapshot fix. - Extend the validated-tree fixture with a `tails += COMMA*` list token label so the `label_token many(skip(n), [...])` arms — previously the only accessor shape never compiled by the workspace test run — are generated, compiled, and exercised at runtime in both the populated and empty cases. - Report catch-all macro diagnostics in source declaration order (`rule broken: bogus(...)`) via structured catch-all arms, keeping the fully generic arms for shapes that do not parse as a declaration. - Comment the intentionally unbalanced brace handling in the generated_parser_api accessor-declaration scan.
1 participant
Summary
antlr-rust-runtimefrom the rootsrc/tree tocrates/antlr-rust-runtime/, alongside the other published cratestools/totests/antlr-rust-runtime-testsuite/This is a package and filesystem layout refactor. It does not change the generated-source/runtime contract, so
__ANTLR4_RUST_CODEGEN_APIis unchanged.Testing
cargo test --locked --workspace --all-featurescargo clippy --locked --workspace --all-targets --all-features -- -D warningscargo +1.95 check --locked --workspace --all-targets --all-featurescargo fmt --all -- --checkactionlintLexerExec/KeywordID