Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a shorthand for "TrueClass | FalseClass" #133

Closed
connorshea opened this issue Dec 23, 2019 · 12 comments
Closed

Add a shorthand for "TrueClass | FalseClass" #133

connorshea opened this issue Dec 23, 2019 · 12 comments
Assignees

Comments

@connorshea
Copy link
Contributor

I see in syntax.md there's a bool type, but it allows "nil or any other values", which seems to defeat the purpose a bit.

I'd expect passing anything other than true or false to warn, because otherwise the type signature doesn't seem to have a point?

# rbs file
def foo: (bool) -> String
# ruby file
baz = 'string'

foo(true) #=> good
foo(baz) #=> this should make the typechecker warn?

If changing bool isn't going to happen, I'd like a simple shorthand for TrueClass | FalseClass that would make passing nil or a string to a method expecting a boolean give a warning. Maybe bool!? I don't think ! has any meaning in the signature syntax right now, but it might not be a good idea to use ! if you plan on using it for something else at some point.

# rbs file
def foo: (bool) -> String
def bar: (bool!) -> String
# ruby file
baz = 'string'

foo(true) #=> good
foo(baz) #=> ok
bar(baz) # warning

Thanks :)

@soutaro
Copy link
Member

soutaro commented Dec 30, 2019

@connorshea

Hi, thank you very much for your proposal and sorry for late reply.

Your proposal makes sense. It keeps the syntax of RBS, and we know there is some cases we need the true | false types. But I'm not sure how many of Ruby developers want this feature or how many of Ruby developers can choose one from the two types correctly.

So, my intuition is that:

  • Some Ruby program need that, but the programmer can easily define the type like: type boolean = true | false.
  • Adding extra checks about implicit conversion to bool types can be done by type checker or other tools like RuboCop.
  • I want to see how many Ruby program wants the strict bool type.

Do these make sense?

@connorshea
Copy link
Contributor Author

That makes sense to me, though I still think a lot of users are going to want a strict bool type.

I'm fine with waiting and seeing if people actually need it before implementing it :)

@sandstrom
Copy link

sandstrom commented Sep 28, 2020

For this use-case, plus for regular in ruby, e.g. my_var.is_a?(Boolean) it would be great with a proper boolean super-class in Ruby.

This has been requested before:
https://bugs.ruby-lang.org/issues/12515

This is what Sorbet does:
https://sorbet.org/docs/class-types

Feels a bit like the Bignum/Fixnum thing that was improved in Ruby 2.4, where they both merged into Integer.

@marcandre
Copy link
Member

marcandre commented Oct 11, 2020

I'm 👍 for a strict bool, especially now that safe navigation is very different between nil and false.

I managed to convince rspec to be stricter and found bugs in their own source code doing so.

@marcandre
Copy link
Member

Like we have string and String, could we have bool (any type but intend to use as truthy/falsy) and Bool (true | false)?

I note that there are many instances in the current code base of bool that are incorrect (e.g. Array#all?):

  def all?: () -> bool
#                 ^^^^ no, should be `true | false`
          | (_Pattern[Elem] pattern) -> bool
#                                       ^^^^ no, should be `true | false`
          | () { (Elem obj) -> bool } -> bool
#                              ^^^^ correct, block can return anything, we use only truthiness
#                                        ^^^^ no, should be `true | false`

@soutaro could you confirm my understanding? I can provide a PR to fix these, but is ::Bool = true | false acceptable, or should the PR use true | false?

@soutaro
Copy link
Member

soutaro commented Oct 17, 2020

@marcandre I assume returning bool from the predicate methods including all? is good. They are predicate so I don't think it's safe to have more assumptions about the value than it's a truth value.

@soutaro
Copy link
Member

soutaro commented Oct 17, 2020

Opened a WIP pull request to add bool!. Any comments? @marcandre @connorshea @sandstrom

@marcandre
Copy link
Member

@marcandre I assume returning bool from the predicate methods including all? is good. They are predicate so I don't think it's safe to have more assumptions about the value than it's a truth value.

The fact that this method returns true or false (and never nil or 42 for example) is in the official documentation, in the tests and in the RubySpec specification that all implementations follow (MRI, JRuby, TruffleRuby, etc).

If Enumerable#any? returned anything else than true or false it would be a bug. Rubyists can and should rely on that method returning true or false, and can thus write code like: enum.tally { |series| series.any? { ... } }. Of course there is nothing special about any?, this is true of the vast majority of predicates (nonzero? and Module#< are some of the few exceptions).

Opened a WIP pull request to add bool!

Looks good to me 👍. Is bool!? supported? (e.g. Module#<)?

@marcandre
Copy link
Member

PS: In the block form, if there is a statement of break 42, then the "result" of any? will be 42. This is of course true most methods accepting a block. I imagine this is not part of the signature and dealt with separately by steep?

@soutaro
Copy link
Member

soutaro commented Oct 18, 2020

@marcandre Thanks! I got the point about #tally. It makes sense and I agree that it should be true | false, not bool.

So, the next problem would be if we should keep the bool semantics. bool! will be used much more than I expected, and then making bool == true | false would make more sense.

My concern is if using top makes sense for truthiness, for methods we write currently as def do_something: (if: bool) -> void. Writing def do_something: (if: top) -> void would be confusing and make it more difficult to reason what the method requires.

There would be three options:

  1. Keep bool == top and use bool! for many places.
  2. Change the semantics that bool == true | false and use top for conditions.
  3. Change the semantics that bool == true | false and introduce another type for conditions, like cond.

(2 seems reasonable to me for now because I haven't found a good name for 3...)

Typing break is in another layer. It's not about the type of methods and Steep will take care of them.

@marcandre
Copy link
Member

I like the idea of "intention", e.g. void vs top. I think it is worthwhile to have a distinction between (any object used for truthiness) and (true | false). So I would prefer 1 or 3.

I'd love to use Bool for strict true | false. I know there is no such class, but there could be / should be / would be if Ruby was not so much duck typed. It would reflect string vs String, etc. Do you feel we can't do this because ::Bool might be actually defined by users?

For other names for option 3, cond is not bad, or maybe test.

We could also have tf == true | false and bool == top.

My preferences (I think):

  1. bool = top, Bool = true | false, and even convince Matz to introduce common Bool subclass to TrueClass and FalseClass.
  2. goto 1 ;-)
  3. bool = top, bool! = true | false

@marcandre
Copy link
Member

I created https://bugs.ruby-lang.org/issues/17265 for addition of Bool ancestor to TrueClass/FalseClass.

mame added a commit to mame/ruby-signature that referenced this issue Oct 19, 2020
For example, the return value of the block of Enumerable#all? is handled
as bool. In this case, it will bring no difference, but I think it is a
good habit to use a more explicit name instead of "untyped".

Note, if the definition of `bool` is changed according to ruby#133, they
should be changed again.
@soutaro soutaro self-assigned this Nov 1, 2020
@soutaro soutaro added this to the Ruby 3.0 Preview 2 milestone Nov 1, 2020
@soutaro soutaro mentioned this issue Nov 1, 2020
@soutaro soutaro closed this as completed Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants