Merge pull request #19448 from d10c/d10c/ruby-printast-order-fix

Ruby printAst: fix order for synth children of real parents

Author: d10c

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.

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()
       )
   }
 

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