From be61041a69a614fd009fd613a70ac1ac8af5d507 Mon Sep 17 00:00:00 2001 From: cparsons Date: Tue, 20 Mar 2018 11:35:33 -0700 Subject: Force @SkylarkCallable Params with defaultValue = "None" to be noneable. Previously, usage was fairly inconsistent. From now on, if the @Param is mandatory, use defaultValue = "" instead. RELNOTES: None. PiperOrigin-RevId: 189777905 --- .../lib/analysis/skylark/SkylarkActionFactory.java | 1 - .../devtools/build/lib/skylarkinterface/Param.java | 22 ++++++++++++++++------ .../processor/SkylarkCallableProcessor.java | 16 ++++++++++++++++ 3 files changed, 32 insertions(+), 7 deletions(-) (limited to 'src/main/java/com/google/devtools/build') diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java index b8f75f07dc..71a597f3cc 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java @@ -517,7 +517,6 @@ public class SkylarkActionFactory implements SkylarkValue { @ParamType(type = SkylarkList.class, generic1 = String.class), @ParamType(type = Runtime.NoneType.class), }, - defaultValue = "None", named = true, positional = false, doc = 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 4757e41fa2..5765205310 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 @@ -23,8 +23,8 @@ import java.lang.annotation.RetentionPolicy; public @interface Param { /** - * Name of the parameter, as viewed from Skylark. Used for named parameters and for generating - * documentation. + * Name of the parameter, as viewed from Skylark. Used for matching keyword arguments and for + * generating documentation. */ String name(); @@ -34,7 +34,16 @@ public @interface Param { String doc() default ""; /** - * Default value for the parameter, as a Skylark value (e.g. "False", "True", "[]", "None"). + * Default value for the parameter, written as a Skylark expression (e.g. "False", "True", "[]", + * "None"). + * + *

If this is empty (the default), the parameter is treated as mandatory. (Thus an exception + * will be thrown if left unspecified by the caller). + * + *

If the function implementation needs to distinguish the case where the caller does not + * supply a value for this parameter, you can set the default to the magic string "unbound", which + * maps to the sentinal object {@link com.google.devtools.build.lib.syntax.Runtime#UNBOUND} + * (which can't appear in normal Skylark code). */ String defaultValue() default ""; @@ -45,8 +54,9 @@ public @interface Param { Class type() default Object.class; /** - * List of allowed types for the parameter if multiple types are allowed, and {@link #type()} is - * set to Object.class. + * List of allowed types for the parameter if multiple types are allowed. + * + *

If using this, {@link #type()} should be set to {@code Object.class}. */ ParamType[] allowedTypes() default {}; @@ -69,7 +79,7 @@ public @interface Param { boolean callbackEnabled() default false; /** - * If true, this parameter can be passed the "None" value. + * If true, this parameter can be passed the "None" value in addition to whatever 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 cd4b288db1..841bdd1d5a 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 @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.skylarkinterface.processor; +import com.google.devtools.build.lib.skylarkinterface.Param; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import java.util.List; import java.util.Set; @@ -81,6 +82,7 @@ public final class SkylarkCallableProcessor extends AbstractProcessor { } try { + verifyParamSemantics(methodElement, annotation); verifyNumberOfParameters(methodElement, annotation); verifyExtraInterpreterParams(methodElement, annotation); } catch (SkylarkCallableProcessorException exception) { @@ -91,6 +93,20 @@ public final class SkylarkCallableProcessor extends AbstractProcessor { return true; } + private void verifyParamSemantics(ExecutableElement methodElement, SkylarkCallable annotation) + throws SkylarkCallableProcessorException { + for (Param parameter : annotation.parameters()) { + if ("None".equals(parameter.defaultValue()) && !parameter.noneable()) { + throw new SkylarkCallableProcessorException( + methodElement, + String.format("Parameter '%s' has 'None' default value but is not noneable. " + + "(If this is intended as a mandatory parameter, leave the defaultValue field " + + "empty)", + parameter.name())); + } + } + } + private void verifyNumberOfParameters(ExecutableElement methodElement, SkylarkCallable annotation) throws SkylarkCallableProcessorException { List methodSignatureParams = methodElement.getParameters(); -- cgit v1.2.3