Add AllowedPredicates configuration option to Rails/Env#1617
Open
dduugg wants to merge 2 commits into
Open
Conversation
Replace the hard-coded `ALLOWED_LIST` constant with a configurable
`AllowedPredicates` option. The default value preserves the previous
behavior — `String` / `StringInquirer` predicates such as `empty?`,
`match?`, and `between?` are still exempt out of the box — but users
can now customize the list, for example to allow `Rails.env.local?`
or a custom predicate monkey-patched onto the environment inquirer.
The configured value fully replaces the default; users who want to
inherit the defaults and add to them can opt in via RuboCop's
`inherit_mode`.
Rails/Env:
AllowedPredicates:
- empty?
- match?
- local?
A new spec block uses `let(:cop_config)` to verify that overriding
`AllowedPredicates` both allows the configured predicate and continues
to flag predicates that are no longer in the list.
6dc5bea to
b7b27ad
Compare
|
Great fix! Thanks for the added versatility. And we need this now as well, because we do allow |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace
Rails/Env's hard-codedALLOWED_LISTconstant with a configurableAllowedPredicatesoption. The default value is the previousALLOWED_LISTcontent —String/StringInquirerpredicates likeempty?,match?,between?— so out-of-the-box behavior is unchanged.The motivation is to let users exempt additional predicates from the cop without forking or disabling it. Two concrete cases this addresses:
Rails.env.local?— the built-in alias for "development or test", introduced in Rails 7.1. Some users treat it as a legitimate guard rail for code that should only ever execute in dev or test (sanity checks, devtools, seed data) rather than as an environment-rollout mechanism, and want it exempt from the cop.Rails.env.beta_user?/Rails.env.internal?style helpers can now opt those in.The configured value fully replaces the default — it is not merged. Users who want to inherit the defaults and add to them can opt in via RuboCop's
inherit_mode:Compatibility with
Rails/EnvLocalRails/EnvLocalautocorrectsRails.env.development? || Rails.env.test?→Rails.env.local?. Under the defaultAllowedPredicates,Rails/Envwill still flag the autocorrectedRails.env.local?. Users who run both cops can now resolve that conflict by includinglocal?in theirAllowedPredicatesrather than disabling either cop.Test coverage
A new context block uses
let(:cop_config)to overrideAllowedPredicatesand verifies (a) a predicate in the override is not flagged, and (b) a predicate omitted from the override is flagged — confirming the override is exhaustive (no implicit merge with defaults).Before submitting the PR make sure the following are checked:
[Fix #issue-number](no related issue).master.bundle exec rake default.