Skip to content

Commit

Permalink
Avoid wrong infinite loop
Browse files Browse the repository at this point in the history
TypeProf caused an infinite loop under the following situation:

```
@A = @b[0]
@b = '/' + @A
```

1. both @A and @b is untyped
2. @b is changed to a String because of the second line (String#+)
3. @A is changed to a String? because of the first line (String#[])
4. @b is changed to nil because String#+ does not accept String?
5. go to 2

This change fixes the loop by making `String#+` to accept `String?`.

TODO: it should emit a warning, but it is a bot complex considering the
situation where passing `Integer | String | nil` to the following foo:

```
def foo: (Integer) -> Integer
       | (String) -> String
``
  • Loading branch information
mame committed Aug 27, 2024
1 parent aab5fac commit 86ca835
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 21 deletions.
9 changes: 6 additions & 3 deletions lib/typeprof/core/graph/vertex.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,17 @@ def check_match(genv, changes, vtx)
end
end

return true if @types.empty?
return true if vtx.types.empty?

each_type do |ty|
next if vtx.types.include?(ty) # fast path
return false unless ty.check_match(genv, changes, vtx)
return true if vtx.types.include?(ty) # fast path
if ty.check_match(genv, changes, vtx)
return true
end
end

return true
return false
end

def show
Expand Down
17 changes: 17 additions & 0 deletions scenario/known-issues/check-return-type.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
## update: test.rbs
class Foo
def foo: -> Foo
def foo=: (Foo) -> Foo
end

## update: test.rb
class Foo
def initialize
@foo = 1
end

attr_accessor :foo
end

## diagnostics: test.rb
(6,2)-(6,20): expected: Foo; actual: (Foo | Integer)
14 changes: 14 additions & 0 deletions scenario/known-issues/unsupported-arg.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
## update: test.rbs
class Object
def foo: (String) -> String
| (Integer) -> Integer
def get1: -> (String | Integer)
def get2: -> (String | Integer | nil)
end

## update: test.rb
def check1 = foo(get1)
def check2 = foo(get2)

## diagnostics
(2,13)-(2,16): expected: (String | Integer); actual: (String | Integer)?
18 changes: 0 additions & 18 deletions scenario/rbs/check-return-type5.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,3 @@ def initialize

## diagnostics: test.rb
(6,2)-(6,18): expected: Foo; actual: Integer

## update: test.rbs
class Foo
def foo: -> Foo
def foo=: (Foo) -> Foo
end

## update: test.rb
class Foo
def initialize
@foo = 1
end

attr_accessor :foo
end

## diagnostics: test.rb
(6,2)-(6,20): expected: Foo; actual: (Foo | Integer)
26 changes: 26 additions & 0 deletions scenario/regressions/avoid-infinte-loop-2.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
## update: test.rbs
class Foo
def foo: -> Foo?
def self.bar: (Foo) -> Foo
end

## update: test.rb
def check
# The old infinite-loop scenario
# 1. both @a and @b are untyped
# 2. @b is now Foo because `@b = Foo.bar(@a)` and `Foo.bar: (Foo) -> Foo` and @a is untyped
# 3. @a is now Foo? because of `@a = @b.foo` and @b is Foo
# 4. @b is now untyped because `@b = Foo.bar(@a)` and `Foo.bar: (Foo) -> Foo` and @a is Foo | nil
# 5. go to 2
#
# How did I fixed:
# `Foo.bar: (Foo) -> Foo` should match against `Foo | nil`
# (TODO: add a diagnostics for this)
@a = @b.foo
@b = Foo.bar(@a)
end

def check2
@a = @b[0]
@b = '/' + @a
end

0 comments on commit 86ca835

Please sign in to comment.