Ruby: Replace CFG with shared implementation by aschackmull · Pull Request #22158 · github/codeql

aschackmull · 2026-07-10T11:46:40Z

This replaces the Ruby control flow graph with an instantiation of the shared library.

+ guardNode.isAfterValue(guard.getAstNode(), s) and
+ s.getValue() = branch and
+ guardNode.getBasicBlock().dominates(bb)


Copilot

Pull request overview

Replaces Ruby’s custom control-flow graph implementation with the shared control-flow library and updates dependent analyses and test baselines.

Changes:

Instantiates the shared CFG framework for Ruby and removes bespoke CFG, completion, splitting, and basic-block implementations.
Migrates SSA, data-flow, security, framework, and IDE consumers to shared CFG APIs.
Refreshes affected expected test results and consistency checks.

Show a summary per file

File	Description
`ruby/ql/test/library-tests/variables/ssa.expected`	Updates SSA results for the shared CFG.
`ruby/ql/test/library-tests/frameworks/active_record/ActiveRecord.expected`	Updates Active Record data-flow results.
`ruby/ql/test/library-tests/dataflow/type-tracker/TypeTracker.expected`	Updates type-tracking results.
`ruby/ql/test/library-tests/dataflow/local/Nodes.expected`	Updates local data-flow nodes.
`ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.ql`	Uses shared basic-block types.
`ruby/ql/test/library-tests/controlflow/graph/Nodes.expected`	Updates CFG node results.
`ruby/ql/test/library-tests/controlflow/graph/Cfg.ql`	Uses shared CFG test output.
`ruby/ql/test/library-tests/controlflow/graph/BasicBlocks.ql`	Adapts basic-block tests.
`ruby/ql/test/library-tests/ast/ValueText.expected`	Updates CFG-backed value results.
`ruby/ql/src/queries/variables/DeadStoreOfLocal.ql`	Uses enclosing callable scope.
`ruby/ql/src/queries/performance/DatabaseQueryInLoop.ql`	Updates loop scope lookup.
`ruby/ql/src/experimental/performance/UseDetect.ql`	Uses direct control-flow-node lookup.
`ruby/ql/lib/ide-contextual-queries/printCfg.ql`	Migrates CFG visualization APIs.
`ruby/ql/lib/codeql/ruby/security/ConditionalBypassCustomizations.qll`	Adapts edge-dominance checks.
`ruby/ql/lib/codeql/ruby/frameworks/Sinatra.qll`	Updates callable scope lookup.
`ruby/ql/lib/codeql/ruby/dataflow/SSA.qll`	Simplifies phi-node rendering.
`ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll`	Migrates SSA construction to shared CFG blocks.
`ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll`	Migrates internal data-flow CFG usage.
`ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll`	Updates call scope and file lookup.
`ruby/ql/lib/codeql/ruby/controlflow/internal/Splitting.qll`	Removes bespoke CFG splitting.
`ruby/ql/lib/codeql/ruby/controlflow/internal/NonReturning.qll`	Maps non-returning calls to shared successor types.
`ruby/ql/lib/codeql/ruby/controlflow/internal/Guards.qll`	Adapts guard dominance logic.
`ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll`	Removes the previous CFG implementation.
`ruby/ql/lib/codeql/ruby/controlflow/internal/Completion.qll`	Removes custom completion modeling.
`ruby/ql/lib/codeql/ruby/controlflow/ControlFlowGraph.qll`	Instantiates and configures the shared CFG.
`ruby/ql/lib/codeql/ruby/controlflow/CfgNodes.qll`	Migrates Ruby CFG node wrappers.
`ruby/ql/lib/codeql/ruby/controlflow/BasicBlocks.qll`	Removes custom basic-block wrappers.
`ruby/ql/lib/codeql/ruby/CFG.qll`	Drops the removed basic-block import.
`ruby/ql/lib/codeql/ruby/ast/Statement.qll`	Exposes shared control-flow-node mappings.
`ruby/ql/lib/codeql/ruby/ast/internal/Synthesis.qll`	Excludes synthesized assignment operands from CFG construction.
`ruby/ql/consistency-queries/DataFlowConsistency.ql`	Removes split-specific exclusions.
`ruby/ql/consistency-queries/CfgConsistency.ql`	Uses shared CFG consistency checks.

Review details

Tip

Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Files reviewed: 34/38 changed files
Comments generated: 0
Review effort level: Balanced

hvitved

Looks great, thanks for doing this. I have a few minor comments.

hvitved · 2026-08-24T08:15:03Z

+ ControlFlowNode getControlFlowNode() { result.injects(this) }
+
 /** Gets a control-flow node for this statement, if any. */
 CfgNodes::AstCfgNode getAControlFlowNode() { result.getAstNode() = this }


I think we should deprecate this predicate.

hvitved · 2026-08-24T08:21:13Z

