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

Add a cop for warning against exit calls #3089

Closed
sgringwe opened this issue Apr 27, 2016 · 4 comments
Closed

Add a cop for warning against exit calls #3089

sgringwe opened this issue Apr 27, 2016 · 4 comments

Comments

@sgringwe
Copy link
Contributor

I ran into this recently which is what prompted this. I couldn't find an existing cop, so figured I'd suggest it and if it is welcomed open a PR.

Generally speaking, exit calls in a rails app are probably not wanted. Rails seems to rescue from SystemExit by default (testing on my own app + code at https://github.com/rails/rails/blob/master/actionpack/lib/action_controller/metal/rescue.rb), but there a few reasons it's still probably not the functionality desired:

  • If you have an exit() in a library file, and you unit test that library file, the exit will exit out of rspec and silently not run tests (and be a "green" build if exit code is 0 and depending on your test suite).
  • If you write a ruby processor outside of the rails controller context, the program will exit and stop running which is probably not desirable if part of your web stack (you instead want to continue running your web app).

This may be enough of an edge case to ignore, but figured I'd recommend.

Expected behavior

A rubocop warning of some sort if I try to use an exit() in rails apps.

Actual behavior

No warning.

@sgringwe
Copy link
Contributor Author

@bbatsov if you are able to provide direction, i'd be interested in trying to add a cop for this.

@bbatsov
Copy link
Collaborator

bbatsov commented May 1, 2016

Sure. A cop like this will be pretty easy to add - you'll just have to sift through all send nodes and look for nodes named exit without a receiver.

@bbatsov
Copy link
Collaborator

bbatsov commented May 1, 2016

Guess you can use the Style/FormatString cop as a sort of reference.

@sgringwe
Copy link
Contributor Author

sgringwe commented May 1, 2016

@bbatsov cool, thank you! Opened #3100

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

2 participants