Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix #757] Fix a false positive for Rails/ReflectionClassName #758

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .rubocop_todo.yml
Expand Up @@ -11,6 +11,7 @@ InternalAffairs/NodeDestructuring:
Exclude:
- 'lib/rubocop/cop/rails/environment_comparison.rb'
- 'lib/rubocop/cop/rails/exit.rb'
- 'lib/rubocop/cop/rails/reflection_class_name.rb'
- 'lib/rubocop/cop/rails/relative_date_constant.rb'
- 'lib/rubocop/cop/rails/time_zone.rb'

Expand Down
@@ -0,0 +1 @@
* [#757](https://github.com/rubocop/rubocop-rails/issues/757): Fix a false positive for `Rails/ReflectionClassName` when using Ruby 3.1's hash shorthand syntax. ([@koic][])
17 changes: 17 additions & 0 deletions lib/rubocop/cop/rails/reflection_class_name.rb
Expand Up @@ -34,12 +34,29 @@ class ReflectionClassName < Base

def on_send(node)
association_with_reflection(node) do |reflection_class_name|
return if reflection_class_name.value.send_type? && reflection_class_name.value.receiver.nil?
return if reflection_class_name.value.lvar_type? && str_assigned?(reflection_class_name)

add_offense(reflection_class_name.loc.expression)
end
end

private

def str_assigned?(reflection_class_name)
lvar = reflection_class_name.value.source

reflection_class_name.ancestors.each do |nodes|
return true if nodes.each_child_node(:lvasgn).detect do |node|
lhs, rhs = *node

lhs.to_s == lvar && ALLOWED_REFLECTION_CLASS_TYPES.include?(rhs.type)
end
end

false
end

def reflection_class_value?(class_value)
if class_value.send_type?
!class_value.method?(:to_s) || class_value.receiver&.const_type?
Expand Down
48 changes: 48 additions & 0 deletions spec/rubocop/cop/rails/reflection_class_name_spec.rb
Expand Up @@ -80,4 +80,52 @@
belongs_to :account, class_name: :Account
RUBY
end

it 'registers an offense when parameter value is a local variable assigned a constant' do
expect_offense(<<~RUBY)
class_name = Account

has_many :accounts, class_name: class_name
^^^^^^^^^^^^^^^^^^^^^^ Use a string value for `class_name`.
RUBY
end

it 'does not register an offense when parameter value is a local variable assigned a string' do
expect_no_offenses(<<~RUBY)
class_name = 'Account'

has_many :accounts, class_name: class_name
RUBY
end

it 'does not register an offense when parameter value is a method call' do
expect_no_offenses(<<~RUBY)
has_many :accounts, class_name: class_name
RUBY
end

context 'Ruby >= 3.1', :ruby31 do
it 'registers an offense when shorthand syntax value is a local variable assigned a constant' do
expect_offense(<<~RUBY)
class_name = Account

has_many :accounts, class_name:
^^^^^^^^^^^ Use a string value for `class_name`.
RUBY
end

it 'does not register an offense when shorthand syntax value is a local variable assigned a string' do
expect_no_offenses(<<~RUBY)
class_name = 'Account'

has_many :accounts, class_name:
RUBY
end

it 'does not register an offense when shorthand syntax value is a local variable assigned a method call' do
expect_no_offenses(<<~RUBY)
has_many :accounts, class_name:
RUBY
end
end
end