From 5fbfe36d602cfc1ceb8c2909c5fee38a0c4c847a Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Fri, 19 Mar 2021 01:03:38 +0900 Subject: [PATCH] Add new `Style/StringChars` cop ## Summary This PR adds new `Style/StringChars` cop. The cop checks for uses of `String#split` with empty string or regexp literal argument. ```ruby # bad string.split(//) string.split('') # good string.chars ``` Since Ruby 2.0, the behavior of `String#chars` and `String#split(//)` is the same. ### Ruby 1.9 and lower ```console % ruby -ve "p 'foo'.split(//)" ruby 1.9.3p551 (2014-11-13 revision 48407) [x86_64-darwin13.0.2] ["f", "o", "o"] % ruby -ve "p 'foo'.chars" ruby 1.9.3p551 (2014-11-13 revision 48407) [x86_64-darwin13.0.2] # ``` ### Ruby 2.0 and higher ```console % ruby -ve "p 'foo'.split(//)" ruby 2.0.0p648 (2015-12-16 revision 53162) [x86_64-darwin13.0.2] ["f", "o", "o"] % ruby -ve "p 'foo'.chars" ruby 2.0.0p648 (2015-12-16 revision 53162) [x86_64-darwin13.0.2] ["f", "o", "o"] ``` Writing `String#chars` would pretty express the intent of the code. ## Additional Information This cop is intended to use `String#chars` method that express readable code. And it has a different purpose than `Performance/RedundantSplitRegexpArgument` cop. https://docs.rubocop.org/rubocop-performance/cops_performance.html#performanceredundantsplitregexpargument Therefore, it will be added as a new `Style` cop. --- .../new_add_new_style_string_chars_cop.md | 1 + config/default.yml | 6 +++ lib/rubocop.rb | 1 + lib/rubocop/cop/mixin/preferred_delimiters.rb | 2 +- lib/rubocop/cop/style/command_literal.rb | 2 +- lib/rubocop/cop/style/regexp_literal.rb | 2 +- lib/rubocop/cop/style/string_chars.rb | 38 +++++++++++++ spec/rubocop/cop/style/string_chars_spec.rb | 54 +++++++++++++++++++ 8 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 changelog/new_add_new_style_string_chars_cop.md create mode 100644 lib/rubocop/cop/style/string_chars.rb create mode 100644 spec/rubocop/cop/style/string_chars_spec.rb diff --git a/changelog/new_add_new_style_string_chars_cop.md b/changelog/new_add_new_style_string_chars_cop.md new file mode 100644 index 00000000000..e6d7046f9c2 --- /dev/null +++ b/changelog/new_add_new_style_string_chars_cop.md @@ -0,0 +1 @@ +* [#9615](https://github.com/rubocop/rubocop/pull/9615): Add new `Style/StringChars` cop. ([@koic][]) diff --git a/config/default.yml b/config/default.yml index ef00de7591e..10dccd57dc0 100644 --- a/config/default.yml +++ b/config/default.yml @@ -4493,6 +4493,12 @@ Style/StderrPuts: Enabled: true VersionAdded: '0.51' +Style/StringChars: + Description: 'Checks for uses of `String#split` with empty string or regexp literal argument.' + Enabled: pending + Safe: false + VersionAdded: '<>' + Style/StringConcatenation: Description: 'Checks for places where string concatenation can be replaced with string interpolation.' StyleGuide: '#string-interpolation' diff --git a/lib/rubocop.rb b/lib/rubocop.rb index c1f1deefec5..493ced4490f 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -582,6 +582,7 @@ require_relative 'rubocop/cop/style/special_global_vars' require_relative 'rubocop/cop/style/stabby_lambda_parentheses' require_relative 'rubocop/cop/style/stderr_puts' +require_relative 'rubocop/cop/style/string_chars' require_relative 'rubocop/cop/style/string_concatenation' require_relative 'rubocop/cop/style/string_hash_keys' require_relative 'rubocop/cop/style/string_literals' diff --git a/lib/rubocop/cop/mixin/preferred_delimiters.rb b/lib/rubocop/cop/mixin/preferred_delimiters.rb index bdee917331b..2da389d5e1c 100644 --- a/lib/rubocop/cop/mixin/preferred_delimiters.rb +++ b/lib/rubocop/cop/mixin/preferred_delimiters.rb @@ -15,7 +15,7 @@ def initialize(type, config, preferred_delimiters) end def delimiters - preferred_delimiters[type].split('') + preferred_delimiters[type].chars end private diff --git a/lib/rubocop/cop/style/command_literal.rb b/lib/rubocop/cop/style/command_literal.rb index 9391715b606..534f223ea09 100644 --- a/lib/rubocop/cop/style/command_literal.rb +++ b/lib/rubocop/cop/style/command_literal.rb @@ -165,7 +165,7 @@ def backtick_literal?(node) end def preferred_delimiter - (command_delimiter || default_delimiter).split('') + (command_delimiter || default_delimiter).chars end def command_delimiter diff --git a/lib/rubocop/cop/style/regexp_literal.rb b/lib/rubocop/cop/style/regexp_literal.rb index 5414da42eac..7c345dc671c 100644 --- a/lib/rubocop/cop/style/regexp_literal.rb +++ b/lib/rubocop/cop/style/regexp_literal.rb @@ -150,7 +150,7 @@ def slash_literal?(node) def preferred_delimiters config.for_cop('Style/PercentLiteralDelimiters') \ - ['PreferredDelimiters']['%r'].split('') + ['PreferredDelimiters']['%r'].chars end def correct_delimiters(node, corrector) diff --git a/lib/rubocop/cop/style/string_chars.rb b/lib/rubocop/cop/style/string_chars.rb new file mode 100644 index 00000000000..6105a9cfe42 --- /dev/null +++ b/lib/rubocop/cop/style/string_chars.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # Checks for uses of `String#split` with empty string or regexp literal argument. + # + # This cop is marked as unsafe. But probably it's quite unlikely that some other class would + # define a `split` method that takes exactly the same arguments. + # + # @example + # # bad + # string.split(//) + # string.split('') + # + # # good + # string.chars + # + class StringChars < Base + extend AutoCorrector + + MSG = 'Use `chars` instead of `%s`.' + RESTRICT_ON_SEND = %i[split].freeze + BAD_ARGUMENTS = %w[// '' ""].freeze + + def on_send(node) + return unless node.arguments.one? && BAD_ARGUMENTS.include?(node.first_argument.source) + + range = node.loc.selector.begin.join(node.loc.end) + + add_offense(range, message: format(MSG, current: range.source)) do |corrector| + corrector.replace(range, 'chars') + end + end + end + end + end +end diff --git a/spec/rubocop/cop/style/string_chars_spec.rb b/spec/rubocop/cop/style/string_chars_spec.rb new file mode 100644 index 00000000000..212ba47839f --- /dev/null +++ b/spec/rubocop/cop/style/string_chars_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::StringChars, :config do + it 'registers and corrects an offense when using `split(//)`' do + expect_offense(<<~RUBY) + string.split(//) + ^^^^^^^^^ Use `chars` instead of `split(//)`. + RUBY + + expect_correction(<<~RUBY) + string.chars + RUBY + end + + it "registers and corrects an offense when using `split('')`" do + expect_offense(<<~RUBY) + string.split('') + ^^^^^^^^^ Use `chars` instead of `split('')`. + RUBY + + expect_correction(<<~RUBY) + string.chars + RUBY + end + + it 'registers and corrects an offense when using `split("")`' do + expect_offense(<<~RUBY) + string.split("") + ^^^^^^^^^ Use `chars` instead of `split("")`. + RUBY + + expect_correction(<<~RUBY) + string.chars + RUBY + end + + it 'does not register an offense when using `chars`' do + expect_no_offenses(<<~RUBY) + string.chars + RUBY + end + + it 'does not register an offense when using `split(/ /)`' do + expect_no_offenses(<<~RUBY) + string.split(/ /) + RUBY + end + + it 'does not register an offense when using `split`' do + expect_no_offenses(<<~RUBY) + string.split + RUBY + end +end