Fix for legacy named arguments

[vladmos]

Replaces legacy keyword arguments with positional arguments in some builtin functions, e.g. bool(x = foo) -> bool(foo).

Fixes bazelbuild/bazel#5010

[laurentlb]

cc @alandonovan, fyi

[alandonovan]

typo: Skylark.
but really: Starlark.

[vladmos]

Thanks, fixed.

[alandonovan]

The two commas in this sentence are ungrammatical.
Replace the first with ";" or "and", and the second with a period and capital.

[vladmos]

Done.

[alandonovan]

Most often I see string.split(maxsplit=-1) or dict.get(key, default=...). Be sure to handle methods too.

[vladmos]

I've collected the potential cases here: bazelbuild/bazel#5010 (comment)

I'll implement more fixes once we agree on which keyword parameters we want to deprecate.

[alandonovan]

To repeat what I said on that issue, the important distinction is between core Starlark (e.g. getattr, splitlines) and the build language (e.g. fail, aspect). In the core, following Python, very few functions accept named arguments; int(base=...) is one of the rare exceptions. In the build language, and for user-defined macros, it's much more common for functions to accept or even require named arguments. Let me know if you disagree.

[laurentlb]

This change looks limited to 1-arg functions?
What about these cases?

int(2, base = 3)
range(2, 5, step = 2)

[vladmos]

It's not limited to 1-arg functions, but for now only converts the first argument (see getattr and hasattr in the tests), it just only converts the first argument. As we haven't agreed yet which parameters to make positional only, I've only implemented the most obvious fixes (e.g. a parameter called x never improves readability of the code).

I'll update the PR (or send a separate if this one gets merged) once we finalize the list of parameters to fix.

[laurentlb]

What's the status of this?

[vladmos]

I think I deleted the branch by mistake and Github closed the pull request. But it shouldn't be hard to restore.

Is there a decision which arguments we're going to make positional-only?

[laurentlb]

Once we have a way to say: "This argument is always positional" or "This argument is always named", we can extend the list of functions to fix.

For the core Starlark builtins, I think the only one that can be named is base in int(x, base = 2). In Blaze functions, we can make the exclude more explicit:

- glob(["*.cc"], ["test*])
+ glob(["*.cc"], exclude = ["test*])

[vladmos]

Reopened as #692

[alandonovan]

For the core Starlark builtins, I think the only one that can be named is base in int(x, base = 2). In Blaze functions, we can make the exclude more explicit:

I think the following built-in functions allow some named arguments: print, sorted, fail, int, min, max.
We should make the docs more explicit.