search

standardrb / standard-rails

A Standard Ruby plugin that configures rubocop-rails
MIT License
163 stars 15 forks source link

Disable Rails/EnvironmentVariableAccess in config/initializers? #46

Closed danielmorrison closed 3 months ago

danielmorrison commented 3 months ago

Upgrading standard-rails to 1.1.0 has started flagging things like: 

config/initializers/stripe.rb:1:18: Rails/EnvironmentVariableAccess: Do not read from `ENV` directly post initialization.

IMHO, an initializer is not "post initialization" and is the correct spot for this.

This comes from https://github.com/rubocop/rubocop-rails/pull/1229 which specifically looks at config/initializers/**/*.rb to apply this rule.

What do others think about removing that directory from this check? Happy to whip up a PR if there's enough interest.

searls commented 3 months ago

I hate this rule across the board (due to the fact Rails has changed the "correct" way to access ENV vars conventionally 4 times in 10 years) and am absolutely supportive of nixing it. Send a PR to disable

dogweather commented 3 months ago

Awesome. I came here with this same issue.

I think this rule has never worked as intended. Rubocop-rails has disabled it by default because of a lack of a full example to migrate code.

I suspect that the Rails config and #config_for helper do not provide boot-time env var checking as someone claimed in your conference voting presentation. I think it's the opposite: Rails config is a silent-failure API. Unknown config names just return nil.

Personally, I ENV.fetch in my initializers and store them in namespaced constants. That way, I get some help from the interpreter if I misspell one. This also enables static analysis tools like Sorbet to provide more help.

dogweather commented 3 months ago

@searls wrote:

Send a PR to disable

https://github.com/standardrb/standard-rails/pull/47

danielmorrison commented 3 months ago

You're a hero, @dogweather!