Sorbet: When `compact.first` cannot be nil, incorrectly produces error 7005

Created on 24 Jun 2019  Â·  5Comments  Â·  Source: sorbet/sorbet

Generally, compact.first can return nil. However, consider this case where it is guaranteed not to.

In this example, there may or may not be an apple, but there is always a banana. Therefore, compact.first can never return nil.

Input

→ View on sorbet.run

# typed: true
# frozen_string_literal: true

class Chooser
  extend T::Sig

  sig { params(hates_apples: T::Boolean).void }
  def initialize(hates_apples)
    @hates_apples = hates_apples
  end

  sig { returns(String) }
  def best
    candidates.compact.first
  end

  private

  sig { returns(::T.nilable(::String)) }
  def apple
    @hates_apples ? nil : 'Malus domestica'
  end

  sig { returns(::String) }
  def banana
    'Musa acuminata'
  end

  sig { returns(T::Array[T.nilable(String)]) }
  def candidates
    [
      apple,
      banana
    ]
  end
end

::Chooser.new(true).best

Observed output

editor.rb:14: Returning value that does not conform to method result type https://srb.help/7005
    14 |    candidates.compact.first
            ^^^^^^^^^^^^^^^^^^^^^^^^
  Expected String
    editor.rb:13: Method best has return type String
    13 |  def best
          ^^^^^^^^
  Got T.nilable(String) originating from:
    editor.rb:14:
    14 |    candidates.compact.first
            ^^^^^^^^^^^^^^^^^^^^^^^^
Errors: 1

Expected behavior

Should pass tc.

Workaround

def best
  T.cast(candidates.compact.first, String)
end
bug

Most helpful comment

T.cast requires that the type be repeated, and could actually get out of sync if for some reason the array changed from being an array of strings to an array of integers. To just remove the T.nilable, you can use T.must(candidates.compact.first) which is both shorter and more type safe.

All 5 comments

Thanks for writing in! You are indeed correct that it could be possible to prove all sorts of things about the internal states of arrays, but Sorbet doesn't track things like emptyness as it processes. There is a future where it could, but these types of checks are traditionally very hard to get right and we haven't deemed the effort to be worth the value yet.

.. Sorbet doesn't track things like emptyness as it processes .. these types of checks are traditionally very hard to get right ..

Totally understandable. Some things that are easy for humans to see at a glance turn out to be really hard to handle with software. Maybe we won't all lose our jobs just yet. :grin:

One way to think about this issue is to say that the signature of candidates is incorrect.

sig { returns(T::Array[T.nilable(String)]) }
def candidates

We could instead give it the following hypothetical signature:

An array with exactly two elements, whose second element is a non-nilable string.

This is similar in my mind to the concept of "shapes". But, even with this improved signature, we still have the problem of, as you say, "tracking emptiness".

... Sorbet doesn't track things like emptyness as it processes. There is a future where it could, but these types of checks are traditionally very hard to get right and we haven't deemed the effort to be worth the value yet.

Closing, given that T.cast is a reasonable workaround for this.

T.cast requires that the type be repeated, and could actually get out of sync if for some reason the array changed from being an array of strings to an array of integers. To just remove the T.nilable, you can use T.must(candidates.compact.first) which is both shorter and more type safe.

Was this page helpful?
0 / 5 - 0 ratings