More Skylark function cleanups
--
MOS_MIGRATED_REVID=91713784
diff --git a/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java
index 9466c69..07988a9 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java
@@ -173,7 +173,7 @@
@Param(name = "old", type = String.class, doc = "The string to be replaced."),
@Param(name = "new", type = String.class, doc = "The string to replace with.")},
optionalPositionals = {
- @Param(name = "maxsplit", type = Integer.class,
+ @Param(name = "maxsplit", type = Integer.class, noneable = true, defaultValue = "None",
doc = "The maximum number of replacements.")},
useLocation = true)
private static Function replace =
@@ -208,9 +208,10 @@
mandatoryPositionals = {
@Param(name = "self", type = String.class, doc = "This string.")},
optionalPositionals = {
- @Param(name = "sep", type = String.class,
+ @Param(name = "sep", type = String.class, defaultValue = "' '",
doc = "The string to split on, default is space (\" \")."),
- @Param(name = "maxsplit", type = Integer.class, doc = "The maximum number of splits.")},
+ @Param(name = "maxsplit", type = Integer.class, noneable = true, defaultValue = "None",
+ doc = "The maximum number of splits.")},
useEnvironment = true)
private static Function split = new MixedModeFunction("split",
ImmutableList.of("this", "sep", "maxsplit"), 1, false) {
@@ -890,10 +891,10 @@
+ "otherwise value of stop and the actual start is 0"),
},
optionalPositionals = {
- @Param(name = "stop", type = Integer.class, noneable = true,
+ @Param(name = "stop", type = Integer.class, noneable = true, defaultValue = "None",
doc = "optional index of the first item <i>not</i> to be included in the "
+ "resulting list; generation of the list stops before <code>stop</code> is reached."),
- @Param(name = "step", type = Integer.class,
+ @Param(name = "step", type = Integer.class, defaultValue = "1",
doc = "The increment (default is 1). It may be negative.")},
useLocation = true)
private static final Function range =
@@ -999,7 +1000,8 @@
@Param(name = "object", doc = "The struct which's field is accessed."),
@Param(name = "name", doc = "The name of the struct field.")},
optionalPositionals = {
- @Param(name = "default", doc = "The default value to return in case the struct "
+ @Param(name = "default", defaultValue = "None",
+ doc = "The default value to return in case the struct "
+ "doesn't have a field of the given name.")},
useLocation = true)
private static final Function getattr = new MixedModeFunction(
@@ -1014,8 +1016,9 @@
if (args[2] != null) {
return args[2];
} else {
- throw new EvalException(ast.getLocation(), "Object of type '"
- + EvalUtils.getDataTypeName(obj) + "' has no field '" + name + "'");
+ throw new EvalException(ast.getLocation(),
+ String.format("Object of type '%s' has no field %s",
+ EvalUtils.getDataTypeName(obj), EvalUtils.prettyPrintValue(name)));
}
}
return result;
@@ -1088,7 +1091,7 @@
@SkylarkSignature(name = "print", returnType = Environment.NoneType.class,
doc = "Prints <code>msg</code> to the console.",
optionalNamedOnly = {
- @Param(name = "sep", type = String.class,
+ @Param(name = "sep", type = String.class, defaultValue = "' '",
doc = "The separator string between the objects, default is space (\" \").")},
// NB: as compared to Python3, we're missing optional named-only arguments 'end' and 'file'
extraPositionals = {@Param(name = "args", doc = "The objects to print.")},
@@ -1222,18 +1225,7 @@
private static final List<Function> skylarkGlobalFunctions = ImmutableList
.<Function>builder()
.addAll(pureGlobalFunctions)
- .add(list)
- .add(struct)
- .add(hasattr)
- .add(getattr)
- .add(set)
- .add(dir)
- .add(enumerate)
- .add(range)
- .add(type)
- .add(fail)
- .add(print)
- .add(zip)
+ .add(list, struct, hasattr, getattr, set, dir, enumerate, range, type, fail, print, zip)
.build();
/**
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 aa9b811..ac669e6 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
@@ -447,8 +447,7 @@
*/
/**
- * Returns a function-value implementing "glob" in the specified package
- * context.
+ * Returns a function-value implementing "glob" in the specified package context.
*
* @param async if true, start globs in the background but don't block on their completion.
* Only use this for heuristic preloading.
@@ -465,7 +464,7 @@
};
}
- static Object callGlob(@Nullable PackageContext originalContext, boolean async,
+ static GlobList<String> callGlob(@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;
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
index cbc308e..a5b438c 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java
@@ -18,6 +18,7 @@
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.GlobList;
import com.google.devtools.build.lib.syntax.SkylarkFunction;
import com.google.devtools.build.lib.syntax.SkylarkList;
import com.google.devtools.build.lib.syntax.SkylarkModule;
@@ -37,7 +38,7 @@
public class SkylarkNativeModule {
@SkylarkSignature(name = "glob", objectType = SkylarkNativeModule.class,
- returnType = SkylarkList.class,
+ returnType = GlobList.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> "
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
index 408c484..7d39e0d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
@@ -16,7 +16,6 @@
import static com.google.devtools.build.lib.syntax.SkylarkType.castList;
-import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
import com.google.devtools.build.lib.packages.Attribute.SkylarkLateBound;
@@ -24,6 +23,7 @@
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.EvalUtils;
import com.google.devtools.build.lib.syntax.FuncallExpression;
import com.google.devtools.build.lib.syntax.Label;
import com.google.devtools.build.lib.syntax.SkylarkCallbackFunction;
@@ -75,15 +75,18 @@
+ "executable file, or to a rule that outputs an executable file. Access the labels with "
+ "<code>ctx.executable.<attribute_name></code>.";
+ private static boolean containsNonNoneKey(Map<String, Object> arguments, String key) {
+ return arguments.containsKey(key) && arguments.get(key) != Environment.NONE;
+ }
+
private static Attribute.Builder<?> createAttribute(Type<?> type, Map<String, Object> arguments,
FuncallExpression ast, SkylarkEnvironment env) throws EvalException, ConversionException {
- final Location loc = ast.getLocation();
// We use an empty name now so that we can set it later.
// This trick makes sense only in the context of Skylark (builtin rules should not use it).
Attribute.Builder<?> builder = Attribute.attr("", type);
Object defaultValue = arguments.get("default");
- if (defaultValue != null) {
+ if (!EvalUtils.isNullOrNone(defaultValue)) {
if (defaultValue instanceof UserDefinedFunction) {
// Late bound attribute. Non label type attributes already caused a type check error.
builder.value(new SkylarkLateBound(
@@ -97,23 +100,23 @@
builder.setPropertyFlag(flag);
}
- if (arguments.containsKey("mandatory") && (Boolean) arguments.get("mandatory")) {
+ if (containsNonNoneKey(arguments, "mandatory") && (Boolean) arguments.get("mandatory")) {
builder.setPropertyFlag("MANDATORY");
}
- if (arguments.containsKey("non_empty") && (Boolean) arguments.get("non_empty")) {
+ if (containsNonNoneKey(arguments, "non_empty") && (Boolean) arguments.get("non_empty")) {
builder.setPropertyFlag("NON_EMPTY");
}
- if (arguments.containsKey("executable") && (Boolean) arguments.get("executable")) {
+ if (containsNonNoneKey(arguments, "executable") && (Boolean) arguments.get("executable")) {
builder.setPropertyFlag("EXECUTABLE");
}
- if (arguments.containsKey("single_file") && (Boolean) arguments.get("single_file")) {
+ if (containsNonNoneKey(arguments, "single_file") && (Boolean) arguments.get("single_file")) {
builder.setPropertyFlag("SINGLE_ARTIFACT");
}
- if (arguments.containsKey("allow_files")) {
+ if (containsNonNoneKey(arguments, "allow_files")) {
Object fileTypesObj = arguments.get("allow_files");
if (fileTypesObj == Boolean.TRUE) {
builder.allowedFileTypes(FileTypeSet.ANY_FILE);
@@ -122,29 +125,30 @@
} else if (fileTypesObj instanceof SkylarkFileType) {
builder.allowedFileTypes(((SkylarkFileType) fileTypesObj).getFileTypeSet());
} else {
- throw new EvalException(loc, "allow_files should be a boolean or a filetype object.");
+ throw new EvalException(ast.getLocation(),
+ "allow_files should be a boolean or a filetype object.");
}
} else if (type.equals(Type.LABEL) || type.equals(Type.LABEL_LIST)) {
builder.allowedFileTypes(FileTypeSet.NO_FILE);
}
Object ruleClassesObj = arguments.get("allow_rules");
- if (ruleClassesObj != null) {
+ if (ruleClassesObj != null && ruleClassesObj != Environment.NONE) {
builder.allowedRuleClasses(castList(ruleClassesObj, String.class,
"allowed rule classes for attribute definition"));
}
- if (arguments.containsKey("providers")) {
+ if (containsNonNoneKey(arguments, "providers")) {
builder.mandatoryProviders(castList(arguments.get("providers"), String.class));
}
- if (arguments.containsKey("cfg")) {
+ if (containsNonNoneKey(arguments, "cfg")) {
builder.cfg((ConfigurationTransition) arguments.get("cfg"));
}
return builder;
}
- private static Object createAttribute(Map<String, Object> kwargs, Type<?> type,
+ private static Attribute.Builder<?> createAttribute(Map<String, Object> kwargs, Type<?> type,
FuncallExpression ast, Environment env) throws EvalException {
try {
return createAttribute(type, kwargs, ast, (SkylarkEnvironment) env);
@@ -181,7 +185,7 @@
returnType = Attribute.Builder.class,
optionalNamedOnly = {
@Param(name = "default", type = String.class,
- defaultValue = "\"\"", doc = DEFAULT_DOC + " If not specified, default is \"\"."),
+ defaultValue = "''", doc = DEFAULT_DOC + " If not specified, default is \"\"."),
@Param(name = "flags", type = SkylarkList.class, generic1 = String.class,
defaultValue = "[]", doc = FLAGS_DOC),
@Param(name = "mandatory", type = Boolean.class,
@@ -239,13 +243,18 @@
objectType = SkylarkAttr.class,
returnType = Attribute.class,
optionalPositionals = {
- @Param(name = "default", type = SkylarkList.class, generic1 = String.class,
- doc = DEFAULT_DOC + " If not specified, default is []."),
- @Param(name = "flags", type = SkylarkList.class, generic1 = String.class, doc = FLAGS_DOC),
- @Param(name = "mandatory", type = Boolean.class, doc = MANDATORY_DOC),
- @Param(name = "non_empty", type = Boolean.class, doc = NON_EMPTY_DOC),
- @Param(name = "cfg", type = ConfigurationTransition.class,
- doc = CONFIGURATION_DOC)})
+ @Param(name = "default", type = SkylarkList.class, generic1 = String.class,
+ defaultValue = "[]",
+ doc = DEFAULT_DOC + " If not specified, default is []."),
+ @Param(name = "flags", type = SkylarkList.class, generic1 = String.class,
+ defaultValue = "[]", doc = FLAGS_DOC),
+ @Param(name = "mandatory", type = Boolean.class, defaultValue = "False",
+ doc = MANDATORY_DOC),
+ @Param(name = "non_empty", type = Boolean.class, defaultValue = "False",
+ doc = NON_EMPTY_DOC),
+ @Param(name = "cfg", type = ConfigurationTransition.class, noneable = true,
+ defaultValue = "None", doc = CONFIGURATION_DOC)},
+ useAst = true, useEnvironment = true)
private static SkylarkFunction stringList = new SkylarkFunction("string_list") {
@Override
public Object call(Map<String, Object> kwargs, FuncallExpression ast, Environment env)
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
index 005b528..a5ddf3d 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
@@ -117,7 +117,7 @@
* May yield a supplier that provides factories for the Preprocessor to apply. Only one of the
* configured modules may return non-null.
*
- * The factory yielded by the supplier will be checked with
+ * <p>The factory yielded by the supplier will be checked with
* {@link Preprocessor.Factory#isStillValid} at the beginning of each incremental build. This
* allows modules to have preprocessors customizable by flags.
*
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
index b09371a..9cd37e7 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
@@ -61,8 +61,8 @@
throw new EvalException(getLocation(), customErrorMessage);
}
}
- throw new EvalException(getLocation(), "Object of type '"
- + EvalUtils.getDataTypeName(objValue) + "' has no field '" + name + "'");
+ throw new EvalException(getLocation(), String.format("Object of type '%s' has no field %s",
+ EvalUtils.getDataTypeName(objValue), EvalUtils.prettyPrintValue(name)));
}
return result;
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java
index 1d9cd81..42946ee 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java
@@ -190,9 +190,12 @@
return new Parameter.Star<>(param.name(), officialType);
} else if (mandatory) {
return new Parameter.Mandatory<>(param.name(), officialType);
- } else {
- return new Parameter.Optional<>(param.name(), officialType, defaultValue);
+ } else if (defaultValue != null && enforcedType != null) {
+ Preconditions.checkArgument(enforcedType.contains(defaultValue),
+ "In function '%s', parameter '%s' has default value %s that isn't of enforced type %s",
+ name, param.name(), EvalUtils.prettyPrintValue(defaultValue), enforcedType);
}
+ return new Parameter.Optional<>(param.name(), officialType, defaultValue);
}
private static Object getDefaultValue(Param param, Iterator<Object> iterator) {
