Rubocop: Rails/UnknownEnv mislead users when having non-default environments

[wata727]

For example, if a user has "staging" environment, by default Rails/UnknownEnv will display the following message:

$ rubocop --only Rails/UnknownEnv app/models/foo.rb
Inspecting 1 file
C

Offenses:

app/models/foo.rb:5:18: C: Rails/UnknownEnv: Unknown environment staging?.
    if Rails.env.staging?
                 ^^^^^^^^

1 file inspected, 1 offense detected

However, if the user is already using the "staging" environment, confusing because it is a known environment for the user.

Should we include messages like the following?

Rails/UnknownEnv: Unknown environment staging?. Please check the setting of "Environments" in `.rubocop.yml`.

RuboCop version

$ rubocop -V
0.52.1 (using Parser 2.4.0.2, running on ruby 2.4.1 x86_64-darwin14)

[bbatsov]

I generally dislike multi-sentence messages. There are just a handful of those in the entire RuboCop codebase and they look like something odd to me. Apart from that - the message you suggested is also misleading IMO, as it doesn't clearly state that this is some configuration of that particular cop.

[mikegee]

I agree with the reported misleading message. Here is my attempt at an improvement confined to one sentence:

Rails/UnknownEnv: Environment "staging" is not one of the allowed environments: ["development", "test", "production"].

(I don't like including that question mark in the message.)

---

Is it possible for this cop to look at the files in #{APP}/config/environments/*.rb, instead of using the hard-coded list? I suspect that would workaround this issue pretty well.

[wata727]

That's good! However, I'd like to avoid "allow". How about the following?

Rails/UnknownEnv: "staging" is not one of the known environments: ["development", "test", "production"].

(I don't like including that question mark in the message.)

👍

Is it possible for this cop to look at the files in #{APP}/config/environments/*.rb, instead of using the hard-coded list? I suspect that would workaround this issue pretty well.

It seems good, but I'm worried about where to write the process of generating the list of environments. (It would be redundant to generate in the on_send every time...)

[bbatsov]

I guess this can simply be stored in some instance variable.

[pocke]

Is it possible for this cop to look at the files in #{APP}/config/environments/*.rb, instead of using the hard-coded list? I suspect that would workaround this issue pretty well.

We should be careful two things.
First, if rubocop runs in a child directory, it does not work well.
For example:

$ cd some_application
$ ls
app/ config/ db/ ...

# In the root directory, it works well.
$ rubocop

# In a child directory, it does not work well.
$ cd app/
$ rubocop # Probably it can't find `config/environments/*.rb`, so it will have false positives/negatives.

Second, config/environments/*.rb for a valid environment name doesn't exist sometimes. See https://github.com/bbatsov/rubocop/pull/4791#issuecomment-331843976 . (But I guess it is a rare case.)

Maybe we can solve the first problem with finding the root directory. For example, some_application/ directory is a root directory in the above example. The cop searches config/environments/*.rb from the root directory.

[bbatsov]

I'm closing this ticket due to no recent activity. Feel free to re-open it if you ever come back to it.

[dgmora]

I landed in this issue after googling the rubocop warning (for staging as well). If you don't know the reason, the current message is not very clear.