Skip to content
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

Naming/FileName: make CheckDefinitionPathHierarchy roots configureable #10224

Merged
merged 1 commit into from Nov 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/change_namingfilename_make.md
@@ -0,0 +1 @@
* [#10220](https://github.com/rubocop/rubocop/issues/10220): Update `Naming/FileName` to make `CheckDefinitionPathHierarchy` roots configurable. ([@grosser][])
7 changes: 7 additions & 0 deletions config/default.yml
Expand Up @@ -2520,6 +2520,13 @@ Naming/FileName:
# whether each source file's class or module name matches the file name --
# not whether the nested module hierarchy matches the subdirectory path.
CheckDefinitionPathHierarchy: true
# paths that are considered root directories, for example "lib" in most ruby projects
# or "app/models" in rails projects
CheckDefinitionPathHierarchyRoots:
- lib
- spec
- test
- src
# If non-`nil`, expect all source file names to match the following regex.
# Only the file name itself is matched, not the entire file path.
# Use anchors as necessary if you want to match the entire name rather than
Expand Down
8 changes: 6 additions & 2 deletions lib/rubocop/cop/naming/file_name.rb
Expand Up @@ -123,6 +123,10 @@ def check_definition_path_hierarchy?
cop_config['CheckDefinitionPathHierarchy']
end

def definition_path_hierarchy_roots
cop_config['CheckDefinitionPathHierarchyRoots'] || []
end

def regex
cop_config['Regex']
end
Expand Down Expand Up @@ -206,13 +210,13 @@ def match_acronym?(expected, name)
allowed_acronyms.any? { |acronym| expected.gsub(acronym.capitalize, acronym) == name }
end

def to_namespace(path)
def to_namespace(path) # rubocop:disable Metrics/AbcSize
components = Pathname(path).each_filename.to_a
# To convert a pathname to a Ruby namespace, we need a starting point
# But RC can be run from any working directory, and can check any path
# We can't assume that the working directory, or any other, is the
# "starting point" to build a namespace.
start = %w[lib spec test src]
start = definition_path_hierarchy_roots
start_index = nil

# To find the closest namespace root take the path components, and
Expand Down
48 changes: 25 additions & 23 deletions spec/rubocop/cop/naming/file_name_spec.rb
Expand Up @@ -8,14 +8,15 @@
'/some/.rubocop.yml'
)
end
let(:cop_config) do
let(:cop_config) do # matches default.yml
{
'IgnoreExecutableScripts' => true,
'ExpectMatchingDefinition' => false,
'Regex' => nil
'Regex' => nil,
'CheckDefinitionPathHierarchy' => true,
'CheckDefinitionPathHierarchyRoots' => %w[lib spec test src]
}
end

let(:includes) { ['**/*.rb'] }

context 'with camelCase file names ending in .rb' do
Expand Down Expand Up @@ -93,7 +94,7 @@
end

context 'when IgnoreExecutableScripts is disabled' do
let(:cop_config) { { 'IgnoreExecutableScripts' => false } }
let(:cop_config) { super().merge('IgnoreExecutableScripts' => false) }

it 'registers an offense' do
expect_offense(<<~RUBY, '/some/dir/test-case')
Expand All @@ -118,13 +119,7 @@
end

context 'when ExpectMatchingDefinition is true' do
let(:cop_config) do
{
'IgnoreExecutableScripts' => true,
'ExpectMatchingDefinition' => true,
'CheckDefinitionPathHierarchy' => 'true'
}
end
let(:cop_config) { super().merge('ExpectMatchingDefinition' => true) }

context 'on a file which defines no class or module at all' do
%w[lib src test spec].each do |dir|
Expand All @@ -138,6 +133,17 @@
end
end

context 'under lib when not added to root' do
let(:cop_config) { super().merge('CheckDefinitionPathHierarchyRoots' => ['foo']) }

it 'registers an offense' do
expect_offense(<<~RUBY, '/some/other/dir/test_case.rb')
print 1
^ `test_case.rb` should define a class or module called `TestCase`.
RUBY
end
end

context 'under some other random directory' do
it 'registers an offense' do
expect_offense(<<~RUBY, '/some/other/dir/test_case.rb')
Expand Down Expand Up @@ -272,11 +278,10 @@ module A

context 'when CheckDefinitionPathHierarchy is false' do
let(:cop_config) do
{
'IgnoreExecutableScripts' => true,
super().merge(
'ExpectMatchingDefinition' => true,
'CheckDefinitionPathHierarchy' => false
}
)
end

context 'on a file with a matching class' do
Expand Down Expand Up @@ -412,11 +417,10 @@ module Foo

context 'with acronym namespace' do
let(:cop_config) do
{
'IgnoreExecutableScripts' => true,
super().merge(
'ExpectMatchingDefinition' => true,
'AllowedAcronyms' => ['CLI']
}
)
end

it 'does not register an offense' do
Expand All @@ -433,11 +437,10 @@ class AdminUser

context 'with acronym class name' do
let(:cop_config) do
{
'IgnoreExecutableScripts' => true,
super().merge(
'ExpectMatchingDefinition' => true,
'AllowedAcronyms' => ['CLI']
}
)
end

it 'does not register an offense' do
Expand All @@ -452,11 +455,10 @@ class CLI

context 'with include acronym name' do
let(:cop_config) do
{
'IgnoreExecutableScripts' => true,
super().merge(
'ExpectMatchingDefinition' => true,
'AllowedAcronyms' => ['HTTP']
}
)
end

it 'does not register an offense' do
Expand Down