Conversation
- Add furthestPos/furthestFailure tracking to AST and CST generated parsers - Replace null checks with Option<T> in CST parser generator - Fix infinite recursion in AST parser whitespace rule generation - Add 3 tests for error position reporting (305 → 308 tests) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds furthest-failure tracking and aggregated expected-token reporting to generated parsers; refactors CST/parse-result types to use Option-wrapped nodes/locations; introduces error-position tests and updates Java25 grammar with cut/lookahead/tokenization improvements. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant Parser as GeneratedParser
participant Context as ParseContext
participant Diagnostics as DiagnosticsBuilder
participant Result as ParseResult
Caller->>Parser: parse(input)
activate Parser
Parser->>Context: init(furthestPos = -1, furthestExpected = empty)
loop try expressions / alternatives
Parser->>Context: tryExpression(expr)
alt success
Context->>Parser: return success(node)
else failure
Parser->>Context: trackFailure(currentPos, expectedMsg)
Context->>Context: update furthestPos / furthestExpected
end
end
alt parse succeeded
Parser->>Result: success(Option.some(node))
else parse failed
Context->>Diagnostics: buildDiagnostics(furthestPos, furthestExpected)
Diagnostics->>Result: withErrors(Option.none(), diagnostics)
end
Result-->>Caller: return parse result (Option node + diagnostics)
deactivate Parser
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-12-30T12:39:57.084ZApplied to files:
🔇 Additional comments (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/org/pragmatica/peg/generator/ParserGeneratorTest.java (1)
440-472: Consider cleanup of temporary directories.The helper method creates temporary directories but doesn't explicitly clean them up. While the OS will eventually clean up temp directories, consider using
@TempDirJUnit annotation or explicitly deleting the directory after tests to avoid accumulation during test runs.Optional cleanup approach
Use JUnit 5's
@TempDirannotation:@TempDir Path tempDir; private Object compileAndInstantiate(String source, String className) throws Exception { // Remove: var tempDir = Files.createTempDirectory("peglib-test"); var packagePath = className.substring(0, className.lastIndexOf('.')).replace('.', '/'); // ... rest of implementation }Alternatively, add explicit cleanup in a try-finally block within the method.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.mdsrc/main/java/org/pragmatica/peg/generator/ParserGenerator.javasrc/test/java/org/pragmatica/peg/generator/ParserGeneratorTest.java
🧰 Additional context used
📓 Path-based instructions (5)
**/*.java
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.java: Use sealed interfaces for abstract types with fixed implementations (Expression, ParseResult, CstNode, AstNode, Trivia, ParseError)
Use record classes for immutable data types (Rule, Grammar, SourceLocation, SourceSpan, ParserConfig)
Files:
src/main/java/org/pragmatica/peg/generator/ParserGenerator.javasrc/test/java/org/pragmatica/peg/generator/ParserGeneratorTest.java
src/main/java/org/pragmatica/peg/**/*.java
📄 CodeRabbit inference engine (CLAUDE.md)
src/main/java/org/pragmatica/peg/**/*.java: Package code under org.pragmatica.peg namespace
Use Result/Option types from pragmatica-lite for return values instead of exceptions or null
Files:
src/main/java/org/pragmatica/peg/generator/ParserGenerator.java
src/main/java/org/pragmatica/peg/{parser,generator}/**/*.java
📄 CodeRabbit inference engine (CLAUDE.md)
Support both runtime compilation (JDK Compiler API with full Java support) and source generation (single self-contained Java file) modes
Files:
src/main/java/org/pragmatica/peg/generator/ParserGenerator.java
src/main/java/org/pragmatica/peg/generator/**/*.java
📄 CodeRabbit inference engine (CLAUDE.md)
Generated parsers should only depend on pragmatica-lite:core, with optional ErrorReporting enum (BASIC/ADVANCED) for Rust-style diagnostics
Files:
src/main/java/org/pragmatica/peg/generator/ParserGenerator.java
src/test/java/org/pragmatica/peg/**/*.java
📄 CodeRabbit inference engine (CLAUDE.md)
Test coverage should include: grammar parser (17), parsing engine (29), actions (6), generator (18), internal types (24), examples (135), and trivia handling (19)
Files:
src/test/java/org/pragmatica/peg/generator/ParserGeneratorTest.java
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/main/java/org/pragmatica/peg/error/**/*.java : Support configurable error recovery with three strategies: NONE (fail immediately), BASIC (report error with context), ADVANCED (continue parsing and collect multiple errors)
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/main/java/org/pragmatica/peg/parser/**/*.java : Implement both CST (Concrete Syntax Tree) and AST (Abstract Syntax Tree) output modes for parsers
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/main/java/org/pragmatica/peg/generator/**/*.java : Generated parsers should only depend on pragmatica-lite:core, with optional ErrorReporting enum (BASIC/ADVANCED) for Rust-style diagnostics
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/main/java/org/pragmatica/peg/grammar/**/*.java : Implement grammar syntax compatible with cpp-peglib, including operators (e1 e2, e1/e2, e*, e+, e?, &e, !e), extensions (< >, ~e, 'text'i, $name, ^), and directives (%whitespace)
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/test/java/org/pragmatica/peg/**/*.java : Test coverage should include: grammar parser (17), parsing engine (29), actions (6), generator (18), internal types (24), examples (135), and trivia handling (19)
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/main/java/org/pragmatica/peg/**/*.java : Use Result/Option types from pragmatica-lite for return values instead of exceptions or null
📚 Learning: 2025-12-30T12:39:57.084Z
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/main/java/org/pragmatica/peg/generator/**/*.java : Generated parsers should only depend on pragmatica-lite:core, with optional ErrorReporting enum (BASIC/ADVANCED) for Rust-style diagnostics
Applied to files:
src/main/java/org/pragmatica/peg/generator/ParserGenerator.javasrc/test/java/org/pragmatica/peg/generator/ParserGeneratorTest.java
📚 Learning: 2025-12-30T12:39:57.084Z
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/main/java/org/pragmatica/peg/parser/**/*.java : Implement both CST (Concrete Syntax Tree) and AST (Abstract Syntax Tree) output modes for parsers
Applied to files:
src/main/java/org/pragmatica/peg/generator/ParserGenerator.javasrc/test/java/org/pragmatica/peg/generator/ParserGeneratorTest.java
📚 Learning: 2025-12-30T12:39:57.084Z
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/main/java/org/pragmatica/peg/grammar/**/*.java : Implement grammar syntax compatible with cpp-peglib, including operators (e1 e2, e1/e2, e*, e+, e?, &e, !e), extensions (< >, ~e, 'text'i, $name, ^), and directives (%whitespace)
Applied to files:
src/main/java/org/pragmatica/peg/generator/ParserGenerator.javasrc/test/java/org/pragmatica/peg/generator/ParserGeneratorTest.java
📚 Learning: 2025-12-30T12:39:57.084Z
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/main/java/org/pragmatica/peg/{parser,generator}/**/*.java : Support both runtime compilation (JDK Compiler API with full Java support) and source generation (single self-contained Java file) modes
Applied to files:
src/main/java/org/pragmatica/peg/generator/ParserGenerator.javasrc/test/java/org/pragmatica/peg/generator/ParserGeneratorTest.java
📚 Learning: 2025-12-30T12:39:57.084Z
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/main/java/org/pragmatica/peg/tree/**/*.java : Group whitespace and comments as Trivia nodes (Trivia.Whitespace, Trivia.LineComment, Trivia.BlockComment) for lossless CST
Applied to files:
src/main/java/org/pragmatica/peg/generator/ParserGenerator.java
📚 Learning: 2025-12-30T12:39:57.084Z
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/main/java/org/pragmatica/peg/**/*.java : Use Result/Option types from pragmatica-lite for return values instead of exceptions or null
Applied to files:
src/main/java/org/pragmatica/peg/generator/ParserGenerator.javasrc/test/java/org/pragmatica/peg/generator/ParserGeneratorTest.java
📚 Learning: 2025-12-30T12:39:57.084Z
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/main/java/org/pragmatica/peg/**/*.java : Package code under org.pragmatica.peg namespace
Applied to files:
src/main/java/org/pragmatica/peg/generator/ParserGenerator.javasrc/test/java/org/pragmatica/peg/generator/ParserGeneratorTest.java
📚 Learning: 2025-12-30T12:39:57.084Z
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/test/java/org/pragmatica/peg/**/*.java : Test coverage should include: grammar parser (17), parsing engine (29), actions (6), generator (18), internal types (24), examples (135), and trivia handling (19)
Applied to files:
src/main/java/org/pragmatica/peg/generator/ParserGenerator.javasrc/test/java/org/pragmatica/peg/generator/ParserGeneratorTest.java
📚 Learning: 2025-12-30T12:39:57.084Z
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/main/java/org/pragmatica/peg/error/**/*.java : Support configurable error recovery with three strategies: NONE (fail immediately), BASIC (report error with context), ADVANCED (continue parsing and collect multiple errors)
Applied to files:
src/main/java/org/pragmatica/peg/generator/ParserGenerator.javasrc/test/java/org/pragmatica/peg/generator/ParserGeneratorTest.java
📚 Learning: 2025-12-30T12:39:57.084Z
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/main/java/org/pragmatica/peg/parser/ParsingContext.java : Use packrat memoization for parsing performance optimization with configurable enable/disable
Applied to files:
src/main/java/org/pragmatica/peg/generator/ParserGenerator.java
📚 Learning: 2025-12-30T12:39:57.084Z
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/main/java/org/pragmatica/peg/error/Diagnostic.java : Implement Rust-style diagnostic formatting with error location (file:line:column), context lines, and help messages for error recovery output
Applied to files:
src/main/java/org/pragmatica/peg/generator/ParserGenerator.javasrc/test/java/org/pragmatica/peg/generator/ParserGeneratorTest.java
📚 Learning: 2025-12-30T12:39:57.084Z
Learnt from: CR
Repo: siy/java-peglib PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T12:39:57.084Z
Learning: Applies to src/main/java/org/pragmatica/peg/tree/Trivia.java : Support trivia classification by content inspection: lines starting with // are LineComment, /* ... */ are BlockComment, otherwise Whitespace
Applied to files:
src/main/java/org/pragmatica/peg/generator/ParserGenerator.java
🧬 Code graph analysis (1)
src/test/java/org/pragmatica/peg/generator/ParserGeneratorTest.java (1)
src/main/java/org/pragmatica/peg/PegParser.java (1)
PegParser(26-158)
🔇 Additional comments (11)
src/test/java/org/pragmatica/peg/generator/ParserGeneratorTest.java (3)
350-380: LGTM! Effective test for furthest failure position tracking.The test correctly verifies that AST parsers report errors at the furthest position reached during parsing (column 7 where '1' is encountered) rather than defaulting to 1:1 after backtracking.
382-407: LGTM! CST BASIC mode error reporting test is well-structured.The test appropriately validates that CST parsers in BASIC mode report errors using the "line:column" format at the furthest failure position.
409-437: LGTM! ADVANCED mode diagnostic formatting test is comprehensive.The test correctly exercises the ADVANCED error reporting mode with Rust-style diagnostics, verifying that formatted error messages include the furthest failure position.
src/main/java/org/pragmatica/peg/generator/ParserGenerator.java (8)
94-94: LGTM! Infinite recursion fix is well-implemented.The
inWhitespaceRuleGenerationflag correctly prevents infinite recursion when the whitespace rule uses the*quantifier. The flag is set appropriately around whitespace rule generation and checked before emittingskipWhitespace()calls in expression generation.Also applies to: 101-101, 1117-1124
200-203: LGTM! AST parser furthest failure tracking is correctly implemented.The tracking fields and
trackFailure()method properly record the farthest position where parsing failed, and theparse()method correctly uses this information to report errors at the most informative location rather than after backtracking.Also applies to: 220-223, 261-270, 290-293
1133-1133: LGTM! trackFailure() calls are consistently added to all AST match methods.All terminal match methods now correctly call
trackFailure()before returning failure, ensuring accurate error position tracking.Also applies to: 1141-1141, 1146-1146, 1167-1167, 1182-1182, 1189-1189, 1202-1202
298-298: LGTM! Option API usage corrected.The change from
result.value.orElse(null)toresult.value.or(null)aligns with the pragmatica-lite Option API.
1764-1765: LGTM! CST parser furthest failure tracking uses idiomatic Option types.The CST implementation properly uses
Option<SourceLocation>andOption<String>for furthest failure tracking, providing better type safety and clearer intent than the AST version's primitive fields.Also applies to: 1790-1791, 1846-1854, 1900-1902, 1948-1951
1696-1696: LGTM! ParseResultWithDiagnostics correctly migrated to Option.The migration from nullable
CstNodetoOption<CstNode>is complete and consistent across factory methods, helper methods, and call sites, improving type safety per JBCT compliance requirements.Also applies to: 1700-1706, 1708-1717
3064-3064: LGTM! trackFailure() calls consistently added to all CST match methods.All CST terminal match methods now correctly call
trackFailure()before returning failure, and the methods are appropriately available in both BASIC and ADVANCED error reporting modes.Also applies to: 3073-3073, 3078-3078, 3101-3101, 3121-3121, 3129-3129, 3145-3145
474-477: LGTM! Conditional skipWhitespace() calls prevent infinite recursion.The conditional
skipWhitespace()calls based oninWhitespaceRuleGenerationare consistently applied across all relevant expression types, properly preventing infinite recursion when the whitespace rule itself contains repetition operators.Also applies to: 684-687, 743-746, 862-867
|
Re: Nitpick - Temporary directory cleanup Rejected with justification:
The suggestion is valid but the cost/benefit doesn't justify the change. |
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
siy
changed the title
1 participant
Summary
furthestPos/furthestFailuretracking to both AST and CST generated parsersOption<T>in CST parser generator for JBCT compliance*quantifierProblem
When parsing fails, PEG parsers backtrack trying alternatives. Previously, errors were often reported at position 1:1 instead of where the actual failure occurred. This made debugging grammar issues difficult.
Solution
Track the farthest position where parsing failed, along with what was expected. Report errors at this position instead of wherever backtracking ended up.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.