diff --git a/changelog/change_enable_basic_autocorrection_for.md b/changelog/change_enable_basic_autocorrection_for.md new file mode 100644 index 00000000000..0482654c19a --- /dev/null +++ b/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][]) diff --git a/lib/rubocop/cop/style/semicolon.rb b/lib/rubocop/cop/style/semicolon.rb index 99a4f8820f3..b6d101e98ac 100644 --- a/lib/rubocop/cop/style/semicolon.rb +++ b/lib/rubocop/cop/style/semicolon.rb @@ -32,38 +32,29 @@ 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 @@ -71,9 +62,9 @@ def on_begin(node) # rubocop:todo Metrics/CyclomaticComplexity 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 @@ -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 diff --git a/spec/rubocop/cli/autocorrect_spec.rb b/spec/rubocop/cli/autocorrect_spec.rb index 0ffc39c3100..92f0f6de26e 100644 --- a/spec/rubocop/cli/autocorrect_spec.rb +++ b/spec/rubocop/cli/autocorrect_spec.rb @@ -902,7 +902,10 @@ def func end def func - x; y; rescue StandardError; z + x + y + rescue StandardError + z end def method @@ -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) @@ -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 @@ -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( @@ -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. @@ -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 diff --git a/spec/rubocop/cop/style/semicolon_spec.rb b/spec/rubocop/cop/style/semicolon_spec.rb index 7eeb73e6bae..b29ca9e6c41 100644 --- a/spec/rubocop/cop/style/semicolon_spec.rb +++ b/spec/rubocop/cop/style/semicolon_spec.rb @@ -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