Provide a lambda interface for custom mock rule class behavior.

This extends the "easy use" idea of MockRule from just custom
attributes to full-on custom behavior.

For a proof of concept, also port Bazel's late-bound attribute
tests.

PiperOrigin-RevId: 163483121

5 files changed, 162 insertions, 126 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/ConfigurationsForLateBoundTargetsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/ConfigurationsForLateBoundTargetsTest.java
index f3e8a1d7c0..352fe31aee 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/ConfigurationsForLateBoundTargetsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/ConfigurationsForLateBoundTargetsTest.java
@@ -25,16 +25,15 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFactory;
 import com.google.devtools.build.lib.analysis.config.PatchTransition;
 import com.google.devtools.build.lib.analysis.util.AnalysisTestCase;
+import com.google.devtools.build.lib.analysis.util.MockRule;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.Rule;
-import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
 import com.google.devtools.build.lib.testutil.Suite;
 import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
 import com.google.devtools.build.lib.testutil.TestSpec;
-import com.google.devtools.build.lib.testutil.UnknownRuleConfiguredTarget;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -67,47 +66,39 @@ public class ConfigurationsForLateBoundTargetsTest extends AnalysisTestCase {
   };
 
   /**
-   * Rule definition with a latebound dependency.
+   * Mock late-bound attribute resolver that returns a fixed label.
    */
-  private static class LateBoundDepRule implements RuleDefinition {
-    private static final Attribute.LateBoundLabel<BuildConfiguration> LATEBOUND_VALUE_RESOLVER =
-        new Attribute.LateBoundLabel<BuildConfiguration>() {
-          @Override
-          public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration config) {
-            return Label.parseAbsoluteUnchecked("//foo:latebound_dep");
-          }
-        };
+  private static final Attribute.LateBoundLabel<BuildConfiguration> LATEBOUND_VALUE_RESOLVER =
+      new Attribute.LateBoundLabel<BuildConfiguration>() {
+        @Override
+        public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration config) {
+          return Label.parseAbsoluteUnchecked("//foo:latebound_dep");
+        }
+      };
 
-    @Override
-    public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
-      return builder
-          .add(
-              attr(":latebound_attr", LABEL)
-                  .value(LATEBOUND_VALUE_RESOLVER)
-                  .cfg(CHANGE_FOO_FLAG_TRANSITION))
-          .requiresConfigurationFragments(LateBoundSplitUtil.TestFragment.class)
-          .build();
-    }
-
-    @Override
-    public Metadata getMetadata() {
-      return RuleDefinition.Metadata.builder()
-          .name("rule_with_latebound_attr")
-          .ancestors(BaseRuleClasses.RuleBase.class)
-          .factoryClass(UnknownRuleConfiguredTarget.class)
-          .build();
-    }
-  }
+  /**
+   * Rule definition with a latebound dependency.
+   */
+  private static final RuleDefinition LATE_BOUND_DEP_RULE = (MockRule) () -> MockRule.define(
+      "rule_with_latebound_attr",
+      (builder, env) -> {
+        builder
+            .add(
+                attr(":latebound_attr", LABEL)
+                    .value(LATEBOUND_VALUE_RESOLVER)
+                    .cfg(CHANGE_FOO_FLAG_TRANSITION))
+            .requiresConfigurationFragments(LateBoundSplitUtil.TestFragment.class);
+      });
 
   @Before
   public void setupCustomLateBoundRules() throws Exception {
     ConfiguredRuleClassProvider.Builder builder = new ConfiguredRuleClassProvider.Builder();
     TestRuleClassProvider.addStandardRules(builder);
-    builder.addRuleDefinition(new LateBoundSplitUtil.RuleWithLateBoundSplitAttribute());
-    builder.addRuleDefinition(new LateBoundSplitUtil.RuleWithTestFragment());
+    builder.addRuleDefinition(LateBoundSplitUtil.RULE_WITH_LATEBOUND_SPLIT_ATTR);
+    builder.addRuleDefinition(LateBoundSplitUtil.RULE_WITH_TEST_FRAGMENT);
     builder.addConfigurationFragment(new LateBoundSplitUtil.FragmentLoader());
     builder.addConfigurationOptions(LateBoundSplitUtil.TestOptions.class);
-    builder.addRuleDefinition(new LateBoundDepRule());
+    builder.addRuleDefinition(LATE_BOUND_DEP_RULE);
     useRuleClassProvider(builder.build());
 
     // Register the latebound split fragment with the config creation environment.
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/LateBoundSplitUtil.java b/src/test/java/com/google/devtools/build/lib/analysis/LateBoundSplitUtil.java
index f9297e0f42..403cb42f37 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/LateBoundSplitUtil.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/LateBoundSplitUtil.java
@@ -24,14 +24,13 @@ import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
+import com.google.devtools.build.lib.analysis.util.MockRule;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.Rule;
-import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
-import com.google.devtools.build.lib.testutil.UnknownRuleConfiguredTarget;
 import com.google.devtools.build.lib.util.FileTypeSet;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionDocumentationCategory;
@@ -122,49 +121,25 @@ public class LateBoundSplitUtil {
   /**
    * A custom rule that applies a late-bound split attribute.
    */
-  static class RuleWithLateBoundSplitAttribute implements RuleDefinition {
-    @Override
-    public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
-      return builder
-          .add(attr(":latebound_split_attr", BuildType.LABEL)
-              .allowedFileTypes(FileTypeSet.ANY_FILE)
-              .allowedRuleClasses(Attribute.ANY_RULE)
-              .cfg(SIMPLE_SPLIT)
-              .value(SIMPLE_LATEBOUND_RESOLVER))
-          .requiresConfigurationFragments(TestFragment.class)
-          .build();
-    }
-
-    @Override
-    public Metadata getMetadata() {
-      return RuleDefinition.Metadata.builder()
-          .name("rule_with_latebound_split")
-          .ancestors(BaseRuleClasses.RuleBase.class)
-          .factoryClass(UnknownRuleConfiguredTarget.class)
-          .build();
-    }
-  }
+  static final RuleDefinition RULE_WITH_LATEBOUND_SPLIT_ATTR = (MockRule) () -> MockRule.define(
+      "rule_with_latebound_split",
+      (builder, env) -> {
+        builder
+            .add(
+                attr(":latebound_split_attr", BuildType.LABEL)
+                    .allowedFileTypes(FileTypeSet.ANY_FILE)
+                    .allowedRuleClasses(Attribute.ANY_RULE)
+                    .cfg(SIMPLE_SPLIT)
+                    .value(SIMPLE_LATEBOUND_RESOLVER))
+            .requiresConfigurationFragments(TestFragment.class);
+      });
 
   /**
    * A custom rule that requires {@link TestFragment}.
    */
-  static class RuleWithTestFragment implements RuleDefinition {
-    @Override
-    public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
-      return builder
-          .requiresConfigurationFragments(TestFragment.class)
-          .build();
-    }
-
-    @Override
-    public Metadata getMetadata() {
-      return RuleDefinition.Metadata.builder()
-          .name("rule_with_test_fragment")
-          .ancestors(BaseRuleClasses.RuleBase.class)
-          .factoryClass(UnknownRuleConfiguredTarget.class)
-          .build();
-    }
-  }
+  static final RuleDefinition RULE_WITH_TEST_FRAGMENT = (MockRule) () -> MockRule.define(
+      "rule_with_test_fragment",
+      (builder, env) -> builder.requiresConfigurationFragments(TestFragment.class));
 
   /**
    * Returns a rule class provider with standard test setup plus the above rules/configs.
@@ -172,8 +147,8 @@ public class LateBoundSplitUtil {
   static ConfiguredRuleClassProvider getRuleClassProvider() {
     ConfiguredRuleClassProvider.Builder builder = new ConfiguredRuleClassProvider.Builder();
     TestRuleClassProvider.addStandardRules(builder);
-    builder.addRuleDefinition(new RuleWithLateBoundSplitAttribute());
-    builder.addRuleDefinition(new RuleWithTestFragment());
+    builder.addRuleDefinition(RULE_WITH_LATEBOUND_SPLIT_ATTR);
+    builder.addRuleDefinition(RULE_WITH_TEST_FRAGMENT);
     builder.addConfigurationFragment(new FragmentLoader());
     builder.addConfigurationOptions(TestOptions.class);
     return builder.build();
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/MockRule.java b/src/test/java/com/google/devtools/build/lib/analysis/util/MockRule.java
index bba074bec4..9c97e7d52d 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/MockRule.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/MockRule.java
@@ -21,6 +21,7 @@ import static com.google.devtools.build.lib.syntax.Type.BOOLEAN;
 import static com.google.devtools.build.lib.syntax.Type.STRING;
 import static com.google.devtools.build.lib.syntax.Type.STRING_LIST;
 
+import com.google.common.base.Preconditions;
 import com.google.devtools.build.lib.analysis.BaseRuleClasses;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
@@ -31,75 +32,94 @@ import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.util.FileTypeSet;
 
 import java.util.Arrays;
-import java.util.List;
 
 /**
  * Provides a simple API for creating custom rule classes for tests.
  *
- * <p>Usage:
+ * <p>Usage (for a custom rule type that just needs to exist):
  *
  * <pre>
  *   MockRule fooRule = () -> MockRule.define("foo_rule");
- *   MockRule ruleWithCustomAttr = () -> MockRule.define("attr_rule", attr("myattr", Type.STRING));
  * </pre>
  *
- * <p>If you need special behavior beyond custom attributes:
+ * <p>Usage (for custom attributes):
  *
  * <pre>
- *   class MyCustomRuleClass implements MockRule {
- *     &#064;Override
- *     public MockRule.State define() {
- *      return MockRule.define("my_custom_rule");
- *     }
+ *   MockRule fooRule = () -> MockRule.define("foo_rule", attr("myattr", Type.STRING));
+ * </pre>
+ *
+ * <p>Usage (for arbitrary customization):
  *
- *     &#064;Override
- *     public void customize(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
- *       builder.depsCfg(HostTransition.INSTANCE);
- *     }
- *  }
+ * <pre>
+ *   MockRule fooRule = () -> MockRule.define(
+ *       "foo_rule",
+ *       (builder, env) ->
+ *           builder
+ *               .removeAttribute("tags")
+ *               .requiresConfigurationFragments(FooConfiguration.class);
+ *       );
  * </pre>
  *
+ *
  * <p>We use lambdas for custom rule classes because {@link ConfiguredRuleClassProvider} indexes
  * rule class definitions by their Java class names. So each definition has to have its own
  * unique Java class.
+ *
+ * <p>Both of the following forms are valid:
+ *
+ * <pre>MockRule fooRule = () -> MockRule.define("foo_rule");</pre>
+ * <pre>RuleDefinition fooRule = (MockRule) () -> MockRule.define("foo_rule");</pre>
+ *
+ * <p>Use discretion in choosing your preferred form. The first is more compact. But the second
+ * makes it clearer that <code>fooRule</code> is a proper rule class definition.
  */
 public interface MockRule extends RuleDefinition {
   /**
-   * Container for the desired name and custom attributes for this rule class.
+   * Container for the desired name and custom settings for this rule class.
    */
   class State {
     private final String name;
-    private final List<Attribute.Builder<?>> attributes;
+    private final MockRuleCustomBehavior customBehavior;
 
-    State(String ruleClassName, Attribute.Builder<?>... attributes) {
-      this.name = ruleClassName;
-      this.attributes = Arrays.asList(attributes);
+    State(String ruleClassName, MockRuleCustomBehavior customBehavior) {
+      this.name = Preconditions.checkNotNull(ruleClassName);
+      this.customBehavior = Preconditions.checkNotNull(customBehavior);
     }
   }
 
   /**
-   * Returns a new {@link State} for this rule class. This is a convenience method for lambda
-   * definitions:
+   * Returns a new {@link State} for this rule class with custom attributes. This is a convenience
+   * method for lambda definitions:
    *
    * <pre>
    *   MockRule myRule = () -> MockRule.define("my_rule", attr("myattr", Type.STRING));
    * </pre>
    */
   static State define(String ruleClassName, Attribute.Builder<?>... attributes) {
-    return new State(ruleClassName, attributes);
+    return new State(
+        ruleClassName,
+        new MockRuleCustomBehavior.CustomAttributes(Arrays.asList(attributes)));
   }
 
   /**
-   * Returns the basic state that defines this rule class. This is the only interface method
-   * implementers must override.
+   * Returns a new {@link State} for this rule class with arbitrary custom behavior. This is a
+   * convenience method for lambda definitions:
+   *
+   * <pre>
+   *   MockRule myRule = () -> MockRule.define(
+   *       "my_rule",
+   *       (builder, env) -> builder.requiresConfigurationFragments(FooConfiguration.class));
+   * </pre>
    */
-  State define();
+  static State define(String ruleClassName, MockRuleCustomBehavior customBehavior) {
+    return new State(ruleClassName, customBehavior);
+  }
 
   /**
-   * Allows for custom builder configuration beyond setting attributes.
+   * Returns the basic state that defines this rule class. This is the only interface method
+   * implementers must override.
    */
-  default void customize(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
-  }
+  State define();
 
   /**
    * Default <code>"deps"</code> attribute for rule classes that don't need special behavior.
@@ -110,10 +130,12 @@ public interface MockRule extends RuleDefinition {
    * Builds out this rule with default attributes Blaze expects of all rules plus the custom
    * attributes defined by this implementation's {@link State}.
    *
-   * <p>Do not override this method. For extra custom behavior, override {@link #customize}.
+   * <p>Do not override this method. For extra custom behavior, use
+   * {@link #define(String, MockRuleCustomBehavior)}
    */
   @Override
   default RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
+    State state = define();
     builder
         .add(attr("testonly", BOOLEAN).nonconfigurable("test").value(false))
         .add(attr("deprecation", STRING).nonconfigurable("test").value((String) null))
@@ -121,13 +143,12 @@ public interface MockRule extends RuleDefinition {
         .add(attr("visibility", NODEP_LABEL_LIST).orderIndependent().cfg(HOST)
             .nonconfigurable("test"))
         .add(attr(RuleClass.COMPATIBLE_ENVIRONMENT_ATTR, LABEL_LIST)
-            .allowedFileTypes(FileTypeSet.NO_FILE))
+            .allowedFileTypes(FileTypeSet.NO_FILE)
+            .dontCheckConstraints())
         .add(attr(RuleClass.RESTRICTED_ENVIRONMENT_ATTR, LABEL_LIST)
-            .allowedFileTypes(FileTypeSet.NO_FILE));
-    for (Attribute.Builder<?> customAttribute : define().attributes) {
-      builder.add(customAttribute);
-    }
-    customize(builder, environment);
+            .allowedFileTypes(FileTypeSet.NO_FILE)
+            .dontCheckConstraints());
+    state.customBehavior.customize(builder, environment);
     return builder.build();
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/MockRuleCustomBehavior.java b/src/test/java/com/google/devtools/build/lib/analysis/util/MockRuleCustomBehavior.java
new file mode 100644
index 0000000000..2d811dcdd9
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/MockRuleCustomBehavior.java
@@ -0,0 +1,54 @@
+// Copyright 2017 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.util;
+
+import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
+import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.RuleClass;
+
+/**
+ * Interface for supporting arbitary custom behavior in mock rule classes.
+ *
+ * <p>See {@link MockRule} for details and usage instructions.
+ */
+public interface MockRuleCustomBehavior {
+
+  /**
+   * Adds custom behavior to a mock rule class.
+   *
+   * <p>It's not necessary to call {@link RuleClass.Builder#build} here.
+   */
+  void customize(RuleClass.Builder builder, RuleDefinitionEnvironment env);
+
+  /* Predefined no-op behavior.  */
+  MockRuleCustomBehavior NOOP = (builder, env) -> {};
+
+  /**
+   * Predefined behavior that populates a list of attributes.
+   */
+  class CustomAttributes implements MockRuleCustomBehavior {
+    private final Iterable<Attribute.Builder<?>> attributes;
+
+    CustomAttributes(Iterable<Attribute.Builder<?>> attributes) {
+      this.attributes = attributes;
+    }
+
+    @Override
+    public void customize(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
+      for (Attribute.Builder<?> attribute : attributes) {
+        builder.add(attribute);
+      }
+    }
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithDynamicConfigurationsTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithDynamicConfigurationsTest.java
index ba5a07cb3c..1907209406 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithDynamicConfigurationsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithDynamicConfigurationsTest.java
@@ -26,7 +26,7 @@ import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.analysis.AspectCollection;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.Dependency;
-import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
+import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.PatchTransition;
@@ -35,7 +35,6 @@ import com.google.devtools.build.lib.analysis.util.TestAspects;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.BuildType;
-import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleTransitionFactory;
 import com.google.devtools.build.lib.testutil.Suite;
 import com.google.devtools.build.lib.testutil.TestSpec;
@@ -428,21 +427,17 @@ public class ConfigurationsForTargetsWithDynamicConfigurationsTest
             return toOptions;
           };
 
-  private static final class RuleWithOutgoingTransition implements MockRule {
-    @Override
-    public State define() {
-      return MockRule.define("change_deps", MockRule.DEPS_ATTRIBUTE);
-    }
-
-    @Override
-    public void customize(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
-      builder.depsCfg(RULE_BASED_TEST_FILTER);
-    }
-  }
+  private static final RuleDefinition RULE_WITH_OUTGOING_TRANSITION = (MockRule) () ->
+      MockRule.define(
+          "change_deps",
+          (builder, env) ->
+            builder
+                .add(MockRule.DEPS_ATTRIBUTE)
+                .depsCfg(RULE_BASED_TEST_FILTER));
 
   @Test
   public void outgoingRuleTransition() throws Exception {
-    setRulesAvailableInTests(new RuleWithOutgoingTransition(),
+    setRulesAvailableInTests(RULE_WITH_OUTGOING_TRANSITION,
         (MockRule) () -> MockRule.define("foo_rule"),
         (MockRule) () -> MockRule.define("bar_rule"));
     scratch.file("outgoing/BUILD",