diff --git a/CHANGELOG.md b/CHANGELOG.md index 32c2cd461c0..8b161200f0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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][]) diff --git a/config/default.yml b/config/default.yml index fc7f50ee3b2..aa9542529bd 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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' diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 0d61d80c856..17a9797ca3f 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -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] diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc index 980e6fe4636..564c88a591b 100644 --- a/docs/modules/ROOT/pages/cops_style.adoc +++ b/docs/modules/ROOT/pages/cops_style.adoc @@ -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 |=== diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 1b617cac079..9628ad53de5 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -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' diff --git a/lib/rubocop/cop/style/array_coercion.rb b/lib/rubocop/cop/style/array_coercion.rb new file mode 100644 index 00000000000..73d75ceead8 --- /dev/null +++ b/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(%s)` instead of `[*%s]`.' + CHECK_MSG = 'Use `Array(%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 diff --git a/lib/rubocop/cop/style/conditional_assignment.rb b/lib/rubocop/cop/style/conditional_assignment.rb index bf88d1030f6..42e2488a5a0 100644 --- a/lib/rubocop/cop/style/conditional_assignment.rb +++ b/lib/rubocop/cop/style/conditional_assignment.rb @@ -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 diff --git a/lib/rubocop/cop/style/parallel_assignment.rb b/lib/rubocop/cop/style/parallel_assignment.rb index 11a24027499..7633cc67e2a 100644 --- a/lib/rubocop/cop/style/parallel_assignment.rb +++ b/lib/rubocop/cop/style/parallel_assignment.rb @@ -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) @@ -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) @@ -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? || diff --git a/spec/rubocop/cop/style/array_coercion_spec.rb b/spec/rubocop/cop/style/array_coercion_spec.rb new file mode 100644 index 00000000000..6a44bab1125 --- /dev/null +++ b/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