Do not access SkylarkProviders anywhere outside of ConfiguredTarget i…

…mplementation.

A first step towards applying the same memory optimizations we do for
native provider representation to Skylark providers (declared and
legacy).

RELNOTES: None.
PiperOrigin-RevId: 156111749

src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java

@@ -89,7 +89,6 @@ public String toString() {
     return "ConfiguredTarget(" + getTarget().getLabel() + ", " + getConfiguration() + ")";
   }
 
-  @Nullable
   @Override
   public <P extends TransitiveInfoProvider> P getProvider(Class<P> provider) {
     AnalysisUtils.checkProvider(provider);
@@ -117,7 +116,7 @@ public Object getValue(String name) {
   }
 
   @Override
-  public Object getIndex(Object key, Location loc) throws EvalException {
+  public final Object getIndex(Object key, Location loc) throws EvalException {
     if (!(key instanceof ClassObjectConstructor)) {
       throw new EvalException(loc, String.format(
           "Type Target only supports indexing by object constructors, got %s instead",
@@ -149,7 +148,7 @@ public String errorMessage(String name) {
   }
 
   @Override
-  public ImmutableCollection<String> getKeys() {
+  public final ImmutableCollection<String> getKeys() {
     ImmutableList.Builder<String> result = ImmutableList.builder();
     result.addAll(ImmutableList.of(
         DATA_RUNFILES_FIELD,
@@ -180,7 +179,7 @@ private DefaultProvider getDefaultProvider() {
 
   @Nullable
   @Override
-  public Object get(SkylarkProviderIdentifier id) {
+  public final Object get(SkylarkProviderIdentifier id) {
     if (id.isLegacy()) {
       return get(id.getLegacyId());
     }
@@ -191,7 +190,7 @@ public Object get(SkylarkProviderIdentifier id) {
   /** Returns a declared provider provided by this target. Only meant to use from Skylark. */
   @Nullable
   @Override
-  public SkylarkClassObject get(ClassObjectConstructor.Key providerKey) {
+  public final SkylarkClassObject get(ClassObjectConstructor.Key providerKey) {
     if (providerKey.equals(DefaultProvider.SKYLARK_CONSTRUCTOR.getKey())) {
       return getDefaultProvider();
     }

src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java

@@ -88,9 +88,14 @@ public TransitiveInfoProviderMap getProviders() {
   @Nullable
   @VisibleForTesting
   public <P extends TransitiveInfoProvider> P getProvider(Class<P> providerClass) {
+    AnalysisUtils.checkProvider(providerClass);
     return providers.getProvider(providerClass);
   }
 
+  SkylarkProviders getSkylarkProviders() {
+    return providers.getProvider(SkylarkProviders.class);
+  }
+
   public Object getProvider(SkylarkProviderIdentifier id) {
     if (id.isLegacy()) {
       return get(id.getLegacyId());
@@ -103,15 +108,15 @@ public SkylarkClassObject get(ClassObjectConstructor.Key key) {
     if (OutputGroupProvider.SKYLARK_CONSTRUCTOR.getKey().equals(key)) {
       return getProvider(OutputGroupProvider.class);
     }
-    SkylarkProviders skylarkProviders = getProvider(SkylarkProviders.class);
+    SkylarkProviders skylarkProviders = providers.getProvider(SkylarkProviders.class);
     return skylarkProviders != null ? skylarkProviders.getDeclaredProvider(key) : null;
   }
 
   public Object get(String legacyKey) {
     if (OutputGroupProvider.SKYLARK_NAME.equals(legacyKey)) {
       return getProvider(OutputGroupProvider.class);
     }
-    SkylarkProviders skylarkProviders = getProvider(SkylarkProviders.class);
+    SkylarkProviders skylarkProviders = providers.getProvider(SkylarkProviders.class);
     return skylarkProviders != null
         ? skylarkProviders.get(SkylarkProviderIdentifier.forLegacy(legacyKey))
         : null;

src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java

@@ -16,6 +16,7 @@
 
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.syntax.ClassObject;
 import javax.annotation.Nullable;
 
 /**
@@ -28,7 +29,7 @@
  * {@link TransitiveInfoCollection}s. Also, {@link ConfiguredTarget} objects should not be
  * accessible from the action graph.
  */
-public interface ConfiguredTarget extends TransitiveInfoCollection {
+public interface ConfiguredTarget extends TransitiveInfoCollection, ClassObject {
 
   /**
    *  All <code>ConfiguredTarget</code>s have a "label" field.

src/main/java/com/google/devtools/build/lib/analysis/EnvironmentGroupConfiguredTarget.java

@@ -14,11 +14,8 @@
 
 package com.google.devtools.build.lib.analysis;
 
-import com.google.devtools.build.lib.packages.ClassObjectConstructor;
 import com.google.devtools.build.lib.packages.EnvironmentGroup;
-import com.google.devtools.build.lib.packages.SkylarkClassObject;
 import com.google.devtools.build.lib.util.Preconditions;
-import javax.annotation.Nullable;
 
 /**
  * Dummy ConfiguredTarget for environment groups. Contains no functionality, since
@@ -34,11 +31,4 @@ public final class EnvironmentGroupConfiguredTarget extends AbstractConfiguredTa
   public EnvironmentGroup getTarget() {
     return (EnvironmentGroup) super.getTarget();
   }
-
-  @Nullable
-  @Override
-  public SkylarkClassObject get(ClassObjectConstructor.Key providerKey) {
-    // No providers.
-    return null;
-  }
 }

src/main/java/com/google/devtools/build/lib/analysis/FileConfiguredTarget.java

@@ -65,7 +65,7 @@ public Artifact getArtifact() {
   }
 
   /**
-   *  Returns the file type of this file target.
+   *  Returns the file name of this file target.
    */
   @Override
   public String getFilename() {

src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java

@@ -18,12 +18,9 @@
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.events.Event;
-import com.google.devtools.build.lib.packages.ClassObjectConstructor;
 import com.google.devtools.build.lib.packages.PackageGroup;
 import com.google.devtools.build.lib.packages.PackageSpecification;
-import com.google.devtools.build.lib.packages.SkylarkClassObject;
 import com.google.devtools.build.lib.util.Preconditions;
-import javax.annotation.Nullable;
 
 /**
  * Dummy ConfiguredTarget for package groups. Contains no functionality, since
@@ -66,12 +63,4 @@ public PackageGroup getTarget() {
   public NestedSet<PackageSpecification> getPackageSpecifications() {
     return packageSpecifications;
   }
-
-
-  @Nullable
-  @Override
-  public SkylarkClassObject get(ClassObjectConstructor.Key providerKey) {
-    // No providers.
-    return null;
-  }
 }

src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java

@@ -19,6 +19,8 @@
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.OutputFile;
 import com.google.devtools.build.lib.packages.Rule;
+import com.google.devtools.build.lib.packages.SkylarkClassObject;
+import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor;
 import com.google.devtools.build.lib.util.Preconditions;
 import javax.annotation.Nullable;
 
@@ -48,7 +50,8 @@ public enum Mode {
   RuleConfiguredTarget(
       RuleContext ruleContext,
       TransitiveInfoProviderMap providers,
-      SkylarkProviders skylarkProviders) {
+      ImmutableMap<String, Object> legacySkylarkProviders,
+      ImmutableMap<SkylarkClassObjectConstructor.Key, SkylarkClassObject> skylarkProviders) {
     super(ruleContext);
     // We don't use ImmutableMap.Builder here to allow augmenting the initial list of 'default'
     // providers by passing them in.
@@ -59,9 +62,11 @@ public enum Mode {
     Preconditions.checkState(providerBuilder.contains(FilesToRunProvider.class));
 
     // Initialize every SkylarkApiProvider
-    if (!skylarkProviders.isEmpty()) {
-      skylarkProviders.init(this);
-      providerBuilder.add(skylarkProviders);
+    if (!legacySkylarkProviders.isEmpty() || !skylarkProviders.isEmpty()) {
+      SkylarkProviders allSkylarkProviders = new SkylarkProviders(legacySkylarkProviders,
+          skylarkProviders);
+      allSkylarkProviders.init(this);
+      providerBuilder.add(allSkylarkProviders);
     }
 
     this.providers = providerBuilder.build();

src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java

@@ -140,7 +140,7 @@ public ConfiguredTarget build() {
     return new RuleConfiguredTarget(
         ruleContext,
         providers,
-        new SkylarkProviders(skylarkProviders.build(), skylarkDeclaredProviders.build()));
+        skylarkProviders.build(), skylarkDeclaredProviders.build());
   }
 
   /** Adds skylark providers from a skylark provider registry, and checks for collisions. */

src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java

@@ -32,9 +32,13 @@
 
 /**
  * A helper class for transitive infos provided by Skylark rule implementations.
+ *
+ * DO NOT USE THIS CLASS to access Skylark providers.
+ * Use {@link ConfiguredTarget#get(SkylarkProviderIdentifier)} or
+ * {@link ConfiguredAspect#getProvider(SkylarkProviderIdentifier)}.
  */
 @Immutable
-public final class SkylarkProviders implements TransitiveInfoProvider {
+final class SkylarkProviders implements TransitiveInfoProvider {
   private final ImmutableMap<ClassObjectConstructor.Key, SkylarkClassObject>
       declaredProviders;
   private final ImmutableMap<String, Object> skylarkProviders;

src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java

@@ -19,7 +19,6 @@
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.FileProvider;
-import com.google.devtools.build.lib.analysis.SkylarkProviders;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -127,24 +126,28 @@ public Object getValue(String name) {
           ? NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER)
           : getProvider(FileProvider.class).getFilesToBuild());
     }
-    if (actual instanceof ClassObject) {
-      return ((ClassObject) actual).getValue(name);
-    }
-    return actual == null ? null : actual.get(name);
+    return actual == null ? null : actual.getValue(name);
   }
 
   @Override
   public ImmutableCollection<String> getKeys() {
-    ImmutableList.Builder<String> result = ImmutableList.<String>builder().add("label", "files");
     if (actual != null) {
-        result.addAll(actual.getProvider(SkylarkProviders.class).getKeys());
+        return actual.getKeys();
     }
-    return result.build();
+    return ImmutableList.of();
   }
 
   @Override
   public String errorMessage(String name) {
     // Use the default error message.
     return null;
   }
+
+  /**
+   *  Returns a target this target aliases.
+   */
+  @Nullable
+  public ConfiguredTarget getActual() {
+    return actual;
+  }
 }

src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java

@@ -37,6 +37,8 @@
 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.BuildType;
+import com.google.devtools.build.lib.packages.ClassObjectConstructor;
+import com.google.devtools.build.lib.packages.SkylarkClassObject;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.rules.cpp.CppCompilationContext;
@@ -831,6 +833,15 @@ public final <P extends TransitiveInfoProvider> Iterable<P> getDependencies(
     return AnalysisUtils.getProviders(getDependencies(), provider);
   }
 
+  /**
+   * Gets all the deps that implement a particular provider.
+   */
+  public final <P extends SkylarkClassObject> Iterable<P> getDependencies(
+      ClassObjectConstructor.Key provider, Class<P> resultClass) {
+    return AnalysisUtils.getProviders(getDependencies(), provider, resultClass);
+  }
+
+
   /**
    * Returns true if and only if this target has the neverlink attribute set to
    * 1, or false if the neverlink attribute does not exist (for example, on

src/main/java/com/google/devtools/build/lib/rules/java/JavaProvider.java

@@ -18,7 +18,6 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.Runfiles;
-import com.google.devtools.build.lib.analysis.SkylarkProviders;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMap;
@@ -133,12 +132,8 @@ public static <T extends TransitiveInfoProvider> T getProvider(
     if (provider != null) {
       return provider;
     }
-    SkylarkProviders skylarkProviders = target.getProvider(SkylarkProviders.class);
-    if (skylarkProviders == null) {
-      return null;
-    }
     JavaProvider javaProvider =
-        (JavaProvider) skylarkProviders.getDeclaredProvider(JavaProvider.JAVA_PROVIDER.getKey());
+        (JavaProvider) target.get(JavaProvider.JAVA_PROVIDER.getKey());
     if (javaProvider == null) {
       return null;
     }

src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java

@@ -30,7 +30,6 @@
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.analysis.RuleContext;
-import com.google.devtools.build.lib.analysis.SkylarkProviders;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.Util;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -294,12 +293,13 @@ private Iterable<? extends TransitiveInfoCollection> getTargetDeps() {
   private NestedSet<Artifact> getTransitivePythonSourcesFromSkylarkProvider(
       TransitiveInfoCollection dep) {
     SkylarkClassObject pythonSkylarkProvider = null;
-    SkylarkProviders skylarkProviders = dep.getProvider(SkylarkProviders.class);
     try {
-      if (skylarkProviders != null) {
-        pythonSkylarkProvider = skylarkProviders.getValue(PYTHON_SKYLARK_PROVIDER_NAME,
-            SkylarkClassObject.class);
-      }
+      pythonSkylarkProvider = SkylarkType.cast(
+              dep.get(PYTHON_SKYLARK_PROVIDER_NAME),
+              SkylarkClassObject.class,
+              null,
+              "%s should be a struct", PYTHON_SKYLARK_PROVIDER_NAME
+      );
 
       if (pythonSkylarkProvider != null) {
         Object sourceFiles = pythonSkylarkProvider.getValue(TRANSITIVE_PYTHON_SRCS);
@@ -472,13 +472,10 @@ public boolean usesSharedLibraries() {
   public static boolean checkForSharedLibraries(Iterable<TransitiveInfoCollection> deps)
           throws EvalException{
     for (TransitiveInfoCollection dep : deps) {
-      SkylarkProviders providers = dep.getProvider(SkylarkProviders.class);
-      SkylarkClassObject provider = null;
-      if (providers != null) {
-        provider = providers.getValue(PYTHON_SKYLARK_PROVIDER_NAME,
-                SkylarkClassObject.class);
-      }
-      if (provider != null) {
+      Object providerObject = dep.get(PYTHON_SKYLARK_PROVIDER_NAME);
+      if (providerObject != null) {
+        SkylarkType.checkType(providerObject, SkylarkClassObject.class, null);
+        SkylarkClassObject provider = (SkylarkClassObject) providerObject;
         Boolean isUsingSharedLibrary = provider.getValue(IS_USING_SHARED_LIBRARY, Boolean.class);
         if (Boolean.TRUE.equals(isUsingSharedLibrary)) {
           return true;

src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationSkylarkTest.java

@@ -16,7 +16,6 @@
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.SkylarkProviders;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.syntax.SkylarkDict;
 import org.junit.Test;
@@ -48,8 +47,7 @@ public void testSkylarkWithTestEnvOptions() throws Exception {
         ")");
 
     ConfiguredTarget skylarkTarget = getConfiguredTarget("//examples/config_skylark:my_target");
-    SkylarkProviders skylarkProviders = skylarkTarget.getProvider(SkylarkProviders.class);
-    assertThat(skylarkProviders.getValue("test_env", SkylarkDict.class).get("TEST_ENV_VAR"))
+    assertThat(((SkylarkDict) skylarkTarget.get("test_env")).get("TEST_ENV_VAR"))
         .isEqualTo("my_value");
   }
 }