Improve error diagnostics for Skylark aspects.

--
MOS_MIGRATED_REVID=105851371

8 files changed, 221 insertions, 121 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Aspect.java b/src/main/java/com/google/devtools/build/lib/analysis/Aspect.java
index bb6c38d9a4..835fb5e7de 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Aspect.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Aspect.java
@@ -23,6 +23,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.syntax.EvalException;
 
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -132,8 +133,9 @@ public final class Aspect implements Iterable<TransitiveInfoProvider> {
       return this;
     }
 
-    public Builder addSkylarkTransitiveInfo(String name, Object value, Location loc) {
-      // TODO(dslomov): add {@link RuleConfiguredTargetBuilder#checkSkylarkObjectSafe}
+    public Builder addSkylarkTransitiveInfo(String name, Object value, Location loc)
+        throws EvalException {
+      SkylarkProviderValidationUtil.validateAndThrowEvalException(name, value, loc);
       skylarkProviderBuilder.put(name, value);
       return this;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
index 6e0936861e..e89e91ccd3 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -37,7 +37,6 @@ import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.License;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.TargetUtils;
-import com.google.devtools.build.lib.rules.SkylarkApiProvider;
 import com.google.devtools.build.lib.rules.extra.ExtraActionMapProvider;
 import com.google.devtools.build.lib.rules.extra.ExtraActionSpec;
 import com.google.devtools.build.lib.rules.test.ExecutionInfoProvider;
@@ -46,12 +45,7 @@ import com.google.devtools.build.lib.rules.test.TestActionBuilder;
 import com.google.devtools.build.lib.rules.test.TestEnvironmentProvider;
 import com.google.devtools.build.lib.rules.test.TestProvider;
 import com.google.devtools.build.lib.rules.test.TestProvider.TestParams;
-import com.google.devtools.build.lib.syntax.ClassObject;
 import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.syntax.EvalUtils;
-import com.google.devtools.build.lib.syntax.Runtime;
-import com.google.devtools.build.lib.syntax.SkylarkList;
-import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
 import com.google.devtools.build.lib.syntax.Type;
 
 import java.util.LinkedHashMap;
@@ -347,12 +341,8 @@ public final class RuleConfiguredTargetBuilder {
    */
   public RuleConfiguredTargetBuilder addSkylarkTransitiveInfo(
       String name, Object value, Location loc) throws EvalException {
-    try {
-      checkSkylarkObjectSafe(value);
-    } catch (IllegalArgumentException e) {
-      throw new EvalException(loc, String.format("Value of provider '%s' is of an illegal type: %s",
-          name, e.getMessage()));
-    }
+
+    SkylarkProviderValidationUtil.validateAndThrowEvalException(name, value, loc);
     skylarkProviders.put(name, value);
     return this;
   }
@@ -362,70 +352,12 @@ public final class RuleConfiguredTargetBuilder {
    */
   public RuleConfiguredTargetBuilder addSkylarkTransitiveInfo(
       String name, Object value) {
-    checkSkylarkObjectSafe(value);
+    SkylarkProviderValidationUtil.checkSkylarkObjectSafe(value);
     skylarkProviders.put(name, value);
     return this;
   }
 
   /**
-   * Check if the value provided by a Skylark provider is safe (i.e. can be a
-   * TransitiveInfoProvider value).
-   */
-  private void checkSkylarkObjectSafe(Object value) {
-    if (!isSimpleSkylarkObjectSafe(value.getClass())
-        // Java transitive Info Providers are accessible from Skylark.
-        && !(value instanceof TransitiveInfoProvider)) {
-      checkCompositeSkylarkObjectSafe(value);
-    }
-  }
-
-  private void checkCompositeSkylarkObjectSafe(Object object) {
-    if (object instanceof SkylarkApiProvider) {
-      return;
-    } else if (object instanceof SkylarkList) {
-      SkylarkList list = (SkylarkList) object;
-      if (list.isEmpty()) {
-        // Try not to iterate over the list if avoidable.
-        return;
-      }
-      // The list can be a tuple or a list of composite items.
-      for (Object listItem : list) {
-        checkSkylarkObjectSafe(listItem);
-      }
-      return;
-    } else if (object instanceof SkylarkNestedSet) {
-      // SkylarkNestedSets cannot have composite items.
-      Class<?> contentType = ((SkylarkNestedSet) object).getContentType().getType();
-      if (!contentType.equals(Object.class) && !isSimpleSkylarkObjectSafe(contentType)) {
-        throw new IllegalArgumentException(EvalUtils.getDataTypeName(contentType));
-      }
-      return;
-    } else if (object instanceof Map<?, ?>) {
-      for (Map.Entry<?, ?> entry : ((Map<?, ?>) object).entrySet()) {
-        checkSkylarkObjectSafe(entry.getKey());
-        checkSkylarkObjectSafe(entry.getValue());
-      }
-      return;
-    } else if (object instanceof ClassObject) {
-      ClassObject struct = (ClassObject) object;
-      for (String key : struct.getKeys()) {
-        checkSkylarkObjectSafe(struct.getValue(key));
-      }
-      return;
-    }
-    throw new IllegalArgumentException(EvalUtils.getDataTypeName(object));
-  }
-
-  private boolean isSimpleSkylarkObjectSafe(Class<?> type) {
-    return type.equals(String.class)
-        || type.equals(Integer.class)
-        || type.equals(Boolean.class)
-        || Artifact.class.isAssignableFrom(type)
-        || type.equals(Label.class)
-        || type.equals(Runtime.NoneType.class);
-  }
-
-  /**
    * Set the runfiles support for executable targets.
    */
   public RuleConfiguredTargetBuilder setRunfilesSupport(
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviderValidationUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviderValidationUtil.java
new file mode 100644
index 0000000000..869a2b0fb5
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviderValidationUtil.java
@@ -0,0 +1,126 @@
+// Copyright 2015 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.analysis;
+
+import com.google.common.base.Function;
+import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.rules.SkylarkApiProvider;
+import com.google.devtools.build.lib.syntax.ClassObject;
+import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.EvalUtils;
+import com.google.devtools.build.lib.syntax.SkylarkList;
+import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
+
+import java.util.Map;
+
+/**
+ * Utility class to validate results of executing Skylark rules and aspects.
+ */
+public class SkylarkProviderValidationUtil {
+  /**
+   * Check if the value provided by a Skylark provider is safe (i.e. can be a
+   * TransitiveInfoProvider value).
+   */
+  public static void checkSkylarkObjectSafe(Object value) {
+    if (!isSimpleSkylarkObjectSafe(value.getClass())
+        // Java transitive Info Providers are accessible from Skylark.
+        && !(value instanceof TransitiveInfoProvider)) {
+      checkCompositeSkylarkObjectSafe(value);
+    }
+  }
+
+  /**
+   * Check if the value provided by a Skylark provider is safe (i.e. can be a
+   * TransitiveInfoProvider value).
+   * Throws {@link EvalException} if not.
+   */
+  public static void validateAndThrowEvalException(String providerName, Object value, Location loc)
+      throws EvalException {
+    try {
+      checkSkylarkObjectSafe(value);
+    } catch (IllegalArgumentException e) {
+      throw new EvalException(
+          loc,
+          String.format(
+              "Value of provider '%s' is of an illegal type: %s", providerName, e.getMessage()));
+    }
+  }
+
+
+  private static void checkCompositeSkylarkObjectSafe(Object object) {
+    if (object instanceof SkylarkApiProvider) {
+      return;
+    } else if (object instanceof SkylarkList) {
+      SkylarkList list = (SkylarkList) object;
+      if (list.isEmpty()) {
+        // Try not to iterate over the list if avoidable.
+        return;
+      }
+      // The list can be a tuple or a list of composite items.
+      for (Object listItem : list) {
+        checkSkylarkObjectSafe(listItem);
+      }
+      return;
+    } else if (object instanceof SkylarkNestedSet) {
+      // SkylarkNestedSets cannot have composite items.
+      Class<?> contentType = ((SkylarkNestedSet) object).getContentType().getType();
+      if (!contentType.equals(Object.class) && !isSimpleSkylarkObjectSafe(contentType)) {
+        throw new IllegalArgumentException(EvalUtils.getDataTypeName(contentType));
+      }
+      return;
+    } else if (object instanceof Map<?, ?>) {
+      for (Map.Entry<?, ?> entry : ((Map<?, ?>) object).entrySet()) {
+        checkSkylarkObjectSafe(entry.getKey());
+        checkSkylarkObjectSafe(entry.getValue());
+      }
+      return;
+    } else if (object instanceof ClassObject) {
+      ClassObject struct = (ClassObject) object;
+      for (String key : struct.getKeys()) {
+        checkSkylarkObjectSafe(struct.getValue(key));
+      }
+      return;
+    }
+    throw new IllegalArgumentException(EvalUtils.getDataTypeName(object));
+  }
+
+  private static boolean isSimpleSkylarkObjectSafe(Class<?> type) {
+    return type.equals(String.class)
+        || type.equals(Integer.class)
+        || type.equals(Boolean.class)
+        || Artifact.class.isAssignableFrom(type)
+        || type.equals(Label.class)
+        || type.equals(com.google.devtools.build.lib.syntax.Runtime.NoneType.class);
+  }
+
+  public static void checkOrphanArtifacts(RuleContext ruleContext) throws EvalException {
+    ImmutableSet<Artifact> orphanArtifacts =
+        ruleContext.getAnalysisEnvironment().getOrphanArtifacts();
+    if (!orphanArtifacts.isEmpty()) {
+      throw new EvalException(null, "The following files have no generating action:\n"
+          + Joiner.on("\n").join(Iterables.transform(orphanArtifacts,
+          new Function<Artifact, String>() {
+            @Override
+            public String apply(Artifact artifact) {
+              return artifact.getRootRelativePathString();
+            }
+          })));
+    }
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
index 9e7ba336e5..31693fc200 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
@@ -13,12 +13,8 @@
 // limitations under the License.
 package com.google.devtools.build.lib.rules;
 
-import com.google.common.base.Function;
-import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
@@ -26,6 +22,7 @@ import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
 import com.google.devtools.build.lib.analysis.RunfilesSupport;
+import com.google.devtools.build.lib.analysis.SkylarkProviderValidationUtil;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.Rule;
@@ -80,7 +77,7 @@ public final class SkylarkRuleConfiguredTargetBuilder {
         return null;
       }
       ConfiguredTarget configuredTarget = createTarget(ruleContext, target);
-      checkOrphanArtifacts(ruleContext);
+      SkylarkProviderValidationUtil.checkOrphanArtifacts(ruleContext);
       return configuredTarget;
     } catch (EvalException e) {
       addRuleToStackTrace(e, ruleContext.getRule(), ruleImplementation);
@@ -100,9 +97,11 @@ public final class SkylarkRuleConfiguredTargetBuilder {
    */
   private static void addRuleToStackTrace(EvalException ex, Rule rule, BaseFunction ruleImpl) {
     if (ex instanceof EvalExceptionWithStackTrace) {
-      ((EvalExceptionWithStackTrace) ex).registerRule(
-          String.format("%s(name = '%s')", rule.getRuleClass(), rule.getName()),
-          rule.getLocation(), ruleImpl);
+      ((EvalExceptionWithStackTrace) ex)
+          .registerPhantomFuncall(
+              String.format("%s(name = '%s')", rule.getRuleClass(), rule.getName()),
+              rule.getLocation(),
+              ruleImpl);
     }
   }
 
@@ -116,20 +115,6 @@ public final class SkylarkRuleConfiguredTargetBuilder {
     return ex.getMessage();
   }
 
-  private static void checkOrphanArtifacts(RuleContext ruleContext) throws EvalException {
-    ImmutableSet<Artifact> orphanArtifacts =
-        ruleContext.getAnalysisEnvironment().getOrphanArtifacts();
-    if (!orphanArtifacts.isEmpty()) {
-      throw new EvalException(null, "The following files have no generating action:\n"
-          + Joiner.on("\n").join(Iterables.transform(orphanArtifacts,
-        new Function<Artifact, String>() {
-          @Override
-          public String apply(Artifact artifact) {
-            return artifact.getRootRelativePathString();
-          }})));
-    }
-  }
-
   // TODO(bazel-team): this whole defaulting - overriding executable, runfiles and files_to_build
   // is getting out of hand. Clean this whole mess up.
   private static ConfiguredTarget createTarget(RuleContext ruleContext, Object target)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
index 5a4052fb8c..884d5dbc72 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -56,6 +56,8 @@ public final class AspectValue extends ActionLookupValue {
 
     public abstract AspectParameters getParameters();
 
+    public abstract String getDescription();
+
     public BuildConfiguration getConfiguration() {
       return configuration;
     }
@@ -88,6 +90,11 @@ public final class AspectValue extends ActionLookupValue {
     }
 
     @Override
+    public String getDescription() {
+      return String.format("%s of %s", aspect.getAspectFactory().getName(), getLabel());
+    }
+
+    @Override
     SkyFunctionName getType() {
       return SkyFunctions.NATIVE_ASPECT;
     }
@@ -152,6 +159,13 @@ public final class AspectValue extends ActionLookupValue {
     }
 
     @Override
+    public String getDescription() {
+      // Skylark aspects are referred to on command line with <file>%<value name>
+      return String.format(
+          "%s%%%s of %s", extensionFile.toString(), skylarkFunctionName, getLabel());
+    }
+
+    @Override
     SkyFunctionName getType() {
       return SkyFunctions.SKYLARK_ASPECT;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index 49766ffc44..f7a85e6902 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -62,6 +62,7 @@ import com.google.devtools.build.lib.packages.RuleClassProvider;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.pkgcache.LoadingPhaseRunner;
 import com.google.devtools.build.lib.skyframe.ActionLookupValue.ActionLookupKey;
+import com.google.devtools.build.lib.skyframe.AspectFunction.AspectCreationException;
 import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
 import com.google.devtools.build.lib.skyframe.BuildInfoCollectionValue.BuildInfoKeyAndConfig;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.ConfiguredValueCreationException;
@@ -313,8 +314,18 @@ public final class SkyframeBuildView {
       Throwable cause = errorInfo.getException();
       Preconditions.checkState(cause != null || !Iterables.isEmpty(errorInfo.getCycleInfo()),
           errorInfo);
-      String errorMsg = "Analysis of target '" + ConfiguredTargetValue.extractLabel(topLevel)
-          + "' failed; build aborted";
+      String errorMsg = null;
+      if (topLevel.argument() instanceof ConfiguredTargetKey) {
+        errorMsg =
+            "Analysis of target '"
+                + ConfiguredTargetValue.extractLabel(topLevel)
+                + "' failed; build aborted";
+      } else if (topLevel.argument() instanceof AspectKey) {
+        AspectKey aspectKey = (AspectKey) topLevel.argument();
+        errorMsg = "Analysis of aspect '" + aspectKey.getDescription() + "' failed; build aborted";
+      } else {
+        assert false;
+      }
       if (cause instanceof ActionConflictException) {
         ((ActionConflictException) cause).reportTo(eventHandler);
       }
@@ -413,8 +424,13 @@ public final class SkyframeBuildView {
     if (cause != null) {
       // We should only be trying to configure targets when the loading phase succeeds, meaning
       // that the only errors should be analysis errors.
-      Preconditions.checkState(cause instanceof ConfiguredValueCreationException
-          || cause instanceof ActionConflictException, "%s -> %s", key, errorInfo);
+      Preconditions.checkState(
+          cause instanceof ConfiguredValueCreationException
+              || cause instanceof AspectCreationException // for top-level aspects
+              || cause instanceof ActionConflictException,
+          "%s -> %s",
+          key,
+          errorInfo);
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
index d7bbf40ef9..916803a795 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
@@ -19,6 +19,7 @@ import com.google.devtools.build.lib.analysis.Aspect;
 import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.SkylarkProviderValidationUtil;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.AspectDefinition;
 import com.google.devtools.build.lib.packages.AspectParameters;
@@ -27,6 +28,7 @@ import com.google.devtools.build.lib.rules.SkylarkRuleContext;
 import com.google.devtools.build.lib.syntax.ClassObject.SkylarkClassObject;
 import com.google.devtools.build.lib.syntax.Environment;
 import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.EvalExceptionWithStackTrace;
 import com.google.devtools.build.lib.syntax.Mutability;
 
 /**
@@ -60,9 +62,9 @@ public class SkylarkAspectFactory implements ConfiguredAspectFactory {
               .setEventHandler(ruleContext.getAnalysisEnvironment().getEventHandler())
               .build(); // NB: loading phase functions are not available: this is analysis already,
                         // so we do *not* setLoadingPhase().
-      Object aspect;
+      Object aspectSkylarkObject;
       try {
-        aspect =
+        aspectSkylarkObject =
             aspectFunction
                 .getImplementation()
                 .call(
@@ -70,21 +72,40 @@ public class SkylarkAspectFactory implements ConfiguredAspectFactory {
                     ImmutableMap.<String, Object>of(),
                     /*ast=*/ null,
                     env);
-      } catch (EvalException e) {
-        ruleContext.ruleError(e.getMessage());
-        return null;
-      }
-      // TODO(dslomov): unify this code with
-      // {@link com.google.devtools.build.lib.rules.SkylarkRuleConfiguredTargetBuilder}
-      Aspect.Builder builder = new Aspect.Builder(name);
-      if (aspect instanceof SkylarkClassObject) {
-        SkylarkClassObject struct = (SkylarkClassObject) aspect;
+
+        if (ruleContext.hasErrors()) {
+          return null;
+        } else if (!(aspectSkylarkObject instanceof SkylarkClassObject)) {
+          ruleContext.ruleError("Aspect implementation doesn't return a struct");
+          return null;
+        }
+
+        Aspect.Builder builder = new Aspect.Builder(name);
+
+        SkylarkClassObject struct = (SkylarkClassObject) aspectSkylarkObject;
         Location loc = struct.getCreationLoc();
         for (String key : struct.getKeys()) {
           builder.addSkylarkTransitiveInfo(key, struct.getValue(key), loc);
         }
+        Aspect aspect = builder.build();
+        SkylarkProviderValidationUtil.checkOrphanArtifacts(ruleContext);
+        return aspect;
+      } catch (EvalException e) {
+        addAspectToStackTrace(base, e);
+        ruleContext.ruleError("\n" + e.print());
+        return null;
       }
-      return builder.build();
+
+    }
+  }
+
+  private void addAspectToStackTrace(ConfiguredTarget base, EvalException e) {
+    if (e instanceof EvalExceptionWithStackTrace) {
+      ((EvalExceptionWithStackTrace) e)
+          .registerPhantomFuncall(
+              String.format("%s(...)", name),
+              base.getTarget().getAssociatedRule().getLocation(),
+              aspectFunction.getImplementation());
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalExceptionWithStackTrace.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalExceptionWithStackTrace.java
index ca65292812..ae9820f142 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalExceptionWithStackTrace.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalExceptionWithStackTrace.java
@@ -71,16 +71,20 @@ public class EvalExceptionWithStackTrace extends EvalException {
   }
 
   /**
-   * Adds the given {@code Rule} to the stack trace.
+   * Makes sure the stack trace is rooted in a function call.
+   *
+   * In some cases (rule implementation application, aspect implementation application)
+   * bazel calls into the function directly (using BaseFunction.call). In that case, since
+   * there is no FuncallExpression to evaluate, stack trace mechanism cannot record this call.
+   * This method allows to augument the stack trace with information about the call.
    */
-  public void registerRule(String rule, Location location, BaseFunction ruleImpl) {
-    /* We have to model the transition from BUILD file to bzl file manually since the stack trace
-     * mechanism cannot do that by itself (because, for example, the rule implementation does not
-     * have a corresponding FuncallExpression).
+  public void registerPhantomFuncall(
+      String funcallDescription, Location location, BaseFunction function) {
+    /*
      *
-     * Consequently, we add two new frames to the stack:
-     * 1. Rule definition
-     * 2. Rule implementation
+     * We add two new frames to the stack:
+     * 1. Pseudo-function call (for example, rule definition)
+     * 2. Function entry (Rule implementation)
      *
      * Similar to Python, all functions that were entered (except for the top-level ones) appear
      * twice in the stack trace output. This would lead to the following trace:
@@ -103,8 +107,8 @@ public class EvalExceptionWithStackTrace extends EvalException {
      *     ...
      *
      * */
-    addStackFrame(ruleImpl.getName(), ruleImpl.getLocation());
-    addStackFrame(rule, location, false);
+    addStackFrame(function.getName(), function.getLocation());
+    addStackFrame(funcallDescription, location, false);
   }
 
   /**