should string·splitlines argument be a bool or be interpreted as a bool?

[josharian]

The spec for string·splitlines reads:

The optional argument, keepends, is interpreted as a Boolean.

Python and starlark-rust require that keepends be a boolean. starlark-go interprets keepends as a boolean:

$ python2 -c "''.splitlines('0')"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: an integer is required
(exit 1)

$ python3 -c "''.splitlines('0')"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: an integer is required (got type str)
(exit 1)

$ starlark-go -c "''.splitlines('0')"

(exit 0)

$ starlark-rust -c "''.splitlines('0')"
error[CV02]: string.splitlines() expect a bool as first parameter whilegot a value of type string.
 --> [command flag]:1:1
  |
1 | ''.splitlines('0')
  | ^^^^^^^^^^^^^^^^^^ type string while expected bool

(exit 2)

Should we switch the spec to match the most common implementation, and then fix the Go implementation?

[alandonovan]

The Java implementation wants exactly a bool. The Pythons actually want a bool or an int in the value range of a C long, not necessarily 0 or 1. Go uses the truth value. What a mess!

[laurentlb]

I don't think we should accept anything else than a boolean. (Python doesn't have a clean distinction between ints and booleans for historical reasons)

[alandonovan]

OK. This may be a breaking change, but the failure mode is clear and it's for the best.

[alandonovan]

Starlark-Java permits the int values 0 or 1, and this is widely used. It rejects other int values. So I think we should allow bool, int(0), or int(1) for this parameter and for all other built-ins that accept a logical boolean.

[laurentlb]

The Java implementation doesn't allow integers in place of booleans.

        File "/tmp/bazel/BUILD", line 2, in print
                "".splitlines(0)
expected value of type 'bool' for parameter 'keepends', in method call splitlines(int) of 'string'

[alandonovan]

Hmmm.

% cat test/BUILD package(default_testonly = 2) % blaze query test/BUILD ERROR: google3/test/BUILD:1:1: boolean is not one of [0, 1]

Perhaps Blaze has more than one way to convert a value to a boolean. We should be consistent.

[laurentlb]

Yes, Bazel native rules do that (with attr.bool), because it was created at a time when there was no booleans at all. I don't think it's useful to support that everywhere in Starlark. It can be more confusing and harder to read.

[alandonovan]

The example was of 'package', a built-in function, not a rule attribute. This suggests that the built-in functions of the build language don't use the same parameter conversion logic as functions in the Starlark standard library. They should, I think, but that would require either changing BUILD files not to pass 0 and 1 for booleans, or making the standard conversion accept these alternative values.

[laurentlb]

The fact that some native functions in Bazel don't behave well shouldn't impact everyone else.

Using 0 and 1 is discouraged: https://docs.bazel.build/versions/master/skylark/bzl-style.html#boolean-values

[alandonovan]

Ah, the build-language package function is not like other functions, and goes to some trouble to use attribute-style argument validation. (Fun fact: every parameter of that function has its own Java class!)

OK, I'm fine with making bool parameters accept only True or False.