rubocop-rspec
rubocop-rspec copied to clipboard
Make `RSpec/FilePath` support ActiveSupport inflections, if defined
Fixes https://github.com/rubocop/rubocop-rspec/issues/740. Implementation is based on the suggestion in this comment: https://github.com/rubocop/rubocop-rspec/issues/740#issuecomment-462932812.
This is especially useful for Rails applications (or any application using ActiveSupport inflections) that define mixed-case custom acronyms, for example:
# config/initializers/inflections.rb
ActiveSupport::Inflector.inflections do |inflect|
inflect.acronym('PvP')
end
# app/models/pvp_class.rb instead of app/models/pv_p_class.rb, thanks to the custom acronym
class PvPClass
...
end
It is also great to be re-using ActiveSupport's inflection configuration instead of having to repeat it in Rubocop as mentioned in https://github.com/rubocop/rubocop-rspec/issues/740.
Other common custom mixed-case acronyms could be StatsD, OAuth, RESTful, etc.
Before submitting the PR make sure the following are checked:
- [x] Feature branch is up-to-date with
master
(if not - rebase it). - [x] Squashed related commits together.
- [x] Added tests.
- [ ] Updated documentation.
- [x] Added an entry to the
CHANGELOG.md
if the new code introduces user-observable changes. - [x] The build (
bundle exec rake
) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).
If you have created a new cop:
- [x] Added the new cop to
config/default.yml
. - [x] The cop is configured as
Enabled: pending
inconfig/default.yml
. - [x] The cop documents examples of good and bad code.
- [x] The tests assert both that bad code is reported and that good code is not reported.
- [x] Set
VersionAdded
indefault/config.yml
to the next minor version.
If you have modified an existing cop's configuration options:
- [x] Set
VersionChanged
inconfig/default.yml
to the next major version.
Nice! Do you mind throwing in a spec that would indicate how it behaves in action?
@pirj Done
The failing example is flaky, due to order dependency. If the example that define the "PvP" acronym runs first, that the other one fails. I suggest adding metadata to the example to work this around:
context 'when ActiveSupport Inflector is defined', order: :defined do
@bquorning
I’ll think about it some more.
Any update on your thoughts?
My guess is that Rails projects that use inflections are either probably just disabling this cop (my project does), so this PR is an attempt to fix that, and make the cop useful again.
I have seen many gems whose goal is to work without Rails, but still provide a Rails::Railtie
file that gets executed if Rails is defined, as an optional enhancement so to speak. In that sense and as long as the Rails specific logic is more or less contained, one could argue that this PR is doing the same, just providing some optional enhancement while keeping the core of the gem intact (indeed, no runtime dependency to ActiveSupport is added in this PR, only a development dependency so my test case can use it).
Any update on your thoughts?
Apologies for the silence on my end.
I do understand the use case, and it would be a neat addition for Rails projects, not having to configure rubocop-rspec with static map of inflections that the Rails application already knows how to generate. My problem is with putting Rails-specific logic into this gem. One might argue that we should then also put logic for other frameworks that might have similar inflection logic – an approach that wouldn’t scale.
It would be neat if we could configure this cop with an “inflector block”, i.e. ->(string) { ActiveSupport::Inflector.underscore(string) }
, but I am not sure we can do that via YAML. @pirj what do you think?
@bquorning I was a secret admirer/dreamer of a plain Ruby RuboCop configuration, especially after our DSL configuration extension and some later problems with merging arrays in YAML.
Did you have something like:
RuboCop::Cop::RSpec::FilePath.add_custom_transform { |string| ActiveSupport::Inflector.underscore(string) }
in mind? I'm all for it. :smiling_imp:
cc @Darhazer