Rust: Apply suggestions from PR comments

github · Nov 22, 2024 · e81c348 · e81c348
1 parent fffeac6
commit e81c348
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 15 deletions.
diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
@@ -187,10 +187,10 @@ module Node {
    * The value of a parameter at function entry, viewed as a node in a data
    * flow graph.
    */
-  final class NormalParameterNode extends ParameterNode, TParameterNode {
+  final class PositionalParameterNode extends ParameterNode, TParameterNode {
     override ParamCfgNode n;
 
-    NormalParameterNode() { this = TParameterNode(n) }
+    PositionalParameterNode() { this = TParameterNode(n) }
 
     /** Gets the parameter in the CFG that this node corresponds to. */
     ParamCfgNode getParameter() { result = n }
@@ -230,10 +230,7 @@ module Node {
 
   /** A data flow node that represents a value returned by a callable. */
   final class ReturnNode extends ExprNode {
-    ReturnNode() {
-      this.getCfgNode().getASuccessor() instanceof ExitCfgNode or
-      this.getCfgNode().getASuccessor() instanceof AnnotatedExitCfgNode
-    }
+    ReturnNode() { this.getCfgNode().getASuccessor() instanceof AnnotatedExitCfgNode }
 
     ReturnKind getKind() { any() }
   }
@@ -270,11 +267,11 @@ module Node {
     /** Gets the node before the state update. */
     Node getPreUpdateNode() { result = TExprNode(n) }
 
-    final override CfgScope getCfgScope() { result = n.getAstNode().getEnclosingCfgScope() }
+    final override CfgScope getCfgScope() { result = n.getScope() }
 
-    final override Location getLocation() { result = n.getAstNode().getLocation() }
+    final override Location getLocation() { result = n.getLocation() }
 
-    final override string toString() { result = n.getAstNode().toString() }
+    final override string toString() { result = n.toString() }
   }
 
   final class CastNode = NaNode;
@@ -287,7 +284,7 @@ module SsaFlow {
   private module SsaFlow = SsaImpl::DataFlowIntegration;
 
   private Node::ParameterNode toParameterNode(ParamCfgNode p) {
-    result.(Node::NormalParameterNode).getParameter() = p
+    result.(Node::PositionalParameterNode).getParameter() = p
   }
 
   /** Converts a control flow node into an SSA control flow node. */
@@ -336,7 +333,8 @@ module LocalFlow {
     nodeFrom.(Node::AstCfgFlowNode).getCfgNode() =
       nodeTo.(Node::SsaNode).getDefinitionExt().(Ssa::WriteDefinition).getControlFlowNode()
     or
-    nodeFrom.(Node::NormalParameterNode).getParameter().getPat() = nodeTo.(Node::PatNode).getPat()
+    nodeFrom.(Node::PositionalParameterNode).getParameter().getPat() =
+      nodeTo.(Node::PatNode).getPat()
     or
     SsaFlow::localFlowStep(_, nodeFrom, nodeTo, _)
     or
@@ -376,7 +374,7 @@ module RustDataFlow implements InputSig<Location> {
 
   /** Holds if `p` is a parameter of `c` at the position `pos`. */
   predicate isParameterNode(ParameterNode p, DataFlowCallable c, ParameterPosition pos) {
-    p.getCfgNode().getAstNode() = pos.getParameterIn(c.asCfgScope().(Function).getParamList())
+    p.getCfgNode().getAstNode() = pos.getParameterIn(c.asCfgScope().(Callable).getParamList())
   }
 
   /** Holds if `n` is an argument of `c` at the position `pos`. */
@@ -590,7 +588,7 @@ private module Cached {
   cached
   newtype TParameterPosition =
     TPositionalParameterPosition(int i) {
-      exists(any(ParamList l).getParam(i)) or exists(any(ArgList l).getArg(i))
+      i in [0 .. max([any(ParamList l).getNumberOfParams(), any(ArgList l).getNumberOfArgs()]) - 1]
     } or
     TSelfParameterPosition()
 }

diff --git a/rust/ql/test/library-tests/dataflow/global/inline-flow.expected b/rust/ql/test/library-tests/dataflow/global/inline-flow.expected
@@ -64,7 +64,6 @@ subpaths
 | main.rs:41:26:44:5 | BlockExpr : unit | main.rs:30:17:30:22 | Param : unit | main.rs:30:32:32:1 | BlockExpr : unit | main.rs:41:13:44:6 | CallExpr : unit |
 | main.rs:90:29:90:29 | a : unit | main.rs:66:28:66:33 | Param : unit | main.rs:66:43:72:5 | BlockExpr : unit | main.rs:90:13:90:30 | ... .data_through(...) : unit |
 testFailures
-| main.rs:45:10:45:10 | a | Fixed missing result: hasValueFlow=14 |
 #select
 | main.rs:18:10:18:10 | a | main.rs:13:5:13:13 | CallExpr : unit | main.rs:18:10:18:10 | a | $@ | main.rs:13:5:13:13 | CallExpr : unit | CallExpr : unit |
 | main.rs:22:10:22:10 | n | main.rs:26:13:26:21 | CallExpr : unit | main.rs:22:10:22:10 | n | $@ | main.rs:26:13:26:21 | CallExpr : unit | CallExpr : unit |

diff --git a/rust/ql/test/library-tests/dataflow/global/main.rs b/rust/ql/test/library-tests/dataflow/global/main.rs
@@ -42,7 +42,7 @@ fn block_expression_as_argument() {
         println!("Hello");
         source(14)
     });
-    sink(a); // $ MISSING: hasValueFlow=14
+    sink(a); // $ hasValueFlow=14
 }
 
 // -----------------------------------------------------------------------------