diff options
author | 2018-04-11 11:09:17 -0700 | |
---|---|---|
committer | 2018-04-11 11:11:22 -0700 | |
commit | 4baafaca73cee2d3237999048acfc0d5f5f59a5b (patch) | |
tree | f67e4fa76f74c9458c579f4bec0e09d892cbb7da /src/main/java/com | |
parent | 51ae67a626aae45163175a1eee518b9e96e33fbc (diff) |
Introduce Param.legacyNamed() to handle migration from @SkylarkSignature to @SkylarkCallable.
@SkylarkSignature.parameters() treat named()=true for parameters regardless of whether named() was actually set to true, and thus for ease of migration we mark migrated parameters as legacyNamed() = true, which is currently semantically equivalent to named() = true.
We make a distinction so that it's easier to bulk fix these later.
RELNOTES: None.
PiperOrigin-RevId: 192477405
Diffstat (limited to 'src/main/java/com')
4 files changed, 31 insertions, 6 deletions
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 8f10c72850..45ad6edc51 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 @@ -98,6 +98,18 @@ public @interface Param { boolean named() default false; /** + * If this true, {@link #named} should be treated as true. + * + * <p>This indicates this parameter is part of a {@link SkylarkCallable} method which + * was migrated from {@link SkylarkSignature}. Due to a pre-migration bug, all parameters were + * treated as if {@link #named} was true, even if it was false. To prevent breakages during + * migration, the interpreter can continue to treat these parameters as named. This is distinct + * from {@link #named}, however, so that a bulk fix/cleanup will be easier later. + */ + // TODO(b/77902276): Remove this after a bulk cleanup/fix. + boolean legacyNamed() default false; + + /** * If true, the parameter may be specified as a positional parameter. For example for an integer * positional parameter {@code foo} of a method {@code bar}, then the method call will look like * {@code bar(1)}. If {@link #named()} is {@code false}, then this will be the only way to call 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 a39acba89a..cfc8958762 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 @@ -141,6 +141,10 @@ public final class SkylarkCallableProcessor extends AbstractProcessor { } } + private static boolean isParamNamed(Param param) { + return param.named() || param.legacyNamed(); + } + private void verifyParamSemantics(ExecutableElement methodElement, SkylarkCallable annotation) throws SkylarkCallableProcessorException { boolean allowPositionalNext = true; @@ -148,7 +152,7 @@ public final class SkylarkCallableProcessor extends AbstractProcessor { boolean allowNonDefaultPositionalNext = true; for (Param parameter : annotation.parameters()) { - if ((!parameter.positional()) && (!parameter.named())) { + if ((!parameter.positional()) && (!isParamNamed(parameter))) { throw new SkylarkCallableProcessorException( methodElement, String.format("Parameter '%s' must be either positional or named", @@ -180,7 +184,7 @@ public final class SkylarkCallableProcessor extends AbstractProcessor { + "non-positonal parameters", parameter.name())); } - if (!parameter.named() && !allowPositionalOnlyNext) { + if (!isParamNamed(parameter) && !allowPositionalOnlyNext) { throw new SkylarkCallableProcessorException( methodElement, String.format( @@ -205,7 +209,7 @@ public final class SkylarkCallableProcessor extends AbstractProcessor { // No positional parameters can come after this parameter. allowPositionalNext = false; } - if (parameter.named()) { + if (isParamNamed(parameter)) { // 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 c9f3aee01d..e19113c93f 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 @@ -484,6 +484,10 @@ public final class FuncallExpression extends Expression { return result; } + private static boolean isParamNamed(Param param) { + return param.named() || param.legacyNamed(); + } + /** * Constructs the parameters list to actually pass to the method, filling with default values if * any. If there is a type or argument mismatch, returns a result containing an error message. @@ -557,13 +561,14 @@ public final class FuncallExpression extends Expression { "expected value of type '%s' for parameter '%s'", type.toString(), param.name())); } - if (param.named() && keys.contains(param.name())) { + if (isParamNamed(param) && keys.contains(param.name())) { return ArgumentListConversionResult.fromError( String.format("got multiple values for keyword argument '%s'", param.name())); } argIndex++; } else { // No more positional arguments, or no more positional parameters. - if (param.named() && keys.remove(param.name())) { // Param specified by keyword argument. + if (isParamNamed(param) && keys.remove(param.name())) { + // Param specified by keyword argument. value = kwargs.get(param.name()); if (!type.contains(value)) { return ArgumentListConversionResult.fromError( 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 cbc653a087..a4302b57ed 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 @@ -112,6 +112,10 @@ public class SkylarkSignatureProcessor { annotation.extraKeywords(), defaultValues, paramDoc, enforcedTypesList); } + private static boolean isParamNamed(Param param) { + return param.named() || param.legacyNamed(); + } + private static FunctionSignature.WithValues<Object, SkylarkType> getSignatureForCallable( String name, boolean documented, ImmutableList<Parameter<Object, SkylarkType>> mandatoryPositionals, @@ -133,7 +137,7 @@ public class SkylarkSignatureProcessor { for (Param param : parameters) { boolean mandatory = param.defaultValue() != null && param.defaultValue().isEmpty(); Object defaultValue = mandatory ? null : getDefaultValue(param, defaultValuesIterator); - if (param.named() && !param.positional() && !named) { + if (isParamNamed(param) && !param.positional() && !named) { named = true; @Nullable Param starParam = null; if (extraPositionals != null && !extraPositionals.name().isEmpty()) { |