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 #13395] False positive for Lint/UselessAssignment when assigning in branch and block #13409

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/fix_false_positive_for_useless_assignment.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#13395](https://github.com/rubocop/rubocop/issues/13395): Fix a false positive for `Lint/UselessAssignment` when assigning in branch and block. ([@pCosta99][])
14 changes: 13 additions & 1 deletion lib/rubocop/cop/variable_force/variable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ class VariableForce
# A Variable represents existence of a local variable.
# This holds a variable declaration node and some states of the variable.
class Variable
extend NodePattern::Macros

VARIABLE_DECLARATION_TYPES = (VARIABLE_ASSIGNMENT_TYPES + ARGUMENT_DECLARATION_TYPES).freeze

attr_reader :name, :declaration_node, :scope, :assignments, :references, :captured_by_block
Expand All @@ -31,11 +33,21 @@ def initialize(name, declaration_node, scope)
def assign(node)
assignment = Assignment.new(node, self)

@assignments.last&.reassigned! unless captured_by_block?
mark_last_as_reassigned!(assignment)

@assignments << assignment
end

def mark_last_as_reassigned!(assignment)
return if captured_by_block?
return if candidate_condition?(assignment.node.parent)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@koic Can I make parent an attribute in RuboCop::Cop::VariableForce::Assignment?
Seems like a worthy refactor, makes sense as an attribute and simplifies a bunch of accesses throughout the class


@assignments.last&.reassigned!
end

# @!method candidate_condition?(node)
def_node_matcher :candidate_condition?, '[{if case case_match when}]'

def referenced?
[email protected]?
end
Expand Down
45 changes: 45 additions & 0 deletions spec/rubocop/cop/lint/useless_assignment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,51 @@ def some_method
end
end

context 'when assigning in branch' do
it 'accepts' do
expect_no_offenses(<<~RUBY)
def some_method
changed = false

if Random.rand > 1
changed = true
end

[].each do
changed = true
end

puts changed
end
RUBY
end
end

context 'when assigning in case' do
it 'accepts' do
expect_no_offenses(<<~RUBY)
def some_method
changed = false

case Random.rand
when 0.5
changed = true
when 1..20
changed = false
when 21..70
changed = true
end

[].each do
changed = true
end

puts changed
end
RUBY
end
end

context "when a variable is reassigned in loop body but won't " \
'be referenced either next iteration or loop condition' do
it 'registers an offense' do
Expand Down