Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Style/Semicolon to add autocorrection #9979

Merged
merged 2 commits into from Aug 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/change_enable_basic_autocorrection_for.md
@@ -0,0 +1 @@
* [#9979](https://github.com/rubocop/rubocop/pull/9979): Enable basic autocorrection for `Style/Semicolon`. ([@dvandersluis][])
56 changes: 32 additions & 24 deletions lib/rubocop/cop/style/semicolon.rb
Expand Up @@ -32,48 +32,39 @@ class Semicolon < Base

MSG = 'Do not use semicolons to terminate expressions.'

def self.autocorrect_incompatible_with
[Style::SingleLineMethods]
end

def on_new_investigation
return if processed_source.blank?

@processed_source = processed_source

check_for_line_terminator_or_opener
end

def on_begin(node) # rubocop:todo Metrics/CyclomaticComplexity
def on_begin(node)
return if cop_config['AllowAsExpressionSeparator']

exprs = node.children

return if exprs.size < 2

# create a map matching lines to the number of expressions on them
exprs_lines = exprs.map(&:first_line)
lines = exprs_lines.group_by(&:itself)

lines.each do |line, expr_on_line|
expressions_per_line(exprs).each do |line, expr_on_line|
# Every line with more than one expression on it is a
# potential offense
next unless expr_on_line.size > 1

# TODO: Find the correct position of the semicolon. We don't know
# if the first semicolon on the line is a separator of
# expressions. It's just a guess.
column = @processed_source[line - 1].index(';')

next unless column

convention_on(line, column, false)
find_semicolon_positions(line) { |pos| register_semicolon(line, pos, true) }
end
end

private

def check_for_line_terminator_or_opener
# Make the obvious check first
return unless @processed_source.raw_source.include?(';')
return unless processed_source.raw_source.include?(';')

each_semicolon { |line, column| convention_on(line, column, true) }
each_semicolon { |line, column| register_semicolon(line, column, false) }
end

def each_semicolon
Expand All @@ -84,15 +75,32 @@ def each_semicolon
end

def tokens_for_lines
@processed_source.tokens.group_by(&:line)
processed_source.tokens.group_by(&:line)
end

def convention_on(line, column, autocorrect)
range = source_range(@processed_source.buffer, line, column)
# Don't attempt to autocorrect if semicolon is separating statements
# on the same line
def register_semicolon(line, column, after_expression)
range = source_range(processed_source.buffer, line, column)

add_offense(range) do |corrector|
corrector.remove(range) if autocorrect
if after_expression
corrector.replace(range, "\n")
else
corrector.remove(range)
end
end
end

def expressions_per_line(exprs)
# create a map matching lines to the number of expressions on them
exprs_lines = exprs.map(&:first_line)
exprs_lines.group_by(&:itself)
end

def find_semicolon_positions(line)
# Scan for all the semicolons on the line
semicolons = processed_source[line - 1].enum_for(:scan, ';')
semicolons.each do
yield Regexp.last_match.begin(0)
end
end
end
Expand Down
54 changes: 42 additions & 12 deletions spec/rubocop/cli/autocorrect_spec.rb
Expand Up @@ -902,7 +902,10 @@ def func
end

def func
x; y; rescue StandardError; z
x
y
rescue StandardError
z
end

def method
Expand Down Expand Up @@ -1285,7 +1288,8 @@ def func2() do_1; do_2; end
exit_status = cli.run(
%w[--auto-correct-all --format offenses --only] << %w[
SingleLineMethods Semicolon EmptyLineBetweenDefs
DefWithParentheses TrailingWhitespace
DefWithParentheses TrailingWhitespace TrailingBodyOnMethodDefinition
DefEndAlignment IndentationConsistency
].join(',')
)
expect(exit_status).to eq(0)
Expand All @@ -1302,13 +1306,16 @@ def func2
RUBY
expect($stdout.string).to eq(<<~RESULT)

6 Layout/TrailingWhitespace
4 Layout/TrailingWhitespace
3 Style/Semicolon
2 Layout/IndentationConsistency
2 Style/SingleLineMethods
1 Layout/DefEndAlignment
1 Layout/EmptyLineBetweenDefs
1 Style/DefWithParentheses
1 Style/TrailingBodyOnMethodDefinition
--
13 Total
15 Total

RESULT
end
Expand Down Expand Up @@ -1529,16 +1536,11 @@ def self.some_method(foo, bar: 1)
log.debug(foo)
end
RUBY
corrected = <<~RUBY
func a do b end
Signal.trap('TERM') { system(cmd); exit }
def self.some_method(foo, bar: 1)
log.debug(foo)
end
RUBY
create_file('.rubocop.yml', <<~YAML)
AllCops:
TargetRubyVersion: 2.5
Style/Semicolon:
AutoCorrect: false
YAML
create_file('example.rb', src)
exit_status = cli.run(
Expand All @@ -1547,7 +1549,7 @@ def self.some_method(foo, bar: 1)
)
expect(exit_status).to eq(1)
expect($stderr.string).to eq('')
expect(File.read('example.rb')).to eq(corrected)
expect(File.read('example.rb')).to eq(src)
expect($stdout.string).to eq(<<~RESULT)
== example.rb ==
C: 1: 8: Style/BlockDelimiters: Prefer {...} over do...end for single-line blocks.
Expand Down Expand Up @@ -2156,4 +2158,32 @@ def my_func
}
RUBY
end

it 'avoids adding extra spaces when both `Style/Semicolon` and `Style/SingleLineMethods`' \
'both apply' do
source_file = Pathname('example.rb')
create_file(source_file, <<~RUBY)
def foo(a) x(1); y(2); z(3); end
RUBY

create_file('.rubocop.yml', <<~YAML)
Style/Semicolon:
AllowAsExpressionSeparator: false
YAML

status = cli.run(
%w[--auto-correct --only] << %w[
Semicolon SingleLineMethods TrailingBodyOnMethodDefinition
DefEndAlignment TrailingWhitespace IndentationConsistency
].join(',')
)
expect(status).to eq(0)
expect(source_file.read).to eq(<<~RUBY)
def foo(a)
x(1)
y(2)
z(3)
end
RUBY
end
end
14 changes: 12 additions & 2 deletions spec/rubocop/cop/style/semicolon_spec.rb
Expand Up @@ -20,16 +20,26 @@
^ Do not use semicolons to terminate expressions.
RUBY

expect_no_corrections
expect_correction(<<~RUBY)
puts "this is a test"
puts "So is this"
RUBY
end

it 'registers an offense for one line method with two statements' do
expect_offense(<<~RUBY)
def foo(a) x(1); y(2); z(3); end
^ Do not use semicolons to terminate expressions.
^ Do not use semicolons to terminate expressions.
^ Do not use semicolons to terminate expressions.
RUBY

expect_no_corrections
expect_correction(<<~RUBY)
def foo(a) x(1)
y(2)
z(3)
end
RUBY
end

it 'accepts semicolon before end if so configured' do
Expand Down