From eb5ad0c086c470a5b847322b58f9a64ef0c502c9 Mon Sep 17 00:00:00 2001 From: Garett Arrowood Date: Fri, 8 Dec 2017 11:13:49 -0500 Subject: [PATCH] [#4811] Add new Layout/SpaceInsideReferenceBrackets cop --- CHANGELOG.md | 1 + config/default.yml | 6 + config/enabled.yml | 4 + lib/rubocop.rb | 1 + .../layout/space_inside_reference_brackets.rb | 136 +++++++++ lib/rubocop/cop/mixin/surrounding_space.rb | 25 ++ manual/cops.md | 1 + manual/cops_layout.md | 46 +++ .../cop/layout/space_around_keyword_spec.rb | 3 + .../space_inside_reference_brackets_spec.rb | 267 ++++++++++++++++++ 10 files changed, 490 insertions(+) create mode 100644 lib/rubocop/cop/layout/space_inside_reference_brackets.rb create mode 100644 spec/rubocop/cop/layout/space_inside_reference_brackets_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index ab9aff70ed79..f45db97b22e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * [#5101](https://github.com/bbatsov/rubocop/pull/5101): Allow to specify `TargetRubyVersion` 2.5. ([@walf443][]) * [#1575](https://github.com/bbatsov/rubocop/issues/1575): Add new `Layout/ClassStructure` cop that checks whether definitions in a class are in the configured order. This cop is disabled by default. ([@jonatas][]) * New cop `Rails/InverseOf` checks for association arguments that require setting the `inverse_of` option manually. ([@bdewater][]) +* [#4811](https://github.com/bbatsov/rubocop/issues/4811): Add new `Layout/SpaceInsideReferenceBrackets` cop. ([@garettarrowood][]) * [#4252](https://github.com/bbatsov/rubocop/issues/4252): Add new `Style/TrailingBodyOnMethodDefinition` cop. ([@garettarrowood][]) * Add new `Style/TrailingMethodEndStatment` cop. ([@garettarrowood][]) * [#4650](https://github.com/bbatsov/rubocop/issues/4650): Add new `Style/StringHashKeys` cop. ([@donjar][]) diff --git a/config/default.yml b/config/default.yml index afea8249a99f..367270e003e6 100644 --- a/config/default.yml +++ b/config/default.yml @@ -526,6 +526,12 @@ Layout/SpaceInsideHashLiteralBraces: - space - no_space +Layout/SpaceInsideReferenceBrackets: + EnforcedStyle: no_space + SupportedStyles: + - space + - no_space + Layout/SpaceInsideStringInterpolation: EnforcedStyle: no_space SupportedStyles: diff --git a/config/enabled.yml b/config/enabled.yml index fdf4ede75e62..f519368479cf 100644 --- a/config/enabled.yml +++ b/config/enabled.yml @@ -361,6 +361,10 @@ Layout/SpaceInsideRangeLiteral: StyleGuide: '#no-space-inside-range-literals' Enabled: true +Layout/SpaceInsideReferenceBrackets: + Description: 'Checks the spacing inside referential brackets.' + Enabled: true + Layout/SpaceInsideStringInterpolation: Description: 'Checks for padding/surrounding spaces inside string interpolation.' StyleGuide: '#string-interpolation' diff --git a/lib/rubocop.rb b/lib/rubocop.rb index bae574e6dc34..d492241b7acb 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -213,6 +213,7 @@ require_relative 'rubocop/cop/layout/space_inside_parens' require_relative 'rubocop/cop/layout/space_inside_percent_literal_delimiters' require_relative 'rubocop/cop/layout/space_inside_range_literal' +require_relative 'rubocop/cop/layout/space_inside_reference_brackets' require_relative 'rubocop/cop/layout/space_inside_string_interpolation' require_relative 'rubocop/cop/layout/tab' require_relative 'rubocop/cop/layout/trailing_blank_lines' diff --git a/lib/rubocop/cop/layout/space_inside_reference_brackets.rb b/lib/rubocop/cop/layout/space_inside_reference_brackets.rb new file mode 100644 index 000000000000..09e2aee68461 --- /dev/null +++ b/lib/rubocop/cop/layout/space_inside_reference_brackets.rb @@ -0,0 +1,136 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Layout + # Checks that reference brackets have or don't have + # surrounding space depending on configuration. + # + # @example EnforcedStyle: no_space (default) + # # The `no_space` style enforces that reference brackets have + # # no surrounding space. + # + # # bad + # hash[ :key ] + # array[ index ] + # + # # good + # hash[:key] + # array[index] + # + # @example EnforcedStyle: space + # # The `space` style enforces that reference brackets have + # # surrounding space. + # + # # bad + # hash[:key] + # array[index] + # + # # good + # hash[ :key ] + # array[ index ] + class SpaceInsideReferenceBrackets < Cop + include SurroundingSpace + include ConfigurableEnforcedStyle + + MSG = '%s space inside reference brackets.'.freeze + + def on_send(node) + return if node.multiline? + return unless left_ref_bracket(node) + left_token = left_ref_bracket(node) + right_token = right_ref_bracket(node, left_token) + + if style == :no_space + no_space_offenses(node, left_token, right_token) + else + space_offenses(node, left_token, right_token) + end + end + + private + + def autocorrect(node) + lambda do |corrector| + left_token = left_ref_bracket(node) + right_token = right_ref_bracket(node, left_token) + + if style == :no_space + no_space_corrector(corrector, left_token, right_token) + else + space_corrector(corrector, left_token, right_token) + end + end + end + + def left_ref_bracket(node) + line_tokens(node).reverse.find { |t| t.type == :tLBRACK2 } + end + + def line_tokens(node) + tokens = processed_source.tokens.select do |t| + t.pos.line == send_line(node) + end + tokens.select { |t| t.pos.end_pos <= node.source_range.end_pos } + end + + def send_line(node) + node.source_range.first_line + end + + def right_ref_bracket(node, token) + i = line_tokens(node).index(token) + line_tokens(node).slice(i..-1).find { |t| t.type == :tRBRACK } + end + + def no_space_offenses(node, left_token, right_token) + if extra_space?(left_token, :left) + range = side_space_range(range: left_token.pos, side: :right) + add_offense(node, location: range, + message: format(MSG, command: 'Do not use')) + end + return unless extra_space?(right_token, :right) + range = side_space_range(range: right_token.pos, side: :left) + add_offense(node, location: range, + message: format(MSG, command: 'Do not use')) + end + + def no_space_corrector(corrector, left_token, right_token) + if space_after?(left_token) + range = side_space_range(range: left_token.pos, side: :right) + corrector.remove(range) + end + return unless space_before?(right_token) + range = side_space_range(range: right_token.pos, side: :left) + corrector.remove(range) + end + + def space_offenses(node, left_token, right_token) + unless extra_space?(left_token, :left) + add_offense(node, location: left_token.pos, + message: format(MSG, command: 'Use')) + end + return if extra_space?(right_token, :right) || right_token.nil? + add_offense(node, location: right_token.pos, + message: format(MSG, command: 'Use')) + end + + def space_corrector(corrector, left_token, right_token) + unless space_after?(left_token) + corrector.insert_after(left_token.pos, ' ') + end + return if space_before?(right_token) + corrector.insert_before(right_token.pos, ' ') + end + + def extra_space?(token, side) + if side == :left + token && space_after?(token) + else + token && space_before?(token) + end + end + end + end + end +end diff --git a/lib/rubocop/cop/mixin/surrounding_space.rb b/lib/rubocop/cop/mixin/surrounding_space.rb index 4c4163a93fb4..546456bf386b 100644 --- a/lib/rubocop/cop/mixin/surrounding_space.rb +++ b/lib/rubocop/cop/mixin/surrounding_space.rb @@ -14,6 +14,23 @@ def space_before?(token) token.pos.source_buffer.source.match(/\G\s/, token.pos.begin_pos - 1) end + def side_space_range(range:, side:) + buffer = @processed_source.buffer + src = buffer.source + + begin_pos = range.begin_pos + end_pos = range.end_pos + if side == :left + begin_pos = reposition(src, begin_pos, -1) + end_pos -= 1 + end + if side == :right + begin_pos += 1 + end_pos = reposition(src, end_pos, 1) + end + Parser::Source::Range.new(buffer, begin_pos, end_pos) + end + def index_of_first_token(node) range = node.source_range token_table[range.line][range.column] @@ -38,6 +55,14 @@ def token_table table end end + + private + + def reposition(src, pos, step) + offset = step == -1 ? -1 : 0 + pos += step while src[pos + offset] =~ /[ \t]/ + pos < 0 ? 0 : pos + end end end end diff --git a/manual/cops.md b/manual/cops.md index f0ac5034c242..19aa6e2929a5 100644 --- a/manual/cops.md +++ b/manual/cops.md @@ -170,6 +170,7 @@ In the following section you find all available cops: * [Layout/SpaceInsideParens](cops_layout.md#layoutspaceinsideparens) * [Layout/SpaceInsidePercentLiteralDelimiters](cops_layout.md#layoutspaceinsidepercentliteraldelimiters) * [Layout/SpaceInsideRangeLiteral](cops_layout.md#layoutspaceinsiderangeliteral) +* [Layout/SpaceInsideReferenceBrackets](cops_layout.md#layoutspaceinsidereferencebrackets) * [Layout/SpaceInsideStringInterpolation](cops_layout.md#layoutspaceinsidestringinterpolation) * [Layout/Tab](cops_layout.md#layouttab) * [Layout/TrailingBlankLines](cops_layout.md#layouttrailingblanklines) diff --git a/manual/cops_layout.md b/manual/cops_layout.md index a2647f65a0f0..ac08b2662b50 100644 --- a/manual/cops_layout.md +++ b/manual/cops_layout.md @@ -2894,6 +2894,52 @@ Checks for spaces inside range literals. * [https://github.com/bbatsov/ruby-style-guide#no-space-inside-range-literals](https://github.com/bbatsov/ruby-style-guide#no-space-inside-range-literals) +## Layout/SpaceInsideReferenceBrackets + +Enabled by default | Supports autocorrection +--- | --- +Enabled | Yes + +Checks that reference brackets have or don't have +surrounding space depending on configuration. + +### Examples + +#### EnforcedStyle: no_space (default) + +```ruby +# The `no_space` style enforces that reference brackets have +# no surrounding space. + +# bad +hash[ :key ] +array[ index ] + +# good +hash[:key] +array[index] +``` +#### EnforcedStyle: space + +```ruby +# The `space` style enforces that reference brackets have +# surrounding space. + +# bad +hash[:key] +array[index] + +# good +hash[ :key ] +array[ index ] +``` + +### Configurable attributes + +Name | Default value | Configurable values +--- | --- | --- +EnforcedStyle | `no_space` | `space`, `no_space` + ## Layout/SpaceInsideStringInterpolation Enabled by default | Supports autocorrection diff --git a/spec/rubocop/cop/layout/space_around_keyword_spec.rb b/spec/rubocop/cop/layout/space_around_keyword_spec.rb index e311323bbcdf..e1fc15e395a5 100644 --- a/spec/rubocop/cop/layout/space_around_keyword_spec.rb +++ b/spec/rubocop/cop/layout/space_around_keyword_spec.rb @@ -206,6 +206,9 @@ # Layout/SpaceInsideHashLiteralBraces it_behaves_like 'accept around', '{}', '{a: begin end}' + # Layout/SpaceInsideReferenceBrackets + it_behaves_like 'accept around', '[]', 'a[begin end]' + # Layout/SpaceInsideStringInterpolation it_behaves_like 'accept around', '{}', '"#{begin end}"' end diff --git a/spec/rubocop/cop/layout/space_inside_reference_brackets_spec.rb b/spec/rubocop/cop/layout/space_inside_reference_brackets_spec.rb new file mode 100644 index 000000000000..cd63d22cf5f7 --- /dev/null +++ b/spec/rubocop/cop/layout/space_inside_reference_brackets_spec.rb @@ -0,0 +1,267 @@ +# frozen_string_literal: true + +describe RuboCop::Cop::Layout::SpaceInsideReferenceBrackets, :config do + subject(:cop) { described_class.new(config) } + + context 'when EnforcedStyle is no_space' do + let(:cop_config) { { 'EnforcedStyle' => 'no_space' } } + + it 'does not register offense for array literals' do + expect_no_offenses(<<-RUBY.strip_indent) + a = [1, 2 ] + b = [ 3, 4] + c = [5, 6] + d = [ 7, 8 ] + RUBY + end + + it 'does not register offense for reference brackets with no spaces' do + expect_no_offenses(<<-RUBY.strip_indent) + a[1] + b[index, 2] + c["foo"] + d[:bar] + RUBY + end + + it 'does not register offense for ref bcts with no spaces that assign' do + expect_no_offenses(<<-RUBY.strip_indent) + a[1] = 2 + b[345] = [ 678, var, "", nil] + c["foo"] = "qux" + d[:bar] = var + RUBY + end + + it 'accepts square brackets as method name' do + expect_no_offenses(<<-RUBY.strip_indent) + def Vector.[](*array) + end + RUBY + end + + it 'accepts square brackets called with method call syntax' do + expect_no_offenses('subject.[](0)') + end + + it 'registers offense in ref brackets with leading whitespace' do + expect_offense(<<-RUBY.strip_indent) + a[ :key] + ^^ Do not use space inside reference brackets. + RUBY + end + + it 'registers offense in ref brackets with trailing whitespace' do + expect_offense(<<-RUBY.strip_indent) + b[:key ] + ^ Do not use space inside reference brackets. + RUBY + end + + it 'registers offense in second ref brackets with leading whitespace' do + expect_offense(<<-RUBY.strip_indent) + a[:key][ "key"] + ^ Do not use space inside reference brackets. + RUBY + end + + it 'registers offense in second ref brackets with trailing whitespace' do + expect_offense(<<-RUBY.strip_indent) + a[1][:key ] + ^^^ Do not use space inside reference brackets. + RUBY + end + + it 'registers offense in third ref brackets with leading whitespace' do + expect_offense(<<-RUBY.strip_indent) + a[:key][3][ :key] + ^ Do not use space inside reference brackets. + RUBY + end + + it 'registers offense in third ref brackets with trailing whitespace' do + expect_offense(<<-RUBY.strip_indent) + a[var]["key", 3][:key ] + ^ Do not use space inside reference brackets. + RUBY + end + + it 'registers multiple offenses in one set of ref brackets' do + inspect_source(<<-RUBY.strip_indent) + b[ 89 ] + RUBY + expect(cop.offenses.size).to eq(2) + expect(cop.messages.uniq) + .to eq(['Do not use space inside reference brackets.']) + end + + it 'registers multiple offenses for multiple sets of ref brackets' do + inspect_source(<<-RUBY.strip_indent) + a[ :key]["foo" ][ 0 ] + RUBY + expect(cop.offenses.size).to eq(4) + expect(cop.messages.uniq) + .to eq(['Do not use space inside reference brackets.']) + end + + context 'auto-correct' do + it 'fixes multiple offenses in one set of ref brackets' do + new_source = autocorrect_source(<<-RUBY.strip_indent) + bar[ 89 ] + RUBY + expect(new_source).to eq(<<-RUBY.strip_indent) + bar[89] + RUBY + end + + it 'fixes multiple offenses for multiple sets of ref brackets' do + new_source = autocorrect_source(<<-RUBY.strip_indent) + b[ :key]["foo" ][ 0 ] + RUBY + expect(new_source).to eq(<<-RUBY.strip_indent) + b[:key]["foo"][0] + RUBY + end + + it 'avoids altering array brackets' do + new_source = autocorrect_source(<<-RUBY.strip_indent) + j[ "pop"] = [89, nil, "" ] + RUBY + expect(new_source).to eq(<<-RUBY.strip_indent) + j["pop"] = [89, nil, "" ] + RUBY + end + end + end + + context 'when EnforcedStyle is no_space' do + let(:cop_config) { { 'EnforcedStyle' => 'space' } } + + it 'does not register offense for array literals' do + expect_no_offenses(<<-RUBY.strip_indent) + a = [1, 2 ] + b = [ 3, 4] + c = [5, 6] + d = [ 7, 8 ] + RUBY + end + + it 'does not register offense for reference brackets with spaces' do + expect_no_offenses(<<-RUBY.strip_indent) + a[ 1 ] + b[ index, 3 ] + c[ "foo" ] + d[ :bar ] + RUBY + end + + it 'does not register offense for ref bcts with spaces that assign' do + expect_no_offenses(<<-RUBY.strip_indent) + a[ 1 ] = 2 + b[ 345 ] = [ 678, var, "", nil] + c[ "foo" ] = "qux" + d[ :bar ] = var + RUBY + end + + it 'accepts square brackets as method name' do + expect_no_offenses(<<-RUBY.strip_indent) + def Vector.[](*array) + end + RUBY + end + + it 'accepts square brackets called with method call syntax' do + expect_no_offenses('subject.[](0)') + end + + it 'registers offense in ref brackets with no leading whitespace' do + expect_offense(<<-RUBY.strip_indent) + a[:key ] + ^ Use space inside reference brackets. + RUBY + end + + it 'registers offense in ref brackets with no trailing whitespace' do + expect_offense(<<-RUBY.strip_indent) + b[ :key] + ^ Use space inside reference brackets. + RUBY + end + + it 'registers offense in second ref brackets with no leading whitespace' do + expect_offense(<<-RUBY.strip_indent) + a[ :key ]["key" ] + ^ Use space inside reference brackets. + RUBY + end + + it 'registers offense in second ref brackets with no trailing whitespace' do + expect_offense(<<-RUBY.strip_indent) + a[ 5, 1 ][ :key] + ^ Use space inside reference brackets. + RUBY + end + + it 'registers offense in third ref brackets with no leading whitespace' do + expect_offense(<<-RUBY.strip_indent) + a[ :key ][ 3 ][:key ] + ^ Use space inside reference brackets. + RUBY + end + + it 'registers offense in third ref brackets with no trailing whitespace' do + expect_offense(<<-RUBY.strip_indent) + a[ var ][ "key" ][ :key] + ^ Use space inside reference brackets. + RUBY + end + + it 'registers multiple offenses in one set of ref brackets' do + inspect_source(<<-RUBY.strip_indent) + b[89] + RUBY + expect(cop.offenses.size).to eq(2) + expect(cop.messages.uniq) + .to eq(['Use space inside reference brackets.']) + end + + it 'registers multiple offenses for multiple sets of ref brackets' do + inspect_source(<<-RUBY.strip_indent) + a[:key]["foo" ][0] + RUBY + expect(cop.offenses.size).to eq(5) + expect(cop.messages.uniq) + .to eq(['Use space inside reference brackets.']) + end + + context 'auto-correct' do + it 'fixes multiple offenses in one set of ref brackets' do + new_source = autocorrect_source(<<-RUBY.strip_indent) + bar[89] + RUBY + expect(new_source).to eq(<<-RUBY.strip_indent) + bar[ 89 ] + RUBY + end + + it 'fixes multiple offenses for multiple sets of ref brackets' do + new_source = autocorrect_source(<<-RUBY.strip_indent) + b[:key][ "foo"][0 ] + RUBY + expect(new_source).to eq(<<-RUBY.strip_indent) + b[ :key ][ "foo" ][ 0 ] + RUBY + end + + it 'avoids altering array brackets' do + new_source = autocorrect_source(<<-RUBY.strip_indent) + j[ "pop"] = [89, nil, "" ] + RUBY + expect(new_source).to eq(<<-RUBY.strip_indent) + j[ "pop" ] = [89, nil, "" ] + RUBY + end + end + end +end