Skylark builtin function cleanups
Clean up related to Skylark builtin functions.
Replace "hidden" field of some annotations with a "documented" field
(with reversed semantics).
--
MOS_MIGRATED_REVID=90827020
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
index c9f49ee..dce075a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
@@ -16,6 +16,7 @@
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.Type.ConversionException;
+import com.google.devtools.build.lib.syntax.SkylarkType.SkylarkFunctionType;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
@@ -231,7 +232,9 @@
int arguments = signature.getSignature().getShape().getArguments();
innerArgumentCount = arguments + (extraArgs == null ? 0 : extraArgs.length);
Class<?>[] parameterTypes = invokeMethod.getParameterTypes();
- Preconditions.checkArgument(innerArgumentCount == parameterTypes.length, getName());
+ Preconditions.checkArgument(innerArgumentCount == parameterTypes.length,
+ "bad argument count for %s: method has %s arguments, type list has %s",
+ getName(), innerArgumentCount, parameterTypes.length);
// TODO(bazel-team): also grab the returnType from the annotations,
// and check it against method return type
@@ -240,10 +243,12 @@
SkylarkType enforcedType = enforcedArgumentTypes.get(i);
if (enforcedType != null) {
Class<?> parameterType = parameterTypes[i];
- String msg = String.format("fun %s, param %s, enforcedType: %s (%s); parameterType: %s",
- getName(), signature.getSignature().getNames().get(i),
+ String msg = String.format(
+ "fun %s(%s), param %s, enforcedType: %s (%s); parameterType: %s",
+ getName(), signature, signature.getSignature().getNames().get(i),
enforcedType, enforcedType.getClass(), parameterType);
- if (enforcedType instanceof SkylarkType.Simple) {
+ if (enforcedType instanceof SkylarkType.Simple
+ || enforcedType instanceof SkylarkFunctionType) {
Preconditions.checkArgument(
enforcedType == SkylarkType.of(parameterType), msg);
// No need to enforce Simple types on the Skylark side, the JVM will do it for us.
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java
index 91b7304..8a8ff9e 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java
@@ -20,7 +20,6 @@
import java.util.logging.Level;
-
/**
* Exceptions thrown during evaluation of BUILD ASTs or Skylark extensions.
*
@@ -72,7 +71,7 @@
message = "";
}
if (cause != null) {
- message = message + (message.isEmpty() ? "" : "\n") + cause.getMessage();
+ message = message + (message.isEmpty() ? "" : ": ") + cause.getMessage();
}
if (message.isEmpty()) {
LoggingUtil.logToRemote(Level.SEVERE, "Invalid EvalException", cause);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkBuiltin.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkBuiltin.java
index a2f0d1b..894a77a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkBuiltin.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkBuiltin.java
@@ -34,7 +34,7 @@
Param[] optionalParams() default {};
- boolean hidden() default false;
+ boolean documented() default true;
Class<?> objectType() default Object.class;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallable.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallable.java
index ae6987f..89d36f1 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallable.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallable.java
@@ -26,9 +26,9 @@
public @interface SkylarkCallable {
String name() default "";
- String doc();
+ String doc() default ""; // only allowed to be empty if documented() is false.
- boolean hidden() default false;
+ boolean documented() default true;
boolean structField() default false;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkModule.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkModule.java
index 96421b2..a9c0f9b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkModule.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkModule.java
@@ -30,7 +30,7 @@
String doc();
- boolean hidden() default false;
+ boolean documented() default true;
boolean namespace() default false;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignature.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignature.java
index 0772c42..8329a09 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignature.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignature.java
@@ -46,7 +46,7 @@
Param[] extraKeywords() default {};
- boolean undocumented() default false;
+ boolean documented() default true;
Class<?> objectType() default Object.class;
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 6d7b1db..bacb78a 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
@@ -54,8 +54,8 @@
? null : new HashMap<String, SkylarkType>();
HashMap<String, String> doc = new HashMap<>();
- boolean undocumented = annotation.undocumented();
- if (annotation.doc().isEmpty() && !undocumented) {
+ boolean documented = annotation.documented();
+ if (annotation.doc().isEmpty() && documented) {
throw new RuntimeException(String.format("function %s is undocumented", name));
}
@@ -63,11 +63,11 @@
? null : defaultValues.iterator();
try {
for (Param param : annotation.mandatoryPositionals()) {
- paramList.add(getParameter(name, param, enforcedTypes, doc, undocumented,
+ paramList.add(getParameter(name, param, enforcedTypes, doc, documented,
/*mandatory=*/true, /*star=*/false, /*starStar=*/false, /*defaultValue=*/null));
}
for (Param param : annotation.optionalPositionals()) {
- paramList.add(getParameter(name, param, enforcedTypes, doc, undocumented,
+ paramList.add(getParameter(name, param, enforcedTypes, doc, documented,
/*mandatory=*/false, /*star=*/false, /*starStar=*/false,
/*defaultValue=*/getDefaultValue(param, defaultValuesIterator)));
}
@@ -79,22 +79,22 @@
Preconditions.checkArgument(annotation.extraPositionals().length == 1);
starParam = annotation.extraPositionals()[0];
}
- paramList.add(getParameter(name, starParam, enforcedTypes, doc, undocumented,
+ paramList.add(getParameter(name, starParam, enforcedTypes, doc, documented,
/*mandatory=*/false, /*star=*/true, /*starStar=*/false, /*defaultValue=*/null));
}
for (Param param : annotation.mandatoryNamedOnly()) {
- paramList.add(getParameter(name, param, enforcedTypes, doc, undocumented,
+ paramList.add(getParameter(name, param, enforcedTypes, doc, documented,
/*mandatory=*/true, /*star=*/false, /*starStar=*/false, /*defaultValue=*/null));
}
for (Param param : annotation.optionalNamedOnly()) {
- paramList.add(getParameter(name, param, enforcedTypes, doc, undocumented,
+ paramList.add(getParameter(name, param, enforcedTypes, doc, documented,
/*mandatory=*/false, /*star=*/false, /*starStar=*/false,
/*defaultValue=*/getDefaultValue(param, defaultValuesIterator)));
}
if (annotation.extraKeywords().length > 0) {
Preconditions.checkArgument(annotation.extraKeywords().length == 1);
paramList.add(
- getParameter(name, annotation.extraKeywords()[0], enforcedTypes, doc, undocumented,
+ getParameter(name, annotation.extraKeywords()[0], enforcedTypes, doc, documented,
/*mandatory=*/false, /*star=*/false, /*starStar=*/true, /*defaultValue=*/null));
}
FunctionSignature.WithValues<Object, SkylarkType> signature =
@@ -124,7 +124,7 @@
// Then the only per-parameter information needed is a documentation string.
private static Parameter<Object, SkylarkType> getParameter(
String name, Param param, Map<String, SkylarkType> enforcedTypes,
- Map<String, String> paramDoc, boolean undocumented,
+ Map<String, String> paramDoc, boolean documented,
boolean mandatory, boolean star, boolean starStar, @Nullable Object defaultValue)
throws FunctionSignature.SignatureException {
@@ -166,7 +166,7 @@
if (enforcedTypes != null) {
enforcedTypes.put(param.name(), enforcedType);
}
- if (param.doc().isEmpty() && !undocumented) {
+ if (param.doc().isEmpty() && documented) {
throw new RuntimeException(String.format("parameter %s is undocumented", name));
}
if (paramDoc != null) {
