Skip to content

Commit f0b9041

Browse files
committed
Improve parser error handling with Megaparsec best practices
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)
1 parent 5799445 commit f0b9041

File tree

11 files changed

+306
-98
lines changed

11 files changed

+306
-98
lines changed

plutus-core/plutus-core/src/PlutusCore/Parser/Type.hs

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -135,29 +135,29 @@ i.e. parse into @Tree Text@ and do the kind checking afterwards, but given that
135135
to do the kind checking of builtins regardless (even for UPLC), we don't win much by deferring
136136
doing it. -}
137137
defaultUni :: Parser (SomeTypeIn (Kinded DefaultUni))
138-
defaultUni = do
139-
choice $
140-
map
141-
try
142-
[ trailingWhitespace (inParens defaultUniApplication)
143-
, someType @_ @Integer <$ symbol "integer"
144-
, someType @_ @ByteString <$ symbol "bytestring"
145-
, someType @_ @Text <$ symbol "string"
146-
, someType @_ @() <$ symbol "unit"
147-
, someType @_ @Bool <$ symbol "bool"
148-
, someType @_ @[] <$ symbol "list"
149-
, someType @_ @Strict.Vector <$ symbol "array"
150-
, someType @_ @(,) <$ symbol "pair"
151-
, someType @_ @Data <$ symbol "data"
152-
, someType @_ @BLS12_381.G1.Element <$ symbol "bls12_381_G1_element"
153-
, someType @_ @BLS12_381.G2.Element <$ symbol "bls12_381_G2_element"
154-
, someType @_ @BLS12_381.Pairing.MlResult <$ symbol "bls12_381_mlresult"
155-
, someType @_ @Value <$ symbol "value"
156-
, -- We include an explicit failure case here to produce clearer error messages.
157-
-- Without this, using `choice` with `symbol` results in error messages that cover the longest possible SrcSpan,
158-
-- which in this context would be 20 characters spanning the entire "bls12_381_G2_element" token.
159-
fail "Unknown type, expected one of: bool, integer, bytestring, string, unit, list, array, pair, data, value, bls12_381_G1_element, bls12_381_G2_element, bls12_381_mlresult, or a type application in parens"
160-
]
138+
defaultUni =
139+
( choice $
140+
map
141+
try
142+
[ trailingWhitespace (inParens defaultUniApplication)
143+
, someType @_ @Integer <$ symbol "integer"
144+
, someType @_ @ByteString <$ symbol "bytestring"
145+
, someType @_ @Text <$ symbol "string"
146+
, someType @_ @() <$ symbol "unit"
147+
, someType @_ @Bool <$ symbol "bool"
148+
, someType @_ @[] <$ symbol "list"
149+
, someType @_ @Strict.Vector <$ symbol "array"
150+
, someType @_ @(,) <$ symbol "pair"
151+
, someType @_ @Data <$ symbol "data"
152+
, someType @_ @BLS12_381.G1.Element <$ symbol "bls12_381_G1_element"
153+
, someType @_ @BLS12_381.G2.Element <$ symbol "bls12_381_G2_element"
154+
, someType @_ @BLS12_381.Pairing.MlResult <$ symbol "bls12_381_mlresult"
155+
, someType @_ @Value <$ symbol "value"
156+
]
157+
)
158+
<?> "type name (integer, bytestring, string, unit, bool, list, array, pair,\
159+
\ data, value, bls12_381_G1_element, bls12_381_G2_element,\
160+
\ bls12_381_mlresult, or type application)"
161161

162162
tyName :: Parser TyName
163163
tyName = TyName <$> name

plutus-core/untyped-plutus-core/src/UntypedPlutusCore/Parser.hs

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -100,31 +100,31 @@ term =
100100
]
101101
where
102102
tryAppTerm :: Parser PTerm
103-
tryAppTerm = do
104-
withSpan $ \sp -> do
105-
_ <- try (symbol "[")
106-
t <- appTerm sp
107-
_ <- char ']'
108-
return t
103+
tryAppTerm =
104+
withSpan $ \sp ->
105+
between
106+
(symbol "[" <?> "opening bracket '['")
107+
(char ']' <?> "closing bracket ']'")
108+
(appTerm sp)
109109

110110
tryTermInParens :: Parser PTerm
111111
tryTermInParens =
112-
withSpan $ \sp -> do
113-
_ <- try (symbol "(")
114-
t <-
115-
choice
116-
[ try (symbol "builtin") *> builtinTerm sp
117-
, try (symbol "lam") *> lamTerm sp
118-
, try (symbol "constr") *> constrTerm sp -- "constr" must come before "con"
119-
, try (symbol "con") *> conTerm sp
120-
, try (symbol "delay") *> delayTerm sp
121-
, try (symbol "force") *> forceTerm sp
122-
, try (symbol "error") *> errorTerm sp
123-
, try (symbol "constr") *> constrTerm sp
124-
, try (symbol "case") *> caseTerm sp
125-
]
126-
_ <- char ')'
127-
return t
112+
withSpan $ \sp ->
113+
between
114+
(symbol "(" <?> "opening parenthesis '('")
115+
(char ')' <?> "closing parenthesis ')'")
116+
( choice
117+
[ symbol "builtin" *> builtinTerm sp
118+
, symbol "lam" *> lamTerm sp
119+
, symbol "constr" *> constrTerm sp -- "constr" must come before "con"
120+
, symbol "con" *> conTerm sp
121+
, symbol "delay" *> delayTerm sp
122+
, symbol "force" *> forceTerm sp
123+
, symbol "error" *> errorTerm sp
124+
, symbol "case" *> caseTerm sp
125+
]
126+
<?> "term keyword (builtin, lam, constr, con, delay, force, error, case)"
127+
)
128128

129129
-- | Parser for UPLC programs.
130130
program :: Parser (UPLC.Program PLC.Name PLC.DefaultUni PLC.DefaultFun SrcSpan)
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
test:1:18:
2+
|
3+
1 | (program 1.1.0 [(var x))
4+
| ^^^^^^^
5+
unexpected "var x))"
6+
expecting term keyword (builtin, lam, constr, con, delay, force, error, case)
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
test:1:17:
2+
|
3+
1 | (program 1.1.0 (foo x))
4+
| ^^^^^^^
5+
unexpected "foo x))"
6+
expecting term keyword (builtin, lam, constr, con, delay, force, error, case)
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
test:3:64:
2+
|
3+
3 | (force (builtin mkCons)) (con integer 4) (con (list integer) [true]) ]
4+
| ^
5+
unexpected 't'
6+
expecting '+', '-', ']', or integer
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
test:1:24:
2+
|
3+
1 | (program 1.1.0 (builtin))
4+
| ^
5+
Unknown built-in function '' at test:1:24.
6+
Parsable functions are [ addInteger
7+
, andByteString
8+
, appendByteString
9+
, appendString
10+
, bData
11+
, blake2b_224
12+
, blake2b_256
13+
, bls12_381_G1_add
14+
, bls12_381_G1_compress
15+
, bls12_381_G1_equal
16+
, bls12_381_G1_hashToGroup
17+
, bls12_381_G1_multiScalarMul
18+
, bls12_381_G1_neg
19+
, bls12_381_G1_scalarMul
20+
, bls12_381_G1_uncompress
21+
, bls12_381_G2_add
22+
, bls12_381_G2_compress
23+
, bls12_381_G2_equal
24+
, bls12_381_G2_hashToGroup
25+
, bls12_381_G2_multiScalarMul
26+
, bls12_381_G2_neg
27+
, bls12_381_G2_scalarMul
28+
, bls12_381_G2_uncompress
29+
, bls12_381_finalVerify
30+
, bls12_381_millerLoop
31+
, bls12_381_mulMlResult
32+
, byteStringToInteger
33+
, chooseData
34+
, chooseList
35+
, chooseUnit
36+
, complementByteString
37+
, consByteString
38+
, constrData
39+
, countSetBits
40+
, decodeUtf8
41+
, divideInteger
42+
, dropList
43+
, encodeUtf8
44+
, equalsByteString
45+
, equalsData
46+
, equalsInteger
47+
, equalsString
48+
, expModInteger
49+
, findFirstSetBit
50+
, fstPair
51+
, headList
52+
, iData
53+
, ifThenElse
54+
, indexArray
55+
, indexByteString
56+
, insertCoin
57+
, integerToByteString
58+
, keccak_256
59+
, lengthOfArray
60+
, lengthOfByteString
61+
, lessThanByteString
62+
, lessThanEqualsByteString
63+
, lessThanEqualsInteger
64+
, lessThanInteger
65+
, listData
66+
, listToArray
67+
, lookupCoin
68+
, mapData
69+
, mkCons
70+
, mkNilData
71+
, mkNilPairData
72+
, mkPairData
73+
, modInteger
74+
, multiplyInteger
75+
, nullList
76+
, orByteString
77+
, quotientInteger
78+
, readBit
79+
, remainderInteger
80+
, replicateByte
81+
, ripemd_160
82+
, rotateByteString
83+
, scaleValue
84+
, serialiseData
85+
, sha2_256
86+
, sha3_256
87+
, shiftByteString
88+
, sliceByteString
89+
, sndPair
90+
, subtractInteger
91+
, tailList
92+
, trace
93+
, unBData
94+
, unConstrData
95+
, unIData
96+
, unListData
97+
, unMapData
98+
, unValueData
99+
, unionValue
100+
, valueContains
101+
, valueData
102+
, verifyEcdsaSecp256k1Signature
103+
, verifyEd25519Signature
104+
, verifySchnorrSecp256k1Signature
105+
, writeBits
106+
, xorByteString ]
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
test:1:69:
2+
|
3+
1 | (program 1.1.0 [(builtin addInteger) (con integer 1) (con integer 2))
4+
| ^
5+
unexpected ')'
6+
expecting '`', closing bracket ']', opening bracket '[', or opening parenthesis '('
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
test:1:24:
2+
|
3+
1 | (program 1.1.0 (lam x (var x))
4+
| ^^^^^^^
5+
unexpected "var x))"
6+
expecting term keyword (builtin, lam, constr, con, delay, force, error, case)
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
test:1:20:
2+
|
3+
1 | (program 1.1.0 (con))
4+
| ^^
5+
unexpected "))"
6+
expecting type name (integer, bytestring, string, unit, bool, list, array, pair, data, value, bls12_381_G1_element, bls12_381_G2_element, bls12_381_mlresult, or type application)
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
test:2:38:
2+
|
3+
2 | [ (builtin integerToByteString) (con boot True) (con integer 0) (con integer 712372356934756347862573452345342345) ]
4+
| ^^^^^^^^^^^^^^^^^^^^
5+
unexpected "boot True) (con inte"
6+
expecting type name (integer, bytestring, string, unit, bool, list, array, pair, data, value, bls12_381_G1_element, bls12_381_G2_element, bls12_381_mlresult, or type application)

0 commit comments

Comments
 (0)