bazel syntax: remove type parameters from Parameter<V, T>
Parameter is a part of the syntax, and is now created only by
the parser. Its "default value" (V) is always an Expression.
Its "type" (T) is no longer needed.
The type parameters were presumably added to achieve an unholy
form of code re-use in SkylarkSignatureProcessor and FunctionSignature:
the @Param annotations from Java methods were converted into
pseudo-syntax and then fed to WithValues.of.
This change forks rather than reuses WithValues.of.
One copy is renamed fromParameters, and is specialized to the parser;
the other copy is inlined into getSignatureForCallable and specialized
to no longer need the construction of transient Parameter syntax nodes.
Both copies will be simplified in follow-up changes that
make further slices at the monster that is FunctionSignature.
Also: remove confusingly named isMandatory and isOptional methods.
They were instanceof tests, and thus not opposites.
PiperOrigin-RevId: 272868319
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DefStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/DefStatement.java
index fb29714..7b1e584 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/DefStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/DefStatement.java
@@ -22,11 +22,11 @@
private final Identifier identifier;
private final FunctionSignature.WithValues<Expression, Expression> signature;
private final ImmutableList<Statement> statements;
- private final ImmutableList<Parameter<Expression, Expression>> parameters;
+ private final ImmutableList<Parameter> parameters;
DefStatement(
Identifier identifier,
- Iterable<Parameter<Expression, Expression>> parameters,
+ Iterable<Parameter> parameters,
FunctionSignature.WithValues<Expression, Expression> signature,
Iterable<Statement> statements) {
this.identifier = identifier;
@@ -42,7 +42,7 @@
identifier.prettyPrint(buffer);
buffer.append('(');
String sep = "";
- for (Parameter<?, ?> param : parameters) {
+ for (Parameter param : parameters) {
buffer.append(sep);
param.prettyPrint(buffer);
sep = ", ";
@@ -64,10 +64,11 @@
return statements;
}
- public ImmutableList<Parameter<Expression, Expression>> getParameters() {
+ public ImmutableList<Parameter> getParameters() {
return parameters;
}
+ // TODO(adonovan): remove this, and sole external call from StatementCodecTest.
public FunctionSignature.WithValues<Expression, Expression> getSignature() {
return signature;
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java b/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
index b67dc14..3a037bf 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
@@ -236,12 +236,9 @@
return new AutoValue_FunctionSignature_WithValues<>(signature, defaultValues, types);
}
- /**
- * Parse a list of Parameter into a FunctionSignature.
- *
- * <p>To be used both by the Parser and by the SkylarkSignature annotation processor.
- */
- public static <V, T> WithValues<V, T> of(Iterable<Parameter<V, T>> parameters)
+ /** Convert a list of Parameter into a FunctionSignature. */
+ // TODO(adonovan): inline into parser (or validator) and simplify.
+ static WithValues<Expression, Expression> fromParameters(Iterable<Parameter> parameters)
throws SignatureException {
int mandatoryPositionals = 0;
int optionalPositionals = 0;
@@ -251,25 +248,20 @@
boolean hasStar = false;
@Nullable String star = null;
@Nullable String starStar = null;
- @Nullable T starType = null;
- @Nullable T starStarType = null;
ArrayList<String> params = new ArrayList<>();
- ArrayList<V> defaults = new ArrayList<>();
- ArrayList<T> types = new ArrayList<>();
+ ArrayList<Expression> defaults = new ArrayList<>();
// optional named-only parameters are kept aside to be spliced after the mandatory ones.
ArrayList<String> optionalNamedOnlyParams = new ArrayList<>();
- ArrayList<T> optionalNamedOnlyTypes = new ArrayList<>();
- ArrayList<V> optionalNamedOnlyDefaultValues = new ArrayList<>();
+ ArrayList<Expression> optionalNamedOnlyDefaultValues = new ArrayList<>();
boolean defaultRequired = false; // true after mandatory positionals and before star.
Set<String> paramNameSet = new HashSet<>(); // set of names, to avoid duplicates
- for (Parameter<V, T> param : parameters) {
+ for (Parameter param : parameters) {
if (hasStarStar) {
throw new SignatureException("illegal parameter after star-star parameter", param);
}
@Nullable String name = param.getName();
- @Nullable T type = param.getType();
- if (param.hasName()) {
+ if (param.getName() != null) {
if (paramNameSet.contains(name)) {
throw new SignatureException("duplicate parameter name in function definition", param);
}
@@ -278,7 +270,6 @@
if (param instanceof Parameter.StarStar) {
hasStarStar = true;
starStar = name;
- starStarType = type;
} else if (param instanceof Parameter.Star) {
if (hasStar) {
throw new SignatureException(
@@ -286,18 +277,15 @@
}
hasStar = true;
defaultRequired = false;
- if (param.hasName()) {
+ if (param.getName() != null) {
star = name;
- starType = type;
}
} else if (hasStar && param instanceof Parameter.Optional) {
optionalNamedOnly++;
optionalNamedOnlyParams.add(name);
- optionalNamedOnlyTypes.add(type);
optionalNamedOnlyDefaultValues.add(param.getDefaultValue());
} else {
params.add(name);
- types.add(type);
if (param instanceof Parameter.Optional) {
optionalPositionals++;
defaults.add(param.getDefaultValue());
@@ -314,16 +302,13 @@
}
}
params.addAll(optionalNamedOnlyParams);
- types.addAll(optionalNamedOnlyTypes);
defaults.addAll(optionalNamedOnlyDefaultValues);
if (star != null) {
params.add(star);
- types.add(starType);
}
if (starStar != null) {
params.add(starStar);
- types.add(starStarType);
}
return WithValues.create(
FunctionSignature.create(
@@ -335,7 +320,7 @@
starStar != null,
ImmutableList.copyOf(params)),
FunctionSignature.valueListOrNull(defaults),
- FunctionSignature.valueListOrNull(types));
+ null);
}
public StringBuilder toStringBuilder(final StringBuilder sb) {
@@ -547,16 +532,17 @@
/** Invalid signature from Parser or from SkylarkSignature annotations */
protected static class SignatureException extends Exception {
- @Nullable private final Parameter<?, ?> parameter;
+ @Nullable private final Parameter parameter;
/** SignatureException from a message and a Parameter */
- public SignatureException(String message, @Nullable Parameter<?, ?> parameter) {
+ public SignatureException(String message, @Nullable Parameter parameter) {
super(message);
this.parameter = parameter;
}
/** what parameter caused the exception, if identified? */
- @Nullable public Parameter<?, ?> getParameter() {
+ @Nullable
+ public Parameter getParameter() {
return parameter;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/NodeVisitor.java b/src/main/java/com/google/devtools/build/lib/syntax/NodeVisitor.java
index e794393..e13c6e6 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/NodeVisitor.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/NodeVisitor.java
@@ -50,7 +50,7 @@
}
// All four subclasses of Parameter are handled together.
- public void visit(Parameter<Expression, Expression> node) {
+ public void visit(Parameter node) {
visit(node.getIdentifier());
if (node.getDefaultValue() != null) {
visit(node.getDefaultValue());
@@ -132,11 +132,7 @@
public void visit(DefStatement node) {
visit(node.getIdentifier());
- // Do not use visitAll for the parameters, because we would lose the type information.
- // Inside the AST, we know that Parameters are using Expressions.
- for (Parameter<Expression, Expression> param : node.getParameters()) {
- visit(param);
- }
+ visitAll(node.getParameters());
visitBlock(node.getStatements());
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parameter.java b/src/main/java/com/google/devtools/build/lib/syntax/Parameter.java
index e585055..13d272a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parameter.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parameter.java
@@ -28,14 +28,12 @@
* <p>V is the class of a defaultValue (Expression at compile-time, Object at runtime), T is the
* class of a type (Expression at compile-time, SkylarkType at runtime).
*/
-public abstract class Parameter<V, T> extends Node {
+public abstract class Parameter extends Node {
- @Nullable protected final Identifier identifier;
- @Nullable protected final T type;
+ @Nullable private final Identifier identifier;
- private Parameter(@Nullable Identifier identifier, @Nullable T type) {
+ private Parameter(@Nullable Identifier identifier) {
this.identifier = identifier;
- this.type = type;
}
@Nullable
@@ -48,17 +46,8 @@
return identifier;
}
- public boolean hasName() {
- return true;
- }
-
@Nullable
- public T getType() {
- return type;
- }
-
- @Nullable
- public V getDefaultValue() {
+ public Expression getDefaultValue() {
return null;
}
@@ -74,14 +63,10 @@
* Syntax node for a mandatory parameter, {@code f(id)}. It may be positional or keyword-only
* depending on its position.
*/
- public static final class Mandatory<V, T> extends Parameter<V, T> {
+ public static final class Mandatory extends Parameter {
Mandatory(Identifier identifier) {
- this(identifier, null);
- }
-
- Mandatory(Identifier identifier, @Nullable T type) {
- super(identifier, type);
+ super(identifier);
}
@Override
@@ -94,22 +79,18 @@
* Syntax node for an optional parameter, {@code f(id=expr).}. It may be positional or
* keyword-only depending on its position.
*/
- public static final class Optional<V, T> extends Parameter<V, T> {
+ public static final class Optional extends Parameter {
- public final V defaultValue;
+ public final Expression defaultValue;
- Optional(Identifier identifier, @Nullable V defaultValue) {
- this(identifier, null, defaultValue);
- }
-
- Optional(Identifier identifier, @Nullable T type, @Nullable V defaultValue) {
- super(identifier, type);
+ Optional(Identifier identifier, @Nullable Expression defaultValue) {
+ super(identifier);
this.defaultValue = defaultValue;
}
@Override
@Nullable
- public V getDefaultValue() {
+ public Expression getDefaultValue() {
return defaultValue;
}
@@ -117,13 +98,9 @@
public void prettyPrint(Appendable buffer) throws IOException {
buffer.append(getName());
buffer.append('=');
- // This should only ever be used on a parameter representing static information, i.e. with V
- // and T instantiated as Expression.
- ((Expression) defaultValue).prettyPrint(buffer);
+ defaultValue.prettyPrint(buffer);
}
- // Keep this as a separate method so that it can be used regardless of what V and T are
- // parameterized with.
@Override
public String toString() {
return getName() + "=" + defaultValue;
@@ -131,19 +108,10 @@
}
/** Syntax node for a star parameter, {@code f(*identifier)} or or {@code f(..., *, ...)}. */
- public static final class Star<V, T> extends Parameter<V, T> {
-
- Star(@Nullable Identifier identifier, @Nullable T type) {
- super(identifier, type);
- }
+ public static final class Star extends Parameter {
Star(@Nullable Identifier identifier) {
- this(identifier, null);
- }
-
- @Override
- public boolean hasName() {
- return getName() != null;
+ super(identifier);
}
@Override
@@ -156,14 +124,10 @@
}
/** Syntax node for a parameter of the form {@code f(**identifier)}. */
- public static final class StarStar<V, T> extends Parameter<V, T> {
-
- StarStar(Identifier identifier, @Nullable T type) {
- super(identifier, type);
- }
+ public static final class StarStar extends Parameter {
StarStar(Identifier identifier) {
- this(identifier, null);
+ super(identifier);
}
@Override
@@ -174,8 +138,7 @@
}
@Override
- @SuppressWarnings("unchecked")
public void accept(NodeVisitor visitor) {
- visitor.visit((Parameter<Expression, Expression>) this);
+ visitor.visit(this);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
index 3918e45..fd70625 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
@@ -444,30 +444,29 @@
// arg ::= IDENTIFIER '=' nontupleexpr
// | IDENTIFIER
- private Parameter<Expression, Expression> parseFunctionParameter() {
- // TODO(bazel-team): optionally support type annotations
+ private Parameter parseFunctionParameter() {
int start = token.left;
if (token.kind == TokenKind.STAR_STAR) { // kwarg
nextToken();
Identifier ident = parseIdent();
- return setLocation(new Parameter.StarStar<>(ident), start, ident);
+ return setLocation(new Parameter.StarStar(ident), start, ident);
} else if (token.kind == TokenKind.STAR) { // stararg
int end = token.right;
nextToken();
if (token.kind == TokenKind.IDENTIFIER) {
Identifier ident = parseIdent();
- return setLocation(new Parameter.Star<>(ident), start, ident);
+ return setLocation(new Parameter.Star(ident), start, ident);
} else {
- return setLocation(new Parameter.Star<>(null), start, end);
+ return setLocation(new Parameter.Star(null), start, end);
}
} else {
Identifier ident = parseIdent();
if (token.kind == TokenKind.EQUALS) { // there's a default value
nextToken();
Expression expr = parseNonTupleExpression();
- return setLocation(new Parameter.Optional<>(ident, expr), start, expr);
+ return setLocation(new Parameter.Optional(ident, expr), start, expr);
} else {
- return setLocation(new Parameter.Mandatory<>(ident), start, ident);
+ return setLocation(new Parameter.Mandatory(ident), start, ident);
}
}
}
@@ -1250,7 +1249,7 @@
expect(TokenKind.DEF);
Identifier ident = parseIdent();
expect(TokenKind.LPAREN);
- List<Parameter<Expression, Expression>> params = parseParameters();
+ List<Parameter> params = parseParameters();
FunctionSignature.WithValues<Expression, Expression> signature = functionSignature(params);
expect(TokenKind.RPAREN);
expect(TokenKind.COLON);
@@ -1261,9 +1260,9 @@
}
private FunctionSignature.WithValues<Expression, Expression> functionSignature(
- List<Parameter<Expression, Expression>> parameters) {
+ List<Parameter> parameters) {
try {
- return FunctionSignature.WithValues.of(parameters);
+ return FunctionSignature.WithValues.fromParameters(parameters);
} catch (FunctionSignature.SignatureException e) {
reportError(e.getParameter().getLocation(), e.getMessage());
// return bogus empty signature
@@ -1276,10 +1275,10 @@
// This parser does minimal validation: it ensures the proper python use of the comma (that can
// terminate before a star but not after) and the fact that **kwargs must appear last. It does
// not validate further ordering constraints. This validation happens in the validator pass.
- private ImmutableList<Parameter<Expression, Expression>> parseParameters() {
+ private ImmutableList<Parameter> parseParameters() {
boolean hasParam = false;
boolean hasStarStar = false;
- ImmutableList.Builder<Parameter<Expression, Expression>> list = ImmutableList.builder();
+ ImmutableList.Builder<Parameter> list = ImmutableList.builder();
while (token.kind != TokenKind.RPAREN && token.kind != TokenKind.EOF) {
if (hasParam) {
@@ -1295,7 +1294,7 @@
break;
}
- Parameter<Expression, Expression> param = parseFunctionParameter();
+ Parameter param = parseFunctionParameter();
hasParam = true;
if (param instanceof Parameter.StarStar) { // TODO(adonovan): not Star too? verify.
hasStarStar = true;
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 d8e7954..557f380 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
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.syntax;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
import com.google.common.primitives.Booleans;
import com.google.devtools.build.lib.skylarkinterface.Param;
import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
@@ -23,8 +24,8 @@
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
-import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.BiFunction;
import javax.annotation.Nullable;
/**
@@ -41,8 +42,8 @@
new ConcurrentHashMap<>();
/**
- * Extracts a {@code FunctionSignature.WithValues<Object, SkylarkType>} from a
- * {@link SkylarkCallable}-annotated method.
+ * Extracts a {@code FunctionSignature.WithValues<Object, SkylarkType>} from a {@link
+ * SkylarkCallable}-annotated method.
*
* @param name the name of the function
* @param descriptor the method descriptor
@@ -50,8 +51,10 @@
* @param enforcedTypesList an optional list into which to store effective types to enforce
*/
public static FunctionSignature.WithValues<Object, SkylarkType> getSignatureForCallable(
- String name, MethodDescriptor descriptor,
- @Nullable List<String> paramDoc, @Nullable List<SkylarkType> enforcedTypesList) {
+ String name,
+ MethodDescriptor descriptor,
+ @Nullable List<String> paramDoc,
+ @Nullable List<SkylarkType> enforcedTypesList) {
SkylarkCallable annotation = descriptor.getAnnotation();
@@ -63,7 +66,7 @@
throw new RuntimeException(String.format("function %s is undocumented", name));
}
- return getSignatureForCallable(
+ return getSignatureForCallableImpl(
name,
documented,
annotation.parameters(),
@@ -73,10 +76,9 @@
enforcedTypesList);
}
-
/**
- * Extracts a {@code FunctionSignature.WithValues<Object, SkylarkType>} from a
- * {@link SkylarkSignature} annotation.
+ * Extracts a {@code FunctionSignature.WithValues<Object, SkylarkType>} from a {@link
+ * SkylarkSignature} annotation.
*
* @param name the name of the function
* @param annotation the annotation
@@ -87,8 +89,10 @@
// side-effects, and that's ugly
// TODO(bazel-team): use AutoValue to declare a value type to use as return value?
public static FunctionSignature.WithValues<Object, SkylarkType> getSignatureForCallable(
- String name, SkylarkSignature annotation,
- @Nullable List<String> paramDoc, @Nullable List<SkylarkType> enforcedTypesList) {
+ String name,
+ SkylarkSignature annotation,
+ @Nullable List<String> paramDoc,
+ @Nullable List<SkylarkType> enforcedTypesList) {
Preconditions.checkArgument(name.equals(annotation.name()),
"%s != %s", name, annotation.name());
@@ -96,7 +100,7 @@
if (annotation.doc().isEmpty() && documented) {
throw new RuntimeException(String.format("function %s is undocumented", name));
}
- return getSignatureForCallable(
+ return getSignatureForCallableImpl(
name,
documented,
annotation.parameters(),
@@ -110,82 +114,150 @@
return param.named() || param.legacyNamed();
}
- private static FunctionSignature.WithValues<Object, SkylarkType> getSignatureForCallable(
- String name, boolean documented,
- Param[] parameters,
- @Nullable Param extraPositionals, @Nullable Param extraKeywords,
- @Nullable List<String> paramDoc, @Nullable List<SkylarkType> enforcedTypesList) {
- ArrayList<Parameter<Object, SkylarkType>> paramList = new ArrayList<>();
- HashMap<String, SkylarkType> enforcedTypes =
- enforcedTypesList == null ? null : new HashMap<>();
-
- HashMap<String, String> doc = new HashMap<>();
- try {
- boolean named = false;
- for (Param param : parameters) {
- boolean mandatory = param.defaultValue() != null && param.defaultValue().isEmpty();
- Object defaultValue = mandatory ? null : getDefaultValue(param);
- if (isParamNamed(param) && !param.positional() && !named) {
- named = true;
- @Nullable Param starParam = null;
- if (extraPositionals != null && !extraPositionals.name().isEmpty()) {
- starParam = extraPositionals;
- }
- paramList.add(getParameter(name, starParam, enforcedTypes, doc, documented,
- /*mandatory=*/false, /*star=*/true, /*starStar=*/false, /*defaultValue=*/null));
- }
- paramList.add(getParameter(name, param, enforcedTypes, doc, documented,
- mandatory, /*star=*/false, /*starStar=*/false, defaultValue));
- }
- if (extraPositionals != null && !extraPositionals.name().isEmpty() && !named) {
- paramList.add(getParameter(name, extraPositionals, enforcedTypes, doc,
- documented, /*mandatory=*/false, /*star=*/true, /*starStar=*/false,
- /*defaultValue=*/null));
- }
- if (extraKeywords != null && !extraKeywords.name().isEmpty()) {
- paramList.add(
- getParameter(name, extraKeywords, enforcedTypes, doc, documented,
- /*mandatory=*/false, /*star=*/false, /*starStar=*/true, /*defaultValue=*/null));
- }
- FunctionSignature.WithValues<Object, SkylarkType> signature =
- FunctionSignature.WithValues.of(paramList);
- for (String paramName : signature.getSignature().getParameterNames()) {
- if (enforcedTypesList != null) {
- enforcedTypesList.add(enforcedTypes.get(paramName));
- }
- if (paramDoc != null) {
- paramDoc.add(doc.get(paramName));
- }
- }
- return signature;
- } catch (FunctionSignature.SignatureException e) {
- throw new RuntimeException(String.format(
- "Invalid signature while configuring BuiltinFunction %s", name), e);
- }
- }
-
- /**
- * Configures the parameter of this Skylark function using the annotation.
- */
// TODO(bazel-team): Maybe have the annotation be a string representing the
// python-style calling convention including default values, and have the regular Parser
// process it? (builtin function call not allowed when evaluating values, but more complex
// values are possible by referencing variables in some definition environment).
// 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 documented,
- boolean mandatory, boolean star, boolean starStar, @Nullable Object defaultValue)
- throws FunctionSignature.SignatureException {
- @Nullable SkylarkType officialType = null;
- @Nullable SkylarkType enforcedType = null;
- if (star && param == null) { // pseudo-parameter to separate positional from named-only
- return new Parameter.Star<>(null);
+ // Build-time annotation processing ensures mandatory parameters do not follow optional ones.
+ private static FunctionSignature.WithValues<Object, SkylarkType> getSignatureForCallableImpl(
+ final String name,
+ final boolean documented,
+ Param[] parameters,
+ @Nullable Param extraPositionals,
+ @Nullable Param extraKeywords,
+ @Nullable List<String> paramDoc,
+ @Nullable List<SkylarkType> enforcedTypesList) {
+ final HashMap<String, SkylarkType> enforcedTypes = new HashMap<>();
+ final HashMap<String, String> doc = new HashMap<>();
+
+ // TODO(adonovan): simplify this logic, possibly sharing or delegating to pieces of the
+ // analogous logic in the parser/validator.
+
+ BiFunction<Param, Object, SkylarkType> getParameterType =
+ (Param param, Object defaultValue) ->
+ getParameterType(name, documented, enforcedTypes, doc, param, defaultValue);
+
+ int mandatoryPositionals = 0;
+ int optionalPositionals = 0;
+ int mandatoryNamedOnly = 0;
+ int optionalNamedOnly = 0;
+ boolean hasStar = false;
+ String star = null;
+ String starStar = null;
+ SkylarkType starType = null;
+ SkylarkType starStarType = null;
+ ArrayList<String> params = new ArrayList<>();
+ ArrayList<Object> defaults = new ArrayList<>();
+ ArrayList<SkylarkType> types = new ArrayList<>();
+ // optional named-only parameters are kept aside to be spliced after the mandatory ones.
+ ArrayList<String> optionalNamedOnlyParams = new ArrayList<>();
+ ArrayList<SkylarkType> optionalNamedOnlyTypes = new ArrayList<>();
+ ArrayList<Object> optionalNamedOnlyDefaultValues = new ArrayList<>();
+
+ for (Param param : parameters) {
+ // Implicit * or *args parameter separates transition from positional to named.
+ // f (..., *, ... ) or f(..., *args, ...)
+ if (isParamNamed(param) && !param.positional() && !hasStar) {
+ hasStar = true;
+ if (extraPositionals != null && !extraPositionals.name().isEmpty()) {
+ starType = getParameterType.apply(extraPositionals, null);
+ star = extraPositionals.name();
+ }
+ }
+
+ boolean mandatory = param.defaultValue().isEmpty();
+ if (mandatory) {
+ // f(..., name, ...): required parameter
+ SkylarkType t = getParameterType.apply(param, null);
+ params.add(param.name());
+ types.add(t);
+ if (hasStar) {
+ mandatoryNamedOnly++;
+ } else {
+ mandatoryPositionals++;
+ }
+
+ } else {
+ // f(..., name=value, ...): optional parameter
+ Object defaultValue = getDefaultValue(param);
+ SkylarkType t = getParameterType.apply(param, defaultValue);
+ if (hasStar) {
+ optionalNamedOnly++;
+ optionalNamedOnlyParams.add(param.name());
+ optionalNamedOnlyTypes.add(t);
+ optionalNamedOnlyDefaultValues.add(defaultValue);
+ } else {
+ optionalPositionals++;
+ params.add(param.name());
+ types.add(t);
+ defaults.add(defaultValue);
+ }
+ }
}
+ params.addAll(optionalNamedOnlyParams);
+ types.addAll(optionalNamedOnlyTypes);
+ defaults.addAll(optionalNamedOnlyDefaultValues);
+
+ // f(..., *args, ...)
+ if (extraPositionals != null && !extraPositionals.name().isEmpty() && !hasStar) {
+ star = extraPositionals.name();
+ starType = getParameterType.apply(extraPositionals, null);
+ }
+ if (star != null) {
+ params.add(star);
+ types.add(starType);
+ }
+
+ // f(..., **kwargs)
+ if (extraKeywords != null && !extraKeywords.name().isEmpty()) {
+ starStar = extraKeywords.name();
+ starStarType = getParameterType.apply(extraKeywords, null);
+ params.add(starStar);
+ types.add(starStarType);
+ }
+
+ FunctionSignature.WithValues<Object, SkylarkType> signature =
+ FunctionSignature.WithValues.create(
+ FunctionSignature.create(
+ mandatoryPositionals,
+ optionalPositionals,
+ mandatoryNamedOnly,
+ optionalNamedOnly,
+ star != null,
+ starStar != null,
+ ImmutableList.copyOf(params)),
+ FunctionSignature.valueListOrNull(defaults),
+ FunctionSignature.valueListOrNull(types));
+
+ for (String paramName : signature.getSignature().getParameterNames()) {
+ if (enforcedTypesList != null) {
+ enforcedTypesList.add(enforcedTypes.get(paramName));
+ }
+ if (paramDoc != null) {
+ paramDoc.add(doc.get(paramName));
+ }
+ }
+ return signature;
+ }
+
+ // getParameterType returns the parameter's type from the @Param annotation,
+ // applies other checks and populates the type and doc mappings.
+ private static SkylarkType getParameterType(
+ // Param-independent:
+ String name,
+ boolean documented,
+ HashMap<String, SkylarkType> enforcedTypes,
+ HashMap<String, String> doc,
+ // Param-specific:
+ Param param,
+ @Nullable Object defaultValue) {
+ SkylarkType officialType = null;
+ SkylarkType enforcedType = null;
if (param.type() != Object.class) {
if (param.generic1() != Object.class) {
- // Enforce the proper parametric type for Skylark list and set objects
+ // Enforce the proper parametric type for Starlark list and set objects
officialType = SkylarkType.of(param.type(), param.generic1());
enforcedType = officialType;
} else {
@@ -193,40 +265,38 @@
enforcedType = officialType;
}
if (param.callbackEnabled()) {
- officialType = SkylarkType.Union.of(
- officialType, SkylarkType.SkylarkFunctionType.of(name, officialType));
- enforcedType = SkylarkType.Union.of(
- enforcedType, SkylarkType.SkylarkFunctionType.of(name, enforcedType));
+ officialType =
+ SkylarkType.Union.of(
+ officialType, SkylarkType.SkylarkFunctionType.of(name, officialType));
+ enforcedType =
+ SkylarkType.Union.of(
+ enforcedType, SkylarkType.SkylarkFunctionType.of(name, enforcedType));
}
if (param.noneable()) {
officialType = SkylarkType.Union.of(officialType, SkylarkType.NONE);
enforcedType = SkylarkType.Union.of(enforcedType, SkylarkType.NONE);
}
}
- if (enforcedTypes != null) {
- enforcedTypes.put(param.name(), enforcedType);
+ if (enforcedTypes.put(param.name(), enforcedType) != null) {
+ throw new IllegalStateException(
+ String.format("duplicate parameter %s on method %s", param.name(), name));
}
if (param.doc().isEmpty() && documented) {
- throw new RuntimeException(
+ throw new IllegalStateException(
String.format("parameter %s on method %s is undocumented", param.name(), name));
}
- if (paramDoc != null) {
- paramDoc.put(param.name(), param.doc());
+ doc.put(param.name(), param.doc());
+ if (defaultValue != null && !defaultValue.equals(Runtime.UNBOUND) && 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(),
+ Printer.repr(defaultValue),
+ enforcedType);
}
- if (starStar) {
- return new Parameter.StarStar<>(Identifier.of(param.name()), officialType);
- } else if (star) {
- return new Parameter.Star<>(Identifier.of(param.name()), officialType);
- } else if (mandatory) {
- return new Parameter.Mandatory<>(Identifier.of(param.name()), officialType);
- } else if (defaultValue != null
- && !defaultValue.equals(Runtime.UNBOUND)
- && 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(), Printer.repr(defaultValue), enforcedType);
- }
- return new Parameter.Optional<>(Identifier.of(param.name()), officialType, defaultValue);
+ return officialType;
}
static Object getDefaultValue(Param param) {
@@ -265,7 +335,7 @@
}
/** Extract additional signature information for BuiltinFunction-s */
- public static ExtraArgKind[] getExtraArgs(SkylarkSignature annotation) {
+ static ExtraArgKind[] getExtraArgs(SkylarkSignature annotation) {
final int numExtraArgs =
Booleans.countTrue(
annotation.useLocation(), annotation.useAst(), annotation.useStarlarkThread());
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
index 451e098..73051ae 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
@@ -340,13 +340,13 @@
node.getLocation(),
"nested functions are not allowed. Move the function to the top level.");
}
- for (Parameter<Expression, Expression> param : node.getParameters()) {
+ for (Parameter param : node.getParameters()) {
if (param instanceof Parameter.Optional) {
visit(param.getDefaultValue());
}
}
openBlock(Scope.Local);
- for (Parameter<Expression, Expression> param : node.getParameters()) {
+ for (Parameter param : node.getParameters()) {
if (param.getIdentifier() != null) {
declare(param.getIdentifier());
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/NodeVisitorTest.java b/src/test/java/com/google/devtools/build/lib/syntax/NodeVisitorTest.java
index 93b03a8..9ae38aa 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/NodeVisitorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/NodeVisitorTest.java
@@ -46,7 +46,7 @@
}
@Override
- public void visit(Parameter<Expression, Expression> node) {
+ public void visit(Parameter node) {
params.add(node.toString());
}
}
