Skip to content

Commit

Permalink
Add new Style/ArrayCoercion cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima authored and bbatsov committed Jul 10, 2020
1 parent 8fafbaf commit 03cd4f6
Show file tree
Hide file tree
Showing 9 changed files with 156 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -6,6 +6,7 @@

* [#7333](https://github.com/rubocop-hq/rubocop/issues/7333): Add new `Style/RedundantFileExtensionInRequire` cop. ([@fatkodima][])
* [#8242](https://github.com/rubocop-hq/rubocop/pull/8242): Internal profiling available with `bin/rubocop-profile` and rake tasks. ([@marcandre][])
* [#8295](https://github.com/rubocop-hq/rubocop/pull/8295): Add new `Style/ArrayCoercion` cop. ([@fatkodima][])
* [#8293](https://github.com/rubocop-hq/rubocop/pull/8293): Add new `Lint/DuplicateElsifCondition` cop. ([@fatkodima][])
* [#7736](https://github.com/rubocop-hq/rubocop/issues/7736): Add new `Style/CaseLikeIf` cop. ([@fatkodima][])
* [#4286](https://github.com/rubocop-hq/rubocop/issues/4286): Add new `Style/HashAsLastArrayItem` cop. ([@fatkodima][])
Expand Down
8 changes: 8 additions & 0 deletions config/default.yml
Expand Up @@ -2345,6 +2345,14 @@ Style/AndOr:
- always
- conditionals

Style/ArrayCoercion:
Description: >-
Use Array() instead of explicit Array check or [*var], when dealing
with a variable you want to treat as an Array, but you're not certain it's an array.
StyleGuide: '#array-coercion'
Enabled: 'pending'
VersionAdded: '0.88'

Style/ArrayJoin:
Description: 'Use Array#join instead of Array#*.'
StyleGuide: '#array-join'
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -317,6 +317,7 @@ In the following section you find all available cops:
* xref:cops_style.adoc#styleaccessorgrouping[Style/AccessorGrouping]
* xref:cops_style.adoc#stylealias[Style/Alias]
* xref:cops_style.adoc#styleandor[Style/AndOr]
* xref:cops_style.adoc#stylearraycoercion[Style/ArrayCoercion]
* xref:cops_style.adoc#stylearrayjoin[Style/ArrayJoin]
* xref:cops_style.adoc#styleasciicomments[Style/AsciiComments]
* xref:cops_style.adoc#styleattr[Style/Attr]
Expand Down
33 changes: 33 additions & 0 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -290,6 +290,39 @@ end

* https://rubystyle.guide#no-and-or-or

== Style/ArrayCoercion

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

| Pending
| Yes
| Yes
| 0.88
| -
|===

This cop enforces the use of `Array()` instead of explicit `Array` check or `[*var]`.

=== Examples

[source,ruby]
----
# bad
paths = [paths] unless paths.is_a?(Array)
paths.each { |path| do_something(path) }
# bad (always creates a new Array instance)
[*paths].each { |path| do_something(path) }
# good (and a bit more readable)
Array(paths).each { |path| do_something(path) }
----

=== References

* https://rubystyle.guide#array-coercion

== Style/ArrayJoin

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -361,6 +361,7 @@
require_relative 'rubocop/cop/style/accessor_grouping'
require_relative 'rubocop/cop/style/alias'
require_relative 'rubocop/cop/style/and_or'
require_relative 'rubocop/cop/style/array_coercion'
require_relative 'rubocop/cop/style/array_join'
require_relative 'rubocop/cop/style/ascii_comments'
require_relative 'rubocop/cop/style/attr'
Expand Down
63 changes: 63 additions & 0 deletions lib/rubocop/cop/style/array_coercion.rb
@@ -0,0 +1,63 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# This cop enforces the use of `Array()` instead of explicit `Array` check or `[*var]`.
#
# @example
# # bad
# paths = [paths] unless paths.is_a?(Array)
# paths.each { |path| do_something(path) }
#
# # bad (always creates a new Array instance)
# [*paths].each { |path| do_something(path) }
#
# # good (and a bit more readable)
# Array(paths).each { |path| do_something(path) }
#
class ArrayCoercion < Base
extend AutoCorrector

SPLAT_MSG = 'Use `Array(%<arg>s)` instead of `[*%<arg>s]`.'
CHECK_MSG = 'Use `Array(%<arg>s)` instead of explicit `Array` check.'

def_node_matcher :array_splat?, <<~PATTERN
(array (splat $_))
PATTERN

def_node_matcher :unless_array?, <<~PATTERN
(if
(send
(lvar $_) :is_a?
(const nil? :Array)) nil?
(lvasgn $_
(array
(lvar $_))))
PATTERN

def on_array(node)
return unless node.square_brackets?

array_splat?(node) do |arg_node|
message = format(SPLAT_MSG, arg: arg_node.source)
add_offense(node, message: message) do |corrector|
corrector.replace(node, "Array(#{arg_node.source})")
end
end
end

def on_if(node)
unless_array?(node) do |var_a, var_b, var_c|
if var_a == var_b && var_c == var_b
message = format(CHECK_MSG, arg: var_a)
add_offense(node, message: message) do |corrector|
corrector.replace(node, "#{var_a} = Array(#{var_a})")
end
end
end
end
end
end
end
end
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/conditional_assignment.rb
Expand Up @@ -30,7 +30,7 @@ def expand_when_branches(when_branches)
end

def tail(branch)
branch.begin_type? ? [*branch].last : branch
branch.begin_type? ? Array(branch).last : branch
end

# rubocop:disable Metrics/AbcSize
Expand Down
6 changes: 3 additions & 3 deletions lib/rubocop/cop/style/parallel_assignment.rb
Expand Up @@ -30,7 +30,7 @@ class ParallelAssignment < Cop
def on_masgn(node)
lhs, rhs = *node
lhs_elements = *lhs
rhs_elements = [*rhs].compact # edge case for one constant
rhs_elements = Array(rhs).compact # edge case for one constant

return if allowed_lhs?(lhs) || allowed_rhs?(rhs) ||
allowed_masign?(lhs_elements, rhs_elements)
Expand All @@ -42,7 +42,7 @@ def autocorrect(node)
lambda do |corrector|
left, right = *node
left_elements = *left
right_elements = [*right].compact
right_elements = Array(right).compact
order = find_valid_order(left_elements, right_elements)
correction = assignment_corrector(node, order)

Expand All @@ -69,7 +69,7 @@ def allowed_lhs?(node)

def allowed_rhs?(node)
# Edge case for one constant
elements = [*node].compact
elements = Array(node).compact

# Account for edge case of `Constant::CONSTANT`
!node.array_type? ||
Expand Down
45 changes: 45 additions & 0 deletions spec/rubocop/cop/style/array_coercion_spec.rb
@@ -0,0 +1,45 @@
# frozen_string_literal: true

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

it 'registers an offense and corrects when splatting variable into array' do
expect_offense(<<~RUBY)
[*paths].each { |path| do_something(path) }
^^^^^^^^ Use `Array(paths)` instead of `[*paths]`.
RUBY

expect_correction(<<~RUBY)
Array(paths).each { |path| do_something(path) }
RUBY
end

it 'registers an offense and corrects when converting variable into array with check' do
expect_offense(<<~RUBY)
paths = [paths] unless paths.is_a?(Array)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `Array(paths)` instead of explicit `Array` check.
RUBY

expect_correction(<<~RUBY)
paths = Array(paths)
RUBY
end

it 'does not register an offense when splat is not in array' do
expect_no_offenses(<<~RUBY)
first_path, rest = *paths
RUBY
end

it 'does not register an offense when splatting multiple variables into array' do
expect_no_offenses(<<~RUBY)
[*paths, '/root'].each { |path| do_something(path) }
RUBY
end

it 'does not register an offense when converting variable into other named array variable with check' do
expect_no_offenses(<<~RUBY)
other_paths = [paths] unless paths.is_a?(Array)
RUBY
end
end

0 comments on commit 03cd4f6

Please sign in to comment.