1 files changed, 34 insertions, 33 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index 26e0617454..ad3cdcd039 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -13,10 +13,11 @@
 // limitations under the License.
 package com.google.devtools.build.lib.analysis;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType.ABSTRACT;
 import static com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType.TEST;
 
-import com.google.common.base.Preconditions;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
@@ -46,7 +47,6 @@ import com.google.devtools.common.options.OptionsClassProvider;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -86,6 +86,8 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider {
     private final  Map<String, Class<? extends RuleDefinition>> ruleDefinitionMap =
         new HashMap<>();
     private final Map<Class<? extends RuleDefinition>, RuleClass> ruleMap = new HashMap<>();
+    private final Map<Class<? extends RuleDefinition>, RuleDefinition> ruleDefinitionInstanceCache =
+        new HashMap<>();
     private final Digraph<Class<? extends RuleDefinition>> dependencyGraph =
         new Digraph<>();
     private ConfigurationCollectionFactory configurationCollectionFactory;
@@ -106,11 +108,12 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider {
       return this;
     }
 
-    public Builder addRuleDefinition(Class<? extends RuleDefinition> ruleDefinition) {
-      dependencyGraph.createNode(ruleDefinition);
-      BlazeRule annotation = ruleDefinition.getAnnotation(BlazeRule.class);
-      for (Class<? extends RuleDefinition> ancestor : annotation.ancestors()) {
-        dependencyGraph.addEdge(ancestor, ruleDefinition);
+    public Builder addRuleDefinition(RuleDefinition ruleDefinition) {
+      Class<? extends RuleDefinition> ruleDefinitionClass = ruleDefinition.getClass();
+      ruleDefinitionInstanceCache.put(ruleDefinitionClass, ruleDefinition);
+      dependencyGraph.createNode(ruleDefinitionClass);
+      for (Class<? extends RuleDefinition> ancestor : ruleDefinition.getMetadata().ancestors()) {
+        dependencyGraph.addEdge(ancestor, ruleDefinitionClass);
       }
 
       return this;
@@ -148,40 +151,38 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider {
     }
 
     private RuleClass commitRuleDefinition(Class<? extends RuleDefinition> definitionClass) {
-      BlazeRule annotation = definitionClass.getAnnotation(BlazeRule.class);
-      Preconditions.checkArgument(ruleClassMap.get(annotation.name()) == null, annotation.name());
-
-      Preconditions.checkArgument(
-          annotation.type() == ABSTRACT ^
-          annotation.factoryClass() != RuleConfiguredTargetFactory.class);
-      Preconditions.checkArgument(
-          (annotation.type() != TEST) ||
-          Arrays.asList(annotation.ancestors()).contains(
-              BaseRuleClasses.TestBaseRule.class));
-
-      RuleDefinition instance;
-      try {
-        instance = definitionClass.newInstance();
-      } catch (IllegalAccessException | InstantiationException e) {
-        throw new IllegalStateException(e);
-      }
-      RuleClass[] ancestorClasses = new RuleClass[annotation.ancestors().length];
-      for (int i = 0; i < annotation.ancestors().length; i++) {
-        ancestorClasses[i] = ruleMap.get(annotation.ancestors()[i]);
+      RuleDefinition instance = checkNotNull(ruleDefinitionInstanceCache.get(definitionClass),
+          "addRuleDefinition(new %s()) should be called before build()", definitionClass.getName());
+
+      RuleDefinition.Metadata metadata = instance.getMetadata();
+      checkArgument(ruleClassMap.get(metadata.name()) == null, metadata.name());
+
+      List<Class<? extends RuleDefinition>> ancestors = metadata.ancestors();
+
+      checkArgument(
+          metadata.type() == ABSTRACT ^ metadata.factoryClass()
+              != RuleConfiguredTargetFactory.class);
+      checkArgument(
+          (metadata.type() != TEST)
+          || ancestors.contains(BaseRuleClasses.TestBaseRule.class));
+
+      RuleClass[] ancestorClasses = new RuleClass[ancestors.size()];
+      for (int i = 0; i < ancestorClasses.length; i++) {
+        ancestorClasses[i] = ruleMap.get(ancestors.get(i));
         if (ancestorClasses[i] == null) {
           // Ancestors should have been initialized by now
-          throw new IllegalStateException("Ancestor " + annotation.ancestors()[i] + " of "
-              + annotation.name() + " is not initialized");
+          throw new IllegalStateException("Ancestor " + ancestors.get(i) + " of "
+              + metadata.name() + " is not initialized");
         }
       }
 
       RuleConfiguredTargetFactory factory = null;
-      if (annotation.type() != ABSTRACT) {
-        factory = createFactory(annotation.factoryClass());
+      if (metadata.type() != ABSTRACT) {
+        factory = createFactory(metadata.factoryClass());
       }
 
       RuleClass.Builder builder = new RuleClass.Builder(
-          annotation.name(), annotation.type(), false, ancestorClasses);
+          metadata.name(), metadata.type(), false, ancestorClasses);
       builder.factory(factory);
       RuleClass ruleClass = instance.build(builder, this);
       ruleMap.put(definitionClass, ruleClass);
@@ -193,7 +194,7 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider {
 
     public ConfiguredRuleClassProvider build() {
       for (Node<Class<? extends RuleDefinition>> ruleDefinition :
-        dependencyGraph.getTopologicalOrder()) {
+          dependencyGraph.getTopologicalOrder()) {
         commitRuleDefinition(ruleDefinition.getLabel());
       }