MockRule enhancements.

- Support custom ancestors and rule implementations.
- Refactor defaults into MockRuleDefaults.
- More thorough documentation.

PiperOrigin-RevId: 177303642

4 files changed, 210 insertions, 91 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/MockConfiguredTargetFactory.java b/src/test/java/com/google/devtools/build/lib/analysis/util/MockConfiguredTargetFactory.java
deleted file mode 100644
index bcc286bd38..0000000000
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/MockConfiguredTargetFactory.java
+++ /dev/null
@@ -1,46 +0,0 @@
-// 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.common.collect.ImmutableList;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
-import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
-import com.google.devtools.build.lib.analysis.RuleContext;
-import com.google.devtools.build.lib.analysis.Runfiles;
-import com.google.devtools.build.lib.analysis.RunfilesProvider;
-import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
-import com.google.devtools.build.lib.collect.nestedset.NestedSet;
-import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
-import com.google.devtools.build.lib.collect.nestedset.Order;
-
-/** A simple rule configured target factory for custom test rules. */
-public class MockConfiguredTargetFactory implements RuleConfiguredTargetFactory {
-  @Override
-  public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedException {
-    NestedSet<Artifact> filesToBuild =
-        NestedSetBuilder.wrap(Order.STABLE_ORDER, ruleContext.getOutputArtifacts());
-    for (Artifact artifact : ruleContext.getOutputArtifacts()) {
-      ruleContext.registerAction(
-          FileWriteAction.createEmptyWithInputs(
-              ruleContext.getActionOwner(), ImmutableList.of(), artifact));
-    }
-    return new RuleConfiguredTargetBuilder(ruleContext)
-        .setFilesToBuild(filesToBuild)
-        .setRunfilesSupport(null, null)
-        .add(RunfilesProvider.class, RunfilesProvider.simple(Runfiles.EMPTY))
-        .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 9c97e7d52d..1534202f4c 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
@@ -13,42 +13,49 @@
 // limitations under the License.
 package com.google.devtools.build.lib.analysis.util;
 
-import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.HOST;
-import static com.google.devtools.build.lib.packages.Attribute.attr;
-import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
-import static com.google.devtools.build.lib.packages.BuildType.NODEP_LABEL_LIST;
-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.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
 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.util.FileTypeSet;
 
 import java.util.Arrays;
 
 /**
  * Provides a simple API for creating custom rule classes for tests.
  *
- * <p>Usage (for a custom rule type that just needs to exist):
+ * <p>Use this whenever you want to test language-agnostic Bazel functionality, i.e. behavior that
+ * isn't specific to individual rule implementations. If you find yourself searching through rule
+ * implementations trying to find one that matches whatever you're trying to test, you probably
+ * want this instead.
+ *
+ * <p>This prevents the anti-pattern of tests with commingled dependencies. For example, when a test
+ * uses <code>cc_library</code> to test generic logic that <code>cc_library</code> happens to
+ * provide, the test can break if the <code>cc_library</code> implementation changes. This means C++
+ * rule developers have to understand the test to change C++ logic: a dependency that helps no one.
+ *
+ * <p>Even if C++ logic doesn't change, <code>cc_library</code> may not make it clear what's being
+ * tested (e.g. "why is the "malloc" attribute used here?"). Using a mock rule class offers the
+ * ability to write a clearer, more focused, easier to understand test (e.g.
+ * <code>mock_rule(name = "foo", attr_that_tests_this_specific_test_logic = ":bar")</code).
+ *
+ * <p>Usage for a custom rule type that just needs to exist (no special attributes or behavior
+ * needed):
  *
  * <pre>
  *   MockRule fooRule = () -> MockRule.define("foo_rule");
  * </pre>
  *
- * <p>Usage (for custom attributes):
+ * <p>Usage for custom attributes:
  *
  * <pre>
- *   MockRule fooRule = () -> MockRule.define("foo_rule", attr("myattr", Type.STRING));
+ *   MockRule fooRule = () -> MockRule.define("foo_rule", attr("some_attr", Type.STRING));
  * </pre>
  *
- * <p>Usage (for arbitrary customization):
+ * <p>Usage for arbitrary customization:
  *
  * <pre>
  *   MockRule fooRule = () -> MockRule.define(
@@ -60,6 +67,17 @@ import java.util.Arrays;
  *       );
  * </pre>
  *
+ * Custom {@link RuleDefinition} ancestors and {@link RuleConfiguredTargetFactory} implementations
+ * can also be specified:
+ *
+ * <pre>
+ *   MockRule customAncestor = () -> MockRule.ancestor(BaseRule.class).define(...);
+ *   MockRule customImpl = () -> MockRule.factory(FooRuleFactory.class).define(...);
+ *   MockRule customEverything = () ->
+ *       MockRule.ancestor(BaseRule.class).factory(FooRuleFactory.class).define(...);
+ * </pre>
+ *
+ * When unspecified, {@link State#DEFAULT_ANCESTOR} and {@link State#DEFAULT_FACTORY} apply.
  *
  * <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
@@ -70,24 +88,90 @@ import java.util.Arrays;
  * <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.
+ * <p>Use discretion in choosing your preferred form. The first is more compact. The second makes
+ * it clearer that <code>fooRule</code> is a proper rule class definition.
  */
 public interface MockRule extends RuleDefinition {
+  // MockRule is designed to be easy to use. That doesn't necessarily mean its implementation is
+  // easy to undestand.
+  //
+  // If you just want to mock a rule, it's best to rely on the interface javadoc above, rather than
+  // trying to parse what's going on below. You really only need to understand the below if you want
+  // to customize MockRule itself.
+
   /**
    * Container for the desired name and custom settings for this rule class.
    */
   class State {
     private final String name;
     private final MockRuleCustomBehavior customBehavior;
+    private final Class<? extends RuleConfiguredTargetFactory> factory;
+    private final Class<? extends RuleDefinition> ancestor;
+
+    /** The default {@link RuleConfiguredTargetFactory} for this rule class. */
+    private static final Class<? extends RuleConfiguredTargetFactory> DEFAULT_FACTORY =
+        MockRuleDefaults.DefaultConfiguredTargetFactory.class;
+    /** The default {@link RuleDefinition} for this rule class. */
+    private static final Class<? extends RuleDefinition> DEFAULT_ANCESTOR =
+        BaseRuleClasses.RootRule.class;
 
-    State(String ruleClassName, MockRuleCustomBehavior customBehavior) {
+    State(String ruleClassName, MockRuleCustomBehavior customBehavior,
+        Class<? extends RuleConfiguredTargetFactory> factory,
+        Class<? extends RuleDefinition> ancestor) {
       this.name = Preconditions.checkNotNull(ruleClassName);
       this.customBehavior = Preconditions.checkNotNull(customBehavior);
+      this.factory = factory;
+      this.ancestor = ancestor;
+    }
+
+    public static class Builder {
+      private Class<? extends RuleConfiguredTargetFactory> factory = DEFAULT_FACTORY;
+      private Class<? extends RuleDefinition> ancestor = DEFAULT_ANCESTOR;
+
+      public Builder factory(Class<? extends RuleConfiguredTargetFactory> factory) {
+        this.factory = factory;
+        return this;
+      }
+
+      public Builder ancestor(Class<? extends RuleDefinition> ancestor) {
+        this.ancestor = ancestor;
+        return this;
+      }
+
+      public State define(String ruleClassName, Attribute.Builder<?>... attributes) {
+        return build(ruleClassName,
+            new MockRuleCustomBehavior.CustomAttributes(Arrays.asList(attributes)));
+      }
+
+      public State define(String ruleClassName, MockRuleCustomBehavior customBehavior) {
+        return build(ruleClassName, customBehavior);
+      }
+
+      private State build(String ruleClassName, MockRuleCustomBehavior customBehavior) {
+        return new State(ruleClassName, customBehavior, factory, ancestor);
+      }
     }
   }
 
   /**
+   * Sets a custom {@link RuleConfiguredTargetFactory} for this mock rule.
+   *
+   * <p>If not set, {@link State#DEFAULT_FACTORY} is used.
+   */
+  static State.Builder factory(Class<? extends RuleConfiguredTargetFactory> factory) {
+    return new State.Builder().factory(factory);
+  }
+
+  /**
+   * Sets a custom ancestor {@link RuleDefinition} for this mock rule.
+   *
+   * <p>If not set, {@link State#DEFAULT_ANCESTOR} is used.
+   */
+  static State.Builder ancestor(Class<? extends RuleDefinition> ancestor) {
+    return new State.Builder().ancestor(ancestor);
+  }
+
+  /**
    * Returns a new {@link State} for this rule class with custom attributes. This is a convenience
    * method for lambda definitions:
    *
@@ -96,9 +180,7 @@ public interface MockRule extends RuleDefinition {
    * </pre>
    */
   static State define(String ruleClassName, Attribute.Builder<?>... attributes) {
-    return new State(
-        ruleClassName,
-        new MockRuleCustomBehavior.CustomAttributes(Arrays.asList(attributes)));
+    return new State.Builder().define(ruleClassName, attributes);
   }
 
   /**
@@ -112,7 +194,7 @@ public interface MockRule extends RuleDefinition {
    * </pre>
    */
   static State define(String ruleClassName, MockRuleCustomBehavior customBehavior) {
-    return new State(ruleClassName, customBehavior);
+    return new State.Builder().define(ruleClassName, customBehavior);
   }
 
   /**
@@ -122,13 +204,9 @@ public interface MockRule extends RuleDefinition {
   State define();
 
   /**
-   * Default <code>"deps"</code> attribute for rule classes that don't need special behavior.
-   */
-  Attribute.Builder<?> DEPS_ATTRIBUTE = attr("deps", BuildType.LABEL_LIST).allowedFileTypes();
-
-  /**
-   * Builds out this rule with default attributes Blaze expects of all rules plus the custom
-   * attributes defined by this implementation's {@link State}.
+   * Builds out this rule with default attributes Blaze expects of all rules
+   * ({@link MockRuleDefaults#DEFAULT_ATTRIBUTES}) plus the custom attributes defined by this
+   * implementation's {@link State}.
    *
    * <p>Do not override this method. For extra custom behavior, use
    * {@link #define(String, MockRuleCustomBehavior)}
@@ -136,32 +214,26 @@ public interface MockRule extends RuleDefinition {
   @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))
-        .add(attr("tags", STRING_LIST))
-        .add(attr("visibility", NODEP_LABEL_LIST).orderIndependent().cfg(HOST)
-            .nonconfigurable("test"))
-        .add(attr(RuleClass.COMPATIBLE_ENVIRONMENT_ATTR, LABEL_LIST)
-            .allowedFileTypes(FileTypeSet.NO_FILE)
-            .dontCheckConstraints())
-        .add(attr(RuleClass.RESTRICTED_ENVIRONMENT_ATTR, LABEL_LIST)
-            .allowedFileTypes(FileTypeSet.NO_FILE)
-            .dontCheckConstraints());
+    if (state.ancestor == State.DEFAULT_ANCESTOR) {
+      MockRuleDefaults.DEFAULT_ATTRIBUTES.stream().forEach(builder::add);
+    }
     state.customBehavior.customize(builder, environment);
     return builder.build();
   }
 
   /**
-   * Sets this rule class's metadata with the name defined by {@link State}.
+   * Sets this rule class's metadata with the name defined by {@link State}, configured target
+   * factory declared by {@link State.Builder#factory}, and ancestor rule class declared by
+   * {@link State.Builder#ancestor}.
    */
   @Override
   default RuleDefinition.Metadata getMetadata() {
+    State state = define();
     return RuleDefinition.Metadata.builder()
-        .name(define().name)
+        .name(state.name)
         .type(RuleClass.Builder.RuleClassType.NORMAL)
-        .factoryClass(MockConfiguredTargetFactory.class)
-        .ancestors(BaseRuleClasses.RootRule.class)
+        .factoryClass(state.factory)
+        .ancestors(state.ancestor)
         .build();
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/MockRuleDefaults.java b/src/test/java/com/google/devtools/build/lib/analysis/util/MockRuleDefaults.java
new file mode 100644
index 0000000000..bcce2b62c0
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/MockRuleDefaults.java
@@ -0,0 +1,92 @@
+// 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 static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.HOST;
+import static com.google.devtools.build.lib.packages.Attribute.attr;
+import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
+import static com.google.devtools.build.lib.packages.BuildType.NODEP_LABEL_LIST;
+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.collect.ImmutableList;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
+import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
+import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.Runfiles;
+import com.google.devtools.build.lib.analysis.RunfilesProvider;
+import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.collect.nestedset.Order;
+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.util.FileTypeSet;
+
+/**
+ * Default behaviors for {@link MockRule}.
+ *
+ * <p>All of these can be optionally added or overridden for specific mock rules.
+ */
+public class MockRuleDefaults {
+  /**
+   * Stock <code>"deps"</code> attribute for rule classes that don't need special behavior.
+   */
+  public static final Attribute.Builder<?> DEPS_ATTRIBUTE =
+      attr("deps", BuildType.LABEL_LIST).allowedFileTypes();
+
+  /**
+   * The default attributes added to all mock rules.
+   *
+   * <p>Does not apply when {@link MockRule#ancestor} is set.
+   */
+  public static final ImmutableList<Attribute.Builder<?>> DEFAULT_ATTRIBUTES = ImmutableList.of(
+      attr("testonly", BOOLEAN).nonconfigurable("test").value(false),
+      attr("deprecation", STRING).nonconfigurable("test").value((String) null),
+      attr("tags", STRING_LIST),
+      attr("visibility", NODEP_LABEL_LIST).orderIndependent().cfg(HOST).nonconfigurable("test"),
+      attr(RuleClass.COMPATIBLE_ENVIRONMENT_ATTR, LABEL_LIST)
+          .allowedFileTypes(FileTypeSet.NO_FILE)
+          .dontCheckConstraints(),
+      attr(RuleClass.RESTRICTED_ENVIRONMENT_ATTR, LABEL_LIST)
+          .allowedFileTypes(FileTypeSet.NO_FILE)
+          .dontCheckConstraints());
+
+  /**
+   * The default configured target factory for mock rules.
+   *
+   * <p>Can be overridden with {@link MockRule#factory}.
+   * */
+  public static class DefaultConfiguredTargetFactory implements RuleConfiguredTargetFactory {
+    @Override
+    public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedException {
+      NestedSet<Artifact> filesToBuild =
+          NestedSetBuilder.wrap(Order.STABLE_ORDER, ruleContext.getOutputArtifacts());
+      for (Artifact artifact : ruleContext.getOutputArtifacts()) {
+        ruleContext.registerAction(
+            FileWriteAction.createEmptyWithInputs(
+                ruleContext.getActionOwner(), ImmutableList.of(), artifact));
+      }
+      return new RuleConfiguredTargetBuilder(ruleContext)
+          .setFilesToBuild(filesToBuild)
+          .setRunfilesSupport(null, null)
+          .add(RunfilesProvider.class, RunfilesProvider.simple(Runfiles.EMPTY))
+          .build();
+    }
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithTrimmedConfigurationsTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithTrimmedConfigurationsTest.java
index 65ef8f9e5e..da21b61a52 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithTrimmedConfigurationsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithTrimmedConfigurationsTest.java
@@ -34,6 +34,7 @@ import com.google.devtools.build.lib.analysis.config.PatchTransition;
 import com.google.devtools.build.lib.analysis.config.TransitionResolver;
 import com.google.devtools.build.lib.analysis.test.TestConfiguration;
 import com.google.devtools.build.lib.analysis.util.MockRule;
+import com.google.devtools.build.lib.analysis.util.MockRuleDefaults;
 import com.google.devtools.build.lib.analysis.util.TestAspects;
 import com.google.devtools.build.lib.analysis.util.TestAspects.DummyRuleFactory;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -548,7 +549,7 @@ public class ConfigurationsForTargetsWithTrimmedConfigurationsTest
                   "change_deps",
                   (builder, env) ->
                       builder
-                          .add(MockRule.DEPS_ATTRIBUTE)
+                          .add(MockRuleDefaults.DEPS_ATTRIBUTE)
                           .requiresConfigurationFragments(TestConfiguration.class)
                           .depsCfg(RULE_BASED_TEST_FILTER));