Add new Rails/UnknownEnv cop
#4791
Conversation
lib/rubocop/cop/rails/env.rb
Outdated
| module RuboCop | ||
| module Cop | ||
| module Rails | ||
| # This cop checks typo of environment name. |
There was a problem hiding this comment.
It technically checks that the specified environment exists, and can also indicate if it's a typo or not. 🙂
There was a problem hiding this comment.
Should I replace it with your comment?
There was a problem hiding this comment.
Perhaps something like:
This cop checks that environments called with
Rails.envpredicates exist.
WDYT?
There was a problem hiding this comment.
Looks good to me 👍 I'll update this message tomorrow.
lib/rubocop/cop/rails/env.rb
Outdated
| # | ||
| # # good | ||
| # Rails.env.production? | ||
| class Env < Cop |
There was a problem hiding this comment.
Would probably want a more descriptive name to make it easier to tell what it does, and to not clash with other potential cops. E.g. might want a cop that checks that Rails.env is used instead of ENV["RAILS_ENV"].
lib/rubocop/cop/rails/env.rb
Outdated
| class Env < Cop | ||
| include NameSimilarity | ||
|
|
||
| MSG = '`%<name>s` is incorrect environment name.'.freeze |
There was a problem hiding this comment.
Would probably say "unknown" instead of "incorrect", as it gives a better hint what's wrong. 🙂
E.g.: "Unknown environment prduction. Did you mean production?"
lib/rubocop/cop/rails/env.rb
Outdated
| PATTERN | ||
|
|
||
| def on_send(node) | ||
| typo_environment?(node) do |name| |
There was a problem hiding this comment.
Might change typo to unknown.
lib/rubocop/cop/rails/env.rb
Outdated
| end | ||
| end | ||
|
|
||
| def incorrect_env_name?(name) |
There was a problem hiding this comment.
Here might also change incorrect to unknown.
lib/rubocop/cop/rails/env.rb
Outdated
| end | ||
|
|
||
| def environments | ||
| cop_config['Environments'].map { |env| env + '?' } |
There was a problem hiding this comment.
Technically the ? isn't part of the environment name. Might want to strip it when before checking include? instead.
|
I think this is a brilliant cop! 😍 It can prevent a lot of catastrophic, silent errors, since Rails' string inquirer doesn't discriminate between what exists and not. |
|
And what about custom environments? |
There's a configuration option named |
👍
If a file for environment does not exist under $ rails new foo --skip-active-record
$ cd foo
$ RAILS_ENV=aaa bin/rails c # It worksAnd I think a cop should not depend other files existence. |
|
Updated |
lib/rubocop/cop/rails/unknown_env.rb
Outdated
| module RuboCop | ||
| module Cop | ||
| module Rails | ||
| # This cop checks that environments called with Rails.env predicates |
There was a problem hiding this comment.
It may be better to enclose Rails.env in backquotes.
BTW, This pragmatic cop is awesome. Thanks always.
There was a problem hiding this comment.
It may be better to enclose Rails.env in backquotes.
Good catch, thanks!
BTW, This pragmatic cop is awesome. Thanks always.
😸
There was a problem hiding this comment.
Thanks for the update. Moreover, it seems necessary to regenerate the document with rake generate_cops_documentation.
https://travis-ci.org/bbatsov/rubocop/jobs/280723348#L590
This cop checks typo of `Rails.env.foobar`. For example: ```ruby if Rails.env.proudction? # ^^^^^^^^^^^ `proudction?` is incorrect environment name. Did you mean `production?`? end ``` And, the incorrect environment name does not raise errors. It just return `false`.
We usually have four rails environments (production, staging, development, test) where only production, development, and test are the default rails-defined environments. There is a new cop which checks for the environment list (rubocop/rubocop#4791) which we need to configure with our additional staging env.
This cop checks typo of
Rails.env.foobar.For example:
And, the incorrect environment name does not raise errors. It just return
false.Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).rake spec) are passing.rake internal_investigation.and description in grammatically correct, complete sentences.
rake generate_cops_documentation(required only when you've added a new cop or changed the configuration/documentation of an existing cop).