diff options
Diffstat (limited to 'src/main/java')
4 files changed, 187 insertions, 119 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java index 2c3e25acbc..4afe8c29d6 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java @@ -821,25 +821,6 @@ public final class SkylarkAttr implements SkylarkValue { + "<a href=\"string.html\">strings</a>.", parameters = { @Param( - name = DEFAULT_ARG, - allowedTypes = { - @ParamType(type = SkylarkList.class, generic1 = String.class), - @ParamType(type = UserDefinedFunction.class) - }, - defaultValue = "[]", - doc = DEFAULT_DOC, - named = true, - positional = false - ), - @Param( - name = DOC_ARG, - type = String.class, - defaultValue = "''", - doc = DOC_DOC, - named = true, - positional = false - ), - @Param( name = MANDATORY_ARG, type = Boolean.class, defaultValue = "False", @@ -859,17 +840,36 @@ public final class SkylarkAttr implements SkylarkValue { defaultValue = "True", doc = ALLOW_EMPTY_DOC, named = true + ), + @Param( + name = DEFAULT_ARG, + allowedTypes = { + @ParamType(type = SkylarkList.class, generic1 = String.class), + @ParamType(type = UserDefinedFunction.class) + }, + defaultValue = "[]", + doc = DEFAULT_DOC, + named = true, + positional = false + ), + @Param( + name = DOC_ARG, + type = String.class, + defaultValue = "''", + doc = DOC_DOC, + named = true, + positional = false ) }, useAst = true, useEnvironment = true ) public Descriptor stringListAttribute( - SkylarkList<?> defaultList, - String doc, Boolean mandatory, Boolean nonEmpty, Boolean allowEmpty, + SkylarkList<?> defaultList, + String doc, FuncallExpression ast, Environment env) throws EvalException { @@ -896,25 +896,6 @@ public final class SkylarkAttr implements SkylarkValue { doc = "Creates an attribute which is a <a href=\"list.html\">list</a> of ints.", parameters = { @Param( - name = DEFAULT_ARG, - allowedTypes = { - @ParamType(type = SkylarkList.class, generic1 = Integer.class), - @ParamType(type = UserDefinedFunction.class) - }, - defaultValue = "[]", - doc = DEFAULT_DOC, - named = true, - positional = false - ), - @Param( - name = DOC_ARG, - type = String.class, - defaultValue = "''", - doc = DOC_DOC, - named = true, - positional = false - ), - @Param( name = MANDATORY_ARG, type = Boolean.class, defaultValue = "False", @@ -934,17 +915,36 @@ public final class SkylarkAttr implements SkylarkValue { defaultValue = "True", doc = ALLOW_EMPTY_DOC, named = true + ), + @Param( + name = DEFAULT_ARG, + allowedTypes = { + @ParamType(type = SkylarkList.class, generic1 = Integer.class), + @ParamType(type = UserDefinedFunction.class) + }, + defaultValue = "[]", + doc = DEFAULT_DOC, + named = true, + positional = false + ), + @Param( + name = DOC_ARG, + type = String.class, + defaultValue = "''", + doc = DOC_DOC, + named = true, + positional = false ) }, useAst = true, useEnvironment = true ) public Descriptor intListAttribute( - SkylarkList<?> defaultList, - String doc, Boolean mandatory, Boolean nonEmpty, Boolean allowEmpty, + SkylarkList<?> defaultList, + String doc, FuncallExpression ast, Environment env) throws EvalException { @@ -974,6 +974,13 @@ public final class SkylarkAttr implements SkylarkValue { + "See <a href=\"attr.html#label\">label</a> for more information.", parameters = { @Param( + name = ALLOW_EMPTY_ARG, + type = Boolean.class, + defaultValue = "True", + doc = ALLOW_EMPTY_DOC, + named = true + ), + @Param( name = DEFAULT_ARG, allowedTypes = { @ParamType(type = SkylarkList.class, generic1 = Label.class), @@ -1049,13 +1056,6 @@ public final class SkylarkAttr implements SkylarkValue { doc = NON_EMPTY_DOC ), @Param( - name = ALLOW_EMPTY_ARG, - type = Boolean.class, - defaultValue = "True", - doc = ALLOW_EMPTY_DOC, - named = true - ), - @Param( name = CONFIGURATION_ARG, type = Object.class, noneable = true, @@ -1078,6 +1078,7 @@ public final class SkylarkAttr implements SkylarkValue { useEnvironment = true ) public Descriptor labelListAttribute( + Boolean allowEmpty, Object defaultList, String doc, Object allowFiles, @@ -1086,7 +1087,6 @@ public final class SkylarkAttr implements SkylarkValue { SkylarkList<?> flags, Boolean mandatory, Boolean nonEmpty, - Boolean allowEmpty, Object cfg, SkylarkList<?> aspects, FuncallExpression ast, @@ -1134,6 +1134,13 @@ public final class SkylarkAttr implements SkylarkValue { + "<a href=\"attr.html#label\">label</a> for more information.", parameters = { @Param( + name = ALLOW_EMPTY_ARG, + type = Boolean.class, + defaultValue = "True", + doc = ALLOW_EMPTY_DOC, + named = true + ), + @Param( name = DEFAULT_ARG, allowedTypes = { @ParamType(type = SkylarkDict.class), @@ -1210,13 +1217,6 @@ public final class SkylarkAttr implements SkylarkValue { doc = NON_EMPTY_DOC ), @Param( - name = ALLOW_EMPTY_ARG, - type = Boolean.class, - defaultValue = "True", - doc = ALLOW_EMPTY_DOC, - named = true - ), - @Param( name = CONFIGURATION_ARG, type = Object.class, noneable = true, @@ -1239,6 +1239,7 @@ public final class SkylarkAttr implements SkylarkValue { useEnvironment = true ) public Descriptor labelKeyedStringDictAttribute( + Boolean allowEmpty, Object defaultList, String doc, Object allowFiles, @@ -1247,7 +1248,6 @@ public final class SkylarkAttr implements SkylarkValue { SkylarkList<?> flags, Boolean mandatory, Boolean nonEmpty, - Boolean allowEmpty, Object cfg, SkylarkList<?> aspects, FuncallExpression ast, @@ -1392,6 +1392,13 @@ public final class SkylarkAttr implements SkylarkValue { + "See <a href=\"attr.html#output\">output</a> for more information.", parameters = { @Param( + name = ALLOW_EMPTY_ARG, + type = Boolean.class, + defaultValue = "True", + doc = ALLOW_EMPTY_DOC, + named = true + ), + @Param( name = DEFAULT_ARG, allowedTypes = { @ParamType(type = SkylarkList.class, generic1 = Label.class), @@ -1426,24 +1433,17 @@ public final class SkylarkAttr implements SkylarkValue { named = true, positional = false, doc = NON_EMPTY_DOC - ), - @Param( - name = ALLOW_EMPTY_ARG, - type = Boolean.class, - defaultValue = "True", - doc = ALLOW_EMPTY_DOC, - named = true ) }, useAst = true, useEnvironment = true ) public Descriptor outputListAttribute( + Boolean allowEmpty, SkylarkList defaultList, String doc, Boolean mandatory, Boolean nonEmpty, - Boolean allowEmpty, FuncallExpression ast, Environment env) throws EvalException { @@ -1472,6 +1472,13 @@ public final class SkylarkAttr implements SkylarkValue { + "<a href=\"string.html\">string</a> to <a href=\"string.html\">string</a>.", parameters = { @Param( + name = ALLOW_EMPTY_ARG, + type = Boolean.class, + defaultValue = "True", + doc = ALLOW_EMPTY_DOC, + named = true + ), + @Param( name = DEFAULT_ARG, allowedTypes = { @ParamType(type = SkylarkDict.class), @@ -1505,24 +1512,17 @@ public final class SkylarkAttr implements SkylarkValue { named = true, positional = false, doc = NON_EMPTY_DOC - ), - @Param( - name = ALLOW_EMPTY_ARG, - type = Boolean.class, - defaultValue = "True", - doc = ALLOW_EMPTY_DOC, - named = true ) }, useAst = true, useEnvironment = true ) public Descriptor stringDictAttribute( + Boolean allowEmpty, SkylarkDict<?, ?> defaultO, String doc, Boolean mandatory, Boolean nonEmpty, - Boolean allowEmpty, FuncallExpression ast, Environment env) throws EvalException { @@ -1552,6 +1552,13 @@ public final class SkylarkAttr implements SkylarkValue { + "<a href=\"string.html\">string</a>.", parameters = { @Param( + name = ALLOW_EMPTY_ARG, + type = Boolean.class, + defaultValue = "True", + doc = ALLOW_EMPTY_DOC, + named = true + ), + @Param( name = DEFAULT_ARG, allowedTypes = { @ParamType(type = SkylarkDict.class), @@ -1585,24 +1592,17 @@ public final class SkylarkAttr implements SkylarkValue { named = true, positional = false, doc = NON_EMPTY_DOC - ), - @Param( - name = ALLOW_EMPTY_ARG, - type = Boolean.class, - defaultValue = "True", - doc = ALLOW_EMPTY_DOC, - named = true ) }, useAst = true, useEnvironment = true ) public Descriptor stringListDictAttribute( + Boolean allowEmpty, SkylarkDict<?, ?> defaultO, String doc, Boolean mandatory, Boolean nonEmpty, - Boolean allowEmpty, FuncallExpression ast, Environment env) throws EvalException { diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/Param.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/Param.java index 5765205310..8abb3448b9 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkinterface/Param.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/Param.java @@ -80,6 +80,7 @@ public @interface Param { /** * If true, this parameter can be passed the "None" value in addition to whatever types it allows. + * If false, this parameter cannot be passed "None", no matter the types it allows. */ boolean noneable() default false; diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java index 10a42e4379..8057b89f0d 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java @@ -52,9 +52,14 @@ import javax.tools.Diagnostic; * Each parameter, if explicitly typed, may only use either 'type' or 'allowedTypes', * not both. * </li> + * <li>Each parameter must be positional or named (or both).</li> + * <li>Positional-only parameters must be specified before any named parameters.</li> + * <li>Positional parameters must be specified before any non-positional parameters.</li> * <li> - * Either the doc string is non-empty, or documented is false. + * Positional parameters without default values must be specified before any + * positional parameters with default values. * </li> + * <li>Either the doc string is non-empty, or documented is false.</li> * </ul> * * <p>These properties can be relied upon at runtime without additional checks. @@ -131,7 +136,17 @@ public final class SkylarkCallableProcessor extends AbstractProcessor { private void verifyParamSemantics(ExecutableElement methodElement, SkylarkCallable annotation) throws SkylarkCallableProcessorException { + boolean allowPositionalNext = true; + boolean allowPositionalOnlyNext = true; + boolean allowNonDefaultPositionalNext = true; + for (Param parameter : annotation.parameters()) { + if ((!parameter.positional()) && (!parameter.named())) { + throw new SkylarkCallableProcessorException( + methodElement, + String.format("Parameter '%s' must be either positional or named", + parameter.name())); + } if ("None".equals(parameter.defaultValue()) && !parameter.noneable()) { throw new SkylarkCallableProcessorException( methodElement, @@ -148,6 +163,45 @@ public final class SkylarkCallableProcessor extends AbstractProcessor { + " one may be specified.", parameter.name())); } + + if (parameter.positional()) { + if (!allowPositionalNext) { + throw new SkylarkCallableProcessorException( + methodElement, + String.format( + "Positional parameter '%s' is specified after one or more " + + "non-positonal parameters", + parameter.name())); + } + if (!parameter.named() && !allowPositionalOnlyNext) { + throw new SkylarkCallableProcessorException( + methodElement, + String.format( + "Positional-only parameter '%s' is specified after one or more " + + "named parameters", + parameter.name())); + } + if (parameter.defaultValue().isEmpty()) { // There is no default value. + if (!allowNonDefaultPositionalNext) { + throw new SkylarkCallableProcessorException( + methodElement, + String.format( + "Positional parameter '%s' has no default value but is specified after one " + + "or more positional parameters with default values", + parameter.name())); + } + } else { // There is a default value. + // No positional parameters without a default value can come after this parameter. + allowNonDefaultPositionalNext = false; + } + } else { // Not positional. + // No positional parameters can come after this parameter. + allowPositionalNext = false; + } + if (parameter.named()) { + // No positional-only parameters can come after this parameter. + allowPositionalOnlyNext = false; + } } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java index 597508f04d..ef8db702ad 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java @@ -460,9 +460,9 @@ public final class FuncallExpression extends Expression { } private static SkylarkType getType(Param param) { + SkylarkType result = SkylarkType.BOTTOM; if (param.allowedTypes().length > 0) { Preconditions.checkState(Object.class.equals(param.type())); - SkylarkType result = SkylarkType.BOTTOM; for (ParamType paramType : param.allowedTypes()) { SkylarkType t = paramType.generic1() != Object.class @@ -470,14 +470,17 @@ public final class FuncallExpression extends Expression { : SkylarkType.of(paramType.type()); result = SkylarkType.Union.of(result, t); } - return result; } else { - SkylarkType type = + result = param.generic1() != Object.class ? SkylarkType.of(param.type(), param.generic1()) : SkylarkType.of(param.type()); - return type; } + + if (param.noneable()) { + result = SkylarkType.Union.of(result, SkylarkType.NONE); + } + return result; } /** @@ -509,74 +512,84 @@ public final class FuncallExpression extends Expression { if (mandatoryPositionals > args.size()) { return ArgumentListConversionResult.fromError("too few arguments"); } - if (args.size() > mandatoryPositionals + callable.parameters().length) { - return ArgumentListConversionResult.fromError("too many arguments"); - } + // First process the legacy positional parameters. - int i = 0; + int argIndex = 0; if (mandatoryPositionals > 0) { for (Class<?> param : javaMethodSignatureParams) { - Object value = args.get(i); + Object value = args.get(argIndex); if (!param.isAssignableFrom(value.getClass())) { return ArgumentListConversionResult.fromError( String.format( "Cannot convert parameter at position %d from type %s to type %s", - i, EvalUtils.getDataTypeName(value), param.toString())); + argIndex, EvalUtils.getDataTypeName(value), param.toString())); } builder.add(value); - i++; - if (i >= mandatoryPositionals) { + argIndex++; + if (argIndex >= mandatoryPositionals) { // Stops for specified parameters instead. break; } } } - // Then the parameters specified in callable.parameters() + // Then process parameters specified in callable.parameters() Set<String> keys = new LinkedHashSet<>(kwargs.keySet()); + int paramIndex = mandatoryPositionals; + // Positional parameters are always enumerated before non-positional parameters, + // And default-valued positional parameters are always enumerated after other positional + // parameters. These invariants are validated by the SkylarkCallable annotation processor. for (Param param : callable.parameters()) { SkylarkType type = getType(param); - if (param.noneable()) { - type = SkylarkType.Union.of(type, SkylarkType.NONE); - } Object value = null; - if (i < args.size()) { - value = args.get(i); + + if (argIndex < args.size()) { // Positional arguments remain. + value = args.get(argIndex); if (!param.positional()) { return ArgumentListConversionResult.fromError( - String.format("Parameter '%s' is not positional", param.name())); - } else if (!type.contains(value)) { - return ArgumentListConversionResult.fromError( - String.format( - "expected value of type '%s' for parameter '%s'", - type.toString(), param.name())); + String.format("expected no more than %s positional arguments, but got %s", + paramIndex, args.size())); } - i++; - } else if (param.named() && keys.remove(param.name())) { - // Named parameters - value = kwargs.get(param.name()); if (!type.contains(value)) { return ArgumentListConversionResult.fromError( String.format( "expected value of type '%s' for parameter '%s'", type.toString(), param.name())); } - } else { - // Use default value - if (param.defaultValue().isEmpty()) { + if (param.named() && keys.contains(param.name())) { return ArgumentListConversionResult.fromError( - String.format("parameter '%s' has no default value", param.name())); + String.format("got multiple values for keyword argument '%s'", param.name())); + } + argIndex++; + } else { // No more positional arguments. + if (param.named() && keys.remove(param.name())) { // Param specified by keyword argument. + value = kwargs.get(param.name()); + if (!type.contains(value)) { + return ArgumentListConversionResult.fromError( + String.format( + "expected value of type '%s' for parameter '%s'", + type.toString(), param.name())); + } + } else { // Param not specified by user. Use default value. + if (param.defaultValue().isEmpty()) { + return ArgumentListConversionResult.fromError( + String.format("parameter '%s' has no default value", param.name())); + } + value = SkylarkSignatureProcessor.getDefaultValue(param, null); } - value = SkylarkSignatureProcessor.getDefaultValue(param, null); } - builder.add(value); if (!param.noneable() && value instanceof NoneType) { return ArgumentListConversionResult.fromError( String.format("parameter '%s' cannot be None", param.name())); } + builder.add(value); + paramIndex++; } - if (i < args.size()) { - return ArgumentListConversionResult.fromError("too many arguments"); + + if (argIndex < args.size()) { + return ArgumentListConversionResult.fromError( + String.format("expected no more than %s positional arguments, but got %s", + paramIndex, args.size())); } if (!keys.isEmpty()) { return ArgumentListConversionResult.fromError( |