Merge branch 'main' into redsun82/rust-tweaks

.devcontainer/devcontainer.json

@@ -1,4 +1,5 @@
 {
+  "image": "mcr.microsoft.com/devcontainers/base:ubuntu-24.04",
   "extensions": [
     "rust-lang.rust-analyzer",
     "bungcip.better-toml",

CODEOWNERS

@@ -42,3 +42,6 @@ MODULE.bazel @github/codeql-ci-reviewers
 # Misc
 /misc/scripts/accept-expected-changes-from-ci.py @RasmusWL
 /misc/scripts/generate-code-scanning-query-list.py @RasmusWL
+
+# .devcontainer
+/.devcontainer/ @github/codeql-ci-reviewers

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ProductFlow.qll

@@ -545,7 +545,7 @@ module ProductFlow {
     private predicate outImpl1(Flow1::PathNode pred1, Flow1::PathNode succ1, DataFlowCall call) {
       Flow1::PathGraph::edges(pred1, succ1, _, _) and
       exists(ReturnKindExt returnKind |
-        succ1.getNode() = returnKind.getAnOutNode(call) and
+        succ1.getNode() = getAnOutNodeExt(call, returnKind) and
         returnKind = getParamReturnPosition(_, pred1.asParameterReturnNode()).getKind()
       )
     }
@@ -573,7 +573,7 @@ module ProductFlow {
     private predicate outImpl2(Flow2::PathNode pred2, Flow2::PathNode succ2, DataFlowCall call) {
       Flow2::PathGraph::edges(pred2, succ2, _, _) and
       exists(ReturnKindExt returnKind |
-        succ2.getNode() = returnKind.getAnOutNode(call) and
+        succ2.getNode() = getAnOutNodeExt(call, returnKind) and
         returnKind = getParamReturnPosition(_, pred2.asParameterReturnNode()).getKind()
       )
     }

ql/ql/src/queries/style/ValidatePredicateGetReturns.ql

@@ -0,0 +1,37 @@
+/**
+ * @name Predicates starting with "get" or "as" should return a value
+ * @description Checks if predicates that start with "get" or "as" actually return a value.
+ * @kind problem
+ * @problem.severity warning
+ * @id ql/predicates-get-should-return-value
+ * @tags correctness
+ *       maintainability
+ * @precision high
+ */
+
+import ql
+import codeql_ql.ast.Ast
+
+/**
+ * Identifies predicates whose names start with "get", "as" followed by an uppercase letter.
+ * This ensures that only predicates like "getValue" are matched, excluding names like "getter".
+ */
+predicate isGetPredicate(Predicate pred, string prefix) {
+  prefix = pred.getName().regexpCapture("(get|as)[A-Z].*", 1)
+}
+
+/**
+ * Checks if a predicate has a return type. This is phrased negatively to not flag unresolved aliases.
+ */
+predicate hasNoReturnType(Predicate pred) {
+  not exists(pred.getReturnTypeExpr()) and
+  not pred.(ClasslessPredicate).getAlias() instanceof PredicateExpr
+  or
+  hasNoReturnType(pred.(ClasslessPredicate).getAlias().(PredicateExpr).getResolvedPredicate())
+}
+
+from Predicate pred, string prefix
+where
+  isGetPredicate(pred, prefix) and
+  hasNoReturnType(pred)
+select pred, "This predicate starts with '" + prefix + "' but does not return a value."

ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected

@@ -0,0 +1,6 @@
+| test.qll:4:11:4:18 | ClasslessPredicate getValue | This predicate starts with 'get' but does not return a value. |
+| test.qll:25:11:25:28 | ClasslessPredicate getImplementation2 | This predicate starts with 'get' but does not return a value. |
+| test.qll:28:11:28:19 | ClasslessPredicate getAlias2 | This predicate starts with 'get' but does not return a value. |
+| test.qll:31:11:31:17 | ClasslessPredicate asValue | This predicate starts with 'as' but does not return a value. |
+| test.qll:48:11:48:19 | ClasslessPredicate getAlias4 | This predicate starts with 'get' but does not return a value. |
+| test.qll:61:11:61:22 | ClasslessPredicate getDistance2 | This predicate starts with 'get' but does not return a value. |

ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.qlref

@@ -0,0 +1 @@
+queries/style/ValidatePredicateGetReturns.ql

ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll

@@ -0,0 +1,67 @@
+import ql
+
+// NOT OK -- Predicate starts with "get" but does not return a value
+predicate getValue() { none() }
+
+// OK -- starts with get and returns a value
+string getData() { result = "data" }
+
+// OK -- starts with get but followed by a lowercase letter, probably should be ignored
+predicate getterFunction() { none() }
+
+// OK -- starts with get and returns a value
+string getImplementation() { result = "implementation" }
+
+// OK -- is an alias
+predicate getAlias = getImplementation/0;
+
+// OK -- Starts with "get" but followed by a lowercase letter, probably be ignored
+predicate getvalue() { none() }
+
+// OK -- Does not start with "get", should be ignored
+predicate retrieveValue() { none() }
+
+// NOT OK -- starts with get and does not return value
+predicate getImplementation2() { none() }
+
+// NOT OK -- is an alias for a predicate which does not have a return value
+predicate getAlias2 = getImplementation2/0;
+
+// NOT OK -- starts with as and does not return value
+predicate asValue() { none() }
+
+// OK -- starts with as but followed by a lowercase letter, probably should be ignored
+predicate assessment() { none() }
+
+// OK -- starts with as and returns a value
+string asString() { result = "string" }
+
+// OK -- starts with get and returns a value
+HiddenType getInjectableCompositeActionNode() {
+  exists(HiddenType hidden | result = hidden.toString())
+}
+
+// OK
+predicate implementation4() { none() }
+
+// NOT OK -- is an alias
+predicate getAlias4 = implementation4/0;
+
+// OK -- is an alias
+predicate alias5 = implementation4/0;
+
+int root() { none() }
+
+predicate edge(int x, int y) { none() }
+
+// OK -- Higher-order predicate
+int getDistance(int x) = shortestDistances(root/0, edge/2)(_, x, result)
+
+// NOT OK -- Higher-order predicate that does not return a value even though has 'get' in the name
+predicate getDistance2(int x, int y) = shortestDistances(root/0, edge/2)(_, x, y)
+
+// OK
+predicate unresolvedAlias = unresolved/0;
+
+// OK -- unresolved alias
+predicate getUnresolvedAlias = unresolved/0;

rust/ql/consistency-queries/DataFlowConsistency.ql

@@ -5,12 +5,4 @@
  * @id rust/diagnostics/data-flow-consistency
  */
 
-import codeql.rust.dataflow.DataFlow::DataFlow as DataFlow
-private import rust
-private import codeql.rust.dataflow.internal.DataFlowImpl
-private import codeql.rust.dataflow.internal.TaintTrackingImpl
-private import codeql.dataflow.internal.DataFlowImplConsistency
-
-private module Input implements InputSig<Location, RustDataFlow> { }
-
-import MakeConsistency<Location, RustDataFlow, RustTaintTracking, Input>
+import codeql.rust.dataflow.internal.DataFlowConsistency

rust/ql/lib/codeql/rust/AstConsistency.qll

@@ -21,6 +21,11 @@ query predicate multipleToStrings(Element e, string cls, string s) {
  */
 query predicate multipleLocations(Locatable e) { strictcount(e.getLocation()) > 1 }
 
+/**
+ * Holds if `e` does not have a `Location`.
+ */
+query predicate noLocation(Locatable e) { not exists(e.getLocation()) }
+
 private predicate multiplePrimaryQlClasses(Element e) {
   strictcount(string cls | cls = e.getAPrimaryQlClass() and cls != "VariableAccess") > 1
 }
@@ -58,6 +63,9 @@ int getAstInconsistencyCounts(string type) {
   type = "Multiple locations" and
   result = count(Element e | multipleLocations(e) | e)
   or
+  type = "No location" and
+  result = count(Element e | noLocation(e) | e)
+  or
   type = "Multiple primary QL classes" and
   result = count(Element e | multiplePrimaryQlClasses(e) | e)
   or

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowConsistency.qll

@@ -0,0 +1,17 @@
+import codeql.rust.dataflow.DataFlow::DataFlow as DataFlow
+private import rust
+private import codeql.rust.dataflow.internal.DataFlowImpl
+private import codeql.rust.dataflow.internal.TaintTrackingImpl
+private import codeql.dataflow.internal.DataFlowImplConsistency
+
+private module Input implements InputSig<Location, RustDataFlow> {
+  predicate uniqueNodeLocationExclude(RustDataFlow::Node n) {
+    // Exclude nodes where the missing location can be explained by the
+    // underlying AST node not having a location.
+    not exists(n.asExpr().getLocation())
+  }
+
+  predicate missingLocationExclude(RustDataFlow::Node n) { not exists(n.asExpr().getLocation()) }
+}
+
+import MakeConsistency<Location, RustDataFlow, RustTaintTracking, Input>

rust/ql/src/queries/diagnostics/DataFlowConsistencyCounts.ql

@@ -5,17 +5,10 @@
  * @id rust/diagnostics/data-flow-consistency-counts
  */
 
-private import rust
-private import codeql.rust.dataflow.internal.DataFlowImpl
-private import codeql.rust.dataflow.internal.TaintTrackingImpl
-private import codeql.dataflow.internal.DataFlowImplConsistency
-
-private module Input implements InputSig<Location, RustDataFlow> { }
+import codeql.rust.dataflow.internal.DataFlowConsistency as Consistency
 
 // see also `rust/diagnostics/data-flow-consistency`, which lists the
 // individual inconsistency results.
 from string type, int num
-where
-  num =
-    MakeConsistency<Location, RustDataFlow, RustTaintTracking, Input>::getInconsistencyCounts(type)
+where num = Consistency::getInconsistencyCounts(type)
 select type, num

rust/ql/src/queries/summary/Stats.qll

@@ -7,7 +7,7 @@ private import codeql.rust.dataflow.internal.DataFlowImpl
 private import codeql.rust.dataflow.internal.TaintTrackingImpl
 private import codeql.rust.AstConsistency as AstConsistency
 private import codeql.rust.controlflow.internal.CfgConsistency as CfgConsistency
-private import codeql.dataflow.internal.DataFlowImplConsistency as DataFlowImplConsistency
+private import codeql.rust.dataflow.internal.DataFlowConsistency as DataFlowConsistency
 
 /**
  * Gets a count of the total number of lines of code in the database.
@@ -35,15 +35,9 @@ int getTotalCfgInconsistencies() {
   result = sum(string type | | CfgConsistency::getCfgInconsistencyCounts(type))
 }
 
-private module Input implements DataFlowImplConsistency::InputSig<Location, RustDataFlow> { }
-
 /**
  * Gets a count of the total number of data flow inconsistencies in the database.
  */
 int getTotalDataFlowInconsistencies() {
-  result =
-    sum(string type |
-      |
-      DataFlowImplConsistency::MakeConsistency<Location, RustDataFlow, RustTaintTracking, Input>::getInconsistencyCounts(type)
-    )
+  result = sum(string type | | DataFlowConsistency::getInconsistencyCounts(type))
 }

rust/ql/test/extractor-tests/generated/MacroItems/CONSISTENCY/AstConsistency.expected

@@ -0,0 +1,42 @@
+noLocation
+| file://:0:0:0:0 | ... .parent(...) |
+| file://:0:0:0:0 | ... .unwrap(...) |
+| file://:0:0:0:0 | ...: ... |
+| file://:0:0:0:0 | ...::Path |
+| file://:0:0:0:0 | ...::path |
+| file://:0:0:0:0 | ArgList |
+| file://:0:0:0:0 | ArgList |
+| file://:0:0:0:0 | MacroItems |
+| file://:0:0:0:0 | ParamList |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | RefType |
+| file://:0:0:0:0 | RefType |
+| file://:0:0:0:0 | RetType |
+| file://:0:0:0:0 | StmtList |
+| file://:0:0:0:0 | Use |
+| file://:0:0:0:0 | UseTree |
+| file://:0:0:0:0 | fn get_parent |
+| file://:0:0:0:0 | get_parent |
+| file://:0:0:0:0 | parent |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | std |
+| file://:0:0:0:0 | std |
+| file://:0:0:0:0 | std |
+| file://:0:0:0:0 | unwrap |
+| file://:0:0:0:0 | { ... } |

rust/ql/test/query-tests/diagnostics/AstConsistencyCounts.expected

@@ -2,3 +2,4 @@
 | Multiple parents | 0 |
 | Multiple primary QL classes | 0 |
 | Multiple toStrings | 0 |
+| No location | 0 |