From c3bf154a033abea040fb42c89c5eae4823b3fa7a Mon Sep 17 00:00:00 2001 From: Daniel Vandersluis Date: Fri, 24 Sep 2021 12:56:12 -0400 Subject: [PATCH 1/2] Fix `Layout/RedundantLineBreak` adding extra space within method chains. --- .../fix_fix_layoutredundantlinebreak_adding.md | 1 + lib/rubocop/cop/layout/redundant_line_break.rb | 1 + .../rubocop/cop/layout/redundant_line_break_spec.rb | 13 +++++++++++++ 3 files changed, 15 insertions(+) create mode 100644 changelog/fix_fix_layoutredundantlinebreak_adding.md diff --git a/changelog/fix_fix_layoutredundantlinebreak_adding.md b/changelog/fix_fix_layoutredundantlinebreak_adding.md new file mode 100644 index 00000000000..b3d662eb015 --- /dev/null +++ b/changelog/fix_fix_layoutredundantlinebreak_adding.md @@ -0,0 +1 @@ +* [#10124](https://github.com/rubocop/rubocop/pull/10124): Fix `Layout/RedundantLineBreak` adding extra space within method chains. ([@dvandersluis][]) diff --git a/lib/rubocop/cop/layout/redundant_line_break.rb b/lib/rubocop/cop/layout/redundant_line_break.rb index 5e1a996c818..863a67041f4 100644 --- a/lib/rubocop/cop/layout/redundant_line_break.rb +++ b/lib/rubocop/cop/layout/redundant_line_break.rb @@ -128,6 +128,7 @@ def to_single_line(source) .gsub(/' *\\\n\s*"/, %q(' + ")) # Single quote, backslash, and then double quote .gsub(/(["']) *\\\n\s*\1/, '') # Double or single quote, backslash, then same quote .gsub(/\s*\\?\n\s*/, ' ') # Any other line break, with or without backslash + .gsub(/\s+(?=\.\w)/, '') # Extra space within method chaining end def max_line_length diff --git a/spec/rubocop/cop/layout/redundant_line_break_spec.rb b/spec/rubocop/cop/layout/redundant_line_break_spec.rb index 9517322d0a0..282500690c4 100644 --- a/spec/rubocop/cop/layout/redundant_line_break_spec.rb +++ b/spec/rubocop/cop/layout/redundant_line_break_spec.rb @@ -271,6 +271,19 @@ def resolve_inheritance_from_gems(hash) m(7 + 8 + 9) RUBY end + + it 'properly corrects a method chain on multiple lines' do + expect_offense(<<~RUBY) + foo + ^^^ Redundant line break detected. + .bar + .baz + RUBY + + expect_correction(<<~RUBY) + foo.bar.baz + RUBY + end end context 'for an expression that does not fit on a single line' do From 472458dbf3bb13aa0955f3a4a149410a5abdc7f3 Mon Sep 17 00:00:00 2001 From: Daniel Vandersluis Date: Fri, 24 Sep 2021 13:02:04 -0400 Subject: [PATCH 2/2] Correct new `Layout/RedundantLineBreak` offenses. --- lib/rubocop/cop/layout/dot_position.rb | 4 +- .../cop/layout/redundant_line_break.rb | 2 +- lib/rubocop/cop/mixin/heredoc.rb | 4 +- spec/rubocop/config_loader_spec.rb | 3 +- spec/rubocop/config_spec.rb | 3 +- .../cop/layout/case_indentation_spec.rb | 3 +- .../cop/layout/redundant_line_break_spec.rb | 61 ++++++++++++++++--- spec/rubocop/options_spec.rb | 3 +- spec/rubocop/result_cache_spec.rb | 3 +- 9 files changed, 59 insertions(+), 27 deletions(-) diff --git a/lib/rubocop/cop/layout/dot_position.rb b/lib/rubocop/cop/layout/dot_position.rb index 567f9590367..27857e27119 100644 --- a/lib/rubocop/cop/layout/dot_position.rb +++ b/lib/rubocop/cop/layout/dot_position.rb @@ -109,9 +109,7 @@ def receiver_end_line(node) def last_heredoc_line(node) if node.send_type? - node.arguments.select { |arg| heredoc?(arg) } - .map { |arg| arg.loc.heredoc_end.line } - .max + node.arguments.select { |arg| heredoc?(arg) }.map { |arg| arg.loc.heredoc_end.line }.max elsif heredoc?(node) node.loc.heredoc_end.line end diff --git a/lib/rubocop/cop/layout/redundant_line_break.rb b/lib/rubocop/cop/layout/redundant_line_break.rb index 863a67041f4..b27ad68db9a 100644 --- a/lib/rubocop/cop/layout/redundant_line_break.rb +++ b/lib/rubocop/cop/layout/redundant_line_break.rb @@ -127,8 +127,8 @@ def to_single_line(source) .gsub(/" *\\\n\s*'/, %q(" + ')) # Double quote, backslash, and then single quote .gsub(/' *\\\n\s*"/, %q(' + ")) # Single quote, backslash, and then double quote .gsub(/(["']) *\\\n\s*\1/, '') # Double or single quote, backslash, then same quote + .gsub(/\n\s*(?=\.\w)/, '') # Extra space within method chaining .gsub(/\s*\\?\n\s*/, ' ') # Any other line break, with or without backslash - .gsub(/\s+(?=\.\w)/, '') # Extra space within method chaining end def max_line_length diff --git a/lib/rubocop/cop/mixin/heredoc.rb b/lib/rubocop/cop/mixin/heredoc.rb index 75f67e1a799..2ed16bb661b 100644 --- a/lib/rubocop/cop/mixin/heredoc.rb +++ b/lib/rubocop/cop/mixin/heredoc.rb @@ -21,9 +21,7 @@ def on_heredoc(_node) private def indent_level(str) - indentations = str.lines - .map { |line| line[/^\s*/] } - .reject { |line| line.end_with?("\n") } + indentations = str.lines.map { |line| line[/^\s*/] }.reject { |line| line.end_with?("\n") } indentations.empty? ? 0 : indentations.min_by(&:size).size end diff --git a/spec/rubocop/config_loader_spec.rb b/spec/rubocop/config_loader_spec.rb index 30c2044d938..a115b60fb3b 100644 --- a/spec/rubocop/config_loader_spec.rb +++ b/spec/rubocop/config_loader_spec.rb @@ -1010,8 +1010,7 @@ class Loop < Cop gem_class = Struct.new(:gem_dir) %w[gemone gemtwo].each do |gem_name| mock_spec = gem_class.new(File.join(gem_root, gem_name)) - allow(Gem::Specification).to receive(:find_by_name) - .with(gem_name).and_return(mock_spec) + allow(Gem::Specification).to receive(:find_by_name).with(gem_name).and_return(mock_spec) end allow(Gem).to receive(:path).and_return([gem_root]) end diff --git a/spec/rubocop/config_spec.rb b/spec/rubocop/config_spec.rb index f17a08d77ea..841d5c63f68 100644 --- a/spec/rubocop/config_spec.rb +++ b/spec/rubocop/config_spec.rb @@ -192,8 +192,7 @@ end it 'raises validation error' do - expect { configuration.validate } - .to raise_error(RuboCop::ValidationError, /trailing_comma/) + expect { configuration.validate }.to raise_error(RuboCop::ValidationError, /trailing_comma/) end end diff --git a/spec/rubocop/cop/layout/case_indentation_spec.rb b/spec/rubocop/cop/layout/case_indentation_spec.rb index 9fc6e90bb46..d034595bfff 100644 --- a/spec/rubocop/cop/layout/case_indentation_spec.rb +++ b/spec/rubocop/cop/layout/case_indentation_spec.rb @@ -2,8 +2,7 @@ RSpec.describe RuboCop::Cop::Layout::CaseIndentation, :config do let(:config) do - merged = RuboCop::ConfigLoader - .default_configuration['Layout/CaseIndentation'].merge(cop_config) + merged = RuboCop::ConfigLoader.default_configuration['Layout/CaseIndentation'].merge(cop_config) RuboCop::Config.new('Layout/CaseIndentation' => merged, 'Layout/IndentationWidth' => { 'Width' => 2 }) end diff --git a/spec/rubocop/cop/layout/redundant_line_break_spec.rb b/spec/rubocop/cop/layout/redundant_line_break_spec.rb index 282500690c4..b314a79c5aa 100644 --- a/spec/rubocop/cop/layout/redundant_line_break_spec.rb +++ b/spec/rubocop/cop/layout/redundant_line_break_spec.rb @@ -272,17 +272,58 @@ def resolve_inheritance_from_gems(hash) RUBY end - it 'properly corrects a method chain on multiple lines' do - expect_offense(<<~RUBY) - foo - ^^^ Redundant line break detected. - .bar - .baz - RUBY + context 'method chains' do + it 'properly corrects a method chain on multiple lines' do + expect_offense(<<~RUBY) + foo(' .x') + ^^^^^^^^^^ Redundant line break detected. + .bar + .baz + RUBY - expect_correction(<<~RUBY) - foo.bar.baz - RUBY + expect_correction(<<~RUBY) + foo(' .x').bar.baz + RUBY + end + + it 'registers an offense and corrects with a arguments on multiple lines' do + expect_offense(<<~RUBY) + foo(x, + ^^^^^^ Redundant line break detected. + y, + z) + .bar + .baz + RUBY + + expect_correction(<<~RUBY) + foo(x, y, z).bar.baz + RUBY + end + + it 'registers an offense and corrects with a string argument on multiple lines' do + expect_offense(<<~RUBY) + foo('....' \\ + ^^^^^^^^^^^^ Redundant line break detected. + '....') + .bar + .baz + RUBY + + expect_correction(<<~RUBY) + foo('........').bar.baz + RUBY + end + + it 'does not register an offense with a heredoc argument' do + expect_no_offenses(<<~RUBY) + foo(<<~EOS) + xyz + EOS + .bar + .baz + RUBY + end end end diff --git a/spec/rubocop/options_spec.rb b/spec/rubocop/options_spec.rb index 4221a6e00c1..b51eba7deb8 100644 --- a/spec/rubocop/options_spec.rb +++ b/spec/rubocop/options_spec.rb @@ -365,8 +365,7 @@ def abs(path) end it 'fails if given without --auto-gen-config' do - expect { options.parse %w[--exclude-limit 10] } - .to raise_error(RuboCop::OptionArgumentError) + expect { options.parse %w[--exclude-limit 10] }.to raise_error(RuboCop::OptionArgumentError) end end diff --git a/spec/rubocop/result_cache_spec.rb b/spec/rubocop/result_cache_spec.rb index 474d5fb0123..7900a922e2e 100644 --- a/spec/rubocop/result_cache_spec.rb +++ b/spec/rubocop/result_cache_spec.rb @@ -226,8 +226,7 @@ def abs(path) cache2.save(offenses) expect(cache2.valid?).to eq(true) - expect($stderr.string) - .not_to match(/Warning: .* is a symlink, which is not allowed.\n/) + expect($stderr.string).not_to match(/Warning: .* is a symlink, which is not allowed.\n/) end end end