Rust: Shorthand record construction in data flow

hvitved · hvitved · commit 707bf16d90f9 · 2025-02-06T19:19:18.000+01:00
diff --git a/rust/ql/lib/codeql/rust/controlflow/CfgNodes.qll b/rust/ql/lib/codeql/rust/controlflow/CfgNodes.qll
@@ -241,7 +241,7 @@ final class RecordExprCfgNode extends Nodes::RecordExprCfgNode {
     exists(RecordExprField ref |
       ref = node.getRecordExprFieldList().getAField() and
       any(ChildMapping mapping).hasCfgChild(node, ref.getExpr(), this, result) and
-      field = ref.getNameRef().getText()
+      field = ref.getFieldName()
     )
   }
 }
diff --git a/rust/ql/lib/codeql/rust/elements/internal/PathImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/PathImpl.qll
@@ -34,4 +34,24 @@ module Impl {
     pragma[nomagic]
     string getText() { result = this.getPart().getNameRef().getText() }
   }
+
+  /** A simple identifier path. */
+  class IdentPath extends Path {
+    private string name;
+
+    IdentPath() {
+      not this.hasQualifier() and
+      exists(PathSegment ps |
+        ps = this.getPart() and
+        not ps.hasGenericArgList() and
+        not ps.hasParenthesizedArgList() and
+        not ps.hasPathType() and
+        not ps.hasReturnTypeSyntax() and
+        name = ps.getNameRef().getText()
+      )
+    }
+
+    /** Gets the identifier name. */
+    string getName() { result = name }
+  }
 }
diff --git a/rust/ql/lib/codeql/rust/elements/internal/RecordExprFieldImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/RecordExprFieldImpl.qll
@@ -11,6 +11,9 @@ private import codeql.rust.elements.internal.generated.RecordExprField
  * be referenced directly.
  */
 module Impl {
+  private import rust
+  private import codeql.rust.elements.internal.PathImpl::Impl as PathImpl
+
   // the following QLdoc is generated: if you need to edit it, do it in the schema file
   /**
    * A field in a record expression. For example `a: 1` in:
@@ -24,7 +27,27 @@ module Impl {
     private string toStringPart(int index) {
       index = 0 and result = this.getNameRef().getText()
       or
-      index = 1 and result = ": " + this.getExpr().toAbbreviatedString()
+      index = 1 and this.hasNameRef() and result = ": "
+      or
+      index = 2 and
+      result = this.getExpr().toAbbreviatedString()
+    }
+
+    /**
+     * Gets the name of the field. This includes the case when shorthand syntax is used:
+     *
+     * ```rust
+     * Foo {
+     *   a: 1, // field name is `a`
+     *   b     // field name is `b`
+     * }
+     * ```
+     */
+    string getFieldName() {
+      result = this.getNameRef().getText()
+      or
+      not this.hasNameRef() and
+      result = this.getExpr().(PathExpr).getPath().(PathImpl::IdentPath).getName()
     }
   }
 }
diff --git a/rust/ql/lib/codeql/rust/elements/internal/RecordExprImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/RecordExprImpl.qll
@@ -32,7 +32,7 @@ module Impl {
     RecordField getRecordField(string name) {
       exists(PathResolution::ItemNode i |
         i = PathResolution::resolvePath(this.getPath()) and
-        name = this.getRecordExprFieldList().getAField().getNameRef().getText()
+        name = this.getRecordExprFieldList().getAField().getFieldName()
       |
         result.isStructField(i, name) or
         result.isVariantField(i, name)
diff --git a/rust/ql/lib/codeql/rust/elements/internal/RecordPatFieldImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/RecordPatFieldImpl.qll
@@ -21,6 +21,17 @@ module Impl {
    * ```
    */
   class RecordPatField extends Generated::RecordPatField {
+    override string toString() { result = concat(int i | | this.toStringPart(i) order by i) }
+
+    private string toStringPart(int index) {
+      index = 0 and result = this.getNameRef().getText()
+      or
+      index = 1 and this.hasNameRef() and result = ": "
+      or
+      index = 2 and
+      result = this.getPat().toAbbreviatedString()
+    }
+
     /**
      * Gets the name of the field. This includes the case when shorthand syntax is used:
      *
diff --git a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll
@@ -1,6 +1,7 @@
 private import rust
 private import codeql.rust.controlflow.ControlFlowGraph
 private import codeql.rust.elements.internal.generated.ParentChild
+private import codeql.rust.elements.internal.PathImpl::Impl as PathImpl
 private import codeql.rust.elements.internal.PathExprBaseImpl::Impl as PathExprBaseImpl
 private import codeql.rust.elements.internal.FormatTemplateVariableAccessImpl::Impl as FormatTemplateVariableAccessImpl
 private import codeql.util.DenseRank
@@ -172,16 +173,7 @@ module Impl {
     string name_;
 
     VariableAccessCand() {
-      exists(Path p, PathSegment ps |
-        p = this.(PathExpr).getPath() and
-        not p.hasQualifier() and
-        ps = p.getPart() and
-        not ps.hasGenericArgList() and
-        not ps.hasParenthesizedArgList() and
-        not ps.hasPathType() and
-        not ps.hasReturnTypeSyntax() and
-        name_ = ps.getNameRef().getText()
-      )
+      name_ = this.(PathExpr).getPath().(PathImpl::IdentPath).getName()
       or
       this.(FormatTemplateVariableAccess).getName() = name_
     }
diff --git a/rust/ql/test/extractor-tests/generated/RecordPatField/RecordPatField.expected b/rust/ql/test/extractor-tests/generated/RecordPatField/RecordPatField.expected
@@ -1,2 +1,2 @@
-| gen_record_pat_field.rs:5:15:5:18 | RecordPatField | getNumberOfAttrs: | 0 | hasNameRef: | yes | hasPat: | yes |
-| gen_record_pat_field.rs:5:21:5:24 | RecordPatField | getNumberOfAttrs: | 0 | hasNameRef: | yes | hasPat: | yes |
+| gen_record_pat_field.rs:5:15:5:18 | a: 1 | getNumberOfAttrs: | 0 | hasNameRef: | yes | hasPat: | yes |
+| gen_record_pat_field.rs:5:21:5:24 | b: 2 | getNumberOfAttrs: | 0 | hasNameRef: | yes | hasPat: | yes |
diff --git a/rust/ql/test/extractor-tests/generated/RecordPatField/RecordPatField_getNameRef.expected b/rust/ql/test/extractor-tests/generated/RecordPatField/RecordPatField_getNameRef.expected
@@ -1,2 +1,2 @@
-| gen_record_pat_field.rs:5:15:5:18 | RecordPatField | gen_record_pat_field.rs:5:15:5:15 | a |
-| gen_record_pat_field.rs:5:21:5:24 | RecordPatField | gen_record_pat_field.rs:5:21:5:21 | b |
+| gen_record_pat_field.rs:5:15:5:18 | a: 1 | gen_record_pat_field.rs:5:15:5:15 | a |
+| gen_record_pat_field.rs:5:21:5:24 | b: 2 | gen_record_pat_field.rs:5:21:5:21 | b |
diff --git a/rust/ql/test/extractor-tests/generated/RecordPatField/RecordPatField_getPat.expected b/rust/ql/test/extractor-tests/generated/RecordPatField/RecordPatField_getPat.expected
@@ -1,2 +1,2 @@
-| gen_record_pat_field.rs:5:15:5:18 | RecordPatField | gen_record_pat_field.rs:5:18:5:18 | 1 |
-| gen_record_pat_field.rs:5:21:5:24 | RecordPatField | gen_record_pat_field.rs:5:24:5:24 | 2 |
+| gen_record_pat_field.rs:5:15:5:18 | a: 1 | gen_record_pat_field.rs:5:18:5:18 | 1 |
+| gen_record_pat_field.rs:5:21:5:24 | b: 2 | gen_record_pat_field.rs:5:24:5:24 | 2 |
diff --git a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected
@@ -570,6 +570,7 @@ storeStep
 | main.rs:167:12:167:12 | 4 | Point3D.z | main.rs:162:13:168:5 | Point3D {...} |
 | main.rs:177:16:177:32 | Point {...} | Point3D.plane | main.rs:176:13:179:5 | Point3D {...} |
 | main.rs:177:27:177:27 | 2 | Point.x | main.rs:177:16:177:32 | Point {...} |
+| main.rs:177:30:177:30 | y | Point.y | main.rs:177:16:177:32 | Point {...} |
 | main.rs:178:12:178:12 | 4 | Point3D.z | main.rs:176:13:179:5 | Point3D {...} |
 | main.rs:195:27:195:36 | source(...) | MyTupleStruct(0) | main.rs:195:13:195:40 | MyTupleStruct(...) |
 | main.rs:195:39:195:39 | 2 | MyTupleStruct(1) | main.rs:195:13:195:40 | MyTupleStruct(...) |
diff --git a/rust/ql/test/library-tests/dataflow/local/inline-flow.expected b/rust/ql/test/library-tests/dataflow/local/inline-flow.expected
@@ -49,6 +49,16 @@ edges
 | main.rs:148:12:148:21 | source(...) | main.rs:147:13:150:5 | Point {...} [Point.x] | provenance |  |
 | main.rs:151:9:151:28 | Point {...} [Point.x] | main.rs:151:20:151:20 | a | provenance |  |
 | main.rs:151:20:151:20 | a | main.rs:152:10:152:10 | a | provenance |  |
+| main.rs:175:9:175:9 | y | main.rs:177:30:177:30 | y | provenance |  |
+| main.rs:175:13:175:22 | source(...) | main.rs:175:9:175:9 | y | provenance |  |
+| main.rs:176:9:176:9 | p [Point3D.plane, Point.y] | main.rs:180:11:180:11 | p [Point3D.plane, Point.y] | provenance |  |
+| main.rs:176:13:179:5 | Point3D {...} [Point3D.plane, Point.y] | main.rs:176:9:176:9 | p [Point3D.plane, Point.y] | provenance |  |
+| main.rs:177:16:177:32 | Point {...} [Point.y] | main.rs:176:13:179:5 | Point3D {...} [Point3D.plane, Point.y] | provenance |  |
+| main.rs:177:30:177:30 | y | main.rs:177:16:177:32 | Point {...} [Point.y] | provenance |  |
+| main.rs:180:11:180:11 | p [Point3D.plane, Point.y] | main.rs:181:9:184:9 | Point3D {...} [Point3D.plane, Point.y] | provenance |  |
+| main.rs:181:9:184:9 | Point3D {...} [Point3D.plane, Point.y] | main.rs:182:20:182:33 | Point {...} [Point.y] | provenance |  |
+| main.rs:182:20:182:33 | Point {...} [Point.y] | main.rs:182:31:182:31 | y | provenance |  |
+| main.rs:182:31:182:31 | y | main.rs:186:18:186:18 | y | provenance |  |
 | main.rs:195:9:195:9 | s [MyTupleStruct(0)] | main.rs:199:11:199:11 | s [MyTupleStruct(0)] | provenance |  |
 | main.rs:195:13:195:40 | MyTupleStruct(...) [MyTupleStruct(0)] | main.rs:195:9:195:9 | s [MyTupleStruct(0)] | provenance |  |
 | main.rs:195:27:195:36 | source(...) | main.rs:195:13:195:40 | MyTupleStruct(...) [MyTupleStruct(0)] | provenance |  |
@@ -232,6 +242,17 @@ nodes
 | main.rs:151:9:151:28 | Point {...} [Point.x] | semmle.label | Point {...} [Point.x] |
 | main.rs:151:20:151:20 | a | semmle.label | a |
 | main.rs:152:10:152:10 | a | semmle.label | a |
+| main.rs:175:9:175:9 | y | semmle.label | y |
+| main.rs:175:13:175:22 | source(...) | semmle.label | source(...) |
+| main.rs:176:9:176:9 | p [Point3D.plane, Point.y] | semmle.label | p [Point3D.plane, Point.y] |
+| main.rs:176:13:179:5 | Point3D {...} [Point3D.plane, Point.y] | semmle.label | Point3D {...} [Point3D.plane, Point.y] |
+| main.rs:177:16:177:32 | Point {...} [Point.y] | semmle.label | Point {...} [Point.y] |
+| main.rs:177:30:177:30 | y | semmle.label | y |
+| main.rs:180:11:180:11 | p [Point3D.plane, Point.y] | semmle.label | p [Point3D.plane, Point.y] |
+| main.rs:181:9:184:9 | Point3D {...} [Point3D.plane, Point.y] | semmle.label | Point3D {...} [Point3D.plane, Point.y] |
+| main.rs:182:20:182:33 | Point {...} [Point.y] | semmle.label | Point {...} [Point.y] |
+| main.rs:182:31:182:31 | y | semmle.label | y |
+| main.rs:186:18:186:18 | y | semmle.label | y |
 | main.rs:195:9:195:9 | s [MyTupleStruct(0)] | semmle.label | s [MyTupleStruct(0)] |
 | main.rs:195:13:195:40 | MyTupleStruct(...) [MyTupleStruct(0)] | semmle.label | MyTupleStruct(...) [MyTupleStruct(0)] |
 | main.rs:195:27:195:36 | source(...) | semmle.label | source(...) |
@@ -397,6 +418,7 @@ testFailures
 | main.rs:113:10:113:12 | a.0 | main.rs:111:11:111:20 | source(...) | main.rs:113:10:113:12 | a.0 | $@ | main.rs:111:11:111:20 | source(...) | source(...) |
 | main.rs:121:10:121:15 | ... .1 | main.rs:118:17:118:26 | source(...) | main.rs:121:10:121:15 | ... .1 | $@ | main.rs:118:17:118:26 | source(...) | source(...) |
 | main.rs:152:10:152:10 | a | main.rs:148:12:148:21 | source(...) | main.rs:152:10:152:10 | a | $@ | main.rs:148:12:148:21 | source(...) | source(...) |
+| main.rs:186:18:186:18 | y | main.rs:175:13:175:22 | source(...) | main.rs:186:18:186:18 | y | $@ | main.rs:175:13:175:22 | source(...) | source(...) |
 | main.rs:201:18:201:18 | x | main.rs:195:27:195:36 | source(...) | main.rs:201:18:201:18 | x | $@ | main.rs:195:27:195:36 | source(...) | source(...) |
 | main.rs:214:33:214:33 | n | main.rs:211:27:211:36 | source(...) | main.rs:214:33:214:33 | n | $@ | main.rs:211:27:211:36 | source(...) | source(...) |
 | main.rs:227:25:227:25 | n | main.rs:224:19:224:28 | source(...) | main.rs:227:25:227:25 | n | $@ | main.rs:224:19:224:28 | source(...) | source(...) |
diff --git a/rust/ql/test/library-tests/dataflow/local/main.rs b/rust/ql/test/library-tests/dataflow/local/main.rs
@@ -183,7 +183,7 @@ fn struct_nested_match() {
             z,
         } => {
             sink(x);
-            sink(y); // $ MISSING: hasValueFlow=93
+            sink(y); // $ hasValueFlow=93
             sink(z);
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -241,7 +241,7 @@ final class RecordExprCfgNode extends Nodes::RecordExprCfgNode {`
`241`	`241`	`exists(RecordExprField ref \|`
`242`	`242`	`ref = node.getRecordExprFieldList().getAField() and`
`243`	`243`	`any(ChildMapping mapping).hasCfgChild(node, ref.getExpr(), this, result) and`
`244`		`- field = ref.getNameRef().getText()`
	`244`	`+ field = ref.getFieldName()`
`245`	`245`	`)`
`246`	`246`	`}`
`247`	`247`	`}`
Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@ module Impl {`
`32`	`32`	`RecordField getRecordField(string name) {`
`33`	`33`	`exists(PathResolution::ItemNode i \|`
`34`	`34`	`i = PathResolution::resolvePath(this.getPath()) and`
`35`		`- name = this.getRecordExprFieldList().getAField().getNameRef().getText()`
	`35`	`+ name = this.getRecordExprFieldList().getAField().getFieldName()`
`36`	`36`	`\|`
`37`	`37`	`result.isStructField(i, name) or`
`38`	`38`	`result.isVariantField(i, name)`