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.

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

Sorbet runtime does not support limitless ranges properly #3093

Closed
alexevanczuk opened this issue May 27, 2020 · 2 comments · Fixed by #3094
Closed

Sorbet runtime does not support limitless ranges properly #3093

alexevanczuk opened this issue May 27, 2020 · 2 comments · Fixed by #3094
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@alexevanczuk
Copy link
Contributor

Input

→ View on sorbet.run

class Test
  extend T::Sig
  sig { returns(T::Range[T.nilable(Date)]) }
  def range
    Range.new(nil, nil)
  end

  sig { returns(T::Range[T.nilable(Date)]) }
  def range_1
    Range.new(nil, Date.today)
  end

  sig { returns(T::Range[T.nilable(Date)]) }
  def range_1
    Range.new(nil, nil)
  end
end

Observed output

Static Output
No errors! Great job.
Runtime output
development 25> Test.new.range
RangeError: cannot get the last element of endless range
from /Users/alex.evanczuk/workspace/app/vendor/bundle/ruby/2.6.0/gems/sorbet-runtime-0.5.5585/lib/types/types/typed_enumerable.rb:127:in `last'

Expected behavior

No error when validating runtime signatures.

@alexevanczuk alexevanczuk added bug Something isn't working unconfirmed This issue has not yet been confirmed by the Sorbet team labels May 27, 2020
@jez jez added good first issue Good for newcomers and removed unconfirmed This issue has not yet been confirmed by the Sorbet team labels May 27, 2020
@jez
Copy link
Collaborator

jez commented May 27, 2020

Probably just a matter of editing this:

when Range
@type.valid?(obj.first) && @type.valid?(obj.last)

to check if obj.begin / obj.end is nil before checking first / last

@alexevanczuk
Copy link
Contributor Author

I created a PR for this: #3094

I'll wait until tests pass before assigning it out.

The main documentation doesn't say how to run tests -- how am I able to run these tests? I want to verify locally these tests fail with the old code and pass with the new.

@jez jez closed this as completed in #3094 Jun 12, 2020
jez added a commit that referenced this issue Jun 12, 2020
* [Issue #3093] Fix T::Range validation on limitless ranges

* add tests

* change valid? method too

* code review

* fix nil to nil test

* add tests for nil

* change type to non-nilable integer

* code review

* code review

Co-authored-by: Jake Zimmerman <zimmerman.jake@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants