Skip to content

Commit

Permalink
Add new Style/StringChars cop
Browse files Browse the repository at this point in the history
## 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]
#<Enumerator: "foo":chars>
```

### 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.
  • Loading branch information
koic committed Mar 18, 2021
1 parent 6746f15 commit 5fbfe36
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 3 deletions.
1 change: 1 addition & 0 deletions 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][])
6 changes: 6 additions & 0 deletions config/default.yml
Expand Up @@ -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: '<<next>>'

Style/StringConcatenation:
Description: 'Checks for places where string concatenation can be replaced with string interpolation.'
StyleGuide: '#string-interpolation'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/mixin/preferred_delimiters.rb
Expand Up @@ -15,7 +15,7 @@ def initialize(type, config, preferred_delimiters)
end

def delimiters
preferred_delimiters[type].split('')
preferred_delimiters[type].chars
end

private
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/command_literal.rb
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/regexp_literal.rb
Expand Up @@ -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)
Expand Down
38 changes: 38 additions & 0 deletions 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 `%<current>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
54 changes: 54 additions & 0 deletions 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

0 comments on commit 5fbfe36

Please sign in to comment.