Skip to content

Commit

Permalink
Remove Param.legacyName, it's no longer used after the deletion of --…
Browse files Browse the repository at this point in the history
…incompatible_restrict_named_params in c0a11dc.

#8147

RELNOTES: None
PiperOrigin-RevId: 303328602
  • Loading branch information
Googler authored and copybara-github committed Mar 27, 2020
1 parent 8073508 commit 810cff4
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 216 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ private static Pair<FunctionSignature, List<String>> getSignature(SkylarkCallabl
for (com.google.devtools.build.lib.skylarkinterface.Param param : annot.parameters()) {
// Implicit * or *args parameter separates transition from positional to named.
// f (..., *, ... ) or f(..., *args, ...)
if ((param.named() || param.legacyNamed()) && !param.positional() && !hasStar) {
if (param.named() && !param.positional() && !hasStar) {
hasStar = true;
if (!annot.extraPositionals().name().isEmpty()) {
star = annot.extraPositionals().name();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ Sequence<?> glob(
@Param(
name = "name",
type = String.class,
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true,
doc = "The name of the target.")
},
useStarlarkThread = true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,6 @@ SkylarkAspectApi aspect(
@Param(
name = "label_string",
type = String.class,
legacyNamed = true,
doc = "the label string."),
@Param(
name = "relative_to_caller_repository",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,18 +101,6 @@
*/
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 {@code 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ private void checkParameters(ExecutableElement method, SkylarkCallable annot) {
"Positional parameter '%s' is specified after one or more non-positional parameters",
paramAnnot.name());
}
if (!isParamNamed(paramAnnot) && !allowPositionalOnlyNext) {
if (!paramAnnot.named() && !allowPositionalOnlyNext) {
errorf(
param,
"Positional-only parameter '%s' is specified after one or more named parameters",
Expand All @@ -297,11 +297,11 @@ private void checkParameters(ExecutableElement method, SkylarkCallable annot) {
// No positional parameters can come after this parameter.
allowPositionalNext = false;

if (!isParamNamed(paramAnnot)) {
if (!paramAnnot.named()) {
errorf(param, "Parameter '%s' must be either positional or named", paramAnnot.name());
}
}
if (isParamNamed(paramAnnot)) {
if (paramAnnot.named()) {
// No positional-only parameters can come after this parameter.
allowPositionalOnlyNext = false;
}
Expand All @@ -310,10 +310,6 @@ private void checkParameters(ExecutableElement method, SkylarkCallable annot) {
checkSpecialParams(method, annot);
}

private static boolean isParamNamed(Param param) {
return param.named() || param.legacyNamed();
}

// Checks consistency of a single parameter with its Param annotation.
private void checkParameter(Element param, Param paramAnnot, TypeMirror objectType) {
TypeMirror paramType = param.asType(); // type of the Java method parameter
Expand Down
121 changes: 18 additions & 103 deletions src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,7 @@ private static Object findExtreme(Sequence<?> args, Ordering<Object> maxOrdering
name = "elements",
type = Object.class,
noneable = true,
doc = "A string or a collection of elements.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true)
doc = "A string or a collection of elements.")
})
public Boolean all(Object collection) throws EvalException {
return !hasElementWithBooleanValue(collection, false);
Expand All @@ -122,9 +120,7 @@ public Boolean all(Object collection) throws EvalException {
name = "elements",
type = Object.class,
noneable = true,
doc = "A string or a collection of elements.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true)
doc = "A string or a collection of elements.")
})
public Boolean any(Object collection) throws EvalException {
return hasElementWithBooleanValue(collection, true);
Expand All @@ -148,12 +144,7 @@ private static boolean hasElementWithBooleanValue(Object seq, boolean value)
+ "It is an error if elements are not comparable (for example int with string)."
+ "<pre class=\"language-python\">sorted([3, 5, 4]) == [3, 4, 5]</pre>",
parameters = {
@Param(
name = "iterable",
type = Object.class,
doc = "The iterable sequence to sort.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true),
@Param(name = "iterable", type = Object.class, doc = "The iterable sequence to sort."),
@Param(
name = "key",
doc = "An optional function applied to each element before comparison.",
Expand All @@ -171,10 +162,7 @@ private static boolean hasElementWithBooleanValue(Object seq, boolean value)
},
useStarlarkThread = true)
public StarlarkList<?> sorted(
Object iterable,
final Object key,
Boolean reverse,
final StarlarkThread thread)
Object iterable, final Object key, Boolean reverse, final StarlarkThread thread)
throws EvalException, InterruptedException {
Object[] array = Starlark.toArray(iterable);
if (key == Starlark.NONE) {
Expand Down Expand Up @@ -247,9 +235,7 @@ private static void reverse(Object[] array) {
@Param(
name = "sequence",
type = Sequence.class,
doc = "The sequence (list or tuple) to be reversed.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true),
doc = "The sequence (list or tuple) to be reversed."),
},
useStarlarkThread = true)
public StarlarkList<?> reversed(Sequence<?> sequence, StarlarkThread thread)
Expand All @@ -266,14 +252,7 @@ public StarlarkList<?> reversed(Sequence<?> sequence, StarlarkThread thread)
+ "<pre class=\"language-python\">tuple([1, 2]) == (1, 2)\n"
+ "tuple((2, 3, 2)) == (2, 3, 2)\n"
+ "tuple({5: \"a\", 2: \"b\", 4: \"c\"}) == (5, 2, 4)</pre>",
parameters = {
@Param(
name = "x",
defaultValue = "()",
doc = "The object to convert.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true)
})
parameters = {@Param(name = "x", defaultValue = "()", doc = "The object to convert.")})
public Tuple<?> tuple(Object x) throws EvalException {
if (x instanceof Tuple) {
return (Tuple<?>) x;
Expand All @@ -288,14 +267,7 @@ public Tuple<?> tuple(Object x) throws EvalException {
+ "<pre class=\"language-python\">list([1, 2]) == [1, 2]\n"
+ "list((2, 3, 2)) == [2, 3, 2]\n"
+ "list({5: \"a\", 2: \"b\", 4: \"c\"}) == [5, 2, 4]</pre>",
parameters = {
@Param(
name = "x",
defaultValue = "[]",
doc = "The object to convert.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true)
},
parameters = {@Param(name = "x", defaultValue = "[]", doc = "The object to convert.")},
useStarlarkThread = true)
public StarlarkList<?> list(Object x, StarlarkThread thread) throws EvalException {
return StarlarkList.wrap(thread.mutability(), Starlark.toArray(x));
Expand All @@ -306,13 +278,7 @@ public StarlarkList<?> list(Object x, StarlarkThread thread) throws EvalExceptio
doc =
"Returns the length of a string, sequence (such as a list or tuple), dict, or other"
+ " iterable.",
parameters = {
@Param(
name = "x",
doc = "The value whose length to report.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true)
},
parameters = {@Param(name = "x", doc = "The value whose length to report.")},
useStarlarkThread = true)
public Integer len(Object x, StarlarkThread thread) throws EvalException {
int len = Starlark.len(x);
Expand All @@ -332,8 +298,6 @@ public Integer len(Object x, StarlarkThread thread) throws EvalException {
@Param(
name = "x",
doc = "The object to convert.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true,
noneable = true)
})
public String str(Object x) throws EvalException {
Expand All @@ -360,8 +324,6 @@ public String str(Object x) throws EvalException {
@Param(
name = "x",
doc = "The object to convert.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true,
noneable = true)
})
public String repr(Object x) {
Expand All @@ -381,8 +343,6 @@ public String repr(Object x) {
name = "x",
defaultValue = "False",
doc = "The variable to convert.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true,
noneable = true)
})
public Boolean bool(Object x) throws EvalException {
Expand Down Expand Up @@ -432,12 +392,7 @@ public Boolean bool(Object x) throws EvalException {
+ "int(\"-0x10\", 0) == -16"
+ "</pre>",
parameters = {
@Param(
name = "x",
type = Object.class,
doc = "The string to convert.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true),
@Param(name = "x", type = Object.class, doc = "The string to convert."),
@Param(
name = "base",
type = Object.class,
Expand Down Expand Up @@ -556,9 +511,7 @@ private String getIntegerPrefix(String value) {
defaultValue = "[]",
doc =
"Either a dictionary or a list of entries. Entries must be tuples or lists with "
+ "exactly two elements: key, value.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true),
+ "exactly two elements: key, value."),
},
extraKeywords = @Param(name = "kwargs", doc = "Dictionary of additional entries."),
useStarlarkThread = true)
Expand Down Expand Up @@ -608,14 +561,7 @@ public StarlarkList<?> enumerate(Object input, Integer start, StarlarkThread thr
// Deterministic hashing is important for the consistency of builds, hence why we
// promise a specific algorithm. This is in contrast to Java (Object.hashCode()) and
// Python, which promise stable hashing only within a given execution of the program.
parameters = {
@Param(
name = "value",
type = String.class,
doc = "String value to hash.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true)
})
parameters = {@Param(name = "value", type = String.class, doc = "String value to hash.")})
public Integer hash(String value) throws EvalException {
return value.hashCode();
}
Expand All @@ -635,26 +581,20 @@ public Integer hash(String value) throws EvalException {
type = Integer.class,
doc =
"Value of the start element if stop is provided, "
+ "otherwise value of stop and the actual start is 0",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true),
+ "otherwise value of stop and the actual start is 0"),
@Param(
name = "stop_or_none",
type = Integer.class,
noneable = true,
defaultValue = "None",
doc =
"optional index of the first item <i>not</i> to be included in the resulting "
+ "list; generation of the list stops before <code>stop</code> is reached.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true),
+ "list; generation of the list stops before <code>stop</code> is reached."),
@Param(
name = "step",
type = Integer.class,
defaultValue = "1",
doc = "The increment (default is 1). It may be negative.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true)
doc = "The increment (default is 1). It may be negative.")
},
useStarlarkThread = true)
public Sequence<Integer> range(
Expand Down Expand Up @@ -685,18 +625,8 @@ public Sequence<Integer> range(
+ "<code>name</code>, otherwise False. Example:<br>"
+ "<pre class=\"language-python\">hasattr(ctx.attr, \"myattr\")</pre>",
parameters = {
@Param(
name = "x",
doc = "The object to check.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true,
noneable = true),
@Param(
name = "name",
type = String.class,
doc = "The name of the attribute.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true)
@Param(name = "x", doc = "The object to check.", noneable = true),
@Param(name = "name", type = String.class, doc = "The name of the attribute.")
},
useStarlarkThread = true)
public Boolean hasattr(Object obj, String name, StarlarkThread thread) throws EvalException {
Expand All @@ -718,25 +648,14 @@ public Boolean hasattr(Object obj, String name, StarlarkThread thread) throws Ev
+ "<pre class=\"language-python\">getattr(ctx.attr, \"myattr\")\n"
+ "getattr(ctx.attr, \"myattr\", \"mydefault\")</pre>",
parameters = {
@Param(
name = "x",
doc = "The struct whose attribute is accessed.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true,
noneable = true),
@Param(
name = "name",
doc = "The name of the struct attribute.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true),
@Param(name = "x", doc = "The struct whose attribute is accessed.", noneable = true),
@Param(name = "name", doc = "The name of the struct attribute."),
@Param(
name = "default",
defaultValue = "unbound",
doc =
"The default value to return in case the struct "
+ "doesn't have an attribute of the given name.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true,
noneable = true)
},
useStarlarkThread = true)
Expand All @@ -761,8 +680,6 @@ public Object getattr(Object obj, String name, Object defaultValue, StarlarkThre
@Param(
name = "x",
doc = "The object to check.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true,
noneable = true)
},
useStarlarkThread = true)
Expand Down Expand Up @@ -878,8 +795,6 @@ public NoneType print(String sep, Sequence<?> args, StarlarkThread thread) throw
@Param(
name = "x",
doc = "The object to check type of.",
// TODO(cparsons): This parameter should be positional-only.
legacyNamed = true,
noneable = true)
})
public String type(Object object) {
Expand Down
Loading

0 comments on commit 810cff4

Please sign in to comment.