Merge branch 'main' into redsun82/codegen-new-parent-child

Author: redsun82

cpp/ql/lib/semmlecode.cpp.dbscheme.stats

@@ -100,7 +100,8 @@ module ControlFlow {
   private newtype TNode =
     TExprNode(Expr e) { hasControlFlow(e) } or
     TStmtNode(Stmt s) or
-    TExitNode(Callable c) { exists(c.getBody()) }
+    TExitNode(Callable c) { exists(c.getBody()) } or
+    TAssertThrowNode(AssertStmt s)
 
   /** A node in the expression-level control-flow graph. */
   class Node extends TNode {
@@ -204,6 +205,25 @@ module ControlFlow {
     /** Gets the source location for this element. */
     override Location getLocation() { result = c.getLocation() }
   }
+
+  /** A control flow node indicating a failing assertion. */
+  class AssertThrowNode extends Node, TAssertThrowNode {
+    AssertStmt s;
+
+    AssertThrowNode() { this = TAssertThrowNode(s) }
+
+    override Stmt getEnclosingStmt() { result = s }
+
+    override Callable getEnclosingCallable() { result = s.getEnclosingCallable() }
+
+    override ExprParent getAstNode() { result = s }
+
+    /** Gets a textual representation of this element. */
+    override string toString() { result = "Assert Throw" }
+
+    /** Gets the source location for this element. */
+    override Location getLocation() { result = s.getLocation() }
+  }
 }
 
 class ControlFlowNode = ControlFlow::Node;
@@ -327,7 +347,17 @@ private module ControlFlowGraphImpl {
     )
   }
 
