ruby: refine rb/uninitialized-local-variable

ruby/ql/src/change-notes/2025-04-02-adjust-uninitialized-local-alert-message.md

@@ -0,0 +1,4 @@
+---
+category: majorAnalysis
+---
+* The query `rb/uninitialized-local-variable` now only produces alerts when it can find local flow; this will produce vastly fewer alerts, eliminating most false positives.

ruby/ql/src/change-notes/2025-04-02-adjust-uninitialized-local-metadata.md

@@ -0,0 +1,4 @@
+---
+category: queryMetadata
+---
+* The query `rb/uninitialized-local-variable` now only produces alerts when it can find local flow; we have adjusted the precision to 'high'.

ruby/ql/src/queries/variables/UninitializedLocal.ql

@@ -5,28 +5,87 @@
  * @kind problem
  * @problem.severity error
  * @id rb/uninitialized-local-variable
- * @tags reliability
+ * @tags quality
+ *       reliability
  *       correctness
- * @precision low
+ * @precision high
  */
 
 import codeql.ruby.AST
 import codeql.ruby.dataflow.SSA
+private import codeql.ruby.dataflow.internal.DataFlowPublic
+
+predicate factor(Expr e, Expr factor) {
+  factor = e
+  or
+  e.(BinaryOperation).getOperator() = ["||", "&&"] and
+  factor = e.(BinaryOperation).getAnOperand()
+  or
+  e.(BinaryOperation).getOperator() = ["||", "&&"] and
+  factor(e.(BinaryOperation).getAnOperand(), factor)
+}
+
+predicate previousConjunct(Expr e, Expr prev) {
+  exists(BinaryOperation b |
+    b.getOperator() = "&&" and
+    b.getRightOperand() = e
+  |
+    // 'prev' && 'e'
+    prev = b.getLeftOperand()
+    or
+    // (... && 'prev') && 'e'
+    b.getLeftOperand().(BinaryOperation).getOperator() = "&&" and
+    prev = b.getLeftOperand().(BinaryOperation).getRightOperand()
+    or
+    // (subtree['prev'] && _) && 'e'
+    b.getLeftOperand().(BinaryOperation).getOperator() = "&&" and
+    previousConjunct(b.getLeftOperand().(BinaryOperation).getRightOperand(), prev)
+  )
+}
+
+Expr evaluatingMention(LocalVariableReadAccess read) {
+  result = read
+  or
+  result.(AssignExpr).getLeftOperand() = read
+  or
+  result.(NotExpr).getOperand() = read
+}
 
 class RelevantLocalVariableReadAccess extends LocalVariableReadAccess {
   RelevantLocalVariableReadAccess() {
     not exists(MethodCall c |
       c.getReceiver() = this and
       c.getMethodName() = "nil?"
+    ) and
+    // 'a' is fine to be uninitialised in 'a || ...'
+    not exists(BinaryOperation b |
+      b.getLeftOperand() = this and
+      b.getOperator() = "||"
+    ) and
+    // The second 'a' cannot be uninitialised in 'a && (...a...)'
+    not exists(Expr parent |
+      parent.getAChild*() = this and
+      previousConjunct(parent, this.getVariable().getAnAccess())
+    ) and
+    // Various guards
+    not exists(ConditionalExpr c | factor(c.getCondition(), evaluatingMention(this))) and
+    not exists(ConditionalExpr c | factor(c.getCondition(), this.getVariable().getAnAccess()) |
+      this = c.getBranch(true).getAChild*()
     )
   }
 }
 
-from RelevantLocalVariableReadAccess read, LocalVariable v
+from RelevantLocalVariableReadAccess read, LocalVariable v, Node source
 where
   v = read.getVariable() and
-  exists(Ssa::Definition def |
-    def.getAnUltimateDefinition() instanceof Ssa::UninitializedDefinition and
-    read = def.getARead().getExpr()
+  exists(Ssa::Definition def, Ssa::UninitializedDefinition uninit, Node sink |
+    uninit = def.getAnUltimateDefinition() and
+    // def.getAnUltimateDefinition() instanceof Ssa::UninitializedDefinition and
+    read = def.getARead().getExpr() and
+    // localFlow(uninit, read)
+    source.asExpr() = uninit.getARead() and
+    sink.asExpr() = read.getAControlFlowNode() and
+    localFlow(source, sink)
   )
-select read, "Local variable $@ may be used before it is initialized.", v, v.getName()
+select read, "Here local variable $@ is using $@.", v, v.getName(), source,
+  "this potentially uninitialized value"