Skip to content

Commit

Permalink
Address reviewer feedback to limit what is scanned for the cop and ap…
Browse files Browse the repository at this point in the history
…ply to non-root gem files
  • Loading branch information
gregfletch committed Aug 22, 2021
1 parent da414c4 commit 591b96b
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 28 deletions.
5 changes: 5 additions & 0 deletions config/default.yml
Expand Up @@ -182,6 +182,11 @@ Bundler/GemFilename:
SupportedStyles:
- 'Gemfile'
- 'gems.rb'
Include:
- '**/Gemfile'
- '**/gems.rb'
- '**/Gemfile.lock'
- '**/gems.locked'

Bundler/GemVersion:
Description: 'Requires or forbids specifying gem versions.'
Expand Down
47 changes: 25 additions & 22 deletions lib/rubocop/cop/bundler/gem_filename.rb
Expand Up @@ -29,62 +29,65 @@ class GemFilename < Base
include ConfigurableEnforcedStyle
include RangeHelp

MSG_GEMFILE_REQUIRED = '`gems.rb` file was found but `Gemfile` is required.'
MSG_GEMS_RB_REQUIRED = '`Gemfile` was found but `gems.rb` file is required.'
MSG_GEMFILE_REQUIRED = '`gems.rb` file was found but `Gemfile` is required '\
'(file path: %<file_path>s).'
MSG_GEMS_RB_REQUIRED = '`Gemfile` was found but `gems.rb` file is required '\
'(file path: %<file_path>s).'
MSG_GEMFILE_MISMATCHED = 'Expected a `Gemfile.lock` with `Gemfile` but found '\
'`gems.locked` file.'
'`gems.locked` file (file path: %<file_path>s).'
MSG_GEMS_RB_MISMATCHED = 'Expected a `gems.locked` file with `gems.rb` but found '\
'`Gemfile.lock`.'
'`Gemfile.lock` (file path: %<file_path>s).'
GEMFILE_FILES = %w[Gemfile Gemfile.lock].freeze
GEMS_RB_FILES = %w[gems.rb gems.locked].freeze

def on_new_investigation
file_path = processed_source.file_path
return if expected_gemfile?(file_path)
basename = File.basename(file_path)
return if expected_gemfile?(basename)

register_offense(processed_source.buffer, file_path)
register_offense(file_path, basename)
end

private

def register_offense(source_buffer, file_path)
register_gemfile_offense(source_buffer, file_path) if gemfile_offense?(file_path)
register_gems_rb_offense(source_buffer, file_path) if gems_rb_offense?(file_path)
def register_offense(file_path, basename)
register_gemfile_offense(file_path, basename) if gemfile_offense?(basename)
register_gems_rb_offense(file_path, basename) if gems_rb_offense?(basename)
end

def register_gemfile_offense(source_buffer, file_path)
message = case file_path
def register_gemfile_offense(file_path, basename)
message = case basename
when 'gems.rb'
MSG_GEMFILE_REQUIRED
when 'gems.locked'
MSG_GEMFILE_MISMATCHED
end

add_offense(source_range(source_buffer, 1, 0), message: message)
add_global_offense(format(message, file_path: file_path))
end

def register_gems_rb_offense(source_buffer, file_path)
message = case file_path
def register_gems_rb_offense(file_path, basename)
message = case basename
when 'Gemfile'
MSG_GEMS_RB_REQUIRED
when 'Gemfile.lock'
MSG_GEMS_RB_MISMATCHED
end

add_offense(source_range(source_buffer, 1, 0), message: message)
add_global_offense(format(message, file_path: file_path))
end

def gemfile_offense?(file_path)
gemfile_required? && %w[gems.rb gems.locked].include?(file_path)
def gemfile_offense?(basename)
gemfile_required? && GEMS_RB_FILES.include?(basename)
end

def gems_rb_offense?(file_path)
gems_rb_required? && %w[Gemfile Gemfile.lock].include?(file_path)
def gems_rb_offense?(basename)
gems_rb_required? && GEMFILE_FILES.include?(basename)
end

def expected_gemfile?(file_path)
(gemfile_required? && GEMFILE_FILES.include?(file_path)) ||
(gems_rb_required? && GEMS_RB_FILES.include?(file_path))
def expected_gemfile?(basename)
(gemfile_required? && GEMFILE_FILES.include?(basename)) ||
(gems_rb_required? && GEMS_RB_FILES.include?(basename))
end

def gemfile_required?
Expand Down
66 changes: 60 additions & 6 deletions spec/rubocop/cop/bundler/gem_filename_spec.rb
Expand Up @@ -27,14 +27,33 @@
context 'with gems.rb file path' do
let(:filename) { 'gems.rb' }

include_examples 'invalid gem file', '`gems.rb` file was found but `Gemfile` is required.'
include_examples 'invalid gem file',
'`gems.rb` file was found but `Gemfile` is required '\
'(file path: gems.rb).'
end

context 'with non-root gems.rb file path' do
let(:filename) { 'spec/gems.rb' }

include_examples 'invalid gem file',
'`gems.rb` file was found but `Gemfile` is required '\
'(file path: spec/gems.rb).'
end

context 'with gems.locked file path' do
let(:filename) { 'gems.locked' }

include_examples 'invalid gem file',
'Expected a `Gemfile.lock` with `Gemfile` but found `gems.locked` file.'
'Expected a `Gemfile.lock` with `Gemfile` but found `gems.locked` file '\
'(file path: gems.locked).'
end

context 'with non-root gems.locked file path' do
let(:filename) { 'spec/gems.locked' }

include_examples 'invalid gem file',
'Expected a `Gemfile.lock` with `Gemfile` but found `gems.locked` file '\
'(file path: spec/gems.locked).'
end

context 'with Gemfile file path' do
Expand All @@ -43,11 +62,23 @@
include_examples 'valid gem file'
end

context 'with non-root Gemfile file path' do
let(:filename) { 'spec/Gemfile' }

include_examples 'valid gem file'
end

context 'with Gemfile.lock file path' do
let(:filename) { 'Gemfile.lock' }

include_examples 'valid gem file'
end

context 'with non-root Gemfile.lock file path' do
let(:filename) { 'spec/Gemfile.lock' }

include_examples 'valid gem file'
end
end

context 'with EnforcedStyle set to `gems.rb`' do
Expand All @@ -60,14 +91,31 @@
context 'with Gemfile file path' do
let(:filename) { 'Gemfile' }

include_examples 'invalid gem file', '`Gemfile` was found but `gems.rb` file is required.'
include_examples 'invalid gem file', '`Gemfile` was found but `gems.rb` file is required '\
'(file path: Gemfile).'
end

context 'with non-root Gemfile file path' do
let(:filename) { 'spec/Gemfile' }

include_examples 'invalid gem file', '`Gemfile` was found but `gems.rb` file is required '\
'(file path: spec/Gemfile).'
end

context 'with Gemfile.lock file path' do
let(:filename) { 'Gemfile.lock' }

include_examples 'invalid gem file',
'Expected a `gems.locked` file with `gems.rb` but found `Gemfile.lock`.'
'Expected a `gems.locked` file with `gems.rb` but found `Gemfile.lock` '\
'(file path: Gemfile.lock).'
end

context 'with non-root Gemfile.lock file path' do
let(:filename) { 'spec/Gemfile.lock' }

include_examples 'invalid gem file',
'Expected a `gems.locked` file with `gems.rb` but found `Gemfile.lock` '\
'(file path: spec/Gemfile.lock).'
end

context 'with gems.rb file path' do
Expand All @@ -76,8 +124,14 @@
include_examples 'valid gem file'
end

context 'with gems.locked file path' do
let(:filename) { 'gems.locked' }
context 'with non-root gems.rb file path' do
let(:filename) { 'spec/gems.rb' }

include_examples 'valid gem file'
end

context 'with non-root gems.locked file path' do
let(:filename) { 'spec/gems.locked' }

include_examples 'valid gem file'
end
Expand Down

0 comments on commit 591b96b

Please sign in to comment.