Skip to content

Commit

Permalink
[Fix rubocop#8582] Add new Layout/BeginEndAlignment cop
Browse files Browse the repository at this point in the history
Fixes rubocop#8582.

This PR adds `Layout/BeginEndAlignment` cop to solve the issue.

The following is an example:

```ruby
% cat example.rb
foo = bar rescue "{}"
```

## Before

The alignment was misaligned because `begin` was not awake.

```ruby
% rubocop -a example.rb
(snip)

% cat example.rb
foo =
  begin
         bar
  rescue StandardError
    '{}'
       end
```

## After

Awakened by `begin`, so aligned.

```ruby
% rubocop -a example.rb
(snip)

% cat example.rb
foo =
  begin
    bar
  rescue StandardError
    '{}'
  end
```

I was first planning to add `begin` processing to `Layout/EndAlignment` cop.
However, `Layout/EndAlignment` cop with `EnforcedStyleAlignWith: keyword (default)`
was not preferred code in the following:

```ruby
longlonglonglonglonglonglonglonglong ||= begin
                                           do_something
                                         end
```

This style has been found to have very long line length with RuboCop's own code.

Therefore, since this PR would like to use `EnforcedStyleAlignWith: start_of_line`
by default against `foo ||= begin`, this PR added the new cop.

```ruby
# EnforcedStyleAlignWith: start_of_line (default)
longlonglonglonglonglonglonglonglong ||= begin
  do_something
end
```

This solution allows configuration with minimal impact on existing code.

It is configurable if user want to align to `begin`.

```ruby
# EnforcedStyleAlignWith: begin
longlonglonglonglonglonglonglonglong ||= begin
                                           do_something
                                         end
```

This PR also makes `Layout/RescueEnsureAlignment` cop aware of new
`Layout/BeginEndAlignment` cop's config to align `rescue` indents.
  • Loading branch information
koic committed Sep 6, 2020
1 parent dfdf8f9 commit 1b21b76
Show file tree
Hide file tree
Showing 14 changed files with 341 additions and 46 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
13 changes: 13 additions & 0 deletions config/default.yml
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -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]
Expand Down
76 changes: 76 additions & 0 deletions docs/modules/ROOT/pages/cops_layout.adoc
Expand Up @@ -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

|===
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -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'
Expand Down
6 changes: 3 additions & 3 deletions lib/rubocop/config_loader.rb
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions lib/rubocop/config_store.rb
Expand Up @@ -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
77 changes: 77 additions & 0 deletions 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
15 changes: 5 additions & 10 deletions lib/rubocop/cop/layout/end_alignment.rb
Expand Up @@ -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
#
Expand Down Expand Up @@ -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
Expand Down
21 changes: 17 additions & 4 deletions lib/rubocop/cop/layout/rescue_ensure_alignment.rb
Expand Up @@ -23,6 +23,7 @@ module Layout
# end
class RescueEnsureAlignment < Base
include RangeHelp
include EndKeywordAlignment
extend AutoCorrector

MSG = '`%<kw_loc>s` at %<kw_loc_line>d, %<kw_loc_column>d is not ' \
Expand Down Expand Up @@ -59,24 +60,24 @@ 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

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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions lib/rubocop/cop/mixin/end_keyword_alignment.rb
Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions lib/rubocop/ext/regexp_node.rb
Expand Up @@ -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)
Expand Down
39 changes: 39 additions & 0 deletions 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

0 comments on commit 1b21b76

Please sign in to comment.