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

Suggestion: Prefer to _not_ use two argument version of raise #747

Closed
rhys-vdw opened this issue May 23, 2019 · 5 comments
Closed

Suggestion: Prefer to _not_ use two argument version of raise #747

rhys-vdw opened this issue May 23, 2019 · 5 comments

Comments

@rhys-vdw
Copy link

rhys-vdw commented May 23, 2019

Just hit this surprising behavior of raise.

These two lines are not equivalent:

raise SomeError, "a", "b"
raise SomeError.new("a", "b")

In the former case the third argument will be silently ignored causing an error like this:

ArgumentError: wrong number of arguments (given 1, expected 2)

This is a confusion can be avoided by simply calling new directly. (Also IMO is clearer about what's really happening when you call raise, a simple object construction.)

Alternatively a Rubocop rule could be introduced to detect when more than two arguments are passed to raise.

@rhys-vdw
Copy link
Author

Upon further investigation we had disabled the rule instead of setting it to "EnforcedStyle: compact". I would still suggest a change in default, but it seems that Rubocop has this covered.

Nice. 👌

@rhys-vdw rhys-vdw reopened this May 23, 2019
@rhys-vdw
Copy link
Author

Actually, can I suggest that this gets split into two rules:

  • One rule that prevents more than two arguments being passed to raise. This is to enforce a non-obvious behaviour of Ruby highlighted above.
  • Another rule that is stylistic and suggests using raise A, a instead of raise A.new(a).

I think this is a more sensible approach. Because now I can't enable the rule without changing all instances.

@bbatsov
Copy link
Collaborator

bbatsov commented Jun 14, 2019

I'm totally open to improving the existing rule.

Keep in mind the following:

With no arguments, raises the exception in $! or raises a RuntimeError if $! is nil. With a single String argument, it raises a RuntimeError with the string as a message. Otherwise, the first parameter should be the name of an Exception class (or an object that returns an Exception when sent exception). The optional second parameter sets the message associated with the exception, and the third parameter is an array of callback information. Exceptions are caught by the rescue clause of begin...end blocks.

I think it's mostly a matter of misaligned expectations.

@pirj
Copy link
Member

pirj commented Feb 23, 2021

Related rubocop/rubocop#8175

@pirj
Copy link
Member

pirj commented Feb 24, 2021

Tracked in #395

@pirj pirj closed this as completed Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants