Show more sensible error messages for invalid placeholders

(like %{invalid}) in implicit outputs. SkylarkImplicitOutputsFunctionWithMap.calculateOutputs now throws an EvalException in case of invalid placeholders in com.google. \ devtools.build.lib.packages.ImplicitOutputsFucntion. Fix #2467 on Github -- Change-Id: I5460388d0b3f83390aed4c45c967c633c2574e3b Reviewed-on: https://cr.bazel.build/9490 PiperOrigin-RevId: 150907001 MOS_MIGRATED_REVID=150907001
bazelbuild · Mar 23, 2017 · 09b5957 · 09b5957
1 parent 9409e8d
commit 09b5957
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 6 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java b/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java
@@ -104,10 +104,18 @@ public ImmutableMap<String, String> calculateOutputs(AttributeMap map)
         ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
         for (Map.Entry<String, String> entry : castMap(callback.call(attrs),
             String.class, String.class, "implicit outputs function return value").entrySet()) {
+
+          // Returns empty string only in case of invalid templates
           Iterable<String> substitutions = fromTemplates(entry.getValue()).getImplicitOutputs(map);
-          if (!Iterables.isEmpty(substitutions)) {
-            builder.put(entry.getKey(), Iterables.getOnlyElement(substitutions));
+          if (Iterables.isEmpty(substitutions)) {
+            throw new EvalException(
+                loc,
+                String.format(
+                    "For attribute '%s' in outputs: %s",
+                    entry.getKey(), "Invalid placeholder(s) in template"));
           }
+
+          builder.put(entry.getKey(), Iterables.getOnlyElement(substitutions));
         }
         return builder.build();
       } catch (IllegalArgumentException e) {
@@ -129,13 +137,22 @@ public SkylarkImplicitOutputsFunctionWithMap(ImmutableMap<String, String> output
     }
 
     @Override
-    public ImmutableMap<String, String> calculateOutputs(AttributeMap map) {
+    public ImmutableMap<String, String> calculateOutputs(AttributeMap map) throws EvalException {
+
       ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
       for (Map.Entry<String, String> entry : outputMap.entrySet()) {
+        // Empty iff invalid placeholders present.
         Iterable<String> substitutions = fromTemplates(entry.getValue()).getImplicitOutputs(map);
-        if (!Iterables.isEmpty(substitutions)) {
-          builder.put(entry.getKey(), Iterables.getOnlyElement(substitutions));
+        if (Iterables.isEmpty(substitutions)) {
+          throw new EvalException(
+              null,
+              String.format(
+                  "For attribute '%s' in outputs: %s",
+                  entry.getKey(), "Invalid placeholder(s) in template"));
+
         }
+
+        builder.put(entry.getKey(), Iterables.getOnlyElement(substitutions));
       }
       return builder.build();
     }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/Rule.java b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
@@ -527,7 +527,7 @@ private void populateImplicitOutputFiles(EventHandler eventHandler, Package.Buil
         }
       }
     } catch (EvalException e) {
-      reportError(e.print(), eventHandler);
+      reportError(String.format("In rule %s: %s", getLabel(), e.print()), eventHandler);
     }
   }