Add exclusions to reduce FP

atorralba · atorralba · commit 36ca97e4f607 · 2022-12-22T11:15:07.000+01:00
Predicate parameters that have a database type are excluded.

Also, uses of the exists variable in an agreggation or another quantifier are excluded.
diff --git a/ql/ql/src/queries/style/OmittableExists.ql b/ql/ql/src/queries/style/OmittableExists.ql
@@ -10,12 +10,22 @@
 
 import ql
 
+class AggregateOrForQuantifier extends AstNode {
+  AggregateOrForQuantifier() {
+    this instanceof FullAggregate or this instanceof Forex or this instanceof Forall
+  }
+}
+
 from VarDecl existsArgument, VarAccess use
 where
   existsArgument = any(Exists e).getAnArgument() and
   use = unique( | | existsArgument.getAnAccess()) and
-  exists(Call c, int argPos | c.getArgument(argPos) = use |
-    existsArgument.getType() = c.getTarget().getParameterType(argPos).getASuperType*()
-  )
+  exists(Call c, int argPos, Type paramType |
+    c.getArgument(argPos) = use and paramType = c.getTarget().getParameterType(argPos)
+  |
+    existsArgument.getType() = paramType.getASuperType*() and
+    not paramType instanceof DatabaseType
+  ) and
+  not use.getParent*() instanceof AggregateOrForQuantifier
 select existsArgument, "This exists variable can be omitted by using a don't-care expression $@.",
   use, "in this argument"
diff --git a/ql/ql/test/queries/style/OmittableExists/OmittableExists.expected b/ql/ql/test/queries/style/OmittableExists/OmittableExists.expected
@@ -1 +1 @@
-| Test.qll:10:10:10:14 | i | This exists variable can be omitted by using a don't-care expression $@. | Test.qll:10:29:10:29 | i | in this argument |
+| Test.qll:18:10:18:14 | i | This exists variable can be omitted by using a don't-care expression $@. | Test.qll:18:29:18:29 | i | in this argument |
diff --git a/ql/ql/test/queries/style/OmittableExists/Test.qll b/ql/ql/test/queries/style/OmittableExists/Test.qll
@@ -2,10 +2,18 @@ predicate aPredicate(int i) { none() }
 
 predicate anotherPredicate(int i) { none() }
 
+predicate yetAnotherPredicate(int i, int y) { none() }
+
+predicate dbTypePredicate(@location l) { none() }
+
 class SmallInt extends int {
   SmallInt() { this = [0 .. 10] }
 }
 
+class Location extends @location {
+  string toString() { result = "" }
+}
+
 predicate test() {
   exists(int i | aPredicate(i)) // BAD
   or
@@ -15,5 +23,13 @@ predicate test() {
   or
   exists(int i | aPredicate(i) and exists(int i2 | i = i2)) // GOOD
   or
+  exists(int i | count(int y | yetAnotherPredicate(i, y)) > 0) // GOOD
+  or
+  exists(int i | forex(int y | yetAnotherPredicate(i, y))) // GOOD
+  or
+  exists(int i | forall(int y | yetAnotherPredicate(i, y))) // GOOD
+  or
   exists(SmallInt i | aPredicate(i)) // GOOD
+  or
+  exists(Location l | dbTypePredicate(l)) // GOOD
 }

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-\| Test.qll:10:10:10:14 \| i \| This exists variable can be omitted by using a don't-care expression $@. \| Test.qll:10:29:10:29 \| i \| in this argument \|`
	`1`	`+\| Test.qll:18:10:18:14 \| i \| This exists variable can be omitted by using a don't-care expression $@. \| Test.qll:18:29:18:29 \| i \| in this argument \|`