Make RSpec/FilePath support ActiveSupport inflections, if defined

[jeromedalbert]

Fixes #740. Implementation is based on the suggestion in this comment: #740 (comment).

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 #740.

Other common custom mixed-case acronyms could be StatsD, OAuth, RESTful, etc.

---

Before submitting the PR make sure the following are checked:

• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Updated documentation.
• Added an entry to the CHANGELOG.md if the new code introduces user-observable changes.
• 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:

• Added the new cop to config/default.yml.
• The cop is configured as Enabled: pending in config/default.yml.
• The cop documents examples of good and bad code.
• The tests assert both that bad code is reported and that good code is not reported.
• Set VersionAdded in default/config.yml to the next minor version.

If you have modified an existing cop's configuration options:

• Set VersionChanged in config/default.yml to the next major version.

[jeromedalbert]

Inflection configuration must be loaded before ActiveSupport::Inflector.underscore is executed, to take acronyms and other inflection rules into account.

That configuration is typically found in the config/initializers/inflections.rb file that is generated when creating a Rails app.

[pirj]

Nice!
Do you mind throwing in a spec that would indicate how it behaves in action?

[jeromedalbert]

@pirj Done

[pirj]

Thank you!

[pirj]

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 am a bit on the fence about adding Rails specific logic into RuboCop-RSpec, so not giving a 👍🏼 right away. I’ll think about it some more.

[bquorning]

I’m afraid this code is a bit slow. Could we do the defined?/File.exists?/require thing just once and store the result perhaps a class variable?

[bquorning]

It feels like this file path should be configurable.

[bquorning]

I wonder if we couldn’t do

ensure
  ActiveSupport::Inflector.inflections.clear(:acronyms)

instead of having the order :defined on the context block.

[jeromedalbert]

@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).

[bquorning]

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?

[pirj]

@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. 😈

cc @Darhazer

[cdhagmann]

There are other libraries that can be used to set inflectors:

• dry-inflector
• zeitwerk

I would suggest providing a script field in the YAML files that could be run such that String#underscore could be defined if desired.

Rspec/FilePath:
  LoadInfector: |-
    require 'active_support/inflector'
    require './config/initializers/inflections'

Alternatively, we could add a field for the Inflector gem and/or definition file:

Rspec/FilePath:
  UseInflectorGem: ActiveSupport
  InflectorDefinition: ./config/initializers/inflections.rb

Doing either of these should then allow if string.respond_to?(:underscore) to be used instead of hard-coding a library and a file location.

[cdhagmann]

Another alternative may simply be to make sure the file name matches the locations in which the class is defined, regardless of inflection.

Example:

app/services/my_html_module/xml_reader.rb

module MyHTMLModule
  class XMLReader

spec/services/my_html_module/xml_reader_spec.rb

RSpec.describe MyHTMLModule::XMLReader do
end

This could sidestep the need for inflection.

[jeromedalbert]

This PR is stale, closing it. If someone wants to take a stab at it, feel free to open a new PR.