From 161c054b87eadfc6e9f0ea3658fe0b994cdfbdb6 Mon Sep 17 00:00:00 2001 From: Jonas Arvidsson Date: Thu, 21 May 2020 09:18:24 +0200 Subject: [PATCH] [Fix #7977] Add Layout/RedundantLineBreak cop Support method calls and assignments. Disable the cop by default, but enable it for RuboCop sources. --- .rubocop.yml | 3 + CHANGELOG.md | 4 + config/default.yml | 7 + legacy-docs/cops.md | 0 legacy-docs/cops_layout.md | 0 lib/rubocop.rb | 1 + .../cop/layout/redundant_line_break.rb | 95 +++++ .../cop/layout/redundant_line_break_spec.rb | 344 ++++++++++++++++++ 8 files changed, 454 insertions(+) create mode 100644 legacy-docs/cops.md create mode 100644 legacy-docs/cops_layout.md create mode 100644 lib/rubocop/cop/layout/redundant_line_break.rb create mode 100644 spec/rubocop/cop/layout/redundant_line_break_spec.rb diff --git a/.rubocop.yml b/.rubocop.yml index e726868692e..f4d81a2ecdd 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -47,6 +47,9 @@ Layout/EndOfLine: Layout/ClassStructure: Enabled: true +Layout/RedundantLineBreak: + Enabled: true + Layout/TrailingWhitespace: AllowInHeredoc: false diff --git a/CHANGELOG.md b/CHANGELOG.md index 4307ca826ad..378b61dcf07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## master (unreleased) +### New features + +* [#7977](https://github.com/rubocop/rubocop/issues/7977): Add `Layout/RedundantLineBreak` cop. ([@jonas054][]) + ## 1.12.1 (2021-04-04) ### Bug fixes diff --git a/config/default.yml b/config/default.yml index 4262bdba358..d29f1371265 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1101,6 +1101,13 @@ Layout/ParameterAlignment: # But it can be overridden by setting this parameter IndentationWidth: ~ +Layout/RedundantLineBreak: + Description: >- + Do not break up an expression into multiple lines when it fits + on a single line. + Enabled: false + VersionAdded: '<>' + Layout/RescueEnsureAlignment: Description: 'Align rescues and ensures correctly.' Enabled: true diff --git a/legacy-docs/cops.md b/legacy-docs/cops.md new file mode 100644 index 00000000000..e69de29bb2d diff --git a/legacy-docs/cops_layout.md b/legacy-docs/cops_layout.md new file mode 100644 index 00000000000..e69de29bb2d diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 180d4ef66c6..e8eb895d4a3 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -221,6 +221,7 @@ require_relative 'rubocop/cop/layout/multiline_method_definition_brace_layout' require_relative 'rubocop/cop/layout/multiline_operation_indentation' require_relative 'rubocop/cop/layout/parameter_alignment' +require_relative 'rubocop/cop/layout/redundant_line_break' require_relative 'rubocop/cop/layout/rescue_ensure_alignment' require_relative 'rubocop/cop/layout/space_after_colon' require_relative 'rubocop/cop/layout/space_after_comma' diff --git a/lib/rubocop/cop/layout/redundant_line_break.rb b/lib/rubocop/cop/layout/redundant_line_break.rb new file mode 100644 index 00000000000..da87196be96 --- /dev/null +++ b/lib/rubocop/cop/layout/redundant_line_break.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Layout + # This cop checks whether certain expressions, e.g. method calls, that could fit + # completely on a single line, are broken up into multiple lines unnecessarily. + # + # @example + # # bad + # foo( + # a, + # b + # ) + # + # # good + # foo(a, b) + class RedundantLineBreak < Cop + include CheckAssignment + + MSG = 'Redundant line break detected.' + + def on_send(node) # rubocop:todo Metrics/CyclomaticComplexity + # Include "the whole expression". + node = node.parent while convertible_block?(node) || + node.parent.is_a?(RuboCop::AST::BinaryOperatorNode) || + node.parent&.send_type? + + return unless offense?(node) && !part_of_ignored_node?(node) + + add_offense(node) + ignore_node(node) + end + + def check_assignment(node, _rhs) + return unless offense?(node) + + add_offense(node) + ignore_node(node) + end + + def autocorrect(node) + ->(corrector) { corrector.replace(node.source_range, to_single_line(node.source).strip) } + end + + private + + def offense?(node) + !single_line?(node) && !too_long?(node) && suitable_as_single_line?(node) + end + + def suitable_as_single_line?(node) + !comment_within?(node) && + node.each_descendant(:if, :case, :kwbegin, :def).none? && + node.each_descendant(:dstr, :str).none?(&:heredoc?) && + node.each_descendant(:begin).none? { |b| b.first_line != b.last_line } + end + + def convertible_block?(node) + return false unless node.parent&.block_type? + + send_node = node.parent&.send_node + send_node.parenthesized? || !send_node.arguments? + end + + def comment_within?(node) + processed_source.comments.map(&:loc).map(&:line).any? do |comment_line_number| + comment_line_number >= node.first_line && comment_line_number <= node.last_line + end + end + + def single_line?(node) + node.first_line == node.last_line + end + + def too_long?(node) + lines = processed_source.lines[(node.first_line - 1)...node.last_line] + to_single_line(lines.join("\n")).length > max_line_length + end + + def to_single_line(source) + 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(/\s*\\?\n\s*/, ' ') # Any other line break, with or without backslash + end + + def max_line_length + config.for_cop('Layout/LineLength')['Max'] + end + end + end + end +end diff --git a/spec/rubocop/cop/layout/redundant_line_break_spec.rb b/spec/rubocop/cop/layout/redundant_line_break_spec.rb new file mode 100644 index 00000000000..7c9ed85c046 --- /dev/null +++ b/spec/rubocop/cop/layout/redundant_line_break_spec.rb @@ -0,0 +1,344 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Layout::RedundantLineBreak, :config do + let(:config) { RuboCop::Config.new('Layout/LineLength' => { 'Max' => 31 }) } + + context 'for an expression that fits on a single line' do + it 'accepts an assignment containing an if expression' do + expect_no_offenses(<<~RUBY) + a = + if x + 1 + else + 2 + end + RUBY + end + + it 'accepts an assignment containing a case expression' do + expect_no_offenses(<<~RUBY) + a = + case x + when :a + 1 + else + 2 + end + RUBY + end + + it 'accepts a binary expression containing an if expression' do + expect_no_offenses(<<~RUBY) + a + + if x + 1 + else + 2 + end + RUBY + end + + it 'accepts a method call with a block' do + expect_no_offenses(<<~RUBY) + a do + x + y + end + RUBY + end + + it 'accepts an assignment containing a begin-end expression' do + expect_no_offenses(<<~RUBY) + a ||= begin + x + y + end + RUBY + end + + it 'accepts a method call on a single line' do + expect_no_offenses(<<~RUBY) + my_method(1, 2, "x") + RUBY + end + + it 'registers an offense for a method call on multiple lines with backslash' do + expect_offense(<<~RUBY) + my_method(1) \\ + ^^^^^^^^^^^^^^ Redundant line break detected. + [:a] + RUBY + + expect_correction(<<~RUBY) + my_method(1) [:a] + RUBY + end + + context 'with LineLength Max 100' do + let(:config) { RuboCop::Config.new('Layout/LineLength' => { 'Max' => 100 }) } + + it 'registers an offense for a method without parentheses on multiple lines' do + expect_offense(<<~RUBY) + def resolve_inheritance_from_gems(hash) + gems = hash.delete('inherit_gem') + (gems || {}).each_pair do |gem_name, config_path| + if gem_name == 'rubocop' + raise ArgumentError, + ^^^^^^^^^^^^^^^^^^^^ Redundant line break detected. + "can't inherit configuration from the rubocop gem" + end + #{' '} + hash['inherit_from'] = Array(hash['inherit_from']) + Array(config_path).reverse_each do |path| + # Put gem configuration first so local configuration overrides it. + hash['inherit_from'].unshift gem_config_path(gem_name, path) + end + end + end + RUBY + + expect_correction(<<~RUBY) + def resolve_inheritance_from_gems(hash) + gems = hash.delete('inherit_gem') + (gems || {}).each_pair do |gem_name, config_path| + if gem_name == 'rubocop' + raise ArgumentError, "can't inherit configuration from the rubocop gem" + end + #{' '} + hash['inherit_from'] = Array(hash['inherit_from']) + Array(config_path).reverse_each do |path| + # Put gem configuration first so local configuration overrides it. + hash['inherit_from'].unshift gem_config_path(gem_name, path) + end + end + end + RUBY + end + end + + it 'registers an offense for a method call on multiple lines' do + expect_offense(<<~RUBY) + my_method(1, + ^^^^^^^^^^^^ Redundant line break detected. + 2, + "x") + RUBY + + expect_correction(<<~RUBY) + my_method(1, 2, "x") + RUBY + end + + it 'accepts a method call on multiple lines if there are comments on them' do + expect_no_offenses(<<~RUBY) + my_method(1, + 2, + "x") # X + RUBY + end + + it 'registers an offense for a method call with a double quoted split string in parentheses' do + expect_offense(<<~RUBY) + my_method("a" \\ + ^^^^^^^^^^^^^^^ Redundant line break detected. + "b") + RUBY + + expect_correction(<<~RUBY) + my_method("ab") + RUBY + end + + it 'registers an offense for a method call with a double quoted split string without parentheses' do + expect_offense(<<~RUBY) + puts "(\#{pl(i)}, " \\ + ^^^^^^^^^^^^^^^^^^^^ Redundant line break detected. + "\#{pl(f)})" + RUBY + + expect_correction(<<~RUBY) + puts "(\#{pl(i)}, \#{pl(f)})" + RUBY + end + + it 'registers an offense for a method call with a single quoted split string' do + expect_offense(<<~RUBY) + my_method('a'\\ + ^^^^^^^^^^^^^^ Redundant line break detected. + 'b') + RUBY + + expect_correction(<<~RUBY) + my_method('ab') + RUBY + end + + it 'registers an offense for a method call with a double and single quoted split string' do + expect_offense(<<~RUBY) + my_method("a" \\ + ^^^^^^^^^^^^^^^ Redundant line break detected. + 'b') + my_method('a' \\ + ^^^^^^^^^^^^^^^ Redundant line break detected. + "b") + RUBY + + expect_correction(<<~RUBY) + my_method("a" + 'b') + my_method('a' + "b") + RUBY + end + + it 'registers an offense for a method call with a split operation' do + expect_offense(<<~RUBY) + my_method(1 + + ^^^^^^^^^^^^^ Redundant line break detected. + 2 + + 3) + RUBY + + expect_correction(<<~RUBY) + my_method(1 + 2 + 3) + RUBY + end + + it 'registers an offense for a method call as right hand side of an assignment' do + expect_offense(<<~RUBY) + a = + ^^^ Redundant line break detected. + m(1 + + 2 + + 3) + b = m(4 + + ^^^^^^^^^ Redundant line break detected. + 5 + + 6) + long_variable_name = + m(7 + + ^^^^^ Redundant line break detected. + 8 + + 9) + RUBY + + expect_correction(<<~RUBY) + a = m(1 + 2 + 3) + b = m(4 + 5 + 6) + long_variable_name = + m(7 + 8 + 9) + RUBY + end + end + + context 'for an expression that does not fit on a single line' do + it 'accepts a method call on a multiple lines' do + expect_no_offenses(<<~RUBY) + my_method(11111, + 22222, + "abcxyz") + my_method(111111 + + 222222 + + 333333) + RUBY + end + + context 'with a longer max line length' do + let(:config) { RuboCop::Config.new('Layout/LineLength' => { 'Max' => 82 }) } + + it 'accepts an assignment containing a method definition' do + expect_no_offenses(<<~RUBY) + VariableReference = Struct.new(:name) do + def assignment? + false + end + end + RUBY + end + + it 'accepts a method call followed by binary operations that are too long taken together' do + expect_no_offenses(<<~RUBY) + File.fnmatch?( + pattern, path, + File::FNM_PATHNAME | File::FNM_EXTGLOB + ) && a && File.basename(path).start_with?('.') && !hidden_dir?(path) + RUBY + expect_no_offenses(<<~RUBY) + File.fnmatch?( + pattern, path, + File::FNM_PATHNAME | File::FNM_EXTGLOB + ) + a + File.basename(path).start_with?('.') + !hidden_dir?(path) + RUBY + end + + it 'accepts an assignment containing a heredoc' do + expect_no_offenses(<<~RUBY) + correct = lambda do + autocorrect_source(<<~EOT1) + <<-EOT2 + foo + EOT2 + EOT1 + end + RUBY + end + + context 'for a block' do + it 'accepts when it is difficult to convert to single line' do + expect_no_offenses(<<~RUBY) + RSpec.shared_context 'ruby 2.4', :ruby24 do + let(:ruby_version) { 2.4 } + end + RUBY + end + + it 'registers an offense when the method call has parentheses' do + expect_offense(<<~RUBY) + RSpec.shared_context('ruby 2.4', :ruby24) do + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Redundant line break detected. + let(:ruby_version) { 2.4 } + end + RUBY + end + + it 'registers an offense when the method call has no argumnets' do + expect_offense(<<~RUBY) + RSpec.shared_context do + ^^^^^^^^^^^^^^^^^^^^^^^ Redundant line break detected. + let(:ruby_version) { 2.4 } + end + RUBY + end + end + + it 'accepts a complex method call on a multiple lines' do + expect_no_offenses(<<~RUBY) + node.each_node(:dstr) + .select(&:heredoc?) + .map { |n| n.loc.heredoc_body } + .flat_map { |b| (b.line...b.last_line).to_a } + RUBY + end + + it 'accepts method call with a do keyword that would just surpass the max line length' do + expect_no_offenses(<<~RUBY) + context 'when the configuration includes ' \\ + 'an unsafe cop that is 123456789012345678' do + end + RUBY + end + + it 'registers an offense for a method call with a do keyword that is just under the max line length' do + expect_offense(<<~RUBY) + context 'when the configuration includes ' \\ + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Redundant line break detected. + 'an unsafe cop that is 123456789012345' do + end + RUBY + + expect_correction(<<~RUBY) + context 'when the configuration includes an unsafe cop that is 123456789012345' do + end + RUBY + end + end + end +end