steep icon indicating copy to clipboard operation
steep copied to clipboard

Steep doesn't handle Object#present? correctly

Open mtsmfm opened this issue 2 years ago • 4 comments

Full demo: https://github.com/mtsmfm/steep-with-present

require "bundler/setup"
require "active_support/all"

class Foo
  def foo
    a = ["a", nil][0]

    if a.present?
      p a * 2
    end
  end
end

Foo.new.foo

__END__
$ ruby lib/foo.rb
"aa"
$ bundle exec steep check
# Type checking files:

................................................................................F...

lib/foo.rb:9:10: [error] Type `(::String | nil)` does not have method `*`
│ Diagnostic ID: Ruby::NoMethod
│
└       p a * 2
            ~

Detected 1 problem from 1 file

Is it intentional?

I think it should behave like if a.

mtsmfm avatar Dec 25 '21 14:12 mtsmfm

@mtsmfm Hi.

Sorry for cutting in, but my understanding is that I don't think it is easy to solve this problem.

Steep has special support for some methods, a feature called logic. Logic feature now defined nil?, is_a?, kind_of?, instance_of?, ! and ===. This feature makes it possible to narrow down the type below the if statement.

https://github.com/soutaro/steep/blob/89d42e06f5a2c77c55756c97a40e1a6c4b8fb3a4/lib/steep/type_inference/logic_type_interpreter.rb#L16-L30

https://github.com/soutaro/steep/blob/89d42e06f5a2c77c55756c97a40e1a6c4b8fb3a4/lib/steep/ast/types/factory.rb#L385-L435

In order to solve this problem, Object#present? needs to support on logic, but I think Steep needs an extension that can add arbitrary methods to the logic.

ksss avatar Dec 26 '21 14:12 ksss

As far as I know rbs doesn't have any specification of narrowing on control flow.

I think what we need is user defined type guard on rbs.

https://www.typescriptlang.org/docs/handbook/2/narrowing.html#using-type-predicates

mtsmfm avatar Jan 03 '22 02:01 mtsmfm

Bumping this. The (rather confusing and minimal) readme seems to indicate that using @type can be used to coerce a variable, but this doesn't seem to work:

    current_assignment = Assignment.
      find_by(merchant_id: self.merchant_id) # this returns [ Assignment | nil ]

    return if current_assignment.nil? # steep doesn't seem to realize that this narrows down to Assignment
    # @type var current_assignment: Assignment

    errors.add(:base, "#{current_assignment.merchant.name} is already assigned!")

Leaving out the @type produces an error:

app/models/assignment.rb:47:37: [error] Type `(::Assignment | nil)` does not have method `merchant`
│ Diagnostic ID: Ruby::NoMethod
│
└    errors.add(:base, "#{current_assignment.merchant.name} is already assigned!")

Putting it in gets a different error:

app/models/assignment.rb:39:4: [error] Cannot assign a value of type `(::Assignment | nil)` to a variable of type `::Assignment`
│   (::Assignment | nil) <: ::Assignment
│     nil <: ::Assignment
│
│ Diagnostic ID: Ruby::IncompatibleAssignment

dorner avatar Sep 30 '22 17:09 dorner

This issue is a game-stopper for any code that uses ActiveSupport.

coorasse avatar Mar 30 '23 11:03 coorasse