Improved error reporting in RuleContext:
- Unified duplicate code from RuleContext and RuleContext.Builder in a new class, RuleContext.ErrorReporter
- Added the BUILD file location to error/warning messages if the offending rule was created by a macro
--
MOS_MIGRATED_REVID=103934375
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 10395cb..b280086 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
@@ -137,6 +137,7 @@
private final ImmutableSet<String> features;
private final Map<String, Attribute> attributeMap;
private final BuildConfiguration hostConfiguration;
+ private final ErrorReporter reporter;
private ActionOwner actionOwner;
@@ -157,6 +158,7 @@
this.features = getEnabledFeatures();
this.attributeMap = attributeMap;
this.hostConfiguration = builder.hostConfiguration;
+ reporter = builder.reporter;
}
private ImmutableSet<String> getEnabledFeatures() {
@@ -391,7 +393,7 @@
*/
@Override
public void ruleError(String message) {
- reportError(rule.getLocation(), prefixRuleMessage(message));
+ reporter.ruleError(message);
}
/**
@@ -400,7 +402,7 @@
*/
@Override
public void ruleWarning(String message) {
- reportWarning(rule.getLocation(), prefixRuleMessage(message));
+ reporter.ruleWarning(message);
}
/**
@@ -412,11 +414,7 @@
*/
@Override
public void attributeError(String attrName, String message) {
- reportError(rule.getAttributeLocation(attrName),
- prefixAttributeMessage(Attribute.isImplicit(attrName)
- ? "(an implicit dependency)"
- : attrName,
- message));
+ reporter.attributeError(attrName, message);
}
/**
@@ -427,30 +425,7 @@
*/
@Override
public void attributeWarning(String attrName, String message) {
- reportWarning(rule.getAttributeLocation(attrName),
- prefixAttributeMessage(Attribute.isImplicit(attrName)
- ? "(an implicit dependency)"
- : attrName,
- message));
- }
-
- private String prefixAttributeMessage(String attrName, String message) {
- return "in " + attrName + " attribute of "
- + rule.getRuleClass() + " rule "
- + getLabel() + ": " + message;
- }
-
- private String prefixRuleMessage(String message) {
- return "in " + rule.getRuleClass() + " rule "
- + getLabel() + ": " + message;
- }
-
- private void reportError(Location location, String message) {
- getAnalysisEnvironment().getEventHandler().handle(Event.error(location, message));
- }
-
- private void reportWarning(Location location, String message) {
- getAnalysisEnvironment().getEventHandler().handle(Event.warn(location, message));
+ reporter.attributeWarning(attrName, message);
}
/**
@@ -1237,15 +1212,21 @@
return features;
}
+ @Override
+ public String toString() {
+ return "RuleContext(" + getLabel() + ", " + getConfiguration() + ")";
+ }
+
/**
* Builder class for a RuleContext.
*/
- public static final class Builder {
+ public static final class Builder implements RuleErrorConsumer {
private final AnalysisEnvironment env;
private final Rule rule;
private final BuildConfiguration configuration;
private final BuildConfiguration hostConfiguration;
private final PrerequisiteValidator prerequisiteValidator;
+ private final ErrorReporter reporter;
private ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap;
private Set<ConfigMatchingProvider> configConditions;
private NestedSet<PackageSpecification> visibility;
@@ -1258,6 +1239,7 @@
this.configuration = Preconditions.checkNotNull(configuration);
this.hostConfiguration = Preconditions.checkNotNull(hostConfiguration);
this.prerequisiteValidator = prerequisiteValidator;
+ reporter = new ErrorReporter(env, rule);
}
RuleContext build() {
@@ -1405,41 +1387,32 @@
return mapBuilder.build();
}
- private String prefixRuleMessage(String message) {
- return String.format("in %s rule %s: %s", rule.getRuleClass(), rule.getLabel(), message);
- }
-
- private String maskInternalAttributeNames(String name) {
- return Attribute.isImplicit(name) ? "(an implicit dependency)" : name;
- }
-
- private String prefixAttributeMessage(String attrName, String message) {
- return String.format("in %s attribute of %s rule %s: %s",
- maskInternalAttributeNames(attrName), rule.getRuleClass(), rule.getLabel(), message);
- }
-
public void reportError(Location location, String message) {
- env.getEventHandler().handle(Event.error(location, message));
+ reporter.reportError(location, message);
}
+ @Override
public void ruleError(String message) {
- reportError(rule.getLocation(), prefixRuleMessage(message));
+ reporter.ruleError(message);
}
+ @Override
public void attributeError(String attrName, String message) {
- reportError(rule.getAttributeLocation(attrName), prefixAttributeMessage(attrName, message));
+ reporter.attributeError(attrName, message);
}
public void reportWarning(Location location, String message) {
- env.getEventHandler().handle(Event.warn(location, message));
+ reporter.reportWarning(location, message);
}
+ @Override
public void ruleWarning(String message) {
- env.getEventHandler().handle(Event.warn(rule.getLocation(), prefixRuleMessage(message)));
+ reporter.ruleWarning(message);
}
+ @Override
public void attributeWarning(String attrName, String message) {
- reportWarning(rule.getAttributeLocation(attrName), prefixAttributeMessage(attrName, message));
+ reporter.attributeWarning(attrName, message);
}
private void reportBadPrerequisite(Attribute attribute, String targetKind,
@@ -1584,8 +1557,92 @@
}
}
- @Override
- public String toString() {
- return "RuleContext(" + getLabel() + ", " + getConfiguration() + ")";
+ /**
+ * Helper class for reporting errors and warnings.
+ */
+ public static final class ErrorReporter implements RuleErrorConsumer {
+ private final AnalysisEnvironment env;
+ private final Rule rule;
+
+ ErrorReporter(AnalysisEnvironment env, Rule rule) {
+ this.env = env;
+ this.rule = rule;
+ }
+
+ public void reportError(Location location, String message) {
+ env.getEventHandler().handle(Event.error(location, message));
+ }
+
+ @Override
+ public void ruleError(String message) {
+ reportError(rule.getLocation(), prefixRuleMessage(message));
+ }
+
+ @Override
+ public void attributeError(String attrName, String message) {
+ reportError(getAttributeLocation(attrName), completeAttributeMessage(attrName, message));
+ }
+
+ 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(rule.getLocation(), 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", rule.getRuleClass(), rule.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.
+ String macroMessageAppendix =
+ 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.getAttributeLocation(attrName))
+ : "";
+
+ return String.format("in %s attribute of %s rule %s: %s%s",
+ maskInternalAttributeNames(attrName), rule.getRuleClass(), rule.getLabel(), message,
+ macroMessageAppendix);
+ }
+
+ /**
+ * Returns the location of the specified attribute.
+ *
+ * <p>If the rule was created by a macro, we return the location from the BUILD file instead.
+ */
+ private Location getAttributeLocation(String attrName) {
+ // TODO(bazel-team): We assume that macros have set the rule location to the generator
+ // location. It would be better to read the "generator_location" attribute (which is currently
+ // not implemented).
+ return wasCreatedByMacro() ? rule.getLocation() : rule.getAttributeLocation(attrName);
+ }
+
+ private boolean wasCreatedByMacro() {
+ String generator = getGeneratorFunction();
+ return generator != null && !generator.isEmpty();
+ }
+
+ private String getGeneratorFunction() {
+ return (String) rule.getAttributeContainer().getAttr("generator_function");
+ }
}
}
