Skip to content

Commit

Permalink
[Fix rubocop-hq#6289] Naming/FileName: Add CheckDefinitionPathHierarc…
Browse files Browse the repository at this point in the history
…hy option
  • Loading branch information
jschneid committed May 29, 2020
1 parent eb12728 commit 640f458
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 11 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,10 @@

## master (unreleased)

### New features

* [#6289](https://github.com/rubocop-hq/rubocop/issues/6289): Add new `CheckDefinitionPathHierarchy` option for `Naming/FileName`. ([@jschneid][])

### Bug fixes

* [#8008](https://github.com/rubocop-hq/rubocop/issues/8008): Fix an error for `Lint/SuppressedException` when empty rescue block in `def`. ([@koic][])
Expand Down Expand Up @@ -4542,3 +4546,4 @@
[@laurmurclar]: https://github.com/laurmurclar
[@jethrodaniel]: https://github.com/jethrodaniel
[@CvX]: https://github.com/CvX
[@jschneid]: https://github.com/jschneid
4 changes: 4 additions & 0 deletions config/default.yml
Expand Up @@ -2031,6 +2031,10 @@ Naming/FileName:
# It further expects it to be nested inside modules which match the names
# of subdirectories in its path.
ExpectMatchingDefinition: false
# When `false`, changes the behavior of ExpectMatchingDefinition to match only
# 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
# 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
35 changes: 25 additions & 10 deletions lib/rubocop/cop/naming/file_name.rb
Expand Up @@ -49,25 +49,36 @@ def investigate(processed_source)

def for_bad_filename(file_path)
basename = File.basename(file_path)
msg = if filename_good?(basename)
return if matching_definition?(file_path)

no_definition_message(basename, file_path)
else
return if bad_filename_allowed?
if filename_good?(basename)
msg = perform_class_and_module_naming_checks(file_path, basename)
else
msg = other_message(basename) unless bad_filename_allowed?
end

other_message(basename)
end
yield source_range(processed_source.buffer, 1, 0), msg if msg
end

yield source_range(processed_source.buffer, 1, 0), msg
def perform_class_and_module_naming_checks(file_path, basename)
return unless expect_matching_definition?

if check_definition_path_hierarchy? &&
!matching_definition?(file_path)
msg = no_definition_message(basename, file_path)
elsif !matching_class?(basename)
msg = no_definition_message(basename, basename)
end
msg
end

def matching_definition?(file_path)
return true unless expect_matching_definition?

find_class_or_module(processed_source.ast, to_namespace(file_path))
end

def matching_class?(file_name)
find_class_or_module(processed_source.ast, to_namespace(file_name))
end

def bad_filename_allowed?
ignore_executable_scripts? && processed_source.start_with?('#!')
end
Expand All @@ -94,6 +105,10 @@ def expect_matching_definition?
cop_config['ExpectMatchingDefinition']
end

def check_definition_path_hierarchy?
cop_config['CheckDefinitionPathHierarchy']
end

def regex
cop_config['Regex']
end
Expand Down
1 change: 1 addition & 0 deletions manual/cops_naming.md
Expand Up @@ -242,6 +242,7 @@ Name | Default value | Configurable values
--- | --- | ---
Exclude | `[]` | Array
ExpectMatchingDefinition | `false` | Boolean
CheckDefinitionPathHierarchy | `true` | Boolean
Regex | `<none>` |
IgnoreExecutableScripts | `true` | Boolean
AllowedAcronyms | `CLI`, `DSL`, `ACL`, `API`, `ASCII`, `CPU`, `CSS`, `DNS`, `EOF`, `GUID`, `HTML`, `HTTP`, `HTTPS`, `ID`, `IP`, `JSON`, `LHS`, `QPS`, `RAM`, `RHS`, `RPC`, `SLA`, `SMTP`, `SQL`, `SSH`, `TCP`, `TLS`, `TTL`, `UDP`, `UI`, `UID`, `UUID`, `URI`, `URL`, `UTF8`, `VM`, `XML`, `XMPP`, `XSRF`, `XSS` | Array
Expand Down
104 changes: 103 additions & 1 deletion spec/rubocop/cop/naming/file_name_spec.rb
Expand Up @@ -126,7 +126,11 @@

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

context 'on a file which defines no class or module at all' do
Expand Down Expand Up @@ -273,6 +277,104 @@ class B
end
end

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

context 'on a file with a matching class' do
let(:source) { <<~RUBY }
begin
class ImageCollection
end
end
RUBY
let(:filename) { '/lib/image_collection.rb' }

it 'does not register an offense' do
expect(cop.offenses.empty?).to be(true)
end
end

context 'on a file with a non-matching class' do
let(:source) { <<~RUBY }
begin
class PictureCollection
end
end
RUBY
let(:filename) { '/lib/image_collection.rb' }

it 'registers an offense' do
expect(cop.offenses.size).to eq(1)
expect(cop.messages).to eq(['image_collection.rb should define a ' \
'class or module called ' \
'`ImageCollection`.'])
end
end

context 'on an empty file' do
let(:source) { '' }
let(:filename) { '/lib/rubocop/foo.rb' }

it 'registers an offense' do
expect(cop.offenses.size).to eq(1)
expect(cop.messages).to eq(['foo.rb should define a class ' \
'or module called `Foo`.'])
end
end

context 'in a non-matching directory, but with a matching class' do
let(:source) { <<~RUBY }
begin
module Foo
end
end
RUBY
let(:filename) { '/lib/some/path/foo.rb' }

it 'does not register an offense' do
expect(cop.offenses.empty?).to be(true)
end
end

context 'with a non-matching module containing a matching class' do
let(:source) { <<~RUBY }
begin
module NonMatching
class Foo
end
end
end
RUBY
let(:filename) { 'lib/foo.rb' }

it 'does not register an offense' do
expect(cop.offenses.empty?).to be(true)
end
end

context 'with a matching module containing a non-matching class' do
let(:source) { <<~RUBY }
begin
module Foo
class NonMatching
end
end
end
RUBY
let(:filename) { 'lib/foo.rb' }

it 'does not register an offense' do
expect(cop.offenses.empty?).to be(true)
end
end
end

context 'when Regex is set' do
let(:cop_config) { { 'Regex' => /\A[aeiou]\z/i } }

Expand Down

0 comments on commit 640f458

Please sign in to comment.