Skip to content

Commit

Permalink
Optimize EmptyLineAfterSig
Browse files Browse the repository at this point in the history
The previous implementation would traverse the tokens in the file each
time it would analyze a `sig` until it found the corresponding method
definition. This would take additional time the further we got into a
file, as we would have to re-traverse all tokens so far.

This new implementation simply looks for the next node after the `sig`,
confirms it is the method definition, and calculates offsets for the
lines in between.
  • Loading branch information
sambostock committed Oct 16, 2023
1 parent fd1e1e0 commit 4f74f27
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 25 deletions.
61 changes: 41 additions & 20 deletions lib/rubocop/cop/sorbet/signatures/empty_line_after_sig.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@ module Cop
module Sorbet
# Checks for blank lines after signatures.
#
# It also suggests an autocorrect
#
# @example
#
# # bad
# sig { void }
#
Expand All @@ -17,35 +14,59 @@ module Sorbet
# # good
# sig { void }
# def foo; end
#
class EmptyLineAfterSig < ::RuboCop::Cop::Base
extend AutoCorrector
include RangeHelp
include SignatureHelp

def on_signature(node)
if (next_method(node).line - node.last_line) > 1
location = source_range(processed_source.buffer, next_method(node).line - 1, 0)
add_offense(location, message: "Extra empty line or comment detected") do |corrector|
offending_range = node.source_range.with(
begin_pos: node.source_range.end_pos + 1,
end_pos: processed_source.buffer.line_range(next_method(node).line).begin_pos,
MSG = "Extra empty line or comment detected"

# @!method signable_method_definition?(node)
def_node_matcher :signable_method_definition?, <<~PATTERN
${
def
defs
(send nil? {:attr_reader :attr_writer :attr_accessor} ...)
}
PATTERN

def on_signature(sig)
signable_method_definition?(next_sibling(sig)) do |definition|
range = lines_between(sig, definition)
next if range.empty? || range.single_line?

add_offense(range) do |corrector|
corrector.insert_before(
range_by_whole_lines(sig.source_range),
range.source
.sub(/\A\n+/, "") # remove initial newline(s)
.gsub(/\n{2,}/, "\n"), # remove empty line(s)
)
corrector.remove(offending_range)
clean_range = offending_range.source.split("\n").reject(&:empty?).join("\n")
offending_line = processed_source.buffer.line_range(node.source_range.first_line)
corrector.insert_before(offending_line, "#{clean_range}\n") unless clean_range.empty?
corrector.remove(range)
end
end
end

private

def next_method(node)
processed_source.tokens.find do |t|
t.line >= node.last_line &&
(t.type == :kDEF || t.text.start_with?("attr_"))
end
def next_sibling(node)
node.parent&.children&.at(node.sibling_index + 1)
end

def lines_between(node1, node2, buffer: processed_source.buffer)
end_of_node1_pos = node1.source_range.end_pos
start_of_node2_pos = node2.source_range.begin_pos

string_in_between = buffer.slice(end_of_node1_pos...start_of_node2_pos)
# Fallbacks handle same line edge case
begin_offset = string_in_between.index("\n") || 0
end_offset = string_in_between.rindex("\n") || string_in_between.length - 1

Parser::Source::Range.new(
buffer,
end_of_node1_pos + begin_offset + 1, # +1 to exclude post-node1 newline
end_of_node1_pos + end_offset + 1, # +1 to include pre-node2 newline
)
end
end
end
Expand Down
2 changes: 0 additions & 2 deletions manual/cops_sorbet.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,6 @@ Enabled | Yes | Yes | 0.7.0 | -

Checks for blank lines after signatures.

It also suggests an autocorrect

### Examples

```ruby
Expand Down
12 changes: 9 additions & 3 deletions spec/rubocop/cop/sorbet/signatures/empty_line_after_sig_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ module Example
).void
end
# Session: string
^ Extra empty line or comment detected
^^^^^^^^^^^^^^^^^^^ Extra empty line or comment detected
def initialize(
session:
)
Expand Down Expand Up @@ -153,14 +153,14 @@ def initialize(
expect_offense(<<~RUBY)
sig { params(session: String).void }
^{} Extra empty line or comment detected
# Session: string
# More stuff
# on more lines
^{} Extra empty line or comment detected
def initialize(session:)
@session = session
end
Expand All @@ -181,7 +181,7 @@ def initialize(session:)
expect_offense(<<~RUBY)
true; sig { void }
# Comment
^ Extra empty line or comment detected
^^^^^^^^^ Extra empty line or comment detected
def m; end
RUBY

Expand All @@ -193,6 +193,12 @@ def m; end
end
end

it "registers no offense when there is only a sig" do
expect_no_offenses(<<~RUBY)
sig { void }
RUBY
end

it "registers no offense when there is only a method definition" do
expect_no_offenses(<<~RUBY)
def foo; end
Expand Down

0 comments on commit 4f74f27

Please sign in to comment.