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

Improve signature for Gem::Version.create #625

Merged
merged 1 commit into from
Mar 14, 2021
Merged

Improve signature for Gem::Version.create #625

merged 1 commit into from
Mar 14, 2021

Conversation

ybiquitous
Copy link
Contributor

This change aims to improve the signature for the Gem::Version.create method.

The current signature always returns a nil-able value, but this is not useful because nil-check is necessary.
I think we can improve the method signature via overloading because this method returns nil only when nil is passed.

See below:

# a.rb
p(Gem::Version.create("2.3.4") >= Gem::Version.create("2.3.0"))
$ bundle exec steep check a.rb
...
a.rb:1:31: [error] Type `(::Gem::Version | nil)` does not have method `>=`
│ Diagnostic ID: Ruby::NoMethod

└ p(Gem::Version.create("2.3.4") >= Gem::Version.create("2.3.0"))
                                 ~~

Environments:

  • RBS 1.1.0
  • Steep 0.42.0

Source code of the create method:
https://github.com/rubygems/rubygems/blob/v3.2.13/lib/rubygems/version.rb#L188-L196

This change aims to improve the signature for the `Gem::Version.create` method.

The current signature always returns a nil-able value, but this is not useful because nil-check is necessary.
I think we can improve the method signature via overloading because this method returns `nil` only when `nil` is passed.

See below:

```ruby
# a.rb
p(Gem::Version.create("2.3.4") >= Gem::Version.create("2.3.0"))
```

```console
$ bundle exec steep check a.rb
...
a.rb:1:31: [error] Type `(::Gem::Version | nil)` does not have method `>=`
│ Diagnostic ID: Ruby::NoMethod
│
└ p(Gem::Version.create("2.3.4") >= Gem::Version.create("2.3.0"))
                                 ~~
```

Environments:
- RBS 1.1.0
- Steep 0.42.0

Source code of the `create` method:
https://github.com/rubygems/rubygems/blob/v3.2.13/lib/rubygems/version.rb#L188-L196
@@ -170,7 +170,8 @@ module Gem
# ver2 = Version.create(ver1) # -> (ver1)
# ver3 = Version.create(nil) # -> nil
#
def self.create: (_ToS | Version | nil input) -> instance?
def self.create: (_ToS | Version input) -> instance
| (nil input) -> nil
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note] The test case is covered when nil is passed:

assert_send_type "(nil) -> nil",
Gem::Version, :create, nil

@ybiquitous ybiquitous marked this pull request as ready for review March 10, 2021 02:31
@ybiquitous
Copy link
Contributor Author

FYI. Gem::Version has been added with PR #610.

Copy link
Member

@pocke pocke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@pocke pocke merged commit a4c2f75 into ruby:master Mar 14, 2021
@ybiquitous ybiquitous deleted the improve-gem-version-create branch March 14, 2021 12:08
@ybiquitous
Copy link
Contributor Author

@pocke Thanks for the review and merge! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants