Fork RuleContext.ErrorReporter to create a Skylark-friendly RuleErrorConsumer

The new error consumer is simple to create from @SkylarkCallable methods, so we
won't need to pass SkylarkRuleContext into those methods for that reason
anymore.

Create a new abstract class, rather than expand the existing
AbstractRuleErrorConsumer class, since the latter is already being extended and
used in a variety of tests, and I prefer not to have to migrate all of them as
needed. Add Javadoc to make clear that this class should not be used similarly.

RELNOTES: none
PiperOrigin-RevId: 197148849

3 files changed, 213 insertions, 63 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/EventHandlingErrorReporter.java b/src/main/java/com/google/devtools/build/lib/analysis/EventHandlingErrorReporter.java
new file mode 100644
index 0000000000..9257a75046
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/EventHandlingErrorReporter.java
@@ -0,0 +1,104 @@
+// Copyright 2018 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.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.packages.AbstractRuleErrorConsumer;
+import com.google.devtools.build.lib.packages.Attribute;
+
+/**
+ * Base class for implementations of {@link
+ * com.google.devtools.build.lib.packages.RuleErrorConsumer}.
+ *
+ * <p>Do not create new implementations of this class - instead, use {@link RuleContext} in Native
+ * rule definitions, and {@link com.google.devtools.build.lib.analysis.skylark.SkylarkErrorReporter}
+ * in Skylark API definitions. For use in testing, extend {@link AbstractRuleErrorConsumer} instead.
+ */
+public abstract class EventHandlingErrorReporter extends AbstractRuleErrorConsumer {
+  private final String ruleClassNameForLogging;
+  private final AnalysisEnvironment env;
+
+  protected EventHandlingErrorReporter(String ruleClassNameForLogging, AnalysisEnvironment env) {
+    this.ruleClassNameForLogging = ruleClassNameForLogging;
+    this.env = env;
+  }
+
+  public void reportError(Location location, String message) {
+    env.getEventHandler().handle(Event.error(location, message));
+  }
+
+  @Override
+  public void ruleError(String message) {
+    reportError(getRuleLocation(), prefixRuleMessage(message));
+  }
+
+  @Override
+  public void attributeError(String attrName, String message) {
+    reportError(getAttributeLocation(attrName), completeAttributeMessage(attrName, message));
+  }
+
+  @Override
+  public boolean hasErrors() {
+    return env.hasErrors();
+  }
+
+  public void reportWarning(Location location, String message) {
+    env.getEventHandler().handle(Event.warn(location, message));
+  }
+
+  @Override
+  public void ruleWarning(String message) {
+    env.getEventHandler().handle(Event.warn(getRuleLocation(), prefixRuleMessage(message)));
+  }
+
+  @Override
+  public void attributeWarning(String attrName, String message) {
+    reportWarning(getAttributeLocation(attrName), completeAttributeMessage(attrName, message));
+  }
+
+  private String prefixRuleMessage(String message) {
+    return String.format("in %s rule %s: %s", ruleClassNameForLogging, getLabel(), message);
+  }
+
+  private String maskInternalAttributeNames(String name) {
+    return Attribute.isImplicit(name) ? "(an implicit dependency)" : name;
+  }
+
+  /**
+   * Prefixes the given message with details about the rule and appends details about the macro that
+   * created this rule, if applicable.
+   */
+  private String completeAttributeMessage(String attrName, String message) {
+    // Appends a note to the given message if the offending rule was created by a macro.
+
+    return String.format(
+        "in %s attribute of %s rule %s: %s%s",
+        maskInternalAttributeNames(attrName),
+        ruleClassNameForLogging,
+        getLabel(),
+        message,
+        getMacroMessageAppendix(attrName));
+  }
+
+  /** Returns a string describing the macro that created this rule, or an empty string. */
+  protected abstract String getMacroMessageAppendix(String attrName);
+
+  protected abstract Label getLabel();
+
+  protected abstract Location getRuleLocation();
+
+  protected abstract Location getAttributeLocation(String attrName);
+}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index d33cd173f7..adfeb58f4e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -62,10 +62,8 @@ import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.collect.ImmutableSortedKeyListMultimap;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
 import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.packages.AbstractRuleErrorConsumer;
 import com.google.devtools.build.lib.packages.Aspect;
 import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.Attribute;
@@ -1913,20 +1911,15 @@ public final class RuleContext extends TargetContext
   }
 
   /** Helper class for reporting errors and warnings. */
-  public static final class ErrorReporter extends AbstractRuleErrorConsumer
+  public static final class ErrorReporter extends EventHandlingErrorReporter
       implements RuleErrorConsumer {
     private final AnalysisEnvironment env;
     private final Rule rule;
-    private final String ruleClassNameForLogging;
 
     ErrorReporter(AnalysisEnvironment env, Rule rule, String ruleClassNameForLogging) {
+      super(ruleClassNameForLogging, env);
       this.env = env;
       this.rule = rule;
-      this.ruleClassNameForLogging = ruleClassNameForLogging;
-    }
-
-    public void reportError(Location location, String message) {
-      env.getEventHandler().handle(Event.error(location, message));
     }
 
     public void post(Postable event) {
@@ -1934,72 +1927,33 @@ public final class RuleContext extends TargetContext
     }
 
     @Override
-    public void ruleError(String message) {
-      reportError(rule.getLocation(), prefixRuleMessage(message));
+    protected String getMacroMessageAppendix(String attrName) {
+      return rule.wasCreatedByMacro()
+          ? String.format(
+              ". Since this rule was created by the macro '%s', the error might have been "
+                  + "caused by the macro implementation in %s",
+              getGeneratorFunction(), rule.getAttributeLocationWithoutMacro(attrName))
+          : "";
     }
 
-    @Override
-    public void attributeError(String attrName, String message) {
-      reportError(rule.getAttributeLocation(attrName), completeAttributeMessage(attrName, message));
+    private String getGeneratorFunction() {
+      return (String) rule.getAttributeContainer().getAttr("generator_function");
     }
 
     @Override
-    public boolean hasErrors() {
-      return env.hasErrors();
-    }
-
-    public void reportWarning(Location location, String message) {
-      env.getEventHandler().handle(Event.warn(location, message));
+    protected Label getLabel() {
+      return rule.getLabel();
     }
 
     @Override
-    public void ruleWarning(String message) {
-      env.getEventHandler().handle(Event.warn(rule.getLocation(), prefixRuleMessage(message)));
+    protected Location getRuleLocation() {
+      return rule.getLocation();
     }
 
     @Override
-    public void attributeWarning(String attrName, String message) {
-      reportWarning(
-          rule.getAttributeLocation(attrName), completeAttributeMessage(attrName, message));
-    }
-
-    private String prefixRuleMessage(String message) {
-      return String.format(
-          "in %s rule %s: %s", getRuleClassNameForLogging(), rule.getLabel(), message);
+    protected Location getAttributeLocation(String attrName) {
+      return rule.getAttributeLocation(attrName);
     }
 
-    private String maskInternalAttributeNames(String name) {
-      return Attribute.isImplicit(name) ? "(an implicit dependency)" : name;
-    }
-
-    /**
-     * Prefixes the given message with details about the rule and appends details about the macro
-     * that created this rule, if applicable.
-     */
-    private String completeAttributeMessage(String attrName, String message) {
-      // Appends a note to the given message if the offending rule was created by a macro.
-      String macroMessageAppendix =
-          rule.wasCreatedByMacro()
-              ? String.format(
-                  ". Since this rule was created by the macro '%s', the error might have been "
-                  + "caused by the macro implementation in %s",
-                  getGeneratorFunction(), rule.getAttributeLocationWithoutMacro(attrName))
-              : "";
-
-      return String.format("in %s attribute of %s rule %s: %s%s",
-          maskInternalAttributeNames(attrName), getRuleClassNameForLogging(), rule.getLabel(),
-          message, macroMessageAppendix);
-    }
-
-    /**
-     * Returns a rule class name suitable for log messages, including an aspect name if applicable.
-     */
-    private String getRuleClassNameForLogging() {
-      return ruleClassNameForLogging;
-    }
-
-    private String getGeneratorFunction() {
-      return (String) rule.getAttributeContainer().getAttr("generator_function");
-    }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkErrorReporter.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkErrorReporter.java
new file mode 100644
index 0000000000..98b908573b
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkErrorReporter.java
@@ -0,0 +1,92 @@
+// Copyright 2018 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.skylark;
+
+import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
+import com.google.devtools.build.lib.analysis.EventHandlingErrorReporter;
+import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
+import com.google.devtools.build.lib.packages.RuleErrorConsumer;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
+import com.google.devtools.build.lib.syntax.Environment;
+import com.google.devtools.build.lib.syntax.EvalException;
+
+/**
+ * {@link RuleErrorConsumer} for Native implementations of Skylark APIs.
+ *
+ * <p>This class largely reproduces the functionality of the error consumer in {@link
+ * com.google.devtools.build.lib.analysis.RuleContext}, but should be easy to create and use from
+ * methods annotated with {@link com.google.devtools.build.lib.skylarkinterface.SkylarkCallable}.
+ * (Environment and Location are automatically provided to those methods be specifying the {@link
+ * SkylarkCallable#useLocation()} and {@link SkylarkCallable#useEnvironment()} fields in the
+ * annotation.
+ *
+ * <p>This class is AutoClosable, to ensure that {@link RuleErrorException} are checked and handled
+ * before leaving native code. The {@link #close()} method will only throw {@link EvalException},
+ * properly wrapping any {@link RuleErrorException} instances if needed.
+ */
+public class SkylarkErrorReporter extends EventHandlingErrorReporter implements AutoCloseable {
+  private final Location location;
+  private final Label label;
+
+  public static SkylarkErrorReporter from(
+      ActionConstructionContext context, Location location, Environment env) {
+    return new SkylarkErrorReporter(
+        context.getActionOwner().getTargetKind(),
+        context.getAnalysisEnvironment(),
+        location,
+        env.getCallerLabel());
+  }
+
+  private SkylarkErrorReporter(
+      String ruleClassNameForLogging,
+      AnalysisEnvironment analysisEnvironment,
+      Location location,
+      Label label) {
+    super(ruleClassNameForLogging, analysisEnvironment);
+    this.label = label;
+    this.location = location;
+  }
+
+  @Override
+  protected String getMacroMessageAppendix(String attrName) {
+    return "";
+  }
+
+  @Override
+  protected Label getLabel() {
+    return label;
+  }
+
+  @Override
+  protected Location getRuleLocation() {
+    return location;
+  }
+
+  @Override
+  protected Location getAttributeLocation(String attrName) {
+    return location;
+  }
+
+  @Override
+  public void close() throws EvalException {
+    try {
+      assertNoErrors();
+    } catch (RuleErrorException e) {
+      throw new EvalException(location, e);
+    }
+  }
+}