-  private ThrowableType assertionError() { result.hasQualifiedName("java.lang", "AssertionError") }
+  private ThrowableType actualAssertionError() {
+    result.hasQualifiedName("java.lang", "AssertionError")
+  }
+
+  private ThrowableType assertionError() {
+    result = actualAssertionError()
+    or
+    // In case `AssertionError` is not extracted, we use `Error` as a fallback.
+    not exists(actualAssertionError()) and
+    result.hasQualifiedName("java.lang", "Error")
+  }
 
   /**
    * Gets an exception type that may be thrown during execution of the
@@ -1123,12 +1153,7 @@ private module ControlFlowGraphImpl {
       or
       // `assert` statements may throw
       completion = ThrowCompletion(assertionError()) and
-      (
-        last(assertstmt.getMessage(), last, NormalCompletion())
-        or
-        not exists(assertstmt.getMessage()) and
-        last(assertstmt.getExpr(), last, BooleanCompletion(false, _))
-      )
+      last.(AssertThrowNode).getAstNode() = assertstmt
     )
     or
     // `throw` statements or throwing calls give rise to `Throw` completion
@@ -1547,7 +1572,15 @@ private module ControlFlowGraphImpl {
       or
       last(assertstmt.getExpr(), n, completion) and
       completion = BooleanCompletion(false, _) and
-      result = first(assertstmt.getMessage())
+      (
+        result = first(assertstmt.getMessage())
+        or
+        not exists(assertstmt.getMessage()) and
+        result.(AssertThrowNode).getAstNode() = assertstmt
+      )
+      or
+      last(assertstmt.getMessage(), n, NormalCompletion()) and
+      result.(AssertThrowNode).getAstNode() = assertstmt
     )
     or
     // When expressions:

java/ql/lib/semmle/code/java/ControlFlowGraph.qll

@@ -294,27 +294,23 @@ private module GuardsInput implements SharedGuards::InputSig<Location> {
     equals.getNumberOfParameters() = 2
   }
 
-  class EqualityTest extends Expr {
-    EqualityTest() {
-      this instanceof J::EqualityTest or
-      this.(MethodCall).getMethod() instanceof EqualsMethod or
-      objectsEquals(this.(MethodCall).getMethod())
-    }
-
-    Expr getAnOperand() {
-      result = this.(J::EqualityTest).getAnOperand()
-      or
-      result = this.(MethodCall).getAnArgument()
-      or
-      this.(MethodCall).getMethod() instanceof EqualsMethod and
-      result = this.(MethodCall).getQualifier()
-    }
-
-    boolean polarity() {
-      result = this.(J::EqualityTest).polarity()
+  pragma[nomagic]
+  predicate equalityTest(Expr eqtest, Expr left, Expr right, boolean polarity) {
+    exists(EqualityTest eq | eq = eqtest |
+      eq.getLeftOperand() = left and
+      eq.getRightOperand() = right and
+      eq.polarity() = polarity
+    )
+    or
+    exists(MethodCall call | call = eqtest and polarity = true |
+      call.getMethod() instanceof EqualsMethod and
+      call.getQualifier() = left and
+      call.getAnArgument() = right
       or
-      result = true and not this instanceof J::EqualityTest
-    }
+      objectsEquals(call.getMethod()) and
+      call.getArgument(0) = left and
+      call.getArgument(1) = right
+    )
   }
 
   class ConditionalExpr extends Expr instanceof J::ConditionalExpr {

java/ql/lib/semmle/code/java/controlflow/Guards.qll

@@ -251,7 +251,7 @@ public void ex18(boolean b, int[] xs, Object related) {
            (b && xs == null && related == null);
     if (b) {
       if (related == null) { return; }
-      xs[0] = 42; // FP - correlated conditions fails to recognize assert edges
+      xs[0] = 42; // OK
     }
   }
 }

java/ql/test/query-tests/Nullness/C.java

@@ -32,9 +32,6 @@
 | C.java:207:9:207:11 | obj | Variable $@ may be null at this access because of $@ assignment. | C.java:201:5:201:22 | Object obj | obj | C.java:201:12:201:21 | obj | this |
 | C.java:219:9:219:10 | o1 | Variable $@ may be null at this access as suggested by $@ null guard. | C.java:212:20:212:28 | o1 | o1 | C.java:213:9:213:18 | ... == ... | this |
 | C.java:233:7:233:8 | xs | Variable $@ may be null at this access because of $@ assignment. | C.java:231:5:231:56 | int[] xs | xs | C.java:231:11:231:55 | xs | this |
-| C.java:254:7:254:8 | xs | Variable $@ may be null at this access as suggested by $@ null guard. | C.java:248:31:248:38 | xs | xs | C.java:249:19:249:28 | ... == ... | this |
-| C.java:254:7:254:8 | xs | Variable $@ may be null at this access as suggested by $@ null guard. | C.java:248:31:248:38 | xs | xs | C.java:250:18:250:27 | ... != ... | this |
-| C.java:254:7:254:8 | xs | Variable $@ may be null at this access as suggested by $@ null guard. | C.java:248:31:248:38 | xs | xs | C.java:251:18:251:27 | ... == ... | this |
 | F.java:11:5:11:7 | obj | Variable $@ may be null at this access as suggested by $@ null guard. | F.java:8:18:8:27 | obj | obj | F.java:9:9:9:19 | ... == ... | this |
 | F.java:17:5:17:7 | obj | Variable $@ may be null at this access as suggested by $@ null guard. | F.java:14:18:14:27 | obj | obj | F.java:15:9:15:19 | ... == ... | this |
 | G.java:20:12:20:12 | s | Variable $@ may be null at this access as suggested by $@ null guard. | G.java:3:27:3:34 | s | s | G.java:5:9:5:17 | ... == ... | this |

java/ql/test/query-tests/Nullness/NullMaybe.expected

@@ -64,7 +64,7 @@ void m4(int[] a, int[] b) {
     int sum = 0;
     for (int i = 0; i < a.length; ) {
       sum += a[i++]; // OK
-      sum += a[i++]; // OK - FP
+      sum += a[i++]; // OK
     }
     int len = b.length;
     if ((len & 1) != 0)

java/ql/test/query-tests/RangeAnalysis/A.java

@@ -3,7 +3,6 @@
 | A.java:45:14:45:22 | ...[...] | This array access might be out of bounds, as the index might be equal to the array length. |
 | A.java:49:14:49:22 | ...[...] | This array access might be out of bounds, as the index might be equal to the array length. |
 | A.java:58:14:58:19 | ...[...] | This array access might be out of bounds, as the index might be equal to the array length. |
-| A.java:67:14:67:19 | ...[...] | This array access might be out of bounds, as the index might be equal to the array length. |
 | A.java:89:12:89:16 | ...[...] | This array access might be out of bounds, as the index might be equal to the array length. |
 | A.java:100:18:100:31 | ...[...] | This array access might be out of bounds, as the index might be equal to the array length + 8. |
 | A.java:113:14:113:21 | ...[...] | This array access might be out of bounds, as the index might be equal to the array length. |

java/ql/test/query-tests/RangeAnalysis/ArrayIndexOutOfBounds.expected

@@ -281,10 +281,6 @@ def get_all_properties_to_be_tested(
                     type=p.type if not p.is_predicate else None,
                     is_indexed=p.is_indexed,
                 )
-                if p.is_repeated and not p.is_optional:
-                    yield ql.PropertyForTest(f"getNumberOf{p.plural}", type="int")
-                elif p.is_optional and not p.is_repeated:
-                    yield ql.PropertyForTest(f"has{p.singular}")
 
     def _is_in_qltest_collapsed_hierarchy(
         self,
@@ -415,16 +411,6 @@ def _get_path_public(cls: schema.Class) -> pathlib.Path:
     ).with_suffix(".qll")
 
 
-def _partition_iter(x, pred):
-    x1, x2 = itertools.tee(x)
-    return filter(pred, x1), itertools.filterfalse(pred, x2)
-
-
-def _partition(l, pred):
-    """partitions a list according to boolean predicate"""
-    return map(list, _partition_iter(l, pred))
-
-
 def _get_stub(
     cls: schema.Class, base_import: str, generated_import_prefix: str
 ) -> ql.Stub:
@@ -650,29 +636,16 @@ def generate(opts, renderer):
                         test_dir / missing_test_source_filename,
                     )
                     continue
-                total_props, partial_props = _partition(
-                    resolver.get_all_properties_to_be_tested(c),
-                    lambda p: p.is_total,
-                )
                 renderer.render(
                     ql.ClassTester(
                         class_name=c.name,
-                        properties=total_props,
+                        properties=list(resolver.get_all_properties_to_be_tested(c)),
                         elements_module=elements_module,
                         # in case of collapsed hierarchies we want to see the actual QL class in results
                         show_ql_class="qltest_collapse_hierarchy" in c.pragmas,
                     ),
                     test_dir / f"{c.name}.ql",
                 )
-                for p in partial_props:
-                    renderer.render(
-                        ql.PropertyTester(
-                            class_name=c.name,
-                            elements_module=elements_module,
-                            property=p,
-                        ),
-                        test_dir / f"{c.name}_{p.getter}.ql",
-                    )
 
         final_synth_types = []
         non_final_synth_types = []

misc/codegen/generators/qlgen.py

@@ -248,13 +248,6 @@ class ClassTester(TesterBase):
     show_ql_class: bool = False
 
 
-@dataclass
-class PropertyTester(TesterBase):
-    template: ClassVar = "ql_test_property"
-
-    property: PropertyForTest
-
-
 @dataclass
 class MissingTestInstructions:
     template: ClassVar = "ql_test_missing"

misc/codegen/lib/ql.py

@@ -3,14 +3,28 @@
 import {{elements_module}}
 import TestUtils
 
-from {{class_name}} x{{#properties}}, {{#type}}{{.}}{{/type}}{{^type}}string{{/type}} {{getter}}{{/properties}}
-where toBeTested(x) and not x.isUnknown()
+query predicate instances({{class_name}} x{{#show_ql_class}}, string primaryQlClasses{{/show_ql_class}}{{#properties}}{{#is_total}}, string {{getter}}__label, {{#type}}{{.}}{{/type}}{{^type}}string{{/type}} {{getter}}{{/is_total}}{{/properties}}) {
+    toBeTested(x) and not x.isUnknown()
+    {{#show_ql_class}}
+    and primaryQlClasses = x.getPrimaryQlClasses()
+    {{/show_ql_class}}
+    {{#properties}}
+    {{#is_total}}
+    and {{getter}}__label = "{{getter}}:"
+    {{#type}}
+    and {{getter}} = x.{{getter}}()
+    {{/type}}
+    {{^type}}
+    and if x.{{getter}}() then {{getter}} = "yes" else {{getter}} = "no"
+    {{/type}}
+    {{/is_total}}
+    {{/properties}}
+}
+
 {{#properties}}
-{{#type}}
-and {{getter}} = x.{{getter}}()
-{{/type}}
-{{^type}}
-and if x.{{getter}}() then {{getter}} = "yes" else {{getter}} = "no"
-{{/type}}
+{{^is_total}}
+query predicate {{getter}}({{class_name}} x{{#is_indexed}}, int index{{/is_indexed}}, {{type}} {{getter}}) {
+    toBeTested(x) and not x.isUnknown() and {{getter}} = x.{{getter}}({{#is_indexed}}index{{/is_indexed}})
+}
+{{/is_total}}
 {{/properties}}
-select x{{#show_ql_class}}, x.getPrimaryQlClasses(){{/show_ql_class}}{{#properties}}, "{{getter}}:", {{getter}}{{/properties}}