Address PR feedback: add query predicate, default maxResults/placeholderString

Co-authored-by: MichaelRFairhurst <1627771+MichaelRFairhurst@users.noreply.github.com>

Author: Copilot

src/qtil/results/LimitResults.qll

@@ -4,33 +4,36 @@
  *
  * This is useful for queries that find multiple related entities per finding (such as fields,
  * parameters, or call sites) but where listing all of them would be too noisy. Instead, only the
- * top `N` entities are reported (ordered by a configurable key), and the message notes how many
+ * top `N` entities are reported (ordered by `placeholderString()`), and the message notes how many
  * were omitted.
  *
  * ## Usage
  *
- * Implement the `LimitResultsConfigSig` signature and instantiate the `LimitResults` module:
+ * Implement the `LimitResultsConfigSig` signature and instantiate the `LimitResults` module. Only
+ * `problem` and `message` are required — `placeholderString` and `maxResults` have sensible
+ * defaults:
  *
  * ```ql
  * module MyConfig implements LimitResultsConfigSig<MyFinding, MyEntity> {
  *   predicate problem(MyFinding finding, MyEntity entity) {
  *     entity = finding.getAnEntity()
  *   }
  *
+ *   bindingset[remaining]
  *   string message(MyFinding finding, MyEntity entity, string remaining) {
  *     result = "Finding $@ has entity $@" + remaining + "."
  *   }
- *
- *   string orderBy(MyEntity entity) { result = entity.getName() }
- *
- *   int maxResults() { result = 3 }
  * }
  *
  * module Results = LimitResults<MyFinding, MyEntity, MyConfig>;
+ * ```
+ *
+ * The instantiated module exposes a `problems` query predicate that can be used directly as
+ * the output of a query without any `from`/`where`/`select` boilerplate:
  *
- * from MyFinding finding, MyEntity entity, string message
- * where Results::hasLimitedResult(finding, entity, message)
- * select finding, message, entity, entity.getName()
+ * ```ql
+ * module Results = LimitResults<MyFinding, MyEntity, MyConfig>;
+ * // The query predicate Results::problems(...) is automatically part of the query output.
  * ```
  */
 
@@ -39,16 +42,16 @@ private import qtil.parameterization.SignatureTypes
 /**
  * A signature for configuring the `LimitResults` module.
  *
- * Implement this signature in a module to define the relationship between findings and entities,
- * the message format, the ordering of entities, and the maximum number of results to show per
- * finding.
+ * Only `problem` and `message` must be implemented. The predicates `placeholderString` and
+ * `maxResults` have defaults and may be overridden.
  */
-signature module LimitResultsConfigSig<FiniteType Finding, FiniteType Entity> {
+signature module LimitResultsConfigSig<FiniteStringableType Finding, FiniteStringableType Entity> {
   /**
    * The relationship between findings and their associated entities.
    *
    * Defines which entities are relevant to a given finding. All entities satisfying this predicate
-   * will be counted, but only the top `maxResults()` (ordered by `orderBy()`) will be reported.
+   * will be counted, but only the top `maxResults()` (ordered by `placeholderString()`) will be
+   * reported.
    */
   predicate problem(Finding finding, Entity entity);
 
@@ -63,40 +66,59 @@ signature module LimitResultsConfigSig<FiniteType Finding, FiniteType Entity> {
   string message(Finding finding, Entity entity, string remaining);
 
   /**
-   * The key to use when ordering entities within a finding (ascending).
+   * The display string for an entity, also used as the ordering key (ascending).
    *
-   * Entities with smaller order keys are reported first. When the total count exceeds
+   * Entities with smaller placeholder strings are reported first. When the total count exceeds
    * `maxResults()`, only the first `maxResults()` entities by this ordering are reported.
+   *
+   * Defaults to `entity.toString()`.
    */
-  string orderBy(Entity entity);
+  default string placeholderString(Entity entity) { result = entity.toString() }
 
   /**
    * The maximum number of entities to report per finding.
    *
    * When the total number of entities for a finding exceeds this value, only the first
-   * `maxResults()` entities (by `orderBy()`) are reported, and the message includes a
+   * `maxResults()` entities (by `placeholderString()`) are reported, and the message includes a
    * `" (and N more)"` suffix indicating the number of omitted entities.
+   *
+   * Defaults to `3`.
    */
-  int maxResults();
+  default int maxResults() { result = 3 }
 }
 
 /**
  * A module that limits the number of results reported per finding, appending an "and N more"
  * suffix when additional entities exist beyond the configured maximum.
  *
- * Use `hasLimitedResult` as the body of a `where` clause in a `select` statement. Each result
- * tuple corresponds to one of the top-ranked entities for a finding, together with the formatted
- * message that includes the remaining count suffix when applicable.
+ * The `problems` query predicate is the main entry point for use in a query. When this module is
+ * instantiated as `module Results = LimitResults<...>`, the predicate `Results::problems` is
+ * automatically part of the query output — no `from`/`where`/`select` boilerplate is needed.
  *
  * See `LimitResultsConfigSig` for configuration details.
  */
-module LimitResults<FiniteType Finding, FiniteType Entity, LimitResultsConfigSig<Finding, Entity> Config> {
+module LimitResults<FiniteStringableType Finding, FiniteStringableType Entity, LimitResultsConfigSig<Finding, Entity> Config> {
+  /**
+   * A query predicate that reports findings alongside one of their top-ranked entities and a
+   * formatted message. This is the primary way to use this module in a query.
+   *
+   * Each result tuple `(finding, msg, entity, entityStr)` corresponds to one of the top-ranked
+   * entities for a finding. `entityStr` is `Config::placeholderString(entity)`, suitable for use
+   * as the placeholder text in a `select` column alongside `entity`.
+   *
+   * At most `Config::maxResults()` entities are reported per finding.
+   */
+  query predicate problems(Finding finding, string msg, Entity entity, string entityStr) {
+    hasLimitedResult(finding, entity, msg) and
+    entityStr = Config::placeholderString(entity)
+  }
+
   /**
    * Holds for each finding and one of its top-ranked entities, providing the formatted message.
    *
    * At most `Config::maxResults()` entities are reported per finding. They are selected by ranking
    * all entities satisfying `Config::problem(finding, entity)` in ascending order of
-   * `Config::orderBy(entity)`, and taking those with rank <= `Config::maxResults()`.
+   * `Config::placeholderString(entity)`, and taking those with rank <= `Config::maxResults()`.
    *
    * The `message` is produced by `Config::message(finding, entity, remaining)`, where `remaining`
    * is `" (and N more)"` if the total exceeds `Config::maxResults()`, or `""` otherwise.
@@ -105,7 +127,9 @@ module LimitResults<FiniteType Finding, FiniteType Entity, LimitResultsConfigSig
     exists(int total, int ranked, string remaining |
       total = count(Entity e | Config::problem(finding, e)) and
       entity =
-        rank[ranked](Entity e | Config::problem(finding, e) | e order by Config::orderBy(e)) and
+        rank[ranked](Entity e | Config::problem(finding, e) |
+          e order by Config::placeholderString(e)
+        ) and
       ranked <= Config::maxResults() and
       (
         total > Config::maxResults() and

test/qtil/results/LimitResultsTest.expected

@@ -1 +1 @@
-| All 7 tests passed. |
+| All 8 tests passed. |

test/qtil/results/LimitResultsTest.ql

@@ -9,15 +9,11 @@ module TestConfig implements LimitResultsConfigSig<Bug, BugField> {
   string message(Bug bug, BugField field, string remaining) {
     result = bug.getName() + " has field " + field.getFieldName() + remaining
   }
-
-  string orderBy(BugField field) { result = field.getFieldName() }
-
-  int maxResults() { result = 3 }
 }
 
 module Results = LimitResults<Bug, BugField, TestConfig>;
 
-/** BugA has 1 field (X), which is fewer than maxResults=3, so all results are shown. */
+/** BugA has 1 field (X), which is fewer than maxResults=3 (default), so all results are shown. */
 class TestBugAResultCount extends Test, Case {
   override predicate run(Qnit test) {
     if count(BugField f, string msg | Results::hasLimitedResult(any(Bug b | b = TBugA()), f, msg)) =
@@ -41,7 +37,7 @@ class TestBugANoSuffix extends Test, Case {
   }
 }
 
-/** BugB has 3 fields (A, B, C), exactly maxResults=3, so all results are shown. */
+/** BugB has 3 fields (A, B, C), exactly maxResults=3 (default), so all results are shown. */
 class TestBugBResultCount extends Test, Case {
   override predicate run(Qnit test) {
     if count(BugField f, string msg | Results::hasLimitedResult(any(Bug b | b = TBugB()), f, msg)) =
@@ -73,7 +69,7 @@ class TestBugBNoSuffix extends Test, Case {
   }
 }
 
-/** BugC has 5 fields (A, B, C, D, E), which exceeds maxResults=3, so only 3 are shown. */
+/** BugC has 5 fields (A, B, C, D, E), which exceeds maxResults=3 (default), so only 3 are shown. */
 class TestBugCResultCount extends Test, Case {
   override predicate run(Qnit test) {
     if count(BugField f, string msg | Results::hasLimitedResult(any(Bug b | b = TBugC()), f, msg)) =
@@ -83,7 +79,10 @@ class TestBugCResultCount extends Test, Case {
   }
 }
 
-/** BugC shows only the first 3 fields alphabetically (A, B, C), not D or E. */
+/**
+ * BugC shows only the first 3 fields alphabetically (A, B, C), not D or E, using the default
+ * placeholderString ordering (toString() = "BugC.A", "BugC.B", ...).
+ */
 class TestBugCTopRanked extends Test, Case {
   override predicate run(Qnit test) {
     if
@@ -117,3 +116,31 @@ class TestBugCSuffix extends Test, Case {
     else test.fail("BugC: some results have wrong suffix")
   }
 }
+
+/**
+ * The `problems` query predicate returns (finding, msg, entity, entityStr) tuples, where
+ * entityStr is the default placeholderString (toString()).
+ */
+class TestProblemsQueryPredicate extends Test, Case {
+  override predicate run(Qnit test) {
+    if
+      // BugC: 3 problems shown, each entityStr = BugField.toString() = "BugC.<field>"
+      count(Bug b, string msg, BugField f, string fstr |
+        Results::problems(b, msg, f, fstr) and b = TBugC()
+      ) = 3 and
+      forall(Bug b, string msg, BugField f, string fstr |
+        Results::problems(b, msg, f, fstr) and b = TBugC()
+      |
+        fstr = "BugC." + f.getFieldName()
+      ) and
+      // BugA: 1 problem, entityStr = "BugA.X"
+      exists(Bug b, string msg, BugField f, string fstr |
+        Results::problems(b, msg, f, fstr) and
+        b = TBugA() and
+        fstr = "BugA.X"
+      )
+    then test.pass("problems query predicate returns correct results with entityStr = toString()")
+    else test.fail("problems query predicate returned unexpected results")
+  }
+}
+