Skylark: the native module is refactored and documented.

 - SkylarkNativeModule is added to handle to native module.
 - Glob function is migrated to be a SkylarkFunction. Note that other functions in the native module are more difficult to migrate since they are not static.

--
MOS_MIGRATED_REVID=89292579

7 files changed, 111 insertions, 51 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index ad3cdcd039..b2f7850f6b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -38,7 +38,6 @@ import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.rules.SkylarkModules;
 import com.google.devtools.build.lib.syntax.Label;
 import com.google.devtools.build.lib.syntax.SkylarkEnvironment;
-import com.google.devtools.build.lib.syntax.SkylarkModule;
 import com.google.devtools.build.lib.syntax.SkylarkType;
 import com.google.devtools.build.lib.syntax.ValidationEnvironment;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -294,7 +293,6 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider {
     this.skylarkValidationEnvironment = SkylarkModules.getValidationEnvironment(
         ImmutableMap.<String, SkylarkType>builder()
             .putAll(skylarkAccessibleJavaClasses)
-            .put("native", SkylarkType.of(NativeModule.class))
             .build());
   }
 
@@ -364,12 +362,6 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider {
     return BuildOptions.of(configurationOptions, optionsProvider);
   }
 
-  @SkylarkModule(name = "native", namespace = true, onlyLoadingPhase = true,
-      doc = "Module for native rules.")
-  private static final class NativeModule {}
-
-  public static final NativeModule nativeModule = new NativeModule();
-
   @Override
   public SkylarkEnvironment createSkylarkRuleClassEnvironment(
       EventHandler eventHandler, String astFileContentHashCode) {
@@ -386,11 +378,6 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider {
   }
 
   @Override
-  public Object getNativeModule() {
-    return nativeModule;
-  }
-
-  @Override
   public List<PathFragment> getWorkspaceFiles() {
     return defaultWorkspaceFiles;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index c8aaefd704..cc84ef286f 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -455,39 +455,44 @@ public final class PackageFactory {
         @Override
         public Object call(Object[] namedArguments, FuncallExpression ast, Environment env)
                 throws EvalException, ConversionException, InterruptedException {
-
-          // Skylark build extensions need to get the PackageContext from the Environment;
-          // async glob functions cannot do the same because the Environment is not thread safe.
-          PackageContext context;
-          if (originalContext == null) {
-            Preconditions.checkArgument(!async);
-            context = getContext(env, ast);
-          } else {
-            context = originalContext;
-          }
-
-          List<String> includes = Type.STRING_LIST.convert(namedArguments[0], "'glob' argument");
-          List<String> excludes = namedArguments[1] == null
-              ? Collections.<String>emptyList()
-              : Type.STRING_LIST.convert(namedArguments[1], "'glob' argument");
-          int excludeDirs = namedArguments[2] == null
-            ? EXCLUDE_DIR_DEFAULT
-            : Type.INTEGER.convert(namedArguments[2], "'glob' argument");
-
-          if (async) {
-            try {
-              context.globber.runAsync(includes, excludes, excludeDirs != 0);
-            } catch (GlobCache.BadGlobException e) {
-              // Ignore: errors will appear during the actual evaluation of the package.
-            }
-            return GlobList.captureResults(includes, excludes, ImmutableList.<String>of());
-          } else {
-            return handleGlob(includes, excludes, excludeDirs != 0, context, ast);
-          }
+          return globCall(originalContext, async, ast, env, namedArguments);
         }
       };
   }
 
+  static Object globCall(@Nullable PackageContext originalContext, boolean async,
+      FuncallExpression ast, Environment env, Object[] namedArguments)
+          throws EvalException, ConversionException, InterruptedException {
+    // Skylark build extensions need to get the PackageContext from the Environment;
+    // async glob functions cannot do the same because the Environment is not thread safe.
+    PackageContext context;
+    if (originalContext == null) {
+      Preconditions.checkArgument(!async);
+      context = getContext(env, ast);
+    } else {
+      context = originalContext;
+    }
+
+    List<String> includes = Type.STRING_LIST.convert(namedArguments[0], "'glob' argument");
+    List<String> excludes = namedArguments[1] == null
+        ? Collections.<String>emptyList()
+        : Type.STRING_LIST.convert(namedArguments[1], "'glob' argument");
+    int excludeDirs = namedArguments[2] == null
+      ? EXCLUDE_DIR_DEFAULT
+      : Type.INTEGER.convert(namedArguments[2], "'glob' argument");
+
+    if (async) {
+      try {
+        context.globber.runAsync(includes, excludes, excludeDirs != 0);
+      } catch (GlobCache.BadGlobException e) {
+        // Ignore: errors will appear during the actual evaluation of the package.
+      }
+      return GlobList.captureResults(includes, excludes, ImmutableList.<String>of());
+    } else {
+      return handleGlob(includes, excludes, excludeDirs != 0, context, ast);
+    }
+  }
+
   /**
    * Adds a glob to the package, reporting any errors it finds.
    *
@@ -1075,7 +1080,6 @@ public final class PackageFactory {
     for (String ruleClass : ruleFactory.getRuleClassNames()) {
       builder.add(newRuleFunction(ruleFactory, ruleClass));
     }
-    builder.add(newGlobFunction(null, false));
     builder.add(newPackageFunction(packageArguments));
     return builder.build();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
index 07b1b18dd9..1f9f25c106 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
@@ -45,11 +45,6 @@ public interface RuleClassProvider {
   ValidationEnvironment getSkylarkValidationEnvironment();
 
   /**
-   * Returns the Skylark module to register the native rules with.
-   */
-  Object getNativeModule();
-
-  /**
    * Returns paths to the WORKSPACE files needed to provide external dependencies for built-in
    * rules.  The PathFragments are relative to Bazel's install directory. Returns an empty list if
    * there are none defined.
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java
new file mode 100644
index 0000000000..67191be628
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java
@@ -0,0 +1,67 @@
+// Copyright 2014 Google Inc. 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.packages.Type.ConversionException;
+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.SkylarkBuiltin;
+import com.google.devtools.build.lib.syntax.SkylarkBuiltin.Param;
+import com.google.devtools.build.lib.syntax.SkylarkFunction;
+import com.google.devtools.build.lib.syntax.SkylarkList;
+import com.google.devtools.build.lib.syntax.SkylarkModule;
+
+import java.util.Map;
+
+/**
+ * A class for the Skylark native module.
+ */
+@SkylarkModule(name = "native", namespace = true, onlyLoadingPhase = true, doc =
+      "A built-in module to support native rules and other package helper functions. "
+    + "All native rules appear as functions in this module. Note that the native module is only "
+    + "available in the loading phase (i.e. for macros, not for rule implementations).<br/>"
+    + "Extra helper functions:")
+public class SkylarkNativeModule {
+
+  @SkylarkBuiltin(name = "glob", objectType = SkylarkNativeModule.class, 
+      doc = "Glob returns a list of every file in the current package that:<ul>\n"
+          + "<li>Matches at least one pattern in <code>include</code>.</li>\n"
+          + "<li>Does not match any of the patterns in <code>exclude</code> "
+          + "(default <code>[]</code>).</li></ul>\n"
+          + "If the <code>exclude_directories</code> argument is enabled (set to <code>1</code>), "
+          + "files of type directory will be omitted from the results (default <code>1</code>).",
+      mandatoryParams = {
+      @Param(name = "includes", type = SkylarkList.class, generic1 = String.class,
+          doc = "The list of glob patterns to include.")},
+      optionalParams = {
+      @Param(name = "excludes", type = SkylarkList.class, generic1 = String.class,
+          doc = "The list of glob patterns to exclude."),
+      @Param(name = "exclude_directories", type = Integer.class,
+          doc = "A flag whether to exclude directories or not.")})
+  private static final SkylarkFunction glob = new SkylarkFunction("glob") {
+    @Override
+    public Object call(Map<String, Object> kwargs, FuncallExpression ast, Environment env)
+        throws EvalException, ConversionException, InterruptedException {
+      return PackageFactory.globCall(null, false, ast, env, new Object[] {
+          kwargs.get("includes"),
+          kwargs.get("excludes"),
+          kwargs.get("exclude_directories")
+      });
+    }
+  };
+
+  public static final SkylarkNativeModule NATIVE_MODULE = new SkylarkNativeModule();
+}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java
index a5dc760976..7d1e17647b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java
@@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.collect.CollectionUtils;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.MethodLibrary;
+import com.google.devtools.build.lib.packages.SkylarkNativeModule;
 import com.google.devtools.build.lib.syntax.Environment;
 import com.google.devtools.build.lib.syntax.Function;
 import com.google.devtools.build.lib.syntax.SkylarkBuiltin;
@@ -41,7 +42,13 @@ import java.util.Map;
 // let each extension register itself in a static { } statement.
 public class SkylarkModules {
 
+  /**
+   * The list of built in Skylark modules. Documentation is generated automatically for all these
+   * modules. They are also registered with the {@link ValidationEnvironment} and the
+   * {@link SkylarkEnvironment}. Note that only {@link SkylarkFunction}s are handled properly.
+   */
   public static final ImmutableList<Class<?>> MODULES = ImmutableList.of(
+      SkylarkNativeModule.class,
       SkylarkAttr.class,
       SkylarkCommandLine.class,
       SkylarkRuleClassFunctions.class,
@@ -123,7 +130,6 @@ public class SkylarkModules {
             SkylarkType.of(moduleClass));
       }
     }
-    global.put("native", SkylarkType.UNKNOWN);
     MethodLibrary.setupValidationEnvironment(builtIn);
     for (Class<?> module : MODULES) {
       collectSkylarkTypesFromFields(module, builtIn);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
index efc241e24a..7031e53d7a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
@@ -21,6 +21,7 @@ import com.google.devtools.build.lib.packages.PackageFactory;
 import com.google.devtools.build.lib.packages.PackageIdentifier;
 import com.google.devtools.build.lib.packages.PackageIdentifier.RepositoryName;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
+import com.google.devtools.build.lib.packages.SkylarkNativeModule;
 import com.google.devtools.build.lib.skyframe.ASTFileLookupValue.ASTLookupInputException;
 import com.google.devtools.build.lib.syntax.BuildFileAST;
 import com.google.devtools.build.lib.syntax.Function;
@@ -172,10 +173,9 @@ public class SkylarkImportLookupFunction implements SkyFunction {
         .createSkylarkRuleClassEnvironment(eventHandler, ast.getContentHashCode());
     // Adding native rules module for build extensions.
     // TODO(bazel-team): this might not be the best place to do this.
-    extensionEnv.update("native", ruleClassProvider.getNativeModule());
     for (Function function : nativeRuleFunctions) {
         extensionEnv.registerFunction(
-            ruleClassProvider.getNativeModule().getClass(), function.getName(), function);
+            SkylarkNativeModule.class, function.getName(), function);
     }
     extensionEnv.setImportedExtensions(importMap);
     ast.exec(extensionEnv, eventHandler);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkFunction.java
index 32cf6cb9b1..b5aa6c9729 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkFunction.java
@@ -184,6 +184,7 @@ public abstract class SkylarkFunction extends AbstractFunction {
       ConversionException,
       IllegalArgumentException,
       IllegalStateException,
+      InterruptedException,
       ClassCastException,
       ClassNotFoundException,
       ExecutionException;