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

A better error message for invalid signatures, when using only sorbet-runtime #3328

Closed
jaredbeck opened this issue Jul 29, 2020 · 2 comments · Fixed by #3524
Closed

A better error message for invalid signatures, when using only sorbet-runtime #3328

jaredbeck opened this issue Jul 29, 2020 · 2 comments · Fixed by #3524
Labels
enhancement New feature or surprising current feature runtime Related to the sorbet-runtime gem

Comments

@jaredbeck
Copy link
Contributor

jaredbeck commented Jul 29, 2020

Problem

The following signature is invalid, but it's easy to see how someone could make this mistake.

sig { params(Integer).void }
def f(i)

The correct signature, of course, is params(i: Integer).

The error message produced by sorbet-runtime is quite poor:

undefined method `keys' for Integer:Class (NoMethodError)

Proposed solution

The error message should instead say something like: "Invalid signature: params expects a Hash"

@jaredbeck jaredbeck added enhancement New feature or surprising current feature unconfirmed This issue has not yet been confirmed by the Sorbet team labels Jul 29, 2020
@jaredbeck
Copy link
Contributor Author

jaredbeck commented Jul 29, 2020

If we put ourselves in the shoes of a junior engineer, I think it would be very difficult for them to interpret the current error message. If we want junior engineers to succeed with sorbet, I think better error messages, generally, should be a priority.

Given this is a runtime-only issue (static analysis produces a much more helpful error) we can probably classify it as a low priority?

I'm happy to make a PR if you can give me a hint of where in sorbet-runtime the improved error should be raised.

@jaredbeck
Copy link
Contributor Author

To be clear, this is only an issue with sorbet-runtime. Here's a runtime-only reproduction:

# issue3328.rb
# typed: true
require 'sorbet-runtime'

module M
  extend T::Sig

  sig { params(Integer).void }
  def self.f(i)
  end
end

M.f(13)
Traceback (most recent call last):
	9: from issue3328.rb:11:in `<main>'
...
... methods/signature.rb:54:in `initialize': undefined method `keys' for Integer:Class (NoMethodError)

Static analysis, by comparison, has a helpful error message:

issue3328.rb:5: params expects keyword arguments https://srb.help/5003
     5 |  sig { params(Integer).void }
                ^^^^^^^^^^^^^^^
  All parameters must be given names in params even if they are positional

@jaredbeck jaredbeck changed the title A better error message for invalid signatures A better error message for invalid signatures, when using only sorbet-runtime Aug 4, 2020
@Morriar Morriar added the runtime Related to the sorbet-runtime gem label Aug 19, 2020
jez added a commit that referenced this issue Oct 18, 2020
@jez jez removed the unconfirmed This issue has not yet been confirmed by the Sorbet team label Oct 18, 2020
jez added a commit that referenced this issue Oct 18, 2020
@jez jez closed this as completed in #3524 Nov 13, 2020
jez added a commit that referenced this issue Nov 13, 2020
* sorbet-runtime: Better error for params misuse

Fixes #3328

* Add another test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or surprising current feature runtime Related to the sorbet-runtime gem
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants