Throw a parsing exception when ninja targets reference undefined rules
RELNOTES: None.
PiperOrigin-RevId: 309066431
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/parser/NinjaTarget.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/parser/NinjaTarget.java
index 6522d17..06b52b1 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/parser/NinjaTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/parser/NinjaTarget.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.Interner;
+import com.google.devtools.build.lib.bazel.rules.ninja.file.GenericParsingException;
import com.google.devtools.build.lib.collect.ImmutableSortedKeyListMultimap;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -80,7 +81,7 @@
return this;
}
- public NinjaTarget build() {
+ public NinjaTarget build() throws GenericParsingException {
Preconditions.checkNotNull(ruleName);
String internedName = nameInterner.intern(ruleName);
ImmutableSortedMap<String, String> variables = variablesBuilder.build();
@@ -91,8 +92,8 @@
} else {
NinjaRule ninjaRule = scope.findRule(offset, ruleName);
if (ninjaRule == null) {
- // TODO(cparsons): This indicates no rule was found, so throw a parse exception instead.
- ruleVariables = ImmutableSortedMap.of();
+ throw new GenericParsingException(
+ String.format("could not resolve rule '%s'", internedName));
} else {
ruleVariables =
reduceRuleVariables(scope, offset, ninjaRule.getVariables(), variables, nameInterner);
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 b53e313..8058ec9 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
@@ -46,6 +46,8 @@
/** Tests for {@link NinjaParserStep}. */
@RunWith(JUnit4.class)
public class NinjaParserStepTest {
+ private static final int LINE_NUM_AFTER_RULE_DEFS = 5;
+
@Test
public void testSimpleVariable() throws Exception {
doTestSimpleVariable("a=b", "a", "b");
@@ -313,15 +315,20 @@
@Test
public void testNinjaTargets() throws Exception {
+ NinjaScope scope = scopeWithStubRule("command");
+
// 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");
+ NinjaTarget target =
+ createParser("build output: command input\n\n")
+ .parseNinjaTarget(scope, LINE_NUM_AFTER_RULE_DEFS);
assertThat(target.getRuleName()).isEqualTo("command");
assertThat(target.getOutputs()).containsExactly(PathFragment.create("output"));
assertThat(target.getUsualInputs()).containsExactly(PathFragment.create("input"));
NinjaTarget target1 =
- parseNinjaTarget("build o1 o2 | io1 io2: command i1 i2 | ii1 ii2 || ooi1 ooi2");
+ createParser("build o1 o2 | io1 io2: command i1 i2 | ii1 ii2 || ooi1 ooi2")
+ .parseNinjaTarget(scope, LINE_NUM_AFTER_RULE_DEFS);
assertThat(target1.getRuleName()).isEqualTo("command");
assertThat(target1.getOutputs())
.containsExactly(PathFragment.create("o1"), PathFragment.create("o2"));
@@ -334,11 +341,14 @@
assertThat(target1.getOrderOnlyInputs())
.containsExactly(PathFragment.create("ooi1"), PathFragment.create("ooi2"));
- NinjaTarget target2 = parseNinjaTarget("build output: phony");
+ NinjaTarget target2 =
+ createParser("build output: phony").parseNinjaTarget(scope, LINE_NUM_AFTER_RULE_DEFS);
assertThat(target2.getRuleName()).isEqualTo("phony");
assertThat(target2.getOutputs()).containsExactly(PathFragment.create("output"));
- NinjaTarget target3 = parseNinjaTarget("build output: command $\n || order-only-input");
+ NinjaTarget target3 =
+ createParser("build output: command $\n || order-only-input")
+ .parseNinjaTarget(scope, LINE_NUM_AFTER_RULE_DEFS);
assertThat(target3.getRuleName()).isEqualTo("command");
assertThat(target3.getOutputs()).containsExactly(PathFragment.create("output"));
assertThat(target3.getOrderOnlyInputs())
@@ -376,7 +386,7 @@
createParser(
"build $output : testRule $input $dir/abcde\n"
+ " dir = def$input\n empty = '$dir'")
- .parseNinjaTarget(scope, 5);
+ .parseNinjaTarget(scope, LINE_NUM_AFTER_RULE_DEFS);
assertThat(target.getRuleName()).isEqualTo("testRule");
assertThat(target.getOutputs()).containsExactly(PathFragment.create("out123"));
assertThat(target.getUsualInputs())
@@ -400,7 +410,12 @@
@Test
public void testNinjaTargetsPathWithEscapedSpace() throws Exception {
- NinjaTarget target = parseNinjaTarget("build output : command input$ with$ space other");
+ NinjaScope scope = scopeWithStubRule("command");
+
+ NinjaTarget target =
+ createParser("build output : command input$ with$ space other")
+ .parseNinjaTarget(scope, LINE_NUM_AFTER_RULE_DEFS);
+
assertThat(target.getRuleName()).isEqualTo("command");
assertThat(target.getOutputs()).containsExactly(PathFragment.create("output"));
assertThat(target.getUsualInputs())
@@ -409,9 +424,12 @@
@Test
public void testNinjaTargetsPathWithEscapedNewline() throws Exception {
+ NinjaScope scope = scopeWithStubRule("command");
+
NinjaTarget target =
- parseNinjaTarget(
- "build $\n" + " output : $\n" + " command input$\n" + " with$\n" + " newline");
+ createParser(
+ "build $\n" + " output : $\n" + " command input$\n" + " with$\n" + " newline")
+ .parseNinjaTarget(scope, LINE_NUM_AFTER_RULE_DEFS);
assertThat(target.getRuleName()).isEqualTo("command");
assertThat(target.getOutputs()).containsExactly(PathFragment.create("output"));
assertThat(target.getUsualInputs())
@@ -423,12 +441,36 @@
@Test
public void testNinjaTargetWithScope() throws Exception {
- NinjaTarget target = parseNinjaTarget("build output : command input\n pool = abc\n");
+ NinjaScope scope = scopeWithStubRule("command");
+
+ NinjaTarget target =
+ createParser("build output : command input\n pool = abc\n")
+ .parseNinjaTarget(scope, LINE_NUM_AFTER_RULE_DEFS);
+
assertThat(target.getRuleName()).isEqualTo("command");
assertThat(target.getOutputs()).containsExactly(PathFragment.create("output"));
assertThat(target.getUsualInputs()).containsExactly(PathFragment.create("input"));
}
+ @Test
+ public void testUndefinedRuleForTarget() throws Exception {
+ GenericParsingException exception =
+ assertThrows(
+ GenericParsingException.class,
+ () -> parseNinjaTarget("build output : myUndefinedRule input\n pool = abc\n"));
+
+ assertThat(exception).hasMessageThat().contains("could not resolve rule 'myUndefinedRule'");
+ }
+
+ private static NinjaScope scopeWithStubRule(String ruleName) throws Exception {
+ NinjaScope scope = new NinjaScope();
+
+ NinjaParserStep parser = createParser("rule " + ruleName + " \n");
+ NinjaRule ninjaRule = parser.parseNinjaRule();
+ scope.setRules(ImmutableSortedMap.of(ruleName, ImmutableList.of(Pair.of(0L, ninjaRule))));
+ return scope;
+ }
+
private static void testNinjaTargetParsingError(String text, String error) {
GenericParsingException exception =
assertThrows(GenericParsingException.class, () -> parseNinjaTarget(text));
diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/NinjaBlackBoxTest.java b/src/test/java/com/google/devtools/build/lib/blackbox/tests/NinjaBlackBoxTest.java
index 32d5680..c427461 100644
--- a/src/test/java/com/google/devtools/build/lib/blackbox/tests/NinjaBlackBoxTest.java
+++ b/src/test/java/com/google/devtools/build/lib/blackbox/tests/NinjaBlackBoxTest.java
@@ -393,7 +393,7 @@
"build group1: phony a b c",
"build group2: phony d e",
"build inputs_alias: phony group1 group2",
- "build hello.txt: echo inputs_alias",
+ "build hello.txt: cat inputs_alias",
"build alias: phony hello.txt");
context()
