New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make RSpec/FilePath
support ActiveSupport inflections, if defined
#1266
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -130,6 +130,13 @@ def expected_path(constant) | |
end | ||
|
||
def camel_to_snake_case(string) | ||
if defined?(ActiveSupport::Inflector) | ||
if File.exist?('./config/initializers/inflections.rb') | ||
require './config/initializers/inflections' | ||
Comment on lines
+134
to
+135
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It feels like this file path should be configurable. |
||
end | ||
Comment on lines
+134
to
+136
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inflection configuration must be loaded before That configuration is typically found in the
Comment on lines
+133
to
+136
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’m afraid this code is a bit slow. Could we do the |
||
return ActiveSupport::Inflector.underscore(string) | ||
end | ||
|
||
string | ||
.gsub(/([^A-Z])([A-Z]+)/, '\1_\2') | ||
.gsub(/([A-Z])([A-Z][^A-Z\d]+)/, '\1_\2') | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -246,6 +246,27 @@ class Foo | |
end | ||
end | ||
|
||
context 'when ActiveSupport Inflector is defined', order: :defined do | ||
before { require 'active_support/inflector' } | ||
|
||
it 'registers an offense for a bad path when there is no custom acronym' do | ||
expect_offense(<<-RUBY, 'pvp_class_foo_spec.rb') | ||
describe PvPClass, 'foo' do; end | ||
^^^^^^^^^^^^^^^^^^^^^^^^ Spec path should end with `pv_p_class*foo*_spec.rb`. | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when class name contains custom acronym' do | ||
ActiveSupport::Inflector.inflections do |inflect| | ||
inflect.acronym('PvP') | ||
end | ||
Comment on lines
+260
to
+262
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we couldn’t do ensure
ActiveSupport::Inflector.inflections.clear(:acronyms) instead of having the |
||
|
||
expect_no_offenses(<<-RUBY, 'pvp_class_foo_spec.rb') | ||
describe PvPClass, 'foo' do; end | ||
RUBY | ||
end | ||
end | ||
|
||
context 'when configured with IgnoreMethods' do | ||
let(:cop_config) { { 'IgnoreMethods' => true } } | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are other libraries that can be used to set inflectors:
I would suggest providing a script field in the YAML files that could be run such that
String#underscore
could be defined if desired.Alternatively, we could add a field for the Inflector gem and/or definition file:
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.rbThis could sidestep the need for inflection.