Skip to content

Commit

Permalink
Return global offenses for Naming/FileName and `Naming/InclusiveLan…
Browse files Browse the repository at this point in the history
…guage`

Having these return something with a location is problematic because files can be empty.

This leads to `IndexError` for 3rd-party consumers when accessing location information.
  • Loading branch information
Earlopain committed Mar 21, 2024
1 parent 66c8276 commit 1468845
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 37 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12801](https://github.com/rubocop/rubocop/pull/12801): Return global offenses for `Naming/FileName` and `Naming/InclusiveLanguage` for empty files. ([@earlopain][])
4 changes: 2 additions & 2 deletions lib/rubocop/cop/naming/file_name.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def on_new_investigation
file_path = processed_source.file_path
return if config.file_to_exclude?(file_path) || config.allowed_camel_case_file?(file_path)

for_bad_filename(file_path) { |range, msg| add_offense(range, message: msg) }
for_bad_filename(file_path)
end

private
Expand All @@ -71,7 +71,7 @@ def for_bad_filename(file_path)
msg = other_message(basename) unless bad_filename_allowed?
end

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

def perform_class_and_module_naming_checks(file_path, basename)
Expand Down
3 changes: 1 addition & 2 deletions lib/rubocop/cop/naming/inclusive_language.rb
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,7 @@ def investigate_filepath
message = create_multiple_word_message_for_file(words)
end

range = source_range(processed_source.buffer, 1, 0)
add_offense(range, message: message)
add_global_offense(message)
end

def create_single_word_message_for_file(word)
Expand Down
28 changes: 14 additions & 14 deletions spec/rubocop/cop/naming/file_name_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
it 'registers an offense' do
expect_offense(<<~RUBY, '/some/dir/testCase.rb')
print 1
^ The name of this source file (`testCase.rb`) should use snake_case.
^{} The name of this source file (`testCase.rb`) should use snake_case.
RUBY
end
end
Expand All @@ -32,7 +32,7 @@
it 'registers an offense' do
expect_offense(<<~RUBY, '/some/dir/testCase')
print 1
^ The name of this source file (`testCase`) should use snake_case.
^{} The name of this source file (`testCase`) should use snake_case.
RUBY
end
end
Expand Down Expand Up @@ -99,7 +99,7 @@
it 'registers an offense' do
expect_offense(<<~RUBY, '/some/dir/test-case')
#!/usr/bin/env ruby
^ The name of this source file (`test-case`) should use snake_case.
^{} The name of this source file (`test-case`) should use snake_case.
print 1
RUBY
end
Expand Down Expand Up @@ -127,7 +127,7 @@
it 'registers an offense' do
expect_offense(<<~RUBY, "/some/dir/#{dir}/file/test_case.rb")
print 1
^ `test_case.rb` should define a class or module called `File::TestCase`.
^{} `test_case.rb` should define a class or module called `File::TestCase`.
RUBY
end
end
Expand All @@ -139,7 +139,7 @@
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`.
^{} `test_case.rb` should define a class or module called `TestCase`.
RUBY
end
end
Expand All @@ -148,7 +148,7 @@
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`.
^{} `test_case.rb` should define a class or module called `TestCase`.
RUBY
end
end
Expand All @@ -157,15 +157,15 @@
context 'on an empty file' do
it 'registers an offense' do
expect_offense(<<~RUBY, '/lib/rubocop/blah.rb')
^ `blah.rb` should define a class or module called `Rubocop::Blah`.
^{} `blah.rb` should define a class or module called `Rubocop::Blah`.
RUBY
end
end

context 'on an empty file with a space in its filename' do
it 'registers an offense' do
expect_offense(<<~RUBY, 'a file.rb')
^ The name of this source file (`a file.rb`) should use snake_case.
^{} The name of this source file (`a file.rb`) should use snake_case.
RUBY
end
end
Expand All @@ -184,7 +184,7 @@
it 'registers an offense' do
expect_offense(<<~RUBY, "/some/dir/#{dir}/c/b.rb")
# b.rb
^ `b.rb` should define a class or module called `C::B`.
^{} `b.rb` should define a class or module called `C::B`.
#{source}
RUBY
end
Expand All @@ -211,7 +211,7 @@
it 'registers an offense' do
expect_offense(<<~RUBY, '/some/dir/e.rb')
# start of file
^ `e.rb` should define a class or module called `E`.
^{} `e.rb` should define a class or module called `E`.
#{source}
RUBY
end
Expand Down Expand Up @@ -296,7 +296,7 @@ class ImageCollection
it 'registers an offense' do
expect_offense(<<~RUBY, '/lib/image_collection.rb')
begin
^ `image_collection.rb` should define a class or module called `ImageCollection`.
^{} `image_collection.rb` should define a class or module called `ImageCollection`.
class PictureCollection
end
end
Expand All @@ -316,15 +316,15 @@ class PictureCollection
it 'registers an offense' do
expect_offense(<<~RUBY, '/lib/image_collection.rb')
PictureCollection = Struct.new
^ `image_collection.rb` should define a class or module called `ImageCollection`.
^{} `image_collection.rb` should define a class or module called `ImageCollection`.
RUBY
end
end

context 'on an empty file' do
it 'registers an offense' do
expect_offense(<<~RUBY, '/lib/rubocop/foo.rb')
^ `foo.rb` should define a class or module called `Foo`.
^{} `foo.rb` should define a class or module called `Foo`.
RUBY
end
end
Expand Down Expand Up @@ -406,7 +406,7 @@ module Foo
it 'registers an offense' do
expect_offense(<<~RUBY, 'z.rb')
print 1
^ `z.rb` should match `(?i-mx:\\A[aeiou]\\z)`.
^{} `z.rb` should match `(?i-mx:\\A[aeiou]\\z)`.
RUBY
end
end
Expand Down
29 changes: 10 additions & 19 deletions spec/rubocop/cop/naming/inclusive_language_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -440,22 +440,15 @@ class Nodewhitelist
end

context 'filepath' do
let(:source) { 'print 1' }
let(:processed_source) { parse_source(source) }
let(:offenses) { _investigate(cop, processed_source) }
let(:messages) { offenses.sort.map(&:message) }

before { allow(processed_source.buffer).to receive(:name).and_return(filename) }

context 'one offense in filename' do
let(:cop_config) do
{ 'FlaggedTerms' => { 'master' => { 'Suggestions' => 'main' } } }
end
let(:filename) { '/some/dir/master.rb' }

it 'registers an offense' do
expect(offenses.size).to eq(1)
expect(messages).to eq(["Consider replacing 'master' in file path with 'main'."])
expect_offense(<<~RUBY, '/some/dir/master.rb')
^{} Consider replacing 'master' in file path with 'main'.
RUBY
end
end

Expand All @@ -466,33 +459,31 @@ class Nodewhitelist
let(:filename) { '/some/config/master-slave.rb' }

it 'registers an offense with all problematic words' do
expect(offenses.size).to eq(1)
expect(messages)
.to eq(["Consider replacing 'master', 'slave' in file path with other terms."])
expect_offense(<<~RUBY, '/some/config/master-slave.rb')
^{} Consider replacing 'master', 'slave' in file path with other terms.
RUBY
end
end

context 'offense in directory name' do
let(:cop_config) do
{ 'FlaggedTerms' => { 'master' => {} } }
end
let(:filename) { '/db/master/config.yml' }

it 'registers an offense for a director' do
expect(offenses.size).to eq(1)
expect(messages).to eq(["Consider replacing 'master' in file path with another term."])
expect_offense(<<~RUBY, '/db/master/config.yml')
^{} Consider replacing 'master' in file path with another term.
RUBY
end
end

context 'CheckFilepaths is false' do
let(:cop_config) do
{ 'CheckFilepaths' => false, 'FlaggedTerms' => { 'master' => {} } }
end
let(:filename) { '/some/dir/master.rb' }

it 'does not register an offense' do
expect(offenses.size).to eq(0)
expect(messages.empty?).to be(true)
expect_no_offenses('', '/some/dir/master.rb')
end
end
end
Expand Down

0 comments on commit 1468845

Please sign in to comment.