Skip to content

Commit

Permalink
Add new Style/SwapValues cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima committed Oct 25, 2020
1 parent 69a0284 commit 13b3433
Show file tree
Hide file tree
Showing 7 changed files with 208 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,7 @@
### New features

* [#8895](https://github.com/rubocop-hq/rubocop/pull/8895): Add new `Lint/EmptyBlock` cop. ([@fatkodima][])
* [#8934](https://github.com/rubocop-hq/rubocop/pull/8934): Add new `Style/SwapValues` cop. ([@fatkodima][])
* [#7549](https://github.com/rubocop-hq/rubocop/issues/7549): Add new `Style/ArgumentsForwarding` cop. ([@koic][])
* [#8859](https://github.com/rubocop-hq/rubocop/issues/8859): Add new `Lint/UnmodifiedReduceAccumulator` cop. ([@dvandersluis][])

Expand Down
7 changes: 7 additions & 0 deletions config/default.yml
Expand Up @@ -4244,6 +4244,13 @@ Style/StructInheritance:
VersionAdded: '0.29'
VersionChanged: '0.86'

Style/SwapValues:
Description: 'This cop enforces the use of shorthand-style swapping of 2 variables.'
StyleGuide: '#values-swapping'
Enabled: pending
VersionAdded: '1.1'
SafeAutoCorrect: false

Style/SymbolArray:
Description: 'Use %i or %I for arrays of symbols.'
StyleGuide: '#percent-i'
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -509,6 +509,7 @@ In the following section you find all available cops:
* xref:cops_style.adoc#stylestringmethods[Style/StringMethods]
* xref:cops_style.adoc#stylestrip[Style/Strip]
* xref:cops_style.adoc#stylestructinheritance[Style/StructInheritance]
* xref:cops_style.adoc#styleswapvalues[Style/SwapValues]
* xref:cops_style.adoc#stylesymbolarray[Style/SymbolArray]
* xref:cops_style.adoc#stylesymbolliteral[Style/SymbolLiteral]
* xref:cops_style.adoc#stylesymbolproc[Style/SymbolProc]
Expand Down
33 changes: 33 additions & 0 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -9897,6 +9897,39 @@ end

* https://rubystyle.guide#no-extend-struct-new

== Style/SwapValues

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| Yes
| Yes (Unsafe)
| 1.1
| -
|===

This cop enforces the use of shorthand-style swapping of 2 variables.
Its autocorrection is marked as unsafe, because it can erroneously remove
the temporary variable which is used later.

=== Examples

[source,ruby]
----
# bad
tmp = x
x = y
y = tmp
# good
x, y = y, x
----

=== References

* https://rubystyle.guide#values-swapping

== Style/SymbolArray

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -557,6 +557,7 @@
require_relative 'rubocop/cop/style/string_methods'
require_relative 'rubocop/cop/style/strip'
require_relative 'rubocop/cop/style/struct_inheritance'
require_relative 'rubocop/cop/style/swap_values'
require_relative 'rubocop/cop/style/symbol_array'
require_relative 'rubocop/cop/style/symbol_literal'
require_relative 'rubocop/cop/style/symbol_proc'
Expand Down
108 changes: 108 additions & 0 deletions lib/rubocop/cop/style/swap_values.rb
@@ -0,0 +1,108 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# This cop enforces the use of shorthand-style swapping of 2 variables.
# Its autocorrection is marked as unsafe, because it can erroneously remove
# the temporary variable which is used later.
#
# @example
# # bad
# tmp = x
# x = y
# y = tmp
#
# # good
# x, y = y, x
#
class SwapValues < Base
include RangeHelp
extend AutoCorrector

MSG = 'Replace this and assignments at lines %<x_line>d '\
'and %<y_line>d with `%<replacement>s`.'

SIMPLE_ASSIGNMENT_TYPES = %i[lvasgn ivasgn cvasgn gvasgn casgn].to_set.freeze

def on_asgn(node)
return if allowed_assignment?(node)

tmp_assign = node
x_assign, y_assign = *node.right_siblings.take(2)
return unless x_assign && y_assign && swapping_values?(tmp_assign, x_assign, y_assign)

add_offense(node, message: message(x_assign, y_assign)) do |corrector|
range = correction_range(tmp_assign, y_assign)
corrector.replace(range, replacement(x_assign))
end
end

SIMPLE_ASSIGNMENT_TYPES.each { |asgn_type| alias_method :"on_#{asgn_type}", :on_asgn }

private

def allowed_assignment?(node)
node.parent&.mlhs_type? || node.parent&.shorthand_asgn?
end

def swapping_values?(tmp_assign, x_assign, y_assign)
simple_assignment?(tmp_assign) &&
simple_assignment?(x_assign) &&
simple_assignment?(y_assign) &&
lhs(x_assign) == rhs(tmp_assign) &&
lhs(y_assign) == rhs(x_assign) &&
rhs(y_assign) == lhs(tmp_assign)
end

def simple_assignment?(node)
SIMPLE_ASSIGNMENT_TYPES.include?(node.type)
end

def message(x_assign, y_assign)
format(
MSG,
x_line: x_assign.first_line,
y_line: y_assign.first_line,
replacement: replacement(x_assign)
)
end

def replacement(x_assign)
x = lhs(x_assign)
y = rhs(x_assign)
"#{x}, #{y} = #{y}, #{x}"
end

def lhs(node)
case node.type
when :casgn
namespace, name, = *node
if namespace
"#{namespace.const_name}::#{name}"
else
name.to_s
end
else
node.children[0].to_s
end
end

def rhs(node)
case node.type
when :casgn
node.children[2].source
else
node.children[1].source
end
end

def correction_range(tmp_assign, y_assign)
range_by_whole_lines(
range_between(tmp_assign.source_range.begin_pos, y_assign.source_range.end_pos)
)
end
end
end
end
end
57 changes: 57 additions & 0 deletions spec/rubocop/cop/style/swap_values_spec.rb
@@ -0,0 +1,57 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Style::SwapValues do
subject(:cop) { described_class.new }

shared_examples 'verbosely swapping' do |type, x, y, correction|
it "registers an offense and corrects when verbosely swapping #{type} variables" do
expect_offense(<<~RUBY, x: x)
tmp = %{x}
^^^^^^^{x} Replace this and assignments at lines 2 and 3 with `#{correction}`.
#{x} = #{y}
#{y} = tmp
RUBY

expect_correction(<<~RUBY)
#{correction}
RUBY
end
end

it_behaves_like('verbosely swapping', 'local', 'x', 'y', 'x, y = y, x')
it_behaves_like('verbosely swapping', 'global', '$x', '$y', '$x, $y = $y, $x')
it_behaves_like('verbosely swapping', 'instance', '@x', '@y', '@x, @y = @y, @x')
it_behaves_like('verbosely swapping', 'class', '@@x', '@@y', '@@x, @@y = @@y, @@x')
it_behaves_like('verbosely swapping', 'constant', 'X', 'Y', 'X, Y = Y, X')
it_behaves_like('verbosely swapping', 'constant with namespaces',
'::X', 'Foo::Y', '::X, Foo::Y = Foo::Y, ::X')
it_behaves_like('verbosely swapping', 'mixed', '@x', '$y', '@x, $y = $y, @x')

it 'handles comments when correcting' do
expect_offense(<<~RUBY)
tmp = x # comment 1
^^^^^^^ Replace this and assignments at lines 3 and 4 with `x, y = y, x`.
# comment 2
x = y
y = tmp # comment 3
RUBY

expect_correction(<<~RUBY)
x, y = y, x
RUBY
end

it 'does not register an offense when idiomatically swapping variables' do
expect_no_offenses(<<~RUBY)
x, y = y, x
RUBY
end

it 'does not register an offense when almost swapping variables' do
expect_no_offenses(<<~RUBY)
tmp = x
x = y
y = not_a_tmp
RUBY
end
end

0 comments on commit 13b3433

Please sign in to comment.