rubocop-rspec
rubocop-rspec copied to clipboard
Proposal: Avoid repetitive definitions of CustomTransform in RSpec/FilePath cop
Summary
I would like to achieve the following 2 things for RSpec/FilePath.
I am willing to implement the solution when we reach an agreement on this issue.
Avoid repetitive CustomTransform definitions for single inflection.
When using ActiveSupport::Inflector, we have to write only 1 definition for 1 word with special inflection. On the other hand, when using rubocop-rspec, we have to define multiple definitions (all the composite words derived from a special word). This makes development a little bit less productive.
For example, let's assume that we use a special word like PvP in a project.
When using ActiveSupport::Inflector, all we have to do is write a single rule on PvP.
ActiveSupport::Inflector.inflections(:en) do |inflect|
inflect.acronym 'PvP'
end
When we use the current CustomTransform, we have to define all the transforms as follows.
RSpec/FilePath:
CustomTransform:
PvP: pvp
SyncPvP: sync_pvp
AsyncPvP: async_pvp
PvPOverPvP: pvp_over_pvp
PvPsController: pvps_controller
This is mainly because the exact-match substitution is handled before snake casing.
https://github.com/rubocop-hq/rubocop-rspec/blob/d69442f17d874cff8f5c11faabd929534c8fc717/lib/rubocop/cop/rspec/file_path.rb#L83-L89
I would like to let the following config just work as ActiveSupport::Inflector config does.
RSpec/FilePath:
CustomTransform:
PvP: pvp
Share the same settings as ActiveSupport::Inflector has
I am not so inclined to achieve this as the first one, but, hopefully, I would like to share the same config as we have in Rails application. This motivation is not applicable to non-Rails user. Therefore, I would like to use less-invasive way to achieve this.
In our project, we are using require directive in config YAML to require config/initializers/inflections.rb, where ActiveSupport::Inflector config exists, but there may be a more sophisticated intuitive way.
How to solve this issue
IMHO, I believe that ActiveSupport::Inflector, which is famous for its sophisticated string handling, can solve this issue. Since this idea has both pros and cons, I have set up this issue.
https://api.rubyonrails.org/classes/ActiveSupport/Inflector.html
If RuboCop policy prioritize less dependencies over reinventing the wheel, we can just partially import the code from ActiveSupport::Inflector
Pros and Cons
- Pros
- Avoiding reinventing the wheel on string inflections.
- Reduced code complexity on current and future code.
- Cons
- Breaking backward compatibilities a little.
- Increased dependencies of
RuboCop::RSpeconActiveSupport::Inflector.
Other Information
I made up a gem rubocop-inflector to solve this issue. However, since the gem touch the internal method of rubocop-rspec, later I have found out it is fragile and not so a recommended way to solve the issue. Although it is very thin and comparatively easy to maintain.
https://github.com/aeroastro/rubocop-inflector
Hi @aeroastro, thanks for the issue. I recently ran across a similar problem in one of my repos as well.
I think this is worth considering, but we should definitely not have a hard dependency on ActiveSupport::Inflector. I'm also not particularly wild about the idea of vendoring the AS code (unless it turns out to be much smaller/simpler than I'm picturing), but one thing we could consider is try to check the AS inflections if ActiveSupport::Inflector is defined and then fall back to our existing transforms/logic, or something like that.
This requires some thought. cc: @bquorning @Darhazer
Since the gem works fine, we could recommend it as an optional dependency for this specific case.
To circumvent the brittleness, can you provide more information on what internal API you use in it? Probably it would be possible to expose some stable interface so that the behaviour can be reliably enhanced by optionally including rubocop-inflector?