+ }
+
+ private R::Ast::AstNode adjustedGetChild(R::Ast::AstNode parent, int index) {
+ exists(R::Ast::WhenClause when | parent = when |


I have a mild preference for parent = any(R::Ast::WhenClause when | ... (same in other cases below).

I tried to see how it looks and it generally becomes longer and more indented, so I'll leave it as-is.

hvitved · 2026-08-24T08:23:40Z

+ or
+ findpattern.getElement(index) = result
+ or
+ index = 1 + max(int i | exists(findpattern.getElement(i))) and


Use count instead as in the ArrayPattern case (not sure if there can be 0 elements)?

Yep - that's simpler. I don't think there can be 0 elements, though.

hvitved · 2026-08-24T08:31:01Z

+ result = c.(R::Ast::Toplevel).getABeginBlock() or
+ result = c.(R::Ast::Toplevel).getAStmt() or


Perhaps result = callableGetBodyPart(c, _, _) instead?

I've factored out the common part.

hvitved · 2026-08-24T08:36:40Z

+ n1.isAfterTrue(ce.getCondition()) and not exists(ce.getThen()) and n2.isAfter(ce)
+ or
+ n1.isAfterFalse(ce.getCondition()) and not exists(ce.getElse()) and n2.isAfter(ce)


Should this instead be done in the shared library?

Sure, done.

aschackmull · 2026-08-25T11:55:01Z

Rerun has been triggered: 3 restarted 🚀

hvitved

🎉

github-actionsBot added the Ruby label Jul 10, 2026

github-advanced-securityBot found potential problems Jul 10, 2026
View reviewed changes

Comment threadruby/ql/lib/codeql/ruby/controlflow/ControlFlowGraph2.qll Fixed

aschackmull force-pushed the ruby/cfg-swap branch 3 times, most recently from 597c62a to e8c7a76Compare July 10, 2026 13:02

aschackmull force-pushed the ruby/cfg-swap branch 3 times, most recently from 10337a2 to 8b0d578Compare July 30, 2026 12:03

github-advanced-securityBot found potential problems Aug 4, 2026
View reviewed changes

Comment threadruby/ql/lib/codeql/ruby/controlflow/internal/Guards.qll Fixed

aschackmull force-pushed the ruby/cfg-swap branch from 0baafe3 to f374034Compare August 6, 2026 08:29

github-advanced-securityBot found potential problems Aug 6, 2026
View reviewed changes

Comment threadruby/ql/lib/codeql/ruby/controlflow/internal/Guards.qll Fixed

aschackmull force-pushed the ruby/cfg-swap branch 4 times, most recently from c74162e to 8dcd6e6Compare August 11, 2026 12:10

github-advanced-securityBot found potential problems Aug 11, 2026
View reviewed changes

Comment threadruby/ql/lib/codeql/ruby/controlflow/internal/Guards.qll
Comment on lines +16 to +18
guardNode.isAfterValue(guard.getAstNode(), s) and
s.getValue() = branch and
guardNode.getBasicBlock().dominates(bb)

aschackmull marked this pull request as ready for review August 11, 2026 12:50

aschackmull requested a review from a team as a code owner August 11, 2026 12:50

CopilotAI balanced review requested due to automatic review settings August 11, 2026 12:50

aschackmull added the no-change-note-required This PR does not need a change note label Aug 11, 2026

Copilotstarted reviewing on behalf of aschackmull August 11, 2026 12:51 View session

CopilotAI reviewed Aug 11, 2026
View reviewed changes

aschackmull force-pushed the ruby/cfg-swap branch 2 times, most recently from 99eb18e to 9859c59Compare August 20, 2026 11:29

aschackmull added 6 commits August 20, 2026 15:05

Ruby: Replace control flow graph with shared implementation.
b1ab236

Ruby: Accept qltest changes due to removed splitting.
1b7dde8

Ruby: Accept qltest changes due to method body inclusion in CFG.
54e4bb3

Ruby: Accept new nodes in END block.
c42cd27

Ruby: Accept location changes for SSA.
9584382

Ruby: Accept remaining qltests.
a293c98

aschackmull added 3 commits August 20, 2026 15:05

Ruby: Fix joinorder.
0ffc606

Ruby: Reintroduce lack of BeginExpr nodes.
496f9e6

Ruby: Accept qltest changes from BeginExpr test.
879280a

aschackmull force-pushed the ruby/cfg-swap branch from 9859c59 to 879280aCompare August 20, 2026 13:08

Ruby: Add change note.
736b110

aschackmull removed the no-change-note-required This PR does not need a change note label Aug 24, 2026

github-actionsBot added the documentation label Aug 24, 2026

hvitved reviewed Aug 24, 2026
View reviewed changes

aschackmull added 2 commits August 25, 2026 13:06

Ruby: Deprecate getAControlFlowNode.
4641bdc

Ruby: Address review comments.
ac0438f

aschackmull requested a review from a team as a code owner August 25, 2026 11:24

aschackmull mentioned this pull request Aug 25, 2026
Ruby: Test exception splitting removal. #22359
Closed

hvitved approved these changes Aug 25, 2026
View reviewed changes

aschackmull merged commit daa37c0 into github:mainAug 25, 2026
113 checks passed

aschackmull deleted the ruby/cfg-swap branch August 25, 2026 12:18

		result = c.(R::Ast::Toplevel).getABeginBlock() or
		result = c.(R::Ast::Toplevel).getAStmt() or

Uh oh!

Conversation

aschackmull commented Jul 10, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

hvitved left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aschackmull commented Aug 25, 2026• edited by codeql-ci-app BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hvitved left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aschackmull commented Jul 10, 2026•
edited
Loading

aschackmull commented Aug 25, 2026•
edited by codeql-ci-app Bot
Loading