Fix when Ninja statements finishing earlier than parser expected
Closes #10758.
PiperOrigin-RevId: 294734381
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/parser/NinjaParserStep.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/parser/NinjaParserStep.java
index c625dc5..8640671 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/parser/NinjaParserStep.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/parser/NinjaParserStep.java
@@ -180,8 +180,7 @@
variablesBuilder.put(NinjaRuleVariable.NAME, NinjaVariableValue.createPlainText(name));
parseExpected(NinjaToken.NEWLINE);
- while (lexer.hasNextToken()) {
- parseExpected(NinjaToken.INDENT);
+ while (parseIndentOrFinishDeclaration()) {
String variableName = asString(parseExpected(NinjaToken.IDENTIFIER));
parseExpected(NinjaToken.EQUALS);
NinjaVariableValue value = parseVariableValue();
@@ -304,9 +303,7 @@
throws GenericParsingException {
Map<String, List<Pair<Integer, String>>> expandedVariables = Maps.newHashMap();
lexer.interpretPoolAsVariable();
- while (lexer.hasNextToken()) {
- parseExpected(NinjaToken.INDENT);
-
+ while (parseIndentOrFinishDeclaration()) {
Pair<String, NinjaVariableValue> pair = parseVariable();
String name = Preconditions.checkNotNull(pair.getFirst());
NinjaVariableValue value = Preconditions.checkNotNull(pair.getSecond());
@@ -402,30 +399,76 @@
return new String(value, StandardCharsets.ISO_8859_1);
}
+ /**
+ * It is expected that indent is preceding to the identifier in the scoped variable declaration.
+ * It can be, however, that it is just an empty line with spaces - in that case, we want to
+ * interpret it as the finish of the currently parsed lexeme.
+ *
+ * @return true if indent was parsed and there is something different than the newline after it.
+ */
+ private boolean parseIndentOrFinishDeclaration() throws GenericParsingException {
+ if (!lexer.hasNextToken()) {
+ return false;
+ }
+ NinjaToken token = lexer.nextToken();
+ boolean isIndent = NinjaToken.INDENT.equals(token);
+ if (!isIndent && !NinjaToken.NEWLINE.equals(token)) {
+ throwNotExpectedTokenError(NinjaToken.INDENT, token);
+ }
+ // Check for indent followed by newline, or end of file.
+ if (lexer.hasNextToken()) {
+ NinjaToken afterIndent = lexer.nextToken();
+ lexer.undo();
+ if (NinjaToken.NEWLINE.equals(afterIndent)) {
+ return false;
+ }
+ } else {
+ return false;
+ }
+ return isIndent;
+ }
+
private byte[] parseExpected(NinjaToken expectedToken) throws GenericParsingException {
+ checkLexerHasNextToken(expectedToken);
+ NinjaToken token = lexer.nextToken();
+ if (!expectedToken.equals(token)) {
+ throwNotExpectedTokenError(expectedToken, token);
+ }
+ return lexer.getTokenBytes();
+ }
+
+ private void throwNotExpectedTokenError(NinjaToken expectedToken, NinjaToken token)
+ throws GenericParsingException {
+ String actual =
+ NinjaToken.ERROR.equals(token)
+ ? String.format("error: '%s'", lexer.getError())
+ : asString(token.getBytes());
+ throw new GenericParsingException(
+ String.format(
+ "Expected %s, but got %s in fragment:\n%s\n",
+ asString(expectedToken.getBytes()),
+ actual,
+ lexer.getFragment().getFragmentAround(lexer.getLastStart())));
+ }
+
+ private void checkLexerHasNextToken(NinjaToken expectedToken) throws GenericParsingException {
if (!lexer.hasNextToken()) {
String message;
if (lexer.haveReadAnyTokens()) {
message =
String.format(
- "Expected %s after '%s'",
- asString(expectedToken.getBytes()), asString(lexer.getTokenBytes()));
+ "Expected %s after '%s' in fragment:\n%s\n",
+ asString(expectedToken.getBytes()),
+ asString(lexer.getTokenBytes()),
+ lexer.getFragment().getFragmentAround(lexer.getLastStart()));
} else {
message =
String.format(
- "Expected %s, but found no text to parse", asString(expectedToken.getBytes()));
+ "Expected %s, but found no text to parse after fragment:\n%s\n",
+ asString(expectedToken.getBytes()),
+ lexer.getFragment().getFragmentAround(lexer.getLastStart()));
}
throw new GenericParsingException(message);
}
- NinjaToken token = lexer.nextToken();
- if (!expectedToken.equals(token)) {
- String actual =
- NinjaToken.ERROR.equals(token)
- ? String.format("error: '%s'", lexer.getError())
- : asString(token.getBytes());
- throw new GenericParsingException(
- String.format("Expected %s, but got %s", asString(expectedToken.getBytes()), actual));
- }
- return lexer.getTokenBytes();
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaParserStepTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaParserStepTest.java
index 5ecdde7..ad54205 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaParserStepTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaParserStepTest.java
@@ -54,12 +54,12 @@
@Test
public void testVariableParsingException() {
- doTestVariableParsingException(" ", "Expected identifier, but got indent");
- doTestVariableParsingException("a", "Expected = after 'a'");
+ doTestVariableParsingException(" ", "Expected identifier, but got indent in fragment:\n \n");
+ doTestVariableParsingException("a", "Expected = after 'a' in fragment:\na\n");
doTestVariableParsingException(
"^a=",
"Expected identifier, but got error: 'Symbol '^' is not allowed in the identifier, "
- + "the text fragment with the symbol:\n^a=\n'");
+ + "the text fragment with the symbol:\n^a=\n' in fragment:\n^a=\n");
}
private static void doTestVariableParsingException(String text, String message) {
@@ -120,7 +120,7 @@
.hasMessageThat()
.isEqualTo(
"Expected newline, but got error: "
- + "'Bad $-escape (literal $ must be written as $$)'");
+ + "'Bad $-escape (literal $ must be written as $$)' in fragment:\ninclude x $\n");
GenericParsingException exception2 =
assertThrows(
@@ -136,6 +136,7 @@
@Test
public void testNinjaRule() throws Exception {
+ // Additionally test the situation when we get more line separators in the end.
NinjaParserStep parser =
createParser(
"rule testRule \n"
@@ -143,7 +144,7 @@
+ " description = Test rule for $TARGET\n"
+ " rspfile = $TARGET.in\n"
+ " deps = ${abc} $\n"
- + " ${cde}\n");
+ + " ${cde}\n\n\n");
NinjaRule ninjaRule = parser.parseNinjaRule();
ImmutableSortedMap<NinjaRuleVariable, NinjaVariableValue> variables = ninjaRule.getVariables();
assertThat(variables.keySet())
@@ -180,19 +181,41 @@
@Test
public void testNinjaRuleParsingException() {
doTestNinjaRuleParsingException(
- "rule testRule extra-word\n", "Expected newline, but got identifier");
+ "rule testRule extra-word\n",
+ String.join(
+ "\n",
+ "Expected newline, but got identifier in fragment:",
+ "rule testRule extra-word",
+ "",
+ ""));
doTestNinjaRuleParsingException(
- "rule testRule\ncommand =", "Expected indent, but got identifier");
+ "rule testRule\ncommand =",
+ String.join(
+ "\n",
+ "Expected indent, but got identifier in fragment:",
+ "rule testRule",
+ "command =",
+ ""));
doTestNinjaRuleParsingException(
"rule testRule\n ^custom = a",
- "Expected identifier, but got error: 'Symbol '^' is not allowed in the identifier, "
- + "the text fragment with the symbol:\nrule testRule\n ^custom = a\n'");
+ String.join(
+ "\n",
+ "Expected identifier, but got error: 'Symbol '^' is not allowed in the identifier, "
+ + "the text fragment with the symbol:",
+ "rule testRule",
+ " ^custom = a",
+ "' in fragment:",
+ "rule testRule",
+ " ^custom = a",
+ ""));
doTestNinjaRuleParsingException("rule testRule\n custom = a", "Unexpected variable 'custom'");
}
@Test
public void testNinjaTargets() throws Exception {
- NinjaTarget target = parseNinjaTarget("build output: command input");
+ // Additionally test the situation when the target does not have the variables section and
+ // we get more line separators in the end.
+ NinjaTarget target = parseNinjaTarget("build output: command input\n\n");
assertThat(target.getRuleName()).isEqualTo("command");
assertThat(target.getOutputs()).containsExactly(PathFragment.create("output"));
assertThat(target.getUsualInputs()).containsExactly(PathFragment.create("input"));
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaPipelineTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaPipelineTest.java
index 1ab3ed3..c1bbb5f 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaPipelineTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaPipelineTest.java
@@ -263,6 +263,36 @@
}
@Test
+ public void testSpacesInEmptyLine() throws Exception {
+ Path vfsPath =
+ tester.writeTmpFile(
+ "test.ninja",
+ "subfile=child",
+ " ",
+ "subninja_file=sub",
+ " ",
+ "rule r1",
+ " command = c $in $out",
+ // This could be interpreted as indent, but should be skipped as the empty line.
+ " ",
+ " \n\n\n\n",
+ "include ${subfile}.ninja",
+ " ",
+ "build t2: r1 in3",
+ " ",
+ "build t1: r1 in1 in2");
+ Path childFile = tester.writeTmpFile("child.ninja");
+ NinjaPipeline pipeline =
+ new NinjaPipeline(
+ vfsPath.getParentDirectory(),
+ tester.getService(),
+ ImmutableList.of(childFile),
+ "ninja_target");
+ List<NinjaTarget> targets = pipeline.pipeline(vfsPath);
+ checkTargets(targets);
+ }
+
+ @Test
public void testBigFile() throws Exception {
String[] lines = new String[1000];
for (int i = 0; i < lines.length - 1; i++) {
