Improve error message about wrong symbol in Ninja identifier
Otherwise, it is quite hard to debug problems.
Closes #10755.
PiperOrigin-RevId: 294455487
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/file/ByteBufferFragment.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/file/ByteBufferFragment.java
index 4996841..4e10509 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/file/ByteBufferFragment.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/file/ByteBufferFragment.java
@@ -91,6 +91,21 @@
return bytes;
}
+ /**
+ * Helper method for forming error messages with text fragment around a place with a problem.
+ *
+ * @param index position of the problematic symbol.
+ * @return a fragment if text around the problematic place, that can be retrieved from this buffer
+ */
+ public String getFragmentAround(int index) {
+ if (index < 0 || index >= length()) {
+ throw new IndexOutOfBoundsException(
+ String.format("Index out of bounds: %d (%d, %d).", index, 0, length()));
+ }
+ byte[] bytes = getBytes(Math.max(0, index - 200), Math.min(length(), index + 200));
+ return new String(bytes, StandardCharsets.ISO_8859_1);
+ }
+
private void getBytes(byte[] dst, int offset) {
if (dst.length - offset < length()) {
throw new IndexOutOfBoundsException(
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/lexer/NinjaLexerStep.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/lexer/NinjaLexerStep.java
index 6635378..650d0a4 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/lexer/NinjaLexerStep.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/lexer/NinjaLexerStep.java
@@ -47,7 +47,7 @@
private static final ImmutableSortedSet<Byte> IDENTIFIER_SYMBOLS =
createByteSet("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-");
private static final ImmutableSortedSet<Byte> TEXT_STOPPERS = createByteSet("\n\r \t#$:\u0000");
- private static final ImmutableSortedSet<Byte> PATH_STOPPERS = createByteSet("\n\r \t#$:|\u0000");
+ private static final ImmutableSortedSet<Byte> PATH_STOPPERS = createByteSet("\n\r \t$:|\u0000");
private static ImmutableSortedSet<Byte> createByteSet(String variants) {
ImmutableSortedSet.Builder<Byte> builder = ImmutableSortedSet.naturalOrder();
@@ -221,7 +221,11 @@
public void tryReadIdentifier() {
end = readIdentifier(position, true);
if (position >= end) {
- error = "Symbol is not allowed in the identifier.";
+ error =
+ String.format(
+ "Symbol '%s' is not allowed in the identifier,"
+ + " the text fragment with the symbol:\n%s\n",
+ fragment.subFragment(position, position + 1), fragment.getFragmentAround(position));
end = position + 1;
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaLexerStepTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaLexerStepTest.java
index d343232..9d88fe0 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaLexerStepTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaLexerStepTest.java
@@ -130,12 +130,18 @@
doTest("abc_d-18", NinjaLexerStep::tryReadIdentifier);
doTest("abc_d-18.ccc", NinjaLexerStep::tryReadIdentifier);
doTest("abc_d-18.ccc=", NinjaLexerStep::tryReadIdentifier, "abc_d-18.ccc", true);
+ // Have a longer text to demonstrate the error output.
doTestError(
- "^abc",
+ "^abc Bazel only rebuilds what is necessary. "
+ + "With advanced local and distributed caching, optimized dependency analysis "
+ + "and parallel execution, you get fast and incremental builds.",
NinjaLexerStep::tryReadIdentifier,
"^",
true,
- "Symbol is not allowed in the identifier.");
+ "Symbol '^' is not allowed in the identifier, the text fragment with the symbol:\n"
+ + "^abc Bazel only rebuilds what is necessary. With advanced local and distributed"
+ + " caching, optimized dependency analysis and parallel execution,"
+ + " you get fast and incremental builds.\n");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaLexerTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaLexerTest.java
index 94787c3..7ea35cd 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaLexerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaLexerTest.java
@@ -79,7 +79,10 @@
@Test
public void testDisallowedSymbols() {
- assertError(createLexer("^"), "Symbol is not allowed in the identifier.", "^");
+ assertError(
+ createLexer("^"),
+ "Symbol '^' is not allowed in the identifier, the text fragment with the symbol:\n^\n",
+ "^");
}
@Test
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 6b6ccfb..5ecdde7 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
@@ -57,7 +57,9 @@
doTestVariableParsingException(" ", "Expected identifier, but got indent");
doTestVariableParsingException("a", "Expected = after 'a'");
doTestVariableParsingException(
- "^a=", "Expected identifier, but got error: 'Symbol is not allowed in the identifier.'");
+ "^a=",
+ "Expected identifier, but got error: 'Symbol '^' is not allowed in the identifier, "
+ + "the text fragment with the symbol:\n^a=\n'");
}
private static void doTestVariableParsingException(String text, String message) {
@@ -183,7 +185,8 @@
"rule testRule\ncommand =", "Expected indent, but got identifier");
doTestNinjaRuleParsingException(
"rule testRule\n ^custom = a",
- "Expected identifier, but got error: 'Symbol is not allowed in the identifier.'");
+ "Expected identifier, but got error: 'Symbol '^' is not allowed in the identifier, "
+ + "the text fragment with the symbol:\nrule testRule\n ^custom = a\n'");
doTestNinjaRuleParsingException("rule testRule\n custom = a", "Unexpected variable 'custom'");
}
