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.

Sign up for GitHub

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

Jump to bottom

Steep doesn't handle Object#present? correctly #472

Open
mtsmfm opened this issue Dec 25, 2021 · 4 comments · May be fixed by #1130
Open

Steep doesn't handle Object#present? correctly #472

mtsmfm opened this issue Dec 25, 2021 · 4 comments · May be fixed by #1130

Comments

@mtsmfm
Copy link

mtsmfm commented Dec 25, 2021

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.
@ksss
Copy link
Contributor

ksss commented Dec 26, 2021

@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.

steep/lib/steep/type_inference/logic_type_interpreter.rb

Lines 16 to 30 in 89d42e0

def guess_type_from_method(node)
if node.type == :send
method = node.children[1]
case method
when :is_a?, :kind_of?, :instance_of?
AST::Types::Logic::ReceiverIsArg.new
when :nil?
AST::Types::Logic::ReceiverIsNil.new
when :!
AST::Types::Logic::Not.new
when :===
AST::Types::Logic::ArgIsReceiver.new
end
end
end

steep/lib/steep/ast/types/factory.rb

Lines 385 to 435 in 89d42e0

case method_name
when :is_a?, :kind_of?, :instance_of?
if defined_in == RBS::BuiltinNames::Object.name && member.instance?
return method_type.with(
type: method_type.type.with(
return_type: AST::Types::Logic::ReceiverIsArg.new(location: method_type.type.return_type.location)
)
)
end
when :nil?
case defined_in
when RBS::BuiltinNames::Object.name,
NilClassName
return method_type.with(
type: method_type.type.with(
return_type: AST::Types::Logic::ReceiverIsNil.new(location: method_type.type.return_type.location)
)
)
end
when :!
case defined_in
when RBS::BuiltinNames::BasicObject.name,
RBS::BuiltinNames::TrueClass.name,
RBS::BuiltinNames::FalseClass.name
return method_type.with(
type: method_type.type.with(
return_type: AST::Types::Logic::Not.new(location: method_type.type.return_type.location)
)
)
end
when :===
case defined_in
when RBS::BuiltinNames::Module.name
return method_type.with(
type: method_type.type.with(
return_type: AST::Types::Logic::ArgIsReceiver.new(location: method_type.type.return_type.location)
)
)
when RBS::BuiltinNames::Object.name, RBS::BuiltinNames::String.name, RBS::BuiltinNames::Integer.name, RBS::BuiltinNames::Symbol.name,
RBS::BuiltinNames::TrueClass.name, RBS::BuiltinNames::FalseClass.name, TypeName("::NilClass")
# Value based type-case works on literal types which is available for String, Integer, Symbol, TrueClass, FalseClass, and NilClass
return method_type.with(
type: method_type.type.with(
return_type: AST::Types::Logic::ArgEqualsReceiver.new(location: method_type.type.return_type.location)
)
)
end
end

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.

@mtsmfm
Copy link
Author

mtsmfm commented Jan 3, 2022 •
edited
Loading

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

@dorner
Copy link

dorner commented Sep 30, 2022 •
edited
Loading

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

@coorasse
Copy link

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

tk0miya added a commit to tk0miya/steep that referenced this issue Sep 1, 2023
@tk0miya
Add method annotations: primitive:nil? and primitive:not_nil?
c455966 
To represent methods like `Object#blank?` and `Object#present?` in
ActiveSupport, this adds method annotations `primitive:nil?` and
`primitive:not_nil?`.

They will be considered as same as `nil?` and not `nil?` internally.

refs: soutaro#472
@tk0miya tk0miya mentioned this issue Sep 1, 2023
Open
This was referenced May 20, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Treat present? as a special method that guarantees the receiver is not nil mtsmfm/steep
4 participants
@ksss @coorasse @mtsmfm @dorner