sorbet / sorbet

A fast, powerful type checker designed for Ruby
https://sorbet.org
Apache License 2.0
3.62k stars 525 forks source link

`tap` in a nilable context leads to wrong typing #2877

Closed jusleg closed 1 year ago

jusleg commented 4 years ago

Input

→ View on sorbet.run%0AT.reveal_type(foo.tap%20%7B%7D))

foo = T.let(nil, T.nilable(Integer))
T.reveal_type(foo.tap {})

Observed output

editor.rb:3: Revealed type: Integer https://srb.help/7014
     3 |T.reveal_type(foo.tap {})
        ^^^^^^^^^^^^^^^^^^^^^^^^^
  From:
    editor.rb:3:
     3 |T.reveal_type(foo.tap {})
                      ^^^^^^^^^^

This can lead to runtime errors as srb tc would pass on the following code but the runtime checks would raise Return value: Expected type Integer, got type NilClass (RuntimeError)

→ View on sorbet.run.returns(Integer)%7D%0A%20%20def%20foo(x)%0A%20%20%20%20x.tap%20do%20%7Cstuff%7C%0A%20%20%20%20end%0A%20%20end%0Aend%0AA.new.foo(nil))

class A
  extend T::Sig
  sig {params(x: T.nilable(Integer)).returns(Integer)}
  def foo(x)
    x.tap do |stuff|
    end
  end
end
A.new.foo(nil)

Expected behavior

The revealed type should be T.nilable(Integer)

This behaviour seem to be present on all released versions of sorbet (0.4.4230 to 0.5.5510)

ric2b commented 4 years ago

Might this be a bug with T.self_type, since that's what kernel#tap returns?

https://github.com/sorbet/sorbet/blob/026a5c437d269267bf99608a4be61d1385631d11/rbi/core/kernel.rbi#L581-L587

elliottt commented 4 years ago

I think that you're right, and that something funny is happening with T.self_type in the presence of T.any. Here's another example:

# typed: true

x = T.let(1, T.any(NilClass, Integer))
T.reveal_type(x.tap{}) # NilClass

y = T.let(1, T.any(Integer, NilClass))
T.reveal_type(y.tap{}) # Integer

sorbet.run%0AT.reveal_type(x.tap%7B%7D)%0A%0Ay%20%3D%20T.let(1%2C%20T.any(Integer%2C%20NilClass))%0AT.reveal_type(y.tap%7B%7D))

jez commented 1 year ago

For what it's worth I don't think this is related to T.self_type. The same issue appears for other methods too:

→ View on sorbet.run

# typed: true

extend T::Sig

class Left
  extend T::Sig

  sig do
    returns(Integer)
  end
  def foo
    0
  end
end

class Right
  extend T::Sig

  sig do
    returns(NilClass)
  end
  def foo
    nil
  end
end

sig {params(x: T.any(Left, Right)).void}
def example1(x)
  # This shows the bug:
  result = x.foo {}
  T.reveal_type(result)

  # This doesn't:
  result = x.foo
  T.reveal_type(result)
end

Observed output

editor.rb:31: Revealed type: Integer https://srb.help/7014
    31 |  T.reveal_type(result)
          ^^^^^^^^^^^^^^^^^^^^^
  Got Integer originating from:
    editor.rb:30:
    30 |  result = x.foo {}
                   ^^^^^^^^

editor.rb:35: Revealed type: T.nilable(Integer) https://srb.help/7014
    35 |  T.reveal_type(result)
          ^^^^^^^^^^^^^^^^^^^^^
  Got T.nilable(Integer) originating from:
    editor.rb:34:
    34 |  result = x.foo
                   ^^^^^
Errors: 2

With that in mind, I'm going to close this in favor of #5409