commit 439a73df6185140dab571bb627597e32a98e5f94
author Laurent Le Brun <laurentlb@google.com> Mon Jun 22 16:11:37 2015 +0000
committer Laszlo Csomor <laszlocsomor@google.com> Tue Jun 23 09:01:59 2015 +0000
tree bf0c6809d9cf4ada6fb9d3ce1fd4fb08641702c8
parent b815432074f669eea59b70736f0b4f20a4fdf6d9

Error message when attribute is of the wrong type is misleading

Fixes #241

--
MOS_MIGRATED_REVID=96573740

src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java

diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java
index d5d1a46..a9408ee 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java
@@ -69,15 +69,21 @@
    * <p>It is the caller's responsibility to add the rule to the package (the caller may choose not
    * to do so if, for example, the rule has errors).</p>
    */
-  static Rule createRule(Package.Builder pkgBuilder, RuleClass ruleClass,
-      Map<String, Object> attributeValues, EventHandler eventHandler, FuncallExpression ast,
+  static Rule createRule(
+      Package.Builder pkgBuilder,
+      RuleClass ruleClass,
+      Map<String, Object> attributeValues,
+      EventHandler eventHandler,
+      FuncallExpression ast,
       Location location)
       throws InvalidRuleException, NameConflictException {
     Preconditions.checkNotNull(ruleClass);
     String ruleClassName = ruleClass.getName();
     Object nameObject = attributeValues.get("name");
-    if (!(nameObject instanceof String)) {
+    if (nameObject == null) {
       throw new InvalidRuleException(ruleClassName + " rule has no 'name' attribute");
+    } else if (!(nameObject instanceof String)) {
+      throw new InvalidRuleException(ruleClassName + " 'name' attribute must be a string");
     }
     String name = (String) nameObject;
     Label label;
@@ -88,14 +94,14 @@
     } catch (Label.SyntaxException e) {
       throw new InvalidRuleException("illegal rule name: " + name + ": " + e.getMessage());
     }
-    boolean inWorkspaceFile = location.getPath() != null
-        && location.getPath().getBaseName().contains("WORKSPACE");
+    boolean inWorkspaceFile =
+        location.getPath() != null && location.getPath().getBaseName().contains("WORKSPACE");
     if (ruleClass.getWorkspaceOnly() && !inWorkspaceFile) {
-      throw new RuleFactory.InvalidRuleException(ruleClass + " must be in the WORKSPACE file "
-          + "(used by " + label + ")");
+      throw new RuleFactory.InvalidRuleException(
+          ruleClass + " must be in the WORKSPACE file " + "(used by " + label + ")");
     } else if (!ruleClass.getWorkspaceOnly() && inWorkspaceFile) {
-      throw new RuleFactory.InvalidRuleException(ruleClass + " cannot be in the WORKSPACE file "
-          + "(used by " + label + ")");
+      throw new RuleFactory.InvalidRuleException(
+          ruleClass + " cannot be in the WORKSPACE file " + "(used by " + label + ")");
     }
 
     try {