Use a new pattern for builtin Provider objects with @SkylarkCallable.

This deprecates the old NativeProvider pattern.
The new pattern is demonstrated using AppleStaticLibraryInfo.

RELNOTES: None.
PiperOrigin-RevId: 194956883

17 files changed, 397 insertions, 111 deletions

diff --git a/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationCollector.java b/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationCollector.java
index 6b614b381b..67c46f0e95 100644
--- a/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationCollector.java
+++ b/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationCollector.java
@@ -37,6 +37,7 @@ import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
+import javax.annotation.Nullable;
 
 /**
  * A helper class that collects Skylark module documentation.
@@ -125,6 +126,10 @@ final class SkylarkDocumentationCollector {
 
     while (!toProcess.isEmpty()) {
       Class<?> c = toProcess.removeFirst();
+      if (done.contains(c)) {
+        continue;
+      }
+
       SkylarkModuleDoc module = getSkylarkModuleDoc(c, modules);
       done.add(c);
 
@@ -132,21 +137,57 @@ final class SkylarkDocumentationCollector {
         ImmutableMap<Method, SkylarkCallable> methods =
             FuncallExpression.collectSkylarkMethodsWithAnnotation(c);
         for (Map.Entry<Method, SkylarkCallable> entry : methods.entrySet()) {
-          module.addMethod(
-              new SkylarkJavaMethodDoc(module.getName(), entry.getKey(), entry.getValue()));
-        }
+          if (entry.getKey().isAnnotationPresent(SkylarkConstructor.class)) {
+            collectConstructor(modules, module.getName(), entry.getKey(), entry.getValue());
+          } else {
+            module.addMethod(
+                new SkylarkJavaMethodDoc(module.getName(), entry.getKey(), entry.getValue()));
+          }
 
-        for (Map.Entry<Method, SkylarkCallable> method : methods.entrySet()) {
-          Class<?> returnClass = method.getKey().getReturnType();
-          if (returnClass.isAnnotationPresent(SkylarkModule.class)
-              && !done.contains(returnClass)) {
+          Class<?> returnClass = entry.getKey().getReturnType();
+          if (returnClass.isAnnotationPresent(SkylarkModule.class)) {
             toProcess.addLast(returnClass);
+          } else {
+            Map.Entry<Method, SkylarkCallable> selfCallConstructor =
+                getSelfCallConstructorMethod(returnClass);
+            if (selfCallConstructor != null) {
+              // If the class to be processed is not annotated with @SkylarkModule, then its
+              // @SkylarkCallable methods are not processed, as it does not have its own
+              // documentation page. However, if it is a callable object (has a selfCall method)
+              // that is also a constructor for another type, we still want to ensure that method
+              // is documented.
+              // This is used for builtin providers, which typically are not marked @SkylarkModule,
+              // but which have selfCall constructors for their corresponding Info class.
+
+              // For example, the "mymodule" module may return a callable object at mymodule.foo
+              // which constructs instances of the Bar class. The type returned by mymodule.foo
+              // may have no documentation, but mymodule.foo should be documented as a
+              // constructor of Bar objects.
+              collectConstructor(modules, module.getName(),
+                  selfCallConstructor.getKey(), selfCallConstructor.getValue());
+            }
           }
         }
       }
     }
   }
 
+  @Nullable
+  private static Map.Entry<Method, SkylarkCallable> getSelfCallConstructorMethod(
+      Class<?> objectClass) {
+    ImmutableMap<Method, SkylarkCallable> methods =
+        FuncallExpression.collectSkylarkMethodsWithAnnotation(objectClass);
+    for (Map.Entry<Method, SkylarkCallable> entry : methods.entrySet()) {
+      if (entry.getValue().selfCall()
+          && entry.getKey().isAnnotationPresent(SkylarkConstructor.class)) {
+        // It's illegal, and checked by the interpreter, for there to be more than one method
+        // annotated with selfCall. Thus, it's valid to return on the first find.
+        return entry;
+      }
+    }
+    return null;
+  }
+
   private static void collectBuiltinDoc(Map<String, SkylarkModuleDoc> modules, Field[] fields) {
     for (Field field : fields) {
       if (field.isAnnotationPresent(SkylarkSignature.class)) {
@@ -168,15 +209,19 @@ final class SkylarkDocumentationCollector {
         FuncallExpression.collectSkylarkMethodsWithAnnotation(moduleClass);
     for (Map.Entry<Method, SkylarkCallable> entry : methods.entrySet()) {
       if (entry.getKey().isAnnotationPresent(SkylarkConstructor.class)) {
-        SkylarkConstructor constructorAnnotation =
-            entry.getKey().getAnnotation(SkylarkConstructor.class);
-        Class<?> objectClass = constructorAnnotation.objectType();
-        SkylarkModuleDoc module = getSkylarkModuleDoc(objectClass, modules);
-        module.addMethod(
-              new SkylarkJavaMethodDoc("", entry.getKey(), entry.getValue()));
+        collectConstructor(modules, "", entry.getKey(), entry.getValue());
       } else {
         topLevelModuleDoc.addMethod(new SkylarkJavaMethodDoc("", entry.getKey(), entry.getValue()));
       }
     }
   }
+
+  private static void collectConstructor(Map<String, SkylarkModuleDoc> modules,
+      String originatingModuleName, Method method, SkylarkCallable callable) {
+    SkylarkConstructor constructorAnnotation =
+        Preconditions.checkNotNull(method.getAnnotation(SkylarkConstructor.class));
+    Class<?> objectClass = constructorAnnotation.objectType();
+    SkylarkModuleDoc module = getSkylarkModuleDoc(objectClass, modules);
+    module.setConstructor(new SkylarkJavaMethodDoc(originatingModuleName, method, callable));
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkDoc.java b/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkDoc.java
index 60c19874fd..95f740e002 100644
--- a/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkDoc.java
+++ b/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkDoc.java
@@ -17,6 +17,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.skylarkinterface.Param;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkInterfaceUtils;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature;
 import com.google.devtools.build.lib.syntax.EvalUtils;
@@ -69,8 +70,8 @@ abstract class SkylarkDoc {
       return "<a class=\"anchor\" href=\"" + TOP_LEVEL_ID + ".html#None\">None</a>";
     } else if (type.equals(NestedSet.class)) {
       return "<a class=\"anchor\" href=\"depset.html\">depset</a>";
-    } else if (type.isAnnotationPresent(SkylarkModule.class)) {
-      SkylarkModule module = type.getAnnotation(SkylarkModule.class);
+    } else if (SkylarkInterfaceUtils.getSkylarkModule(type) != null) {
+      SkylarkModule module = SkylarkInterfaceUtils.getSkylarkModule(type);
       if (module.documented()) {
         return String.format("<a class=\"anchor\" href=\"%1$s.html\">%1$s</a>",
                              module.name());
diff --git a/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkModuleDoc.java b/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkModuleDoc.java
index 898c841085..24da2f3e96 100644
--- a/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkModuleDoc.java
+++ b/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkModuleDoc.java
@@ -15,6 +15,7 @@ package com.google.devtools.build.docgen.skylark;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.HashMultimap;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Multimap;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
@@ -25,10 +26,12 @@ import java.util.Collection;
 import java.util.Locale;
 import java.util.Map;
 import java.util.TreeMap;
+import javax.annotation.Nullable;
 
 /**
- * A class representing a Skylark built-in object with its {@link SkylarkSignature} annotation
- * and the {@link SkylarkCallable} methods it might have.
+ * A class representing documentation for a Skylark built-in object with its {@link SkylarkModule}
+ * annotation and with the {@link SkylarkCallable} methods and {@link SkylarkSignature} fields it
+ * documents.
  */
 public final class SkylarkModuleDoc extends SkylarkDoc {
   private final SkylarkModule module;
@@ -37,6 +40,7 @@ public final class SkylarkModuleDoc extends SkylarkDoc {
   private final Multimap<String, SkylarkJavaMethodDoc> javaMethods;
   private TreeMap<String, SkylarkMethodDoc> methodMap;
   private final String title;
+  @Nullable private SkylarkJavaMethodDoc javaConstructor;
 
   public SkylarkModuleDoc(SkylarkModule module, Class<?> classObject) {
     this.module = Preconditions.checkNotNull(
@@ -74,6 +78,12 @@ public final class SkylarkModuleDoc extends SkylarkDoc {
     return classObject;
   }
 
+  public void setConstructor(SkylarkJavaMethodDoc method) {
+    Preconditions.checkState(javaConstructor == null);
+    javaConstructor = method;
+    methodMap.put(method.getName(), method);
+  }
+
   public void addMethod(SkylarkBuiltinMethodDoc method) {
     methodMap.put(method.getName(), method);
     builtinMethodMap.put(method.getName(), method);
@@ -114,7 +124,11 @@ public final class SkylarkModuleDoc extends SkylarkDoc {
   }
 
   public Collection<SkylarkJavaMethodDoc> getJavaMethods() {
-    return javaMethods.values();
+    ImmutableList.Builder<SkylarkJavaMethodDoc> returnedMethods = ImmutableList.builder();
+    if (javaConstructor != null) {
+      returnedMethods.add(javaConstructor);
+    }
+    return returnedMethods.addAll(javaMethods.values()).build();
   }
 
   public Collection<SkylarkMethodDoc> getMethods() {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviderCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviderCollection.java
index 2187da0cae..7e2a59bb29 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviderCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviderCollection.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.analysis;
 
+import com.google.devtools.build.lib.packages.BuiltinProvider;
 import com.google.devtools.build.lib.packages.Info;
 import com.google.devtools.build.lib.packages.NativeProvider;
 import com.google.devtools.build.lib.packages.Provider;
@@ -52,6 +53,16 @@ public interface SkylarkProviderCollection {
   }
 
   /**
+   * Returns the native declared provider requested, or null, if the information is not found.
+   *
+   * <p>Type-safe version of {@link #get(Provider.Key)} for native providers.
+   */
+  @Nullable
+  default <T extends Info> T get(BuiltinProvider<T> provider) {
+    return provider.getValueClass().cast(get(provider.getKey()));
+  }
+
+  /**
    * Returns the provider defined in Skylark, or null, if the information is not found. The
    * transitive information has to have been added using the Skylark framework.
    *
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
index 18db4d3849..deab57c3c2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
@@ -223,7 +223,7 @@ public class SkylarkRuleClassFunctions {
     extraKeywords = @Param(name = "kwargs", doc = "the struct attributes."),
     useLocation = true
   )
-  private static final Provider struct = NativeProvider.STRUCT;
+  private static final NativeProvider<?> struct = NativeProvider.STRUCT;
 
   @SkylarkSignature(
     name = "DefaultInfo",
@@ -257,7 +257,7 @@ public class SkylarkRuleClassFunctions {
             + "</ul>"
             + "See the <a href='../rules.$DOC_EXT'>rules</a> page for more information."
   )
-  private static final Provider defaultInfo = DefaultInfo.PROVIDER;
+  private static final NativeProvider<?> defaultInfo = DefaultInfo.PROVIDER;
 
   @SkylarkSignature(
     name = "OutputGroupInfo",
@@ -270,7 +270,7 @@ public class SkylarkRuleClassFunctions {
             + "See <a href=\"../rules.$DOC_EXT#requesting-output-files\">Requesting output files"
             + "</a> for more information."
   )
-  private static final Provider outputGroupInfo = OutputGroupInfo.SKYLARK_CONSTRUCTOR;
+  private static final NativeProvider<?> outputGroupInfo = OutputGroupInfo.SKYLARK_CONSTRUCTOR;
 
   // TODO(bazel-team): Move to a "testing" namespace module. Normally we'd pass an objectType
   // to @SkylarkSignature to do this, but that doesn't work here because we're exposing an already-
@@ -292,7 +292,7 @@ public class SkylarkRuleClassFunctions {
             + " that have <a href=\"globals.html#rule._skylark_testable\">_skylark_testable</a> "
             + "set to <code>True</code>."
   )
-  private static final Provider actions = ActionsProvider.SKYLARK_CONSTRUCTOR;
+  private static final NativeProvider<?> actions = ActionsProvider.SKYLARK_CONSTRUCTOR;
 
   @SkylarkCallable(
     name = "provider",
diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuiltinProvider.java b/src/main/java/com/google/devtools/build/lib/packages/BuiltinProvider.java
new file mode 100644
index 0000000000..bb12ae37e6
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/packages/BuiltinProvider.java
@@ -0,0 +1,91 @@
+// Copyright 2018 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.packages;
+
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.packages.NativeProvider.NativeKey;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
+import javax.annotation.Nullable;
+
+/**
+ * Base class for declared providers {@see Provider} defined in native code.
+ *
+ * <p>Every subclass of {@link BuiltinProvider} corresponds to a single declared
+ * provider. This is enforced by final {@link #equals(Object)} and {@link #hashCode()}.
+ *
+ * <p>Implementations of native declared providers should subclass this class, and define a method
+ * in the subclass definition to create instances of its corresponding Info object. The method
+ * should be annotated with {@link SkylarkCallable} with {@link SkylarkCallable#selfCall} set to
+ * true, and with {@link SkylarkConstructor} for the info type it constructs.
+ */
+@Immutable
+public abstract class BuiltinProvider<T extends Info> implements Provider {
+  private final NativeKey key;
+  private final String name;
+  private final Class<T> valueClass;
+
+  public Class<T> getValueClass() {
+    return valueClass;
+  }
+
+  public BuiltinProvider(String name, Class<T> valueClass) {
+    @SuppressWarnings("unchecked")
+    Class<? extends BuiltinProvider<?>> clazz = (Class<? extends BuiltinProvider<?>>) getClass();
+    key = new NativeKey(name, clazz);
+    this.name = name;
+    this.valueClass = valueClass;
+  }
+
+  /**
+   * equals() implements singleton class semantics.
+   */
+  @Override
+  public final boolean equals(@Nullable Object other) {
+    return other != null && this.getClass().equals(other.getClass());
+  }
+
+  /**
+   * hashCode() implements singleton class semantics.
+   */
+  @Override
+  public final int hashCode() {
+    return getClass().hashCode();
+  }
+
+  @Override
+  public boolean isExported() {
+    return true;
+  }
+
+  @Override
+  public NativeKey getKey() {
+    return key;
+  }
+
+  @Override
+  public Location getLocation() {
+    return Location.BUILTIN;
+  }
+
+  @Override
+  public String getPrintableName() {
+    return name;
+  }
+
+  @Override
+  public void repr(SkylarkPrinter printer) {
+    printer.append("<function " + getPrintableName() + ">");
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java b/src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java
index c2cb348b37..186a0d473e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java
@@ -67,17 +67,17 @@ public class NativeInfo extends Info {
     return fieldNames;
   }
 
-  public NativeInfo(NativeProvider<?> provider) {
+  public NativeInfo(Provider provider) {
     this(provider, Location.BUILTIN);
   }
 
-  public NativeInfo(NativeProvider<?> provider, Location loc) {
+  public NativeInfo(Provider provider, Location loc) {
     this(provider, ImmutableMap.of(), loc);
   }
 
   // TODO(cparsons): Remove this constructor once DefaultInfo and ToolchainInfo stop using it.
   @Deprecated
-  public NativeInfo(NativeProvider<?> provider, Map<String, Object> values, Location loc) {
+  public NativeInfo(Provider provider, Map<String, Object> values, Location loc) {
     super(provider, loc);
     this.values = copyValues(values);
   }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java b/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java
index 372d85ec7c..20e649b0e4 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java
@@ -42,9 +42,12 @@ import javax.annotation.Nullable;
  * To allow construction from Skylark and custom construction logic, override {@link
  * Provider#createInstanceFromSkylark(Object[], Environment, Location)} (see {@link #STRUCT} for an
  * example.
+ *
+ * @deprecated use {@link BuiltinProvider} instead.
  */
 @Immutable
-public abstract class NativeProvider<V extends Info> extends Provider {
+@Deprecated
+public abstract class NativeProvider<V extends Info> extends ProviderFromFunction {
   private final NativeKey key;
   private final String errorMessageFormatForUnknownField;
 
@@ -180,9 +183,7 @@ public abstract class NativeProvider<V extends Info> extends Provider {
   @SuppressWarnings("unchecked")
   public static NativeKey getNativeKeyFromSerializedRepresentation(Pair<String, String> serialized)
       throws ClassNotFoundException {
-    Class<? extends NativeProvider<?>> aClass =
-        (Class<? extends NativeProvider<?>>)
-            Class.forName(serialized.second).asSubclass(NativeProvider.class);
+    Class<? extends Provider> aClass = Class.forName(serialized.second).asSubclass(Provider.class);
     return new NativeKey(serialized.first, aClass);
   }
 
@@ -193,12 +194,13 @@ public abstract class NativeProvider<V extends Info> extends Provider {
    */
   @AutoCodec
   @Immutable
+  // TODO(cparsons): Move this class, as NativeProvider is deprecated.
   public static final class NativeKey extends Key {
     private final String name;
-    private final Class<? extends NativeProvider<?>> aClass;
+    private final Class<? extends Provider> aClass;
 
     @VisibleForSerialization
-    NativeKey(String name, Class<? extends NativeProvider<?>> aClass) {
+    NativeKey(String name, Class<? extends Provider> aClass) {
       this.name = name;
       this.aClass = aClass;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Provider.java b/src/main/java/com/google/devtools/build/lib/packages/Provider.java
index 698b978976..5afb0fbad6 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Provider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Provider.java
@@ -16,14 +16,8 @@ package com.google.devtools.build.lib.packages;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
-import com.google.devtools.build.lib.syntax.BaseFunction;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
 import com.google.devtools.build.lib.syntax.ClassObject;
-import com.google.devtools.build.lib.syntax.Environment;
-import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.syntax.FuncallExpression;
-import com.google.devtools.build.lib.syntax.FunctionSignature;
-import com.google.devtools.build.lib.syntax.SkylarkType;
-import javax.annotation.Nullable;
 
 /**
  * Declared Provider (a constructor for {@link Info}).
@@ -63,35 +57,19 @@ import javax.annotation.Nullable;
           + "<a href=\"globals.html#provider\">provider</a> function."
 )
 @Immutable
-public abstract class Provider extends BaseFunction {
-
-  /**
-   * Constructs a provider.
-   *
-   * @param name provider name; should be null iff the subclass overrides {@link #getName}
-   * @param signature the signature for calling this provider as a Skylark function (to construct an
-   *     instance of the provider)
-   * @param location the location of this provider's Skylark definition. Use {@link
-   *     Location#BUILTIN} if it is a native provider.
-   */
-  protected Provider(
-      @Nullable String name,
-      FunctionSignature.WithValues<Object, SkylarkType> signature,
-      Location location) {
-    super(name, signature, location);
-  }
+public interface Provider extends SkylarkValue {
 
   /**
    * Has this {@link Provider} been exported? All native providers are always exported. Skylark
    * providers are exported if they are assigned to top-level name in a Skylark module.
    */
-  public abstract boolean isExported();
+  boolean isExported();
 
   /** Returns a serializable representation of this {@link Provider}. */
-  public abstract Key getKey();
+  Key getKey();
 
   /** Returns a name of this {@link Provider} that should be used in error messages. */
-  public abstract String getPrintableName();
+  String getPrintableName();
 
   /**
    * Returns an error message format string for instances to use for their {@link
@@ -99,30 +77,14 @@ public abstract class Provider extends BaseFunction {
    *
    * <p>The format string must contain one {@code '%s'} placeholder for the field name.
    */
-  public abstract String getErrorMessageFormatForUnknownField();
-
-  public SkylarkProviderIdentifier id() {
-    return SkylarkProviderIdentifier.forKey(getKey());
-  }
-
-  @Override
-  protected Object call(Object[] args, @Nullable FuncallExpression ast, Environment env)
-      throws EvalException, InterruptedException {
-    Location loc = ast != null ? ast.getLocation() : Location.BUILTIN;
-    return createInstanceFromSkylark(args, env, loc);
+  default String getErrorMessageFormatForUnknownField() {
+    return String.format("'%s' object has no attribute '%%s'", getPrintableName());
   }
 
   /**
-   * Override this method to provide logic that is used to instantiate a declared provider from
-   * Skylark.
-   *
-   * <p>This is a method that is called when a constructor {@code c} is invoked as<br>
-   * {@code c(arg1 = val1, arg2 = val2, ...)}.
-   *
-   * @param args an array of argument values sorted as per the signature ({@see BaseFunction#call})
+   * Returns the location at which provider was defined.
    */
-  protected abstract Info createInstanceFromSkylark(Object[] args, Environment env, Location loc)
-      throws EvalException;
+  Location getLocation();
 
   /** A serializable representation of {@link Provider}. */
   public abstract static class Key {}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/ProviderFromFunction.java b/src/main/java/com/google/devtools/build/lib/packages/ProviderFromFunction.java
new file mode 100644
index 0000000000..134ed49175
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/packages/ProviderFromFunction.java
@@ -0,0 +1,71 @@
+// Copyright 2018 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.packages;
+
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.syntax.BaseFunction;
+import com.google.devtools.build.lib.syntax.Environment;
+import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.FuncallExpression;
+import com.google.devtools.build.lib.syntax.FunctionSignature;
+import com.google.devtools.build.lib.syntax.SkylarkType;
+import javax.annotation.Nullable;
+
+/**
+ * Implementation of {@link Provider} that is represented by a {@link BaseFunction}, and thus
+ * callable from Skylark.
+ */
+@Immutable
+abstract class ProviderFromFunction extends BaseFunction implements Provider {
+
+  /**
+   * Constructs a provider.
+   *
+   * @param name provider name; should be null iff the subclass overrides {@link #getName}
+   * @param signature the signature for calling this provider as a Skylark function (to construct an
+   *     instance of the provider)
+   * @param location the location of this provider's Skylark definition. Use {@link
+   *     Location#BUILTIN} if it is a native provider.
+   */
+  protected ProviderFromFunction(
+      @Nullable String name,
+      FunctionSignature.WithValues<Object, SkylarkType> signature,
+      Location location) {
+    super(name, signature, location);
+  }
+
+  public SkylarkProviderIdentifier id() {
+    return SkylarkProviderIdentifier.forKey(getKey());
+  }
+
+  @Override
+  protected Object call(Object[] args, @Nullable FuncallExpression ast, Environment env)
+      throws EvalException, InterruptedException {
+    Location loc = ast != null ? ast.getLocation() : Location.BUILTIN;
+    return createInstanceFromSkylark(args, env, loc);
+  }
+
+  /**
+   * Override this method to provide logic that is used to instantiate a declared provider from
+   * Skylark.
+   *
+   * <p>This is a method that is called when a constructor {@code c} is invoked as<br>
+   * {@code c(arg1 = val1, arg2 = val2, ...)}.
+   *
+   * @param args an array of argument values sorted as per the signature ({@see BaseFunction#call})
+   */
+  protected abstract Info createInstanceFromSkylark(Object[] args, Environment env, Location loc)
+      throws EvalException;
+}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java
index c981d6e5d3..4d42b451bc 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java
@@ -45,7 +45,7 @@ import javax.annotation.Nullable;
  * pre-exported provider directly. Exported providers use only their key for {@link #equals} and
  * {@link #hashCode}.
  */
-public class SkylarkProvider extends Provider implements SkylarkExportable {
+public class SkylarkProvider extends ProviderFromFunction implements SkylarkExportable {
 
   private static final FunctionSignature.WithValues<Object, SkylarkType> SCHEMALESS_SIGNATURE =
       FunctionSignature.WithValues.create(FunctionSignature.KWARGS);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleSkylarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleSkylarkCommon.java
index f073292516..9adf309989 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleSkylarkCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleSkylarkCommon.java
@@ -254,7 +254,7 @@ public class AppleSkylarkCommon {
             + "</pre>",
     structField = true
   )
-  public Provider getAppleStaticLibraryProvider() {
+  public AppleStaticLibraryInfo.Provider getAppleStaticLibraryProvider() {
     return AppleStaticLibraryInfo.SKYLARK_CONSTRUCTOR;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryInfo.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryInfo.java
index 2280bd7487..4fe9844e4c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryInfo.java
@@ -15,11 +15,14 @@
 package com.google.devtools.build.lib.rules.objc;
 
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.packages.BuiltinProvider;
 import com.google.devtools.build.lib.packages.NativeInfo;
-import com.google.devtools.build.lib.packages.NativeProvider;
+import com.google.devtools.build.lib.skylarkinterface.Param;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkConstructor;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
+import com.google.devtools.build.lib.syntax.EvalException;
 
 /**
  * Provider containing information regarding multi-architecture Apple static libraries, as is
@@ -46,9 +49,7 @@ public final class AppleStaticLibraryInfo extends NativeInfo {
   public static final String SKYLARK_NAME = "AppleStaticLibrary";
 
   /** Skylark constructor and identifier for AppleStaticLibraryInfo. */
-  public static final NativeProvider<AppleStaticLibraryInfo> SKYLARK_CONSTRUCTOR =
-      new NativeProvider<AppleStaticLibraryInfo>(
-          AppleStaticLibraryInfo.class, SKYLARK_NAME) {};
+  public static final Provider SKYLARK_CONSTRUCTOR = new Provider();
 
   private final Artifact multiArchArchive;
   private final ObjcProvider depsObjcProvider;
@@ -87,4 +88,38 @@ public final class AppleStaticLibraryInfo extends NativeInfo {
   public ObjcProvider getDepsObjcProvider() {
     return depsObjcProvider;
   }
+
+   /**
+    * Provider class for {@link AppleStaticLibraryInfo} objects.
+    */
+  public static class Provider extends BuiltinProvider<AppleStaticLibraryInfo> {
+    private Provider() {
+      super(SKYLARK_NAME, AppleStaticLibraryInfo.class);
+    }
+
+    @SkylarkCallable(
+        name = SKYLARK_NAME,
+        doc = "The <code>AppleStaticLibrary</code> constructor.",
+        parameters = {
+          @Param(
+              name = "archive",
+              type = Artifact.class,
+              named = true,
+              positional = false,
+              doc = "Multi-architecture archive (.a) representing a static library"),
+          @Param(
+              name = "objc",
+              type = ObjcProvider.class,
+              named = true,
+              positional = false,
+              doc = "A provider which contains information about the transitive dependencies "
+                  + "linked into the archive."),
+        },
+        selfCall = true)
+    @SkylarkConstructor(objectType = AppleStaticLibraryInfo.class)
+    public AppleStaticLibraryInfo appleStaticLibrary(
+        Artifact archive, ObjcProvider objcProvider) throws EvalException {
+      return new AppleStaticLibraryInfo(archive, objcProvider);
+    }
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkInterfaceUtils.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkInterfaceUtils.java
index 3ee297993a..7788b478d8 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkInterfaceUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkInterfaceUtils.java
@@ -95,15 +95,16 @@ public class SkylarkInterfaceUtils {
    * Returns the {@link SkylarkCallable} annotation for the given method, if it exists, and
    * null otherwise. The first annotation of an overridden version of the method that is found
    * will be returned, starting with {@code classObj} and following its base classes and
-   * interfaces recursively, skipping any annotation inside a class not marked
+   * interfaces recursively. This skips any method annotated inside a class that is not
+   * marked {@link SkylarkModule} or is not a subclass of a class or interface marked
    * {@link SkylarkModule}.
    */
   @Nullable
   public static SkylarkCallable getSkylarkCallable(Class<?> classObj, Method method) {
     try {
       Method superMethod = classObj.getMethod(method.getName(), method.getParameterTypes());
-      boolean classAnnotatedForCallables = classObj.isAnnotationPresent(SkylarkModule.class)
-          || classObj.isAnnotationPresent(SkylarkGlobalLibrary.class);
+      boolean classAnnotatedForCallables = getParentWithSkylarkModule(classObj) != null
+          || hasSkylarkGlobalLibrary(classObj);
       if (classAnnotatedForCallables
           && superMethod.isAnnotationPresent(SkylarkCallable.class)) {
         return superMethod.getAnnotation(SkylarkCallable.class);
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java
index 0086a3f89d..f8d056ad9e 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java
@@ -17,6 +17,7 @@ package com.google.devtools.build.lib.rules.objc;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.getFirstArtifactEndingWith;
 import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.LIPO;
+import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
@@ -482,7 +483,7 @@ public class AppleStaticLibraryTest extends ObjcRuleTestCase {
   }
 
   @Test
-  public void testAppleStaticLibraryProvider() throws Exception {
+  public void testAppleStaticLibraryInfo() throws Exception {
     RULE_TYPE.scratchTarget(scratch, "platform_type", "'ios'");
     ConfiguredTarget binTarget = getConfiguredTarget("//x:x");
     AppleStaticLibraryInfo provider =
@@ -654,4 +655,79 @@ public class AppleStaticLibraryTest extends ObjcRuleTestCase {
     assertThat(compileArgs2).contains("FLAG_1_ON");
     assertThat(compileArgs2).contains("FLAG_2_ON");
   }
+
+  @Test
+  public void testAppleStaticLibraryProvider() throws Exception {
+    RULE_TYPE.scratchTarget(scratch, "platform_type", "'ios'");
+    scratch.file("examples/rule/BUILD");
+    scratch.file(
+        "examples/rule/apple_rules.bzl",
+        "def skylark_static_lib_impl(ctx):",
+        "   dep_provider = ctx.attr.proxy[apple_common.AppleStaticLibrary]",
+        "   my_provider = apple_common.AppleStaticLibrary(archive = dep_provider.archive,",
+        "       objc = dep_provider.objc)",
+        "   return struct(",
+        "      providers = [my_provider]",
+        "   )",
+        "",
+        "skylark_static_lib = rule(",
+        "  implementation = skylark_static_lib_impl,",
+        "  attrs = {'proxy': attr.label()},",
+        ")");
+
+    scratch.file(
+        "examples/apple_skylark/BUILD",
+        "package(default_visibility = ['//visibility:public'])",
+        "load('//examples/rule:apple_rules.bzl', 'skylark_static_lib')",
+        "skylark_static_lib(",
+        "   name='my_target',",
+        "   proxy='//x:x'",
+        ")");
+
+    ConfiguredTarget binTarget = getConfiguredTarget("//examples/apple_skylark:my_target");
+
+    AppleStaticLibraryInfo provider =
+        binTarget.get(AppleStaticLibraryInfo.SKYLARK_CONSTRUCTOR);
+    assertThat(provider).isNotNull();
+    assertThat(provider.getMultiArchArchive()).isNotNull();
+    assertThat(provider.getDepsObjcProvider()).isNotNull();
+    assertThat(provider.getMultiArchArchive()).isEqualTo(
+        Iterables.getOnlyElement(
+            provider.getDepsObjcProvider().get(ObjcProvider.MULTI_ARCH_LINKED_ARCHIVES)));
+  }
+
+  @Test
+  public void testAppleStaticLibraryProvider_invalidArgs() throws Exception {
+    RULE_TYPE.scratchTarget(scratch, "platform_type", "'ios'");
+    scratch.file("examples/rule/BUILD");
+    scratch.file(
+        "examples/rule/apple_rules.bzl",
+        "def skylark_static_lib_impl(ctx):",
+        "   dep_provider = ctx.attr.proxy[apple_common.AppleStaticLibrary]",
+        "   my_provider = apple_common.AppleStaticLibrary(archive = dep_provider.archive,",
+        "       objc = dep_provider.objc, foo = 'bar')",
+        "   return struct(",
+        "      providers = [my_provider]",
+        "   )",
+        "",
+        "skylark_static_lib = rule(",
+        "  implementation = skylark_static_lib_impl,",
+        "  attrs = {'proxy': attr.label()},",
+        ")");
+
+    scratch.file(
+        "examples/apple_skylark/BUILD",
+        "package(default_visibility = ['//visibility:public'])",
+        "load('//examples/rule:apple_rules.bzl', 'skylark_static_lib')",
+        "skylark_static_lib(",
+        "   name='my_target',",
+        "   proxy='//x:x'",
+        ")");
+
+    AssertionError expected =
+        assertThrows(AssertionError.class, () ->
+            getConfiguredTarget("//examples/apple_skylark:my_target"));
+    assertThat(expected).hasMessageThat()
+        .contains("unexpected keyword 'foo' in call");
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index cb09f8d103..36100727bf 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -485,10 +485,6 @@ public class SkylarkEvaluationTest extends EvaluationTest {
     public Boolean isEmptyInterface(String str) {
       return str.isEmpty();
     }
-    @SkylarkCallable(documented = false)
-    public Boolean isEmptyClassNotAnnotated(String str) {
-      return str.isEmpty();
-    }
   }
 
   @SkylarkModule(name = "MockClassObject", documented = false, doc = "")
@@ -1188,15 +1184,6 @@ public class SkylarkEvaluationTest extends EvaluationTest {
   }
 
   @Test
-  public void testNoJavaCallsIfClassNotAnnotated() throws Exception {
-    new SkylarkTest()
-        .update("mock", new MockSubClass())
-        .testIfExactError(
-            "type 'Mock' has no method is_empty_class_not_annotated(string)",
-            "b = mock.is_empty_class_not_annotated('a')");
-  }
-
-  @Test
   public void testStructAccess() throws Exception {
     new SkylarkTest()
         .update("mock", new Mock())
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkInterfaceUtilsTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkInterfaceUtilsTest.java
index 12aaf0ba58..6479f58041 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkInterfaceUtilsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkInterfaceUtilsTest.java
@@ -201,14 +201,4 @@ public class SkylarkInterfaceUtilsTest {
     assertThat(ann).isNotNull();
     assertThat(ann.doc()).isEqualTo("MockInterfaceB2#qux");
   }
-
-  @Test
-  public void testGetSkylarkCallableIgnoreNonModules() throws Exception {
-    // Don't return SkylarkCallable annotations in classes and interfaces
-    // not marked @SkylarkModule.
-    Method method = MockClassD.class.getMethod("foo");
-    SkylarkCallable ann = SkylarkInterfaceUtils.getSkylarkCallable(method);
-    assertThat(ann).isNotNull();
-    assertThat(ann.doc()).isEqualTo("MockClassC#foo");
-  }
 }