Skip to content

Comments

Apply Megaparsec best practices to parser error handling#7505

Merged
zeme-wana merged 1 commit intomasterfrom
yura/parser-improvements-megaparsec-best-practices
Jan 15, 2026
Merged

Apply Megaparsec best practices to parser error handling#7505
zeme-wana merged 1 commit intomasterfrom
yura/parser-improvements-megaparsec-best-practices

Conversation

@Unisay
Copy link
Contributor

@Unisay Unisay commented Dec 24, 2025

This PR applies Megaparsec best practices to improve the parser error handling implementation from PR #7489.

Summary of Improvements

1. Remove Redundant try Wrappers

  • Issue: Keywords wrapped in try (symbol ...) despite symbol being auto-backtracking since Megaparsec 4.4.0 (current version: 9.7.0)
  • Fix: Removed 10 redundant try wrappers from keyword and delimiter parsing
  • Impact: Improved performance, clearer code, follows Megaparsec idioms

2. Fix Duplicate Entry Bug

  • Issue: "constr" keyword appeared twice in choice list (lines 118 and 123)
  • Fix: Removed duplicate entry
  • Impact: Correctness fix

3. Add Strategic Labels for Better Error Messages

  • Issue: No labels used, resulting in generic error messages
  • Fix: Added two-level labeling with <?> operator
    • Delimiter level: "opening parenthesis '('", "closing bracket ']'"
    • Choice level: "term keyword (builtin, lam, ...)"
  • Impact: Clearer, more helpful error messages for users

4. Replace fail with <?> in Type Parser

  • Issue: Type parser used explicit fail instead of idiomatic <?>
  • Fix: Replaced with label operator
  • Impact: More idiomatic Megaparsec code, cleaner implementation

5. Refactor to Use between Combinator

  • Issue: Manual delimiter handling with intermediate bindings
  • Fix: Use standard between combinator
  • Impact: More declarative, less error-prone, follows parser combinator patterns

6. Expand Test Coverage

  • Added: 6 new error message tests
    • Missing closing parenthesis
    • Missing closing bracket
    • Missing builtin operand
    • Missing con operands
    • Invalid keyword
    • Bracket mismatch
  • Refactor: Extracted testParseError helper to eliminate duplication
  • Organized: Created "Error Messages" test group
  • Impact: Comprehensive coverage, maintainable test code

@Unisay Unisay self-assigned this Dec 24, 2025
@Unisay Unisay force-pushed the yura/parser-improvements-megaparsec-best-practices branch 2 times, most recently from d833dc6 to f0b9041 Compare December 24, 2025 14:25
@Unisay Unisay requested a review from zeme-wana December 24, 2025 15:01
@kwxm
Copy link
Contributor

kwxm commented Dec 26, 2025

Note also #7391.

Base automatically changed from bad-error-messages to master January 13, 2026 10:53
Remove redundant try wrappers:
- symbol is auto-backtracking since Megaparsec 4.4.0
- Removed 10 redundant try wrappers from keyword and delimiter parsing
- Fixed duplicate "constr" entry bug

Add strategic labels for better error messages:
- Use <?> operator for two-level labeling (delimiters + keywords)
- Replace explicit fail with <?> in type parser (more idiomatic)
- Labels provide clearer context in parse errors

Refactor to use between combinator:
- Cleaner, more declarative code structure
- Standard Megaparsec pattern for delimited parsing
- Eliminates intermediate bindings

Expand test coverage:
- Add 6 new error message tests covering common error scenarios
- Extract testParseError helper to reduce duplication
- Organize error tests into dedicated test group
- Verify labels appear correctly in error messages

All 2,973 tests pass (781 UPLC + 1,864 PLC + 328 PlutusIR)
@Unisay Unisay force-pushed the yura/parser-improvements-megaparsec-best-practices branch from f0b9041 to ec31d16 Compare January 14, 2026 14:33
@Unisay Unisay enabled auto-merge (squash) January 15, 2026 12:52
@zeme-wana zeme-wana disabled auto-merge January 15, 2026 13:12
@zeme-wana zeme-wana merged commit 0a906af into master Jan 15, 2026
344 of 479 checks passed
@zeme-wana zeme-wana deleted the yura/parser-improvements-megaparsec-best-practices branch January 15, 2026 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants