Skip to content

Commit ae33f00

Browse files
Fix ||| block parsing (#433)
This PR fixes 2 edges cases that sjsonnet did not handle properly. 1. ||| blocks with \r\n 2. ||| blocks only with empty - indented - lines. The only practical way I found to fix this is to half-rewrite the parsing code myself to better get a handle on how fastparse works. This closes #430
1 parent 908cc50 commit ae33f00

File tree

7 files changed

+46
-77
lines changed

7 files changed

+46
-77
lines changed

sjsonnet/src/sjsonnet/Parser.scala

Lines changed: 38 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -126,25 +126,6 @@ class Parser(
126126
def literalSingleString[$: P]: P[Seq[String]] =
127127
P((CharsWhile(_ != '\'').! | "''".!.map(_ => "'")).repX ~~ "'")
128128

129-
def tripleBarStringLines[$: P]: P[Seq[String]] = P(
130-
tripleBarStringHead.flatMapX { case (pre, w, head) =>
131-
tripleBarStringBody(w).map(pre ++ Seq(head, "\n") ++ _)
132-
}
133-
)
134-
135-
def maybeChompedTripleBarString[$: P]: P[Seq[String]] = tripleBarString.map {
136-
case (true, lines) =>
137-
Seq(lines.mkString.stripLineEnd)
138-
case (false, lines) =>
139-
lines
140-
}
141-
142-
def tripleBarString[$: P]: P[(Boolean, Seq[String])] = P(
143-
("||-" | "||").?.!.map(_.last == '-')./ ~~ CharsWhileIn(" \t", 0) ~~
144-
"\n" ~~ tripleBarStringLines ~~ "\n" ~~
145-
CharsWhileIn(" \t", 0) ~~ "|||"
146-
)
147-
148129
def string[$: P]: P[String] = P(
149130
SingleChar.flatMapX {
150131
case '\"' => doubleString
@@ -160,18 +141,47 @@ class Parser(
160141
}
161142
).map(_.mkString)
162143

163-
def tripleBarStringHead[$: P]: P[(Seq[String], String, String)] = P(
164-
(CharsWhileIn(" \t", 0) ~~ "\n".!).repX ~~
165-
CharsWhileIn(" \t", 1).! ~~
166-
CharsWhile(_ != '\n').!
144+
def maybeChompedTripleBarString[$: P]: P[Seq[String]] = tripleBarString.map {
145+
case (true, lines) =>
146+
lines.dropRight(1) ++ Seq(lines.last.stripLineEnd)
147+
case (false, lines) =>
148+
lines
149+
}
150+
151+
def tripleBarJunk[$: P]: P[Unit] = CharsWhileIn(" \t", 0)
152+
153+
def tripleBarIndent[$: P]: P[String] = P(
154+
CharsWhileIn(" \t", 1)
155+
.opaque("|||-block line must either be an empty line or start with at least one whitespace")
156+
.!
157+
)
158+
159+
def tripleBarString[$: P]: P[(Boolean, Seq[String])] = P(
160+
("||-" | "||").!.map(
161+
_.last == '-'
162+
)./ ~~ tripleBarJunk ~~ tripleBarStringLines ~~ tripleBarJunk ~~ "|||"
167163
)
168-
def tripleBarBlankHead[$: P]: P[String] =
169-
P(CharsWhileIn(" \t", 0) ~~ &("\n").map(_ => "\n"))
170164

171-
def tripleBarBlank[$: P]: P[String] = P("\n" ~~ tripleBarBlankHead)
165+
def tripleBarStringLines[$: P]: P[Seq[String]] = P(
166+
// First, we skip an empty lines until we reach the first line with indentation.
167+
// This will become the indentation for the rest of the block.
168+
("\r\n" | "\n").opaque("|||-blocks require multiple lines").!./.flatMapX { case sep =>
169+
(sep.!.repX ~~ tripleBarIndent)
170+
.flatMapX { case (before, indent) =>
171+
tripleBarStringBody(indent, sep).map(before ++ _)
172+
}
173+
}
174+
)
172175

173-
def tripleBarStringBody[$: P](w: String): P[Seq[String]] = P(
174-
(tripleBarBlank | "\n" ~~ w ~~ CharsWhile(_ != '\n').!.map(_ + "\n")).repX
176+
def tripleBarStringBody[$: P](indent: String, sep: String): P[Seq[String]] = P(
177+
// Because we already parsed the indentation, we special case the first line and only look for the content.
178+
(CharsWhile(!sep.contains(_), 0) ~~ sep).!.flatMapX { firstLine =>
179+
// That's the core of the parsing. Either we have an empty line or we have indentation + content + new line separator.
180+
(sep.! | (indent.! ~~ (CharsWhile(!sep.contains(_), 0) ~~ sep).!).map(_._2))
181+
.opaque("|||-block line must either be an empty line or start with at least one whitespace")
182+
.repX
183+
.map(Seq(firstLine) ++ _)
184+
}
175185
)
176186

177187
def arr[$: P]: P[Expr] = P((Pos ~~ &("]")).map(Val.Arr(_, emptyLazyArray)) | arrBody)
Lines changed: 1 addition & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1 @@
1-
RUNTIME ERROR: max stack frames exceeded.
2-
-------------------------------------------------
3-
4-
5-
-------------------------------------------------
6-
testdata/std.makeArray_recursive_evalutation_order_matters:1:51-57 function <anonymous>
7-
8-
local arr = [0] + std.makeArray(2000, function(i) arr[i] + 1); arr[500]
9-
10-
-------------------------------------------------
11-
12-
13-
-------------------------------------------------
14-
testdata/std.makeArray_recursive_evalutation_order_matters:1:51-57 function <anonymous>
15-
16-
local arr = [0] + std.makeArray(2000, function(i) arr[i] + 1); arr[500]
17-
18-
-------------------------------------------------
19-
... (skipped 492 frames)
20-
-------------------------------------------------
21-
22-
23-
-------------------------------------------------
24-
testdata/std.makeArray_recursive_evalutation_order_matters:1:51-57 function <anonymous>
25-
26-
local arr = [0] + std.makeArray(2000, function(i) arr[i] + 1); arr[500]
27-
28-
-------------------------------------------------
29-
30-
31-
-------------------------------------------------
32-
testdata/std.makeArray_recursive_evalutation_order_matters:1:64-72 $
33-
34-
local arr = [0] + std.makeArray(2000, function(i) arr[i] + 1); arr[500]
35-
36-
-------------------------------------------------
37-
During evaluation
38-
39-
1+
500
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
sjsonnet.ParseError: Expected "\n":18:12, found "\r\n A pa"
2-
at .(sjsonnet/test/resources/test_suite/dos_line_endings.jsonnet:18:12)
3-
1+
{
2+
"text": "A paragraph\r\nof text.\r\n"
3+
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
sjsonnet.ParseError: Expected "\n":17:5, found "|||\n"
1+
sjsonnet.ParseError: Expected |||-blocks require multiple lines:17:5, found "|||\n"
22
at .(sjsonnet/test/resources/test_suite/error.parse.string_multi_no_newline.jsonnet:17:5)
33

sjsonnet/test/src-js/sjsonnet/FileTests.scala

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ object FileTests extends BaseFileTests {
66
val skippedTests = Set(
77
"stdlib.jsonnet",
88
"regex.jsonnet",
9-
"dos_line_endings.jsonnet",
109

1110
// Stack size issues with the JS runner
1211
"recursive_function.jsonnet",
@@ -23,10 +22,8 @@ object FileTests extends BaseFileTests {
2322
val goTestDataSkippedTests: Set[String] = Set(
2423
"bitwise_or9.jsonnet",
2524
"builtinChar6.jsonnet",
26-
"escaped_fields.jsonnet",
2725
"pow6.jsonnet",
2826
"object_invariant_plus.jsonnet",
29-
"std.makeArray_recursive_evalutation_order_matters.jsonnet",
3027
"tailstrict3.jsonnet",
3128
"stdlib_smoke_test.jsonnet",
3229
"builtinSha1.jsonnet",

sjsonnet/test/src-jvm-native/sjsonnet/BaseFileTests.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,9 @@ abstract class BaseFileTests extends TestSuite {
9797
println(res.left.getOrElse(""))
9898
assert(e.getMessage == goldenContent.stripLineEnd)
9999
}
100+
case e: Throwable =>
101+
println(stderr.toString)
102+
throw e
100103
}
101104
}
102105
}

sjsonnet/test/src-jvm-native/sjsonnet/FileTests.scala

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import utest.*
44

55
object FileTests extends BaseFileTests {
66
val testDataSkippedTests: Set[String] = Set(
7-
"dos_line_endings.jsonnet",
87
"regex_js.jsonnet",
98
"stdlib_js.jsonnet"
109
) ++ (if (isScalaNative) {
@@ -22,10 +21,8 @@ object FileTests extends BaseFileTests {
2221
val goTestDataSkippedTests: Set[String] = Set(
2322
"bitwise_or9.jsonnet",
2423
"builtinChar6.jsonnet",
25-
"escaped_fields.jsonnet",
2624
"pow6.jsonnet",
2725
"object_invariant_plus.jsonnet",
28-
"std.makeArray_recursive_evalutation_order_matters.jsonnet",
2926
"tailstrict3.jsonnet"
3027
) ++ (if (isScalaNative)
3128
Set(

0 commit comments

Comments
 (0)