Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new Style/ArrayCoercion cop #8295

Merged
merged 1 commit into from Jul 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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