Skip to content

Commit

Permalink
[Fix #9353] Add new Lint/SymbolConversion cop.
Browse files Browse the repository at this point in the history
  • Loading branch information
dvandersluis authored and bbatsov committed Jan 11, 2021
1 parent 562682a commit 1903522
Show file tree
Hide file tree
Showing 5 changed files with 241 additions and 2 deletions.
1 change: 1 addition & 0 deletions changelog/new_add_new_lintsymbolconversion_cop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#9353](https://github.com/rubocop-hq/rubocop/issues/9353): Add new `Lint/SymbolConversion` cop. ([@dvandersluis][])
8 changes: 6 additions & 2 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2032,12 +2032,16 @@ Lint/SuppressedException:
VersionAdded: '0.9'
VersionChanged: '0.81'

Lint/SymbolConversion:
Description: 'Checks for unnecessary symbol conversions.'
Enabled: pending
VersionAdded: <<next>>

Lint/Syntax:
Description: 'Checks syntax error.'
Description: 'Checks for syntax errors.'
Enabled: true
VersionAdded: '0.9'


Lint/ToEnumArguments:
Description: 'This cop ensures that `to_enum`/`enum_for`, called for the current method, has correct arguments.'
Enabled: pending
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@
require_relative 'rubocop/cop/lint/shadowing_outer_local_variable'
require_relative 'rubocop/cop/lint/struct_new_override'
require_relative 'rubocop/cop/lint/suppressed_exception'
require_relative 'rubocop/cop/lint/symbol_conversion'
require_relative 'rubocop/cop/lint/syntax'
require_relative 'rubocop/cop/lint/to_enum_arguments'
require_relative 'rubocop/cop/lint/to_json'
Expand Down
102 changes: 102 additions & 0 deletions lib/rubocop/cop/lint/symbol_conversion.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Lint
# This cop checks for uses of literal strings converted to
# a symbol where a literal symbol could be used instead.
#
# @example
# # bad
# 'string'.to_sym
# :symbol.to_sym
# 'underscored_string'.to_sym
# :'underscored_symbol'
# 'hyphenated-string'.to_sym
#
# # good
# :string
# :symbol
# :underscored_string
# :underscored_symbol
# :'hyphenated-string'
#
class SymbolConversion < Base
extend AutoCorrector

MSG = 'Unnecessary symbol conversion; use `%<correction>s` instead.'
RESTRICT_ON_SEND = %i[to_sym intern].freeze

def on_send(node)
return unless node.receiver.str_type? || node.receiver.sym_type?

register_offense(node, correction: node.receiver.value.to_sym.inspect)
end

def on_sym(node)
return if properly_quoted?(node.source, node.value.inspect)

# `alias` arguments are symbols but since a symbol that requires
# being quoted is not a valid method identifier, it can be ignored
return if in_alias?(node)

# The `%I[]` and `%i[]` macros are parsed as normal arrays of symbols
# so they need to be ignored.
return if in_percent_literal_array?(node)

# Symbol hash keys have a different format and need to be handled separately
return correct_hash_key(node) if hash_key?(node)

register_offense(node, correction: node.value.inspect)
end

private

def register_offense(node, correction:, message: format(MSG, correction: correction))
add_offense(node, message: message) do |corrector|
corrector.replace(node, correction)
end
end

def properly_quoted?(source, value)
return true unless source.match?(/['"]/)

source == value ||
# `Symbol#inspect` uses double quotes, but allow single-quoted
# symbols to work as well.
source.tr("'", '"') == value
end

def in_alias?(node)
node.parent&.alias_type?
end

def in_percent_literal_array?(node)
node.parent&.array_type? && node.parent&.percent_literal?
end

def hash_key?(node)
node.parent&.pair_type? && node == node.parent.child_nodes.first
end

def correct_hash_key(node)
# Although some operators can be converted to symbols normally
# (ie. `:==`), these are not accepted as hash keys and will
# raise a syntax error (eg. `{ ==: ... }`). Therefore, if the
# symbol does not start with an alpha-numeric or underscore, it
# will be ignored.
return unless node.value.to_s.match?(/\A[a-z0-9_]/i)

correction = node.value.inspect.delete(':')
return if properly_quoted?(node.source, correction)

register_offense(
node,
correction: correction,
message: format(MSG, correction: "#{correction}:")
)
end
end
end
end
end
131 changes: 131 additions & 0 deletions spec/rubocop/cop/lint/symbol_conversion_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Lint::SymbolConversion do
subject(:cop) { described_class.new }

shared_examples 'offense' do |from, to|
it "registers an offense for #{from}" do
expect_offense(<<~RUBY, from: from)
#{from}
^{from} Unnecessary symbol conversion; use `#{to}` instead.
RUBY

expect_correction(<<~RUBY, loop: false)
#{to}
RUBY
end
end

# Unnecessary `to_sym`
it_behaves_like 'offense', ':foo.to_sym', ':foo'
it_behaves_like 'offense', '"foo".to_sym', ':foo'
it_behaves_like 'offense', '"foo_bar".to_sym', ':foo_bar'
it_behaves_like 'offense', '"foo-bar".to_sym', ':"foo-bar"'

# Unnecessary `intern`
it_behaves_like 'offense', ':foo.intern', ':foo'
it_behaves_like 'offense', '"foo".intern', ':foo'
it_behaves_like 'offense', '"foo_bar".intern', ':foo_bar'
it_behaves_like 'offense', '"foo-bar".intern', ':"foo-bar"'

# Unnecessary quoted symbol
it_behaves_like 'offense', ':"foo"', ':foo'
it_behaves_like 'offense', ':"foo_bar"', ':foo_bar'

it 'does not register an offense for a normal symbol' do
expect_no_offenses(<<~RUBY)
:foo
RUBY
end

it 'does not register an offense for a dstr' do
expect_no_offenses(<<~'RUBY')
"#{foo}".to_sym
RUBY
end

it 'does not register an offense for a symbol that requires quotes' do
expect_no_offenses(<<~RUBY)
:"foo-bar"
RUBY
end

context 'in a hash' do
context 'keys' do
it 'does not register an offense for a normal symbol' do
expect_no_offenses(<<~RUBY)
{ foo: 'bar' }
RUBY
end

it 'does not register an offense for a require quoted symbol' do
expect_no_offenses(<<~RUBY)
{ 'foo-bar': 'bar' }
RUBY
end

it 'registers an offense for a quoted symbol' do
expect_offense(<<~RUBY)
{ 'foo': 'bar' }
^^^^^ Unnecessary symbol conversion; use `foo:` instead.
RUBY

expect_correction(<<~RUBY)
{ foo: 'bar' }
RUBY
end

it 'does not register an offense for operators' do
expect_no_offenses(<<~RUBY)
{ '==': 'bar' }
RUBY
end
end

context 'values' do
it 'does not register an offense for a normal symbol' do
expect_no_offenses(<<~RUBY)
{ foo: :bar }
RUBY
end

it 'registers an offense for a quoted symbol' do
expect_offense(<<~RUBY)
{ foo: :'bar' }
^^^^^^ Unnecessary symbol conversion; use `:bar` instead.
RUBY

expect_correction(<<~RUBY)
{ foo: :bar }
RUBY
end
end
end

context 'in an alias' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
alias foo bar
alias == equal
alias eq? ==
RUBY
end
end

context 'inside a percent literal array' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
%i(foo bar foo-bar)
%I(foo bar foo-bar)
RUBY
end
end

context 'single quoted symbol' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
:'Foo/Bar/Baz'
RUBY
end
end
end

0 comments on commit 1903522

Please sign in to comment.