From: merch-redmine@... Date: 2020-11-17T21:46:08+00:00 Subject: [ruby-core:100920] [Ruby master Feature#17326] Add Kernel#must! to the standard library Issue #17326 has been updated by jeremyevans0 (Jeremy Evans). jez (Jake Zimmerman) wrote in #note-13: > jeremyevans0 (Jeremy Evans) wrote in #note-12: > > > the benefit of adding it is smaller than the cost of adding another method to Kernel > > Can you speak more on the const of adding a method to Kernel? While I understand the costs of something new syntax would bring, I am less familiar with the cost to adding something to the standard library. The cost to adding a method to a core class is in the conceptual overhead, backwards compatibility, and need to support in perpetuity, since core class methods are almost never removed. This is especially true of Kernel methods, since they are available on all objects other than `BasicObject` instances. I hope we should all agree that there should be a high bar before adding a method to Kernel. Otherwise Kernel would be flooded with many methods of limited utility. We may disagree about how high the bar should be, and whether this method is over or under the bar. But the fact that there should be a high bar is something I would hope we all understand. > The benefit to having something in the standard library instead of a gem is specifically to make it a common idiom shared across typed and non-typed Ruby codebases. `T.must` already exists in `sorbet-runtime` for the people who want to only use Sorbet. I am of the belief that third party gems should refrain from monkey patching new methods into standard library classes, which is why I proposed a change to the standard library directly. I understand that advantage. However, for non-typed Ruby, the advantage is minimal in my opinion, and not enough to clear the high bar. I understand for Sorbet users, the advantage is quite large, but I don't think we should add a method to Kernel unless there is a significant advantage even for non-Sorbet users, considering that Sorbet users can easily add such a method themselves. I agree with you that third party gems should refrain from monkey patching by default, unless that is the sole purpose of the gem. Which is why I said that making this an optional part of Sorbet (e.g. `require 'sorbet/core_ext/must'`) or an external gem may be best. Looks like the gem name `must` is already taken. The `must` gem adds `Object#must`, but not `Object#must!`. ---------------------------------------- Feature #17326: Add Kernel#must! to the standard library https://bugs.ruby-lang.org/issues/17326#change-88572 * Author: jez (Jake Zimmerman) * Status: Open * Priority: Normal ---------------------------------------- # Abstract We should add a method `Kernel#must!` (name TBD) which raises if `self` is `nil` and returns `self` otherwise. # Background Ruby 3 introduces type annotations for the standard library. Type checkers consume these annotations, and report errors for type mismatches. One of the most common and most valuable type errors is whether `nil` is allowed as an argument or return value. Sorbet's type system tracks this, and RBS files have syntax for annotating whether `nil` is allowed or not. Since Sorbet checks proper usage of `nil`, it requires code that looks like this: ```ruby if thing.nil? raise "The thing was nil" end thing.do_something ``` This is good because it forces the programmer to acknowledge that the thing might be `nil`, and declare that they'd rather raise an exception in that case than handle the `nil` (of course, there are many other times where `nil` is both possible and valid, which is why Sorbet forces at least considering in all cases). It is annoying and repetitive to have to write these `if .nil?` checks everywhere to ignore the type error, so Sorbet provides it as a library function, called `T.must`: ```ruby T.must(thing).do_something ``` Sorbet knows that the call to `T.must` raises if `thing` is `nil`. To make this very concrete, here's a Sorbet playground where you can see this in action: [��� View on sorbet.run](https://sorbet.run/#%23%20typed%3A%20true%0Aextend%20T%3A%3ASig%0A%0Aclass%20Thing%0A%20%20def%20do_something%3B%20end%0Aend%0A%0Asig%20%7Bparams(thing%3A%20T.nilable(Thing)).void%7D%0Adef%20example1(thing)%0A%20%20%23%20error%2C%20might%20be%20nil%3A%0A%20%20thing.do_something%0Aend%0A%0Asig%20%7Bparams(thing%3A%20T.nilable(Thing)).void%7D%0Adef%20example2(thing)%0A%20%20if%20thing.nil%3F%0A%20%20%20%20raise%20%22The%20thing%20was%20nil%22%0A%20%20end%0A%0A%20%20%23%20no%20error%2C%20because%20it's%20after%20the%20%60if%20.nil%3F%60%20check%3A%0A%20%20thing.do_something%0Aend%0A%0Asig%20%7Bparams(thing%3A%20T.nilable(Thing)).void%7D%0Adef%20example3(thing)%0A%20%20%23%20no%20error%2C%20because%20it's%20after%20the%20%60if%20.nil%3F%60%20check%3A%0A%20%20T.must(thing).do_something%0Aend) You can read more about `T.must` in the [Sorbet documentation](https://sorbet.org/docs/type-assertions#tmust). # Problem While `T.must` works, it is not ideal for a couple reasons: 1. It leads to a weird outward spiral of flow control, which disrupts method chains: ```ruby # ��������������������������������������������������������� # ��� ������������������ ��� # ��� ��� ��� ��� T.must(T.must(task).mailing_params).fetch('template_context') # ��� ��� ��� ��� # ��� ������������������������������������ ��� # ��������������������������������������������������������������������������������������������������������� ``` compare that control flow with this: ```ruby # ��������������������������������������������������������������������������������������������������� # ��� ������ ������ ������ ��� task.must!.mailing_params.must!.fetch('template_context') ``` 2. It is not a method, so you can't `map` it over a list using `Symbol#to_proc`. Instead, you have to expand the block: ```ruby array_of_integers = array_of_nilable_integers.map {|x| T.must(x) } ``` Compare that with this: ```ruby array_of_integers = array_of_nilable_integers.map(&:must!) ``` 3. It is in a Sorbet-specific gem. We do not intend for Sorbet to be the only type checker. It would be nice to have such a method in the Ruby standard library so that it can be shared by all type checkers. 4. This method can make Ruby codebases that **don't** use type checkers more robust! `Kernel#must!` could be an easy way to assert invariants early. Failing early makes it more likely that a test will fail, rather than getting `TypeError`'s and `NoMethodError`'s in production. This makes all Ruby code better, not just the Ruby code using types. # Proposal We should extend the Ruby standard library with something like this:: ```ruby module Kernel def must!; self; end end class NilClass def must! raise TypeError.new("nil.must!") end end ``` These methods would get type annotations that look like this: (using Sorbet's RBI syntax, because I don't know RBS well yet) ```ruby module Kernel sig {returns(T.self_type)} def must!; end end class NilClass sig {returns(T.noreturn)} def must!; end end ``` What these annotations say: - In `Kernel#must!`, the return value is `T.self_type`, or "whatever the type of the receiver was." That means that `0.must!` will have type `Integer`, `"".must!` will have type `String`, etc. - In `NilClass#must!`, there is an override of `Kernel#must!` with return type `T.noreturn`. This is a fancy type that says "this code either infinitely loops or raises an exception." This is the name for Sorbet's [bottom type](https://en.wikipedia.org/wiki/Bottom_type), if you are familiar with that terminology. Here is a Sorbet example where you can see how these annotations behave: [��� View on sorbet.run](https://sorbet.run/#%23%20typed%3A%20true%0A%0Amodule%20Kernel%0A%20%20T%3A%3ASig%3A%3AWithoutRuntime.sig%20%7Breturns(T.self_type)%7D%0A%20%20def%20must!%3B%20self%3B%20end%0Aend%0A%0Aclass%20NilClass%0A%20%20T%3A%3ASig%3A%3AWithoutRuntime.sig%20%7Breturns(T.noreturn)%7D%0A%20%20def%20must!%0A%20%20%20%20raise%20TypeError.new(%22nil.must!%22)%0A%20%20end%0Aend%0A%0Axs%20%3D%20T%3A%3AArray%5BInteger%5D.new(%5B0%5D)%0AT.reveal_type(xs.first)%20%20%20%20%20%20%20%23%20T.nilable(Integer)%0AT.reveal_type(xs.first.must!)%20%23%20Integer%0A%0Ays%20%3D%20T%3A%3AArray%5BT.nilable(Integer)%5D.new(%5B0%2C%20nil%2C%201%2C%20nil%2C%202%5D)%0AT.reveal_type(ys)%20%20%20%20%20%20%20%20%20%20%20%20%20%20%23%20T%3A%3AArray%5BT.nilable(Integer)%5D%0AT.reveal_type(ys.map(%26%3Amust!))%20%23%20T%3A%3AArray%5BInteger%5D) # Alternatives considered There was some discussion of this feature at the Feb 2020 Ruby Types discussion: Summarizing: - Sorbet team frequently recommends people to use `xs.fetch(0)` instead of `T.must(xs[0])` on `Array`'s and `Hash`'s because it chains and reads better. `.fetch` not available on other classes. - It's intentional that `T.must` requires as many characters as it does. Making it slightly annoying to type encourages developers to refactor their code so that `nil` never occurs. - There was a proposal to introduce new syntax like `thing.!!`. This is currently a syntax error. **Rebuttal**: There is burden to introducing new syntax. Tools like Rubocop, Sorbet, and syntax highlighting plugins have to be updated. Also: it is hard to search for on Google (as a new Ruby developer). Also: it is very short���having something slightly shorter makes people think about whether they want to type it out instead of changing the code so that `nil` can't occur. Another alternative would be to dismiss this as "not useful / common enough". I don't think that's true. Here are some statistics from Stripe's Ruby monolith (~10 million lines of code): | methood | percentage of files mentioning method | number of occurrences of method | | --- | --- | --- | | `.nil?` | 16.69% | 31340 | | `T.must` | 23.89% | 74742 | From this, we see that - `T.must` is in 1.43x more files than `.nil?` - `T.must` occurs 2.38x more often than `.nil?` # Naming I prefer `must!` because it is what the method in Sorbet is already called. I am open to naming suggestions. Please provide reasoning. # Discussion In the above example, I used `T.must` twice. An alternative way to have written that would have been using save navigation: ```ruby T.must(task&.mailing_params).fetch('template_context') ``` This works as well. The proposed `.must!` method works just as well when chaining methods with safe navigation: ```ruby task&.mailing_params.must!.fetch('template_context') ``` However, there is still merit in using `T.must` (or `.must!`) twice���it calls out that the programmer intended neither location to be `nil`. In fact, if this method had been chained across multiple lines, the backtrace would include line numbers saying specifically **which** `.must!` failed: ```ruby task.must! .mailing_params.must! .fetch('template_context') ``` -- https://bugs.ruby-lang.org/ Unsubscribe: