Skip to content

Commit

Permalink
update PatchCop to support spanning errors
Browse files Browse the repository at this point in the history
We want to keep an error if it _includes_ any lines in a given patch. We also
want only the first such line, since it likely includes more than one (this
singularism was implicit before, because each line was only passed through once)

Also factor out some private methods to simplify the original flat_map logic.
  • Loading branch information
nevinera authored Jan 17, 2023
1 parent fd3041d commit 6e1703c
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 7 deletions.
27 changes: 21 additions & 6 deletions lib/pronto/rubocop/patch_cop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,9 @@ def initialize(patch, runner)
def messages
return [] unless valid?

offenses.flat_map do |offense|
patch
.added_lines
.select { |line| line.new_lineno == offense.line }
.map { |line| OffenseLine.new(self, offense, line).message }
end
offenses
.map { |offense| first_relevant_message(patch, offense) }
.compact
end

def processed_source
Expand Down Expand Up @@ -67,6 +64,11 @@ def offenses
.reject(&:disabled?)
end

def offense_includes?(offense, line_number)
offense_range = (offense.location.first_line..offense.location.last_line)
offense_range.include?(line_number)
end

def team
@team ||=
if ::RuboCop::Cop::Team.respond_to?(:mobilize)
Expand All @@ -76,6 +78,19 @@ def team
::RuboCop::Cop::Team.new(registry, rubocop_config)
end
end

def first_relevant_line(patch, offense)
patch.added_lines.detect do |line|
offense_includes?(offense, line.new_lineno)
end
end

def first_relevant_message(patch, offense)
offending_line = first_relevant_line(patch, offense)
return nil unless offending_line

OffenseLine.new(self, offense, offending_line).message
end
end
end
end
11 changes: 10 additions & 1 deletion spec/pronto/rubocop/patch_cop_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
let(:ast) { double :ast, each_node: nil }
let(:processed_source) { double :processed_source, ast: ast }
let(:team) { double :team, inspect_file: [offense] }
let(:offense) { double :offense, disabled?: false, line: 42 }
let(:offense_location) { double :location, first_line: 42, last_line: 43 }
let(:offense) { double :offense, disabled?: false, location: offense_location }
let(:offense_line) { double :offense_line, message: 'Err' }

before do
Expand All @@ -31,5 +32,13 @@
it do
expect(patch_cop.messages).to eq(['Err'])
end

context 'when there is an error including the patch, but not starting inside it' do
let(:offense_location) { double :location, first_line: 40, last_line: 43 }

it do
expect(patch_cop.messages).to eq(['Err'])
end
end
end
end

0 comments on commit 6e1703c

Please sign in to comment.