Skip to content

Commit 5fc5730

Browse files
authored
chore: emit better errors for strings with bad indentation port google/jsonnet#1261 (#596)
refs: #578
1 parent 77cb322 commit 5fc5730

6 files changed

Lines changed: 110 additions & 18 deletions

File tree

sjsonnet/src/sjsonnet/Error.scala

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,14 +86,18 @@ object Error {
8686
}
8787
}
8888

89-
class ParseError(msg: String, stack: List[Error.Frame] = Nil, underlying: Option[Throwable] = None)
89+
class ParseError(
90+
msg: String,
91+
stack: List[Error.Frame] = Nil,
92+
underlying: Option[Throwable] = None,
93+
val offset: Int = -1)
9094
extends Error(msg, stack, underlying) {
9195

9296
override protected def copy(
9397
msg: String = msg,
9498
stack: List[Error.Frame] = stack,
9599
underlying: Option[Throwable] = underlying): sjsonnet.ParseError =
96-
new ParseError(msg, stack, underlying)
100+
new ParseError(msg, stack, underlying, offset)
97101
}
98102

99103
class StaticError(msg: String, stack: List[Error.Frame] = Nil, underlying: Option[Throwable] = None)

sjsonnet/src/sjsonnet/Importer.scala

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -209,16 +209,23 @@ class CachedResolver(
209209
ev: EvalErrorScope): Either[Error, (Expr, FileScope)] = {
210210
parseCache.getOrElseUpdate(
211211
(path, content.contentHash()), {
212-
val parsed = fastparse.parse(
213-
content.getParserInput(),
214-
parser(path).document(_)
215-
) match {
216-
case f @ Parsed.Failure(_, _, _) =>
217-
val traced = f.trace()
218-
val pos = new Position(new FileScope(path), traced.index)
219-
Left(new ParseError(traced.msg).addFrame(pos))
220-
case Parsed.Success(r, _) => Right(r)
221-
}
212+
val parsed =
213+
try {
214+
fastparse.parse(
215+
content.getParserInput(),
216+
parser(path).document(_)
217+
) match {
218+
case f @ Parsed.Failure(_, _, _) =>
219+
val traced = f.trace()
220+
val pos = new Position(new FileScope(path), traced.index)
221+
Left(new ParseError(traced.msg).addFrame(pos))
222+
case Parsed.Success(r, _) => Right(r)
223+
}
224+
} catch {
225+
case e: ParseError if e.offset >= 0 =>
226+
val pos = new Position(new FileScope(path), e.offset)
227+
Left(new ParseError(e.getMessage).addFrame(pos))
228+
}
222229
parsed.flatMap { case (e, fs) => process(e, fs) }
223230
}
224231
)

sjsonnet/src/sjsonnet/Parser.scala

Lines changed: 64 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ object Parser {
6868

6969
private val emptyLazyArray = new Array[Lazy](0)
7070
private val isSpaceOrTab: Char => Boolean = c => c == ' ' || c == '\t'
71+
7172
}
7273

7374
class Parser(
@@ -91,6 +92,10 @@ class Parser(
9192
}
9293
}
9394

95+
private def failParse(msg: String, offset: Int): Nothing = {
96+
throw new ParseError(msg, offset = offset)
97+
}
98+
9499
def Pos[$: P]: P[Position] = Index.map(offset => new Position(fileScope, offset))
95100

96101
def id[$: P]: P[String] = P(
@@ -284,7 +289,8 @@ class Parser(
284289

285290
def maybeChompedTripleBarString[$: P]: P[Seq[String]] = tripleBarString.map {
286291
case (true, lines) =>
287-
lines.dropRight(1) ++ Seq(lines.last.stripLineEnd)
292+
val last = lines.length - 1
293+
lines.updated(last, lines(last).stripLineEnd)
288294
case (false, lines) =>
289295
lines
290296
}
@@ -314,14 +320,67 @@ class Parser(
314320
}
315321
)
316322

323+
private def describeWhitespace(whitespace: String): String = {
324+
var spaces = 0
325+
var tabs = 0
326+
for (c <- whitespace) {
327+
if (c == ' ') spaces += 1
328+
else if (c == '\t') tabs += 1
329+
}
330+
if (spaces > 0 && tabs > 0) {
331+
s"${spaces} ${if (spaces == 1) "space" else "spaces"} and ${tabs} ${
332+
if (tabs == 1) "tab" else "tabs"
333+
}"
334+
} else if (spaces > 0) {
335+
s"${spaces} ${if (spaces == 1) "space" else "spaces"}"
336+
} else if (tabs > 0) {
337+
s"${tabs} ${if (tabs == 1) "tab" else "tabs"}"
338+
} else {
339+
"no indentation"
340+
}
341+
}
342+
317343
def tripleBarStringBody[$: P](indent: String, sep: String): P[Seq[String]] = P(
318-
// Because we already parsed the indentation, we special case the first line and only look for the content.
344+
// First line: indentation already consumed, just capture content up to the line separator.
319345
(CharsWhile(!sep.contains(_), 0) ~~ sep).!.flatMapX { firstLine =>
320-
// That's the core of the parsing. Either we have an empty line or we have indentation + content + new line separator.
321-
(sep.! | (indent.! ~~ (CharsWhile(!sep.contains(_), 0) ~~ sep).!).map(_._2))
346+
// Subsequent lines: empty line (just separator) | indented line with whitespace check.
347+
(sep.! | tripleBarStringIndentedLine(indent, sep))
322348
.opaque("|||-block line must either be an empty line or start with at least one whitespace")
323349
.repX
324-
.map(Seq(firstLine) ++ _)
350+
.map(firstLine +: _)
351+
}
352+
)
353+
354+
private def tripleBarStringIndentedLine[$: P](indent: String, sep: String): P[String] = P(
355+
// Parse whitespace once, then check if it matches the expected indentation.
356+
(CharsWhile(isSpaceOrTab, 0).! ~~ Index).flatMapX { case (whitespace, wsEndOffset) =>
357+
if (whitespace.isEmpty) {
358+
// No whitespace at all — not an indented line (e.g. ||| terminator without leading spaces).
359+
Fail.opaque("expected indentation for text block line")
360+
} else if (whitespace.startsWith(indent)) {
361+
// Indentation matches — parse the rest of the line content after the indent.
362+
(CharsWhile(!sep.contains(_), 0) ~~ sep).!.map { content =>
363+
whitespace.substring(indent.length) + content
364+
}
365+
} else {
366+
val isTerminator = P.current.input.isReachable(wsEndOffset + 2) &&
367+
P.current.input(wsEndOffset) == '|' &&
368+
P.current.input(wsEndOffset + 1) == '|' &&
369+
P.current.input(wsEndOffset + 2) == '|'
370+
if (isTerminator) {
371+
// This is the ||| terminator line — fail to let the outer parser match |||.
372+
Fail.opaque("text block terminator |||")
373+
} else {
374+
// Indentation mismatch — emit a detailed error.
375+
val expectedDescription = describeWhitespace(indent)
376+
val actualDescription = describeWhitespace(whitespace)
377+
failParse(
378+
"text block indentation mismatch: expected at least " +
379+
expectedDescription + ", found " + actualDescription,
380+
wsEndOffset
381+
)
382+
}
383+
}
325384
}
326385
)
327386

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
sjsonnet.ParseError: Expected "|||":19:2, found "bar\n|||\n"
1+
sjsonnet.ParseError: text block indentation mismatch: expected at least 4 spaces, found 1 tab
22
at .(error.parse.text_block_bad_whitespace.jsonnet:19:2)
33

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/*
2+
Copyright 2026 Google Inc. All rights reserved.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
|||
18+
foo
19+
bar
20+
|||
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
sjsonnet.ParseError: text block indentation mismatch: expected at least 4 spaces, found 2 spaces
2+
at .(error.parse.text_block_indent_spaces.jsonnet:19:3)

0 commit comments

Comments
 (0)