From 22d5d6ae632e09a430695379a5b77390f691d076 Mon Sep 17 00:00:00 2001 From: Dylan Thacker-Smith Date: Wed, 27 Nov 2019 18:04:35 -0500 Subject: [PATCH] [Fix #6918] Add support for variable alignment to Layout/RescueEnsureAlignment --- CHANGELOG.md | 1 + config/default.yml | 4 + docs/modules/ROOT/pages/cops_layout.adoc | 44 +++++- .../cop/layout/rescue_ensure_alignment.rb | 40 +++++- .../layout/rescue_ensure_alignment_spec.rb | 132 +++++++++++------- 5 files changed, 165 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 118d5226254..bbb36b5455a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * [#8164](https://github.com/rubocop-hq/rubocop/pull/8164): Support auto-correction for `Lint/InterpolationCheck`. ([@koic][]) * [#8223](https://github.com/rubocop-hq/rubocop/pull/8223): Support auto-correction for `Style/IfUnlessModifierOfIfUnless`. ([@koic][]) * [#8172](https://github.com/rubocop-hq/rubocop/pull/8172): Support auto-correction for `Lint/SafeNavigationWithEmpty`. ([@koic][]) +* [#7531](https://github.com/rubocop-hq/rubocop/pull/7531): Add support for variable alignment to Layout/RescueEnsureAlignment. ([@dylanahsmith][]) ### Bug fixes diff --git a/config/default.yml b/config/default.yml index 4b3af5b554c..6f769e7e84c 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1059,6 +1059,10 @@ Layout/ParameterAlignment: Layout/RescueEnsureAlignment: Description: 'Align rescues and ensures correctly.' + EnforcedStyleAlignWith: keyword + SupportedStylesAlignWith: + - keyword + - variable Enabled: true VersionAdded: '0.49' diff --git a/docs/modules/ROOT/pages/cops_layout.adoc b/docs/modules/ROOT/pages/cops_layout.adoc index 3d6aaa0060b..7d06b161dfd 100644 --- a/docs/modules/ROOT/pages/cops_layout.adoc +++ b/docs/modules/ROOT/pages/cops_layout.adoc @@ -4757,25 +4757,65 @@ end This cop checks whether the rescue and ensure keywords are aligned properly. +Two modes are supported through the `EnforcedStyleAlignWith` +configuration parameter: + +If it's set to `keyword` (which is the default), the `rescue` +shall be aligned with the start of the begin keyword. + +If it's set to `variable` the `rescue` shall be aligned with the +left-hand-side of the variable assignment, if there is one. + === Examples +==== EnforcedStyleAlignWith: keyword (default) + [source,ruby] ---- # bad -begin +result = begin + something + rescue + puts 'error' + end + +# good +result = begin + something + rescue + puts 'error' + end +---- + +==== EnforcedStyleAlignWith: variable + +[source,ruby] +---- +# bad +result = begin something rescue puts 'error' end # good -begin +result = begin something rescue puts 'error' end ---- +=== Configurable attributes + +|=== +| Name | Default value | Configurable values + +| EnforcedStyleAlignWith +| `keyword` +| `keyword`, `variable` +|=== + == Layout/SpaceAfterColon |=== diff --git a/lib/rubocop/cop/layout/rescue_ensure_alignment.rb b/lib/rubocop/cop/layout/rescue_ensure_alignment.rb index 4932d7d5433..ca8f3ce5867 100644 --- a/lib/rubocop/cop/layout/rescue_ensure_alignment.rb +++ b/lib/rubocop/cop/layout/rescue_ensure_alignment.rb @@ -6,22 +6,48 @@ module Layout # This cop checks whether the rescue and ensure keywords are aligned # properly. # - # @example + # Two modes are supported through the `EnforcedStyleAlignWith` + # configuration parameter: + # + # If it's set to `keyword` (which is the default), the `rescue` + # shall be aligned with the start of the begin keyword. + # + # If it's set to `variable` the `rescue` shall be aligned with the + # left-hand-side of the variable assignment, if there is one. + # + # @example EnforcedStyleAlignWith: keyword (default) + # + # # bad + # result = begin + # something + # rescue + # puts 'error' + # end + # + # # good + # result = begin + # something + # rescue + # puts 'error' + # end + # + # @example EnforcedStyleAlignWith: variable # # # bad - # begin + # result = begin # something # rescue # puts 'error' # end # # # good - # begin + # result = begin # something # rescue # puts 'error' # end class RescueEnsureAlignment < Cop + include ConfigurableEnforcedStyle include RangeHelp MSG = '`%s` at %d, %d is not ' \ @@ -41,6 +67,10 @@ def on_ensure(node) check(node) end + def style_parameter_name + 'EnforcedStyleAlignWith' + end + def autocorrect(node) whitespace = whitespace_range(node) # Some inline node is sitting before current node. @@ -122,8 +152,8 @@ def alignment_source(node, starting_loc) def alignment_node(node) ancestor_node = ancestor_node(node) - return ancestor_node if ancestor_node.nil? || - ancestor_node.kwbegin_type? + return ancestor_node if ancestor_node.nil? + return ancestor_node if style == :keyword && ancestor_node.kwbegin_type? assignment_node = assignment_node(ancestor_node) return assignment_node if same_line?(ancestor_node, assignment_node) diff --git a/spec/rubocop/cop/layout/rescue_ensure_alignment_spec.rb b/spec/rubocop/cop/layout/rescue_ensure_alignment_spec.rb index 0c2c7961175..d6f004122b3 100644 --- a/spec/rubocop/cop/layout/rescue_ensure_alignment_spec.rb +++ b/spec/rubocop/cop/layout/rescue_ensure_alignment_spec.rb @@ -1,62 +1,106 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Layout::RescueEnsureAlignment, :config do + let(:cop_config) do + { 'EnforcedStyleAlignWith' => 'keyword', 'AutoCorrect' => true } + end + it 'accepts the modifier form' do expect_no_offenses('test rescue nil') end - context 'rescue with begin' do - it 'registers an offense' do - expect_offense(<<~RUBY) - begin - something - rescue - ^^^^^^ `rescue` at 3, 4 is not aligned with `begin` at 1, 0. - error - end + it 'rescue not aligned with begin registers an offense' do + expect_offense(<<~RUBY) + begin + something + rescue + ^^^^^^ `rescue` at 3, 4 is not aligned with `begin` at 1, 0. + error + end + RUBY + + expect_correction(<<~RUBY) + begin + something + rescue + error + end + RUBY + end + + context 'on assignment with keyword style' do + let(:cop_config) do + { 'EnforcedStyleAlignWith' => 'keyword', 'AutoCorrect' => true } + end + + it 'accepts multi-line, aligned with keyword' do + expect_no_offenses(<<~RUBY) + x ||= begin + 1 + rescue + 2 + end RUBY + end - expect_correction(<<~RUBY) - begin - something + it 'accepts multi-line, indented' do + expect_no_offenses(<<~RUBY) + x ||= + begin + 1 + rescue + 2 + end + RUBY + end + + it 'registers offense for incorrect alignment' do + expect_offense(<<~RUBY) + x ||= begin + 1 rescue - error + ^^^^^^ `rescue` at 3, 0 is not aligned with `begin` at 1, 6. + 2 end RUBY end + end - context 'as RHS of assignment' do - it 'accepts multi-line, aligned' do - expect_no_offenses(<<~RUBY) - x ||= begin - 1 - rescue - 2 - end - RUBY - end + context 'on assignment with variable style' do + let(:cop_config) do + { 'EnforcedStyleAlignWith' => 'variable', 'AutoCorrect' => true } + end - it 'accepts multi-line, indented' do - expect_no_offenses(<<~RUBY) - x ||= - begin - 1 - rescue - 2 - end - RUBY - end + it 'accepts multi-line, aligned with variable' do + expect_no_offenses(<<~RUBY) + x ||= begin + 1 + rescue + 2 + end + RUBY + end - it 'registers offense for incorrect alignment' do - expect_offense(<<~RUBY) - x ||= begin + it 'registers offense for multi-line, indented' do + expect_no_offenses(<<~RUBY) + x ||= + begin 1 rescue - ^^^^^^ `rescue` at 3, 0 is not aligned with `begin` at 1, 6. 2 end - RUBY - end + RUBY + end + + it 'registers offense for keyword alignment' do + expect_offense(<<~RUBY) + x ||= begin + 1 + rescue + ^^^^^^ `rescue` at 3, 6 is not aligned with `x` at 1, 0. + 2 + end + RUBY end end @@ -286,16 +330,6 @@ def method2 RUBY end - it 'accepts correctly aligned rescue in assigned begin-end block' do - expect_no_offenses(<<-RUBY) - foo = begin - bar - rescue BazError - qux - end - RUBY - end - context '>= Ruby 2.5', :ruby25 do it 'accepts aligned rescue in do-end block' do expect_no_offenses(<<~RUBY)