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 00000000000..b71b60c22b3 --- /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 c15b717610a..97c7119eae2 100644 --- a/ruby/ql/lib/codeql/ruby/printAst.qll +++ b/ruby/ql/lib/codeql/ruby/printAst.qll @@ -121,17 +121,22 @@ 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 + not exists(AstNode parent | + shouldPrintAstEdge(parent, _, astNode) and + synthChild(parent, _, astNode) + ) and result = 0 } + private int getSynthAstNodeIndexForSynthParent() { + if this.parentIsSynthesized() then result = this.getSynthAstNodeIndex() else result = 0 + } + override int getOrder() { this = rank[result](PrintRegularAstNode p, Location l, File f | @@ -140,8 +145,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 294edfdab1e..6263cb8919b 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