diff --git a/CHANGELOG.md b/CHANGELOG.md index de739cfefb0..9756572af66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### New features * New option `--cache-root` and support for the `RUBOCOP_CACHE_ROOT` environment variable. Both can be used to override the `AllCops: CacheRootDirectory` config, especially in a CI setting. ([@sascha-wolf][]) +* [#8582](https://github.com/rubocop-hq/rubocop/issues/8582): Add new `Layout/BeginEndAlignment` cop. ([@koic][]) ### Bug fixes diff --git a/config/default.yml b/config/default.yml index ddcaa3ddc05..cb362fcd2e8 100644 --- a/config/default.yml +++ b/config/default.yml @@ -311,6 +311,19 @@ Layout/AssignmentIndentation: # But it can be overridden by setting this parameter IndentationWidth: ~ +Layout/BeginEndAlignment: + Description: 'Align ends corresponding to begins correctly.' + Enabled: pending + VersionAdded: '0.91' + # The value `start_of_line` means that `end` should be aligned the start of the line + # where the `begin` keyword is. + # The value `begin` means that `end` should be aligned with the `begin` keyword. + EnforcedStyleAlignWith: start_of_line + SupportedStylesAlignWith: + - start_of_line + - begin + Severity: warning + Layout/BlockAlignment: Description: 'Align block ends correctly.' Enabled: true diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 549891f6004..03add164fdb 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -91,6 +91,7 @@ In the following section you find all available cops: * xref:cops_layout.adoc#layoutargumentalignment[Layout/ArgumentAlignment] * xref:cops_layout.adoc#layoutarrayalignment[Layout/ArrayAlignment] * xref:cops_layout.adoc#layoutassignmentindentation[Layout/AssignmentIndentation] +* xref:cops_layout.adoc#layoutbeginendalignment[Layout/BeginEndAlignment] * xref:cops_layout.adoc#layoutblockalignment[Layout/BlockAlignment] * xref:cops_layout.adoc#layoutblockendnewline[Layout/BlockEndNewline] * xref:cops_layout.adoc#layoutcaseindentation[Layout/CaseIndentation] diff --git a/docs/modules/ROOT/pages/cops_layout.adoc b/docs/modules/ROOT/pages/cops_layout.adoc index 1f7627a61c2..747910d30e2 100644 --- a/docs/modules/ROOT/pages/cops_layout.adoc +++ b/docs/modules/ROOT/pages/cops_layout.adoc @@ -261,6 +261,77 @@ value = | Integer |=== +== Layout/BeginEndAlignment + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| Yes +| 0.91 +| - +|=== + +This cop checks whether the end keyword of `begin` is aligned properly. + +Two modes are supported through the `EnforcedStyleAlignWith` configuration +parameter. If it's set to `start_of_line` (which is the default), the +`end` shall be aligned with the start of the line where the `begin` +keyword is. If it's set to `begin`, the `end` shall be aligned with the +`begin` keyword. + +`Layout/EndAlignment` cop aligns with keywords (e.g. `if`, `while`, `case`) +by default. On the other hand, `||= begin` that this cop targets tends to +align with the start of the line, it defaults to `EnforcedStyleAlignWith: start_of_line`. +These style can be configured by each cop. + +=== Examples + +==== EnforcedStyleAlignWith: start_of_line (default) + +[source,ruby] +---- +# bad +foo ||= begin + do_something + end + +# good +foo ||= begin + do_something +end +---- + +==== EnforcedStyleAlignWith: begin + +[source,ruby] +---- +# bad +foo ||= begin + do_something +end + +# good +foo ||= begin + do_something + end +---- + +=== Configurable attributes + +|=== +| Name | Default value | Configurable values + +| EnforcedStyleAlignWith +| `start_of_line` +| `start_of_line`, `begin` + +| Severity +| `warning` +| String +|=== + == Layout/BlockAlignment |=== @@ -2064,6 +2135,11 @@ left-hand-side of the variable assignment, if there is one. If it's set to `start_of_line`, the `end` shall be aligned with the start of the line where the matching keyword appears. +This `Layout/EndAlignment` cop aligns with keywords (e.g. `if`, `while`, `case`) +by default. On the other hand, `Layout/BeginEndAlignment` cop aligns with +`EnforcedStyleAlignWith: start_of_line` by default due to `||= begin` tends +to align with the start of the line. These style can be configured by each cop. + === Examples ==== EnforcedStyleAlignWith: keyword (default) diff --git a/lib/rubocop.rb b/lib/rubocop.rb index a67bcf8b6d6..52147beac19 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -155,6 +155,7 @@ require_relative 'rubocop/cop/layout/argument_alignment' require_relative 'rubocop/cop/layout/array_alignment' require_relative 'rubocop/cop/layout/assignment_indentation' +require_relative 'rubocop/cop/layout/begin_end_alignment' require_relative 'rubocop/cop/layout/block_alignment' require_relative 'rubocop/cop/layout/block_end_newline' require_relative 'rubocop/cop/layout/case_indentation' diff --git a/lib/rubocop/config_loader.rb b/lib/rubocop/config_loader.rb index 27e13106c55..77d15ee3211 100644 --- a/lib/rubocop/config_loader.rb +++ b/lib/rubocop/config_loader.rb @@ -129,9 +129,9 @@ def add_excludes_from_files(config, config_file) def default_configuration @default_configuration ||= begin - print 'Default ' if debug? - load_file(DEFAULT_FILE) - end + print 'Default ' if debug? + load_file(DEFAULT_FILE) + end end # Returns the path rubocop inferred as the root of the project. No file diff --git a/lib/rubocop/config_store.rb b/lib/rubocop/config_store.rb index 018e7b0c949..7c01f7b58ee 100644 --- a/lib/rubocop/config_store.rb +++ b/lib/rubocop/config_store.rb @@ -54,9 +54,9 @@ def for_dir(dir) @path_cache[dir] ||= ConfigLoader.configuration_file_for(dir) path = @path_cache[dir] @object_cache[path] ||= begin - print "For #{dir}: " if ConfigLoader.debug? - ConfigLoader.configuration_from_file(path) - end + print "For #{dir}: " if ConfigLoader.debug? + ConfigLoader.configuration_from_file(path) + end end end end diff --git a/lib/rubocop/cop/layout/begin_end_alignment.rb b/lib/rubocop/cop/layout/begin_end_alignment.rb new file mode 100644 index 00000000000..ce0750ee86b --- /dev/null +++ b/lib/rubocop/cop/layout/begin_end_alignment.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Layout + # This cop checks whether the end keyword of `begin` is aligned properly. + # + # Two modes are supported through the `EnforcedStyleAlignWith` configuration + # parameter. If it's set to `start_of_line` (which is the default), the + # `end` shall be aligned with the start of the line where the `begin` + # keyword is. If it's set to `begin`, the `end` shall be aligned with the + # `begin` keyword. + # + # `Layout/EndAlignment` cop aligns with keywords (e.g. `if`, `while`, `case`) + # by default. On the other hand, `||= begin` that this cop targets tends to + # align with the start of the line, it defaults to `EnforcedStyleAlignWith: start_of_line`. + # These style can be configured by each cop. + # + # @example EnforcedStyleAlignWith: start_of_line (default) + # # bad + # foo ||= begin + # do_something + # end + # + # # good + # foo ||= begin + # do_something + # end + # + # @example EnforcedStyleAlignWith: begin + # # bad + # foo ||= begin + # do_something + # end + # + # # good + # foo ||= begin + # do_something + # end + # + class BeginEndAlignment < Base + include EndKeywordAlignment + include RangeHelp + extend AutoCorrector + + MSG = '`end` at %d, %d is not aligned with `%s` at %d, %d.' + + def on_kwbegin(node) + check_begin_alignment(node) + end + + private + + def check_begin_alignment(node) + align_with = { + begin: node.loc.begin, + start_of_line: start_line_range(node) + } + check_end_kw_alignment(node, align_with) + end + + def autocorrect(corrector, node) + AlignmentCorrector.align_end(corrector, processed_source, node, alignment_node(node)) + end + + def alignment_node(node) + case style + when :begin + node + else + start_line_range(node) + end + end + end + end + end +end diff --git a/lib/rubocop/cop/layout/end_alignment.rb b/lib/rubocop/cop/layout/end_alignment.rb index 2c073c77c6e..4c850705d1f 100644 --- a/lib/rubocop/cop/layout/end_alignment.rb +++ b/lib/rubocop/cop/layout/end_alignment.rb @@ -17,6 +17,11 @@ module Layout # If it's set to `start_of_line`, the `end` shall be aligned with the # start of the line where the matching keyword appears. # + # This `Layout/EndAlignment` cop aligns with keywords (e.g. `if`, `while`, `case`) + # by default. On the other hand, `Layout/BeginEndAlignment` cop aligns with + # `EnforcedStyleAlignWith: start_of_line` by default due to `||= begin` tends + # to align with the start of the line. These style can be configured by each cop. + # # @example EnforcedStyleAlignWith: keyword (default) # # bad # @@ -173,16 +178,6 @@ def alignment_node_for_variable_style(node) node end end - - def start_line_range(node) - expr = node.source_range - buffer = expr.source_buffer - source = buffer.source_line(expr.line) - range = buffer.line_range(expr.line) - - range_between(range.begin_pos + (source =~ /\S/), - range.begin_pos + (source =~ /\s*\z/)) - end end end end diff --git a/lib/rubocop/cop/layout/rescue_ensure_alignment.rb b/lib/rubocop/cop/layout/rescue_ensure_alignment.rb index ad0889bbc20..cae37dc5a75 100644 --- a/lib/rubocop/cop/layout/rescue_ensure_alignment.rb +++ b/lib/rubocop/cop/layout/rescue_ensure_alignment.rb @@ -23,6 +23,7 @@ module Layout # end class RescueEnsureAlignment < Base include RangeHelp + include EndKeywordAlignment extend AutoCorrector MSG = '`%s` at %d, %d is not ' \ @@ -59,7 +60,7 @@ def check(node) alignment_node = alignment_node(node) return if alignment_node.nil? - alignment_loc = alignment_node.loc.expression + alignment_loc = alignment_location(alignment_node) kw_loc = node.loc.keyword return if alignment_loc.column == kw_loc.column || alignment_loc.line == kw_loc.line @@ -67,16 +68,16 @@ def check(node) add_offense( kw_loc, message: format_message(alignment_node, alignment_loc, kw_loc) ) do |corrector| - autocorrect(corrector, node, alignment_node) + autocorrect(corrector, node, alignment_loc) end end - def autocorrect(corrector, node, alignment_node) + def autocorrect(corrector, node, alignment_location) whitespace = whitespace_range(node) # Some inline node is sitting before current node. return nil unless whitespace.source.strip.empty? - new_column = alignment_node.loc.column + new_column = alignment_location.column corrector.replace(whitespace, ' ' * new_column) end @@ -180,6 +181,18 @@ def access_modifier?(node) false end + + def alignment_location(alignment_node) + if begin_end_alignment_style == 'start_of_line' + start_line_range(alignment_node) + else + alignment_node.loc.expression + end + end + + def begin_end_alignment_style + config.for_cop('Layout/BeginEndAlignment')['EnforcedStyleAlignWith'] + end end end end diff --git a/lib/rubocop/cop/mixin/end_keyword_alignment.rb b/lib/rubocop/cop/mixin/end_keyword_alignment.rb index 14523d414bb..84b0aa8d7cb 100644 --- a/lib/rubocop/cop/mixin/end_keyword_alignment.rb +++ b/lib/rubocop/cop/mixin/end_keyword_alignment.rb @@ -39,6 +39,15 @@ def matching_ranges(end_loc, align_ranges) end end + def start_line_range(node) + expr = node.source_range + buffer = expr.source_buffer + source = buffer.source_line(expr.line) + range = buffer.line_range(expr.line) + + range_between(range.begin_pos + (source =~ /\S/), range.begin_pos + (source =~ /\s*\z/)) + end + def add_offense_for_misalignment(node, align_with) end_loc = node.loc.end msg = format(MSG, end_line: end_loc.line, diff --git a/lib/rubocop/ext/regexp_node.rb b/lib/rubocop/ext/regexp_node.rb index 3c8ede20f42..a4b37240609 100644 --- a/lib/rubocop/ext/regexp_node.rb +++ b/lib/rubocop/ext/regexp_node.rb @@ -21,10 +21,10 @@ def parsed_tree str = content Ext::RegexpNode.parsed_cache[str] ||= begin - Regexp::Parser.parse(str) - rescue StandardError - nil - end + Regexp::Parser.parse(str) + rescue StandardError + nil + end end def each_capture(named: ANY) diff --git a/spec/rubocop/cop/layout/begin_end_alignment_spec.rb b/spec/rubocop/cop/layout/begin_end_alignment_spec.rb new file mode 100644 index 00000000000..9bbcadd052f --- /dev/null +++ b/spec/rubocop/cop/layout/begin_end_alignment_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Layout::BeginEndAlignment, :config do + let(:cop_config) do + { 'EnforcedStyleAlignWith' => 'begin' } + end + + include_examples 'aligned', 'begin', '', 'end' + + include_examples 'misaligned', <<~RUBY, false + begin + end + ^^^ `end` at 2, 2 is not aligned with `begin` at 1, 0. + RUBY + + include_examples 'aligned', 'puts 1; begin', '', ' end' + + context 'when EnforcedStyleAlignWith is start_of_line' do + let(:cop_config) do + { 'EnforcedStyleAlignWith' => 'start_of_line' } + end + + include_examples 'aligned', 'puts 1; begin', '', 'end' + + include_examples 'misaligned', <<~RUBY, false + begin + end + ^^^ `end` at 2, 2 is not aligned with `begin` at 1, 0. + RUBY + + include_examples 'misaligned', <<~RUBY, :begin + var << begin + end + ^^^ `end` at 2, 7 is not aligned with `var << begin` at 1, 0. + RUBY + + include_examples 'aligned', 'var = begin', '', 'end' + end +end diff --git a/spec/rubocop/cop/layout/rescue_ensure_alignment_spec.rb b/spec/rubocop/cop/layout/rescue_ensure_alignment_spec.rb index 0c2c7961175..87ae2fd6cc4 100644 --- a/spec/rubocop/cop/layout/rescue_ensure_alignment_spec.rb +++ b/spec/rubocop/cop/layout/rescue_ensure_alignment_spec.rb @@ -26,36 +26,106 @@ end context 'as RHS of assignment' do - it 'accepts multi-line, aligned' do - expect_no_offenses(<<~RUBY) - x ||= begin - 1 - rescue - 2 - end - RUBY + let(:cop_config) do + { 'EnforcedStyle' => 'require_parentheses' } end - it 'accepts multi-line, indented' do - expect_no_offenses(<<~RUBY) - x ||= - begin + context 'when `EnforcedStyleAlignWith: start_of_line` of `Layout/BeginEndAlignment` cop' do + let(:other_cops) do + { + 'Layout/BeginEndAlignment' => { 'EnforcedStyleAlignWith' => 'start_of_line' } + } + end + + it 'accepts multi-line, aligned' do + expect_no_offenses(<<~RUBY) + x ||= begin 1 rescue 2 end - RUBY + RUBY + end + + it 'accepts multi-line, indented' do + expect_no_offenses(<<~RUBY) + x ||= + begin + 1 + rescue + 2 + end + RUBY + end + + it 'registers an offense and corrects for incorrect alignment' do + expect_offense(<<~RUBY) + x ||= begin + 1 + rescue + ^^^^^^ `rescue` at 3, 6 is not aligned with `x ||= begin` at 1, 0. + 2 + end + RUBY + + # Except for `rescue`, it will be aligned by `Layout/BeginEndAlignment` auto-correction. + expect_correction(<<~RUBY) + x ||= begin + 1 + rescue + 2 + end + RUBY + end end - it 'registers offense for incorrect alignment' do - expect_offense(<<~RUBY) - x ||= begin - 1 - rescue - ^^^^^^ `rescue` at 3, 0 is not aligned with `begin` at 1, 6. - 2 - end - RUBY + context 'when `EnforcedStyleAlignWith: begin` of `Layout/BeginEndAlignment` cop' do + let(:other_cops) do + { + 'Layout/BeginEndAlignment' => { 'EnforcedStyleAlignWith' => 'begin' } + } + end + + it 'accepts multi-line, aligned' do + expect_no_offenses(<<~RUBY) + x ||= begin + 1 + rescue + 2 + end + RUBY + end + + it 'accepts multi-line, indented' do + expect_no_offenses(<<~RUBY) + x ||= + begin + 1 + rescue + 2 + end + RUBY + end + + it 'registers an offense and corrects for incorrect alignment' do + expect_offense(<<~RUBY) + x ||= begin + 1 + rescue + ^^^^^^ `rescue` at 3, 0 is not aligned with `begin` at 1, 6. + 2 + end + RUBY + + # Except for `rescue`, it will be aligned by `Layout/BeginEndAlignment` auto-correction. + expect_correction(<<~RUBY) + x ||= begin + 1 + rescue + 2 + end + RUBY + end end end end