Skip to content

Commit

Permalink
[Fix rubocop#6132] Fix a false negative for Naming/FileName
Browse files Browse the repository at this point in the history
Fixes rubocop#6132.

### Summary

This PR fixes a false negative for `Naming/FileName`
when `Include` of `AllCops` is the default setting.

The important change in this PR is below.

```diff
diff --git a/lib/rubocop/cop/naming/file_name.rb
b/lib/rubocop/cop/naming/file_name.rb
index a025a73b7..be3f1c5aa 100644
--- a/lib/rubocop/cop/naming/file_name.rb
+++ b/lib/rubocop/cop/naming/file_name.rb
@@ -32,7 +32,7 @@ module RuboCop

         def investigate(processed_source)
           file_path = processed_source.file_path
-          return if config.file_to_include?(file_path)
+          return if config.file_to_exclude?(file_path)
```

The problem is that the target to be excluded was
`Include` instead of `Exclude`.

Also this PR adds the `RuboCop::Config#allowed_camel_case_file?`
method to judge ignoring `Gemfile`, `Rakefile`, etc
described in` Include`.

### Other Information

This false negative was noticed by adding `**/*.rb` to
config/default.yml at rubocop#5882.

https://github.com/rubocop-hq/rubocop/pull/5882/files#diff-e93280b3b31a6438c533a5f3232340d8R18
  • Loading branch information
koic committed Aug 2, 2018
1 parent c853715 commit 28d3daf
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 6 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## master (unreleased)

### Bug fixes

* [#6132](https://github.com/rubocop-hq/rubocop/issues/6132): Fix a false negative for `Naming/FileName` when `Include` of `AllCops` is the default setting. ([@koic][])

### Changes

* [#6137](https://github.com/rubocop-hq/rubocop/pull/6137): Allow `db` to allowed names of `Naming/UncommunicativeMethodParamName` cop in default config. ([@mkenyon][])
Expand Down
20 changes: 16 additions & 4 deletions lib/rubocop/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,20 @@ def file_to_include?(file)
absolute_file_path = File.expand_path(file)

patterns_to_include.any? do |pattern|
match_path?(pattern, relative_file_path) ||
match_path?(pattern, absolute_file_path)
if block_given?
yield pattern, relative_file_path, absolute_file_path
else
match_path?(pattern, relative_file_path) ||
match_path?(pattern, absolute_file_path)
end
end
end

def allowed_camel_case_file?(file)
file_to_include?(file) do |pattern, relative_path, absolute_path|
pattern.to_s =~ /[A-Z]/ &&
(match_path?(pattern, relative_path) ||
match_path?(pattern, absolute_path))
end
end

Expand All @@ -396,11 +408,11 @@ def file_to_exclude?(file)
end

def patterns_to_include
for_all_cops['Include']
for_all_cops['Include'] || []
end

def patterns_to_exclude
for_all_cops['Exclude']
for_all_cops['Exclude'] || []
end

def path_relative_to_config(path)
Expand Down
3 changes: 2 additions & 1 deletion lib/rubocop/cop/naming/file_name.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ class FileName < Cop

def investigate(processed_source)
file_path = processed_source.file_path
return if config.file_to_include?(file_path)
return if config.file_to_exclude?(file_path) ||
config.allowed_camel_case_file?(file_path)

for_bad_filename(file_path) do |range, msg|
add_offense(nil, location: range, message: msg)
Expand Down
28 changes: 28 additions & 0 deletions spec/rubocop/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,7 @@

describe '#file_to_exclude?' do
before { $stderr = StringIO.new }

after { $stderr = STDERR }

let(:hash) do
Expand Down Expand Up @@ -433,6 +434,32 @@
end
end

describe '#allowed_camel_case_file?' do
subject { configuration.allowed_camel_case_file?(file_path) }

let(:hash) do
{
'AllCops' => {
'Include' => ['**/Gemfile']
}
}
end

context 'when the passed path matches allowed camel case patterns ' \
'to include' do
let(:file_path) { '/home/foo/project/Gemfile' }

it { is_expected.to be true }
end

context 'when the passed path does not match allowed camel case patterns ' \
'to include' do
let(:file_path) { '/home/foo/project/testCase' }

it { is_expected.to be false }
end
end

describe '#patterns_to_include' do
subject(:patterns_to_include) do
configuration = described_class.new(hash, loaded_path)
Expand Down Expand Up @@ -527,6 +554,7 @@
let(:hash) { { 'AllCops' => { 'Includes' => [] } } }

before { $stderr = StringIO.new }

after { $stderr = STDERR }

it 'prints a warning message for the loaded path' do
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cop/naming/file_name_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
}
end

let(:includes) { [] }
let(:includes) { ['**/*.rb'] }
let(:source) { 'print 1' }
let(:processed_source) { parse_source(source) }

Expand Down

0 comments on commit 28d3daf

Please sign in to comment.