crystal
crystal copied to clipboard
Fix `TypeNode#nilable?` for root types
This companion to #12353 makes TypeNode#nilable? return true for all supertypes of Nil:
{{ Value.nilable? }} # => true
{{ Object.nilable? }} # => true
module Foo; end
struct Nil; include Foo; end
{{ Foo.nilable? }} # => true
Value and Object are mainly for theoretical correctness. The module case has little influence because TypeNode#nilable? is often used in tandem with Object#nil?, and the latter doesn't perform type filtering on variables of module types:
module Foo; end
struct Int32; include Foo; end
struct Nil; include Foo; end
# before:
(1.as(Foo)..nil.as(Foo)).sample # Error: no overload matches 'Int32#<' with type Foo
(1.as(Foo)..100.as(Foo)).sample # Error: no overload matches 'Int32#<' with type Foo
# after:
(1.as(Foo)..nil.as(Foo)).sample # raises ArgumentError
(1.as(Foo)..100.as(Foo)).sample # Stack overflow (e.g., infinite or very deep recursion)
It happens because of the following implementation:
struct Range(B, E)
def sample(random = Random::DEFAULT)
# ...
{% elsif B.nilable? || E.nilable? %}
b = self.begin
e = self.end
if b.nil? || e.nil?
raise ArgumentError.new("Can't sample an open range")
end
# no type filtering occurs above; `b` still has type `Foo`
# therefore the `Range` below is inferred to be `Range(Foo, Foo)`
Range.new(b, e, @exclusive).sample(random)
{% else %}
super
{% end %}
end
end
Calling b.not_nil! works as the module is expanded into its including types during method lookup, but it looks awkward when a nil check is already present. (That said, this issue also happens without this PR, as Range#each(&) would show.)
The other use of TypeNode#nilable? is in JSON / YAML deserialization. This PR is inadequate because instance variables of module types will still complain about a missing or disallowed .new on module types.
Thanks! But I don't understand... Value.nilable? should be false. Object.nilable? should be false. As I explained in the other PR, nilable? means it's either Nil, or it's a Union containing the type Nil. For Value and Object that doesn't hold.
Oh, I guess Value can be nil as one of its sublcasses is Nil... Hmm...
I guess I never thought about including a module in Nil, then using that module in a range. But maybe there's a valid use case for that... 🤔
For illustration, this is what we're talking about:
def foo(v : Value)
v.nil? # => true
end
def bar(o : Object)
o.nil? # => true
end
foo nil
bar nil