Skip to content

Ruby: Fix bug in implicitAssignmentNode#21907

Open
hvitved wants to merge 2 commits into
github:mainfrom
hvitved:ruby/implicit-local-fix
Open

Ruby: Fix bug in implicitAssignmentNode#21907
hvitved wants to merge 2 commits into
github:mainfrom
hvitved:ruby/implicit-local-fix

Conversation

@hvitved
Copy link
Copy Markdown
Contributor

@hvitved hvitved commented May 29, 2026

A small bug fix where in cases like

class RescueSetter
  def name
    @name
  end

  def name=(value)
    @name = value
  end

  def foo(msg)
    raise msg
  rescue => self.name # calls `name=`
    :caught
  end
end

self.name was incorrectly marked as updating variables named self and name, instead of calling the name= setter.

DCA confirms that we remove some rb/useless-assignment-to-local FPs.

@github-actions github-actions Bot added Ruby Rust Pull requests that update Rust code labels May 29, 2026
Comment thread ruby/ql/lib/codeql/ruby/ast/internal/Variable.qll Fixed
Comment thread rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll Fixed
Comment thread shared/namebinding/codeql/namebinding/LocalNameBinding.qll Fixed
Comment thread shared/namebinding/codeql/namebinding/LocalNameBinding.qll Fixed
Comment thread shared/namebinding/codeql/namebinding/LocalNameBinding.qll Fixed
@hvitved hvitved force-pushed the ruby/implicit-local-fix branch from 0ef4595 to c319680 Compare June 2, 2026 07:04
@github-actions github-actions Bot removed the Rust Pull requests that update Rust code label Jun 2, 2026
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Jun 2, 2026
@hvitved hvitved marked this pull request as ready for review June 2, 2026 19:17
@hvitved hvitved requested a review from a team as a code owner June 2, 2026 19:17
Copilot AI review requested due to automatic review settings June 2, 2026 19:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes Ruby variable-scope/namebinding for the rescue ... => <target> exception-variable assignment when the target is a setter-like expression (e.g. self.name), avoiding incorrectly treating sub-expressions as implicit variable writes and reducing rb/useless-assignment-to-local false positives.

Changes:

  • Refines implicit/explicit assignment ancestor traversal in Variable.qll to only propagate through LHS-assignment container nodes.
  • Adds a Ruby regression test (RescueSetter) covering rescue => self.name setter behavior.
  • Updates variable/SSA/access expected outputs for the new test coverage and corrected binding.
Show a summary per file
File Description
ruby/ql/lib/codeql/ruby/ast/internal/Variable.qll Restricts recursive “inside assignment” detection to assignment-LHS container parents to avoid misclassifying setter targets.
ruby/ql/test/library-tests/variables/scopes.rb Adds RescueSetter demonstrating exception assignment to a setter-like target in rescue => ....
ruby/ql/test/library-tests/variables/varscopes.expected Updates expected scope ranges to include the new test class/methods.
ruby/ql/test/library-tests/variables/variable.expected Updates expected variable entries/scopes reflecting corrected implicit assignment behavior.
ruby/ql/test/library-tests/variables/varaccess.expected Updates expected variable access/write classifications for the new scenario.
ruby/ql/test/library-tests/variables/ssa.expected Updates expected SSA definitions/reads to match the corrected binding and new test code.
ruby/ql/test/library-tests/variables/parameter.expected Adds expected parameter-variable entries for value and msg from the new test methods.

Copilot's findings

  • Files reviewed: 7/7 changed files
  • Comments generated: 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note Ruby

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants