Skip to content

Commit

Permalink
[Fix #12373] Fix a false negative for Lint/SymbolConversion
Browse files Browse the repository at this point in the history
This PR fixes a false negative for `Lint/SymbolConversion`
when using string interpolation
  • Loading branch information
Earlopain authored and bbatsov committed Nov 20, 2023
1 parent 7449316 commit 2c0f759
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 11 deletions.
1 change: 1 addition & 0 deletions changelog/fix_false_negative_for_lint_symbol_conversion.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12373](https://github.com/rubocop/rubocop/issues/12373): Fix a false negative for `Lint/SymbolConversion` when using string interpolation. ([@earlopain][])
2 changes: 1 addition & 1 deletion lib/rubocop/cop/gemspec/deprecated_attribute_assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def node_and_method_name(node, attribute)
lhs, _op, _rhs = *node
[lhs, attribute]
else
[node, "#{attribute}=".to_sym]
[node, :"#{attribute}="]
end
end

Expand Down
9 changes: 7 additions & 2 deletions lib/rubocop/cop/lint/symbol_conversion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ module Lint
# 'underscored_string'.to_sym
# :'underscored_symbol'
# 'hyphenated-string'.to_sym
# "string_#{interpolation}".to_sym
#
# # good
# :string
# :symbol
# :underscored_string
# :underscored_symbol
# :'hyphenated-string'
# :"string_#{interpolation}"
#
# @example EnforcedStyle: strict (default)
#
Expand Down Expand Up @@ -75,9 +77,12 @@ class SymbolConversion < Base

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

register_offense(node, correction: node.receiver.value.to_sym.inspect)
if node.receiver.str_type? || node.receiver.sym_type?
register_offense(node, correction: node.receiver.value.to_sym.inspect)
elsif node.receiver.dstr_type?
register_offense(node, correction: ":\"#{node.receiver.value.to_sym}\"")
end
end

def on_sym(node)
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/lint/useless_access_modifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ def method_definition?(child)

def any_method_definition?(child)
cop_config.fetch('MethodCreatingMethods', []).any? do |m|
matcher_name = "#{m}_method?".to_sym
matcher_name = :"#{m}_method?"
unless respond_to?(matcher_name)
self.class.def_node_matcher matcher_name, <<~PATTERN
{def (send nil? :#{m} ...)}
Expand All @@ -279,7 +279,7 @@ def eval_call?(child)

def any_context_creating_methods?(child)
cop_config.fetch('ContextCreatingMethods', []).any? do |m|
matcher_name = "#{m}_block?".to_sym
matcher_name = :"#{m}_block?"
unless respond_to?(matcher_name)
self.class.def_node_matcher matcher_name, <<~PATTERN
({block numblock} (send {nil? const} {:#{m}} ...) ...)
Expand Down
25 changes: 19 additions & 6 deletions spec/rubocop/cop/lint/symbol_conversion_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@
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"'
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"'
it_behaves_like 'offense', '"foo-#{bar}".intern', ':"foo-#{bar}"'

# Unnecessary quoted symbol
it_behaves_like 'offense', ':"foo"', ':foo'
Expand All @@ -36,12 +38,6 @@
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 double quotes' do
expect_no_offenses(<<~RUBY)
:"foo-bar"
Expand Down Expand Up @@ -124,6 +120,12 @@
{ '==': 'bar' }
RUBY
end

it 'does not register an offense for dstr' do
expect_no_offenses(<<~'RUBY')
{ "foo-#{'bar'}": 'baz' }
RUBY
end
end

context 'values' do
Expand Down Expand Up @@ -154,6 +156,17 @@
{ foo: :bar }
RUBY
end

it 'registers an offense for dstr' do
expect_offense(<<~'RUBY')
{ foo: "bar-#{'baz'}".to_sym }
^^^^^^^^^^^^^^^^^^^^^ Unnecessary symbol conversion; use `:"bar-#{'baz'}"` instead.
RUBY

expect_correction(<<~'RUBY')
{ foo: :"bar-#{'baz'}" }
RUBY
end
end
end

Expand Down

0 comments on commit 2c0f759

Please sign in to comment.