diff --git a/ruby/ql/lib/change-notes/2025-05-02-ruby-printast-order-fix.md b/ruby/ql/lib/change-notes/2025-05-02-ruby-printast-order-fix.md new file mode 100644 index 000000000000..b71b60c22b3d --- /dev/null +++ b/ruby/ql/lib/change-notes/2025-05-02-ruby-printast-order-fix.md @@ -0,0 +1,6 @@ +--- +category: fix +--- +### Bug Fixes + +* The Ruby printAst.qll library now orders AST nodes slightly differently: child nodes that do not literally appear in the source code, but whose parent nodes do, are assigned a deterministic order based on a combination of source location and logical order within the parent. This fixes the non-deterministic ordering that sometimes occurred depending on evaluation order. The effect may also be visible in downstream uses of the printAst library, such as the AST view in the VSCode extension. diff --git a/ruby/ql/lib/codeql/ruby/printAst.qll b/ruby/ql/lib/codeql/ruby/printAst.qll index c15b717610ab..947f8de06ef2 100644 --- a/ruby/ql/lib/codeql/ruby/printAst.qll +++ b/ruby/ql/lib/codeql/ruby/printAst.qll @@ -121,15 +121,21 @@ class PrintRegularAstNode extends PrintAstNode, TPrintRegularAstNode { } private int getSynthAstNodeIndex() { - this.parentIsSynthesized() and - exists(AstNode parent | - shouldPrintAstEdge(parent, _, astNode) and - parent.isSynthesized() and - synthChild(parent, result, astNode) - ) - or - not this.parentIsSynthesized() and - result = 0 + if + exists(AstNode parent | + shouldPrintAstEdge(parent, _, astNode) and + synthChild(parent, _, astNode) + ) + then + exists(AstNode parent | + shouldPrintAstEdge(parent, _, astNode) and + synthChild(parent, result, astNode) + ) + else result = 0 + } + + private int getSynthAstNodeIndexForSynthParent() { + if this.parentIsSynthesized() then result = this.getSynthAstNodeIndex() else result = 0 } override int getOrder() { @@ -140,8 +146,9 @@ class PrintRegularAstNode extends PrintAstNode, TPrintRegularAstNode { | p order by - f.getBaseName(), f.getAbsolutePath(), l.getStartLine(), p.getSynthAstNodeIndex(), - l.getStartColumn(), l.getEndLine(), l.getEndColumn() + f.getBaseName(), f.getAbsolutePath(), l.getStartLine(), + p.getSynthAstNodeIndexForSynthParent(), l.getStartColumn(), p.getSynthAstNodeIndex(), + l.getEndLine(), l.getEndColumn() ) } diff --git a/ruby/ql/test/library-tests/ast/Ast.expected b/ruby/ql/test/library-tests/ast/Ast.expected index 294edfdab1e6..6263cb8919b5 100644 --- a/ruby/ql/test/library-tests/ast/Ast.expected +++ b/ruby/ql/test/library-tests/ast/Ast.expected @@ -3185,14 +3185,14 @@ params/params.rb: # 106| getParameter: [HashSplatParameter] **kwargs # 106| getDefiningAccess: [LocalVariableAccess] kwargs # 107| getStmt: [SuperCall] super call to m -# 107| getArgument: [HashSplatExpr] ** ... -# 107| getAnOperand/getOperand/getReceiver: [LocalVariableAccess] kwargs +# 107| getArgument: [LocalVariableAccess] y +# 107| getArgument: [SplatExpr] * ... +# 107| getAnOperand/getOperand/getReceiver: [LocalVariableAccess] rest # 107| getArgument: [Pair] Pair # 107| getKey: [SymbolLiteral] k # 107| getValue: [LocalVariableAccess] k -# 107| getArgument: [SplatExpr] * ... -# 107| getAnOperand/getOperand/getReceiver: [LocalVariableAccess] rest -# 107| getArgument: [LocalVariableAccess] y +# 107| getArgument: [HashSplatExpr] ** ... +# 107| getAnOperand/getOperand/getReceiver: [LocalVariableAccess] kwargs # 111| getStmt: [MethodCall] call to m # 111| getReceiver: [MethodCall] call to new # 111| getReceiver: [ConstantReadAccess] Sub