-
Notifications
You must be signed in to change notification settings - Fork 0
Add ranked "and N more" reporting module #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
290295d
49c6b01
ba7bd9d
d58a5a0
8477200
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| codeql_home/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| /** | ||
| * A module for limiting the number of results reported per finding to at most `N`, while appending | ||
| * an "and M more" suffix to the message when there are additional results beyond the limit. | ||
| * | ||
| * 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 `placeholderString()`), and the message notes how many | ||
| * were omitted. | ||
| * | ||
| * ## Usage | ||
| * | ||
| * 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 + "." | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. documentation error: should be
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in d58a5a0 — the example now uses |
||
| * } | ||
| * } | ||
| * | ||
| * 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: | ||
| * | ||
| * ```ql | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Delete this code block, the above paragraph covers it already.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed in d58a5a0. |
||
| * module Results = LimitResults<MyFinding, MyEntity, MyConfig>; | ||
| * // The query predicate Results::problems(...) is automatically part of the query output. | ||
| * ``` | ||
| */ | ||
|
|
||
| private import qtil.parameterization.SignatureTypes | ||
|
|
||
| /** | ||
| * A signature for configuring the `LimitResults` module. | ||
| * | ||
| * Only `problem` and `message` must be implemented. The predicates `placeholderString` and | ||
| * `maxResults` have defaults and may be overridden. | ||
| */ | ||
| 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 `placeholderString()`) will be | ||
| * reported. | ||
| */ | ||
| predicate problem(Finding finding, Entity entity); | ||
|
|
||
| /** | ||
| * Builds the message for the finding, incorporating the "and N more" remaining string. | ||
| * | ||
| * The `remaining` parameter is either an empty string (when all entities are shown) or a string | ||
| * like `" (and 2 more)"` when some entities are omitted. The message should embed `remaining` | ||
| * appropriately, for example: `result = "Foo is broken" + remaining + "."`. | ||
| */ | ||
| bindingset[remaining] | ||
| string message(Finding finding, Entity entity, string remaining); | ||
|
|
||
| /** | ||
| * The display string for an entity, also used as the ordering key (ascending). | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make "also used as the ordering key (ascending)" the default behavior. E.g., add a new
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in d58a5a0 — added |
||
| * | ||
| * 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()`. | ||
| */ | ||
| 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 `placeholderString()`) are reported, and the message includes a | ||
| * `" (and N more)"` suffix indicating the number of omitted entities. | ||
| * | ||
| * Defaults to `3`. | ||
| */ | ||
| default int maxResults() { result = 3 } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a default
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in d58a5a0 — added |
||
| } | ||
|
|
||
| /** | ||
| * 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. | ||
| * | ||
| * 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<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::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. | ||
| */ | ||
| predicate hasLimitedResult(Finding finding, Entity entity, string message) { | ||
| 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::placeholderString(e) | ||
| ) and | ||
| ranked <= Config::maxResults() and | ||
| ( | ||
| total > Config::maxResults() and | ||
| remaining = " (and " + (total - Config::maxResults()) + " more)" | ||
| or | ||
| total <= Config::maxResults() and remaining = "" | ||
| ) and | ||
| message = Config::message(finding, entity, remaining) | ||
| ) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| /** | ||
| * Test data for LimitResultsTest.ql. | ||
| * | ||
| * Models a simple set of "bugs", each associated with a set of "fields". | ||
| * | ||
| * - BugA: 1 field (X) - fewer than maxResults | ||
| * - BugB: 3 fields (A, B, C) - exactly maxResults | ||
| * - BugC: 5 fields (A, B, C, D, E) - more than maxResults | ||
| */ | ||
| newtype TBug = | ||
| TBugA() or | ||
| TBugB() or | ||
| TBugC() | ||
|
|
||
| class Bug extends TBug { | ||
| string getName() { | ||
| this = TBugA() and result = "BugA" | ||
| or | ||
| this = TBugB() and result = "BugB" | ||
| or | ||
| this = TBugC() and result = "BugC" | ||
| } | ||
|
|
||
| string toString() { result = getName() } | ||
| } | ||
|
|
||
| newtype TBugField = | ||
| TBugFieldPair(string bugName, string fieldName) { | ||
| bugName = "BugA" and fieldName = "X" | ||
| or | ||
| bugName = "BugB" and fieldName = ["A", "B", "C"] | ||
| or | ||
| bugName = "BugC" and fieldName = ["A", "B", "C", "D", "E"] | ||
| } | ||
|
|
||
| class BugField extends TBugField { | ||
| string getBugName() { this = TBugFieldPair(result, _) } | ||
|
|
||
| string getFieldName() { this = TBugFieldPair(_, result) } | ||
|
|
||
| string toString() { result = getBugName() + "." + getFieldName() } | ||
|
|
||
| Bug getBug() { result.getName() = getBugName() } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| | All 8 tests passed. | |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| import qtil.results.LimitResults | ||
| import qtil.testing.Qnit | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this case, we shouldn't use unit tests and should use a query test. Simply import
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Converted in d58a5a0. The test now just implements |
||
| import Bugs | ||
|
|
||
| module TestConfig implements LimitResultsConfigSig<Bug, BugField> { | ||
| predicate problem(Bug bug, BugField field) { field.getBug() = bug } | ||
|
|
||
| bindingset[remaining] | ||
| string message(Bug bug, BugField field, string remaining) { | ||
| result = bug.getName() + " has field " + field.getFieldName() + remaining | ||
| } | ||
| } | ||
|
|
||
| module Results = LimitResults<Bug, BugField, TestConfig>; | ||
|
|
||
| /** 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)) = | ||
| 1 | ||
| then test.pass("BugA: 1 result shown (fewer than maxResults)") | ||
| else test.fail("BugA: wrong result count") | ||
| } | ||
| } | ||
|
|
||
| /** BugA shows field X with no suffix since total <= maxResults. */ | ||
| class TestBugANoSuffix extends Test, Case { | ||
| override predicate run(Qnit test) { | ||
| if | ||
| exists(BugField f, string msg | | ||
| Results::hasLimitedResult(any(Bug b | b = TBugA()), f, msg) and | ||
| f.getFieldName() = "X" and | ||
| msg = "BugA has field X" | ||
| ) | ||
| then test.pass("BugA: correct message with no suffix") | ||
| else test.fail("BugA: message incorrect") | ||
| } | ||
| } | ||
|
|
||
| /** 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)) = | ||
| 3 | ||
| then test.pass("BugB: 3 results shown (exactly maxResults)") | ||
| else test.fail("BugB: wrong result count") | ||
| } | ||
| } | ||
|
|
||
| /** BugB shows all 3 fields with no suffix since total <= maxResults. */ | ||
| class TestBugBNoSuffix extends Test, Case { | ||
| override predicate run(Qnit test) { | ||
| if | ||
| forall(string fieldName | | ||
| fieldName = ["A", "B", "C"] and | ||
| exists(BugField f | | ||
| f.getFieldName() = fieldName and | ||
| f.getBugName() = "BugB" | ||
| ) | ||
| | | ||
| exists(BugField f, string msg | | ||
| Results::hasLimitedResult(any(Bug b | b = TBugB()), f, msg) and | ||
| f.getFieldName() = fieldName and | ||
| msg = "BugB has field " + fieldName | ||
| ) | ||
| ) | ||
| then test.pass("BugB: all 3 results shown with no suffix") | ||
| else test.fail("BugB: some results have wrong message or are missing") | ||
| } | ||
| } | ||
|
|
||
| /** 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)) = | ||
| 3 | ||
| then test.pass("BugC: 3 results shown (capped at maxResults)") | ||
| else test.fail("BugC: wrong result count") | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * 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 | ||
| forall(string fieldName | fieldName = ["A", "B", "C"] | | ||
| exists(BugField f | | ||
| Results::hasLimitedResult(any(Bug b | b = TBugC()), f, _) and | ||
| f.getFieldName() = fieldName and | ||
| f.getBugName() = "BugC" | ||
| ) | ||
| ) and | ||
| not exists(BugField f | | ||
| Results::hasLimitedResult(any(Bug b | b = TBugC()), f, _) and | ||
| f.getFieldName() = ["D", "E"] and | ||
| f.getBugName() = "BugC" | ||
| ) | ||
| then test.pass("BugC: top 3 alphabetical fields shown, D and E omitted") | ||
| else test.fail("BugC: wrong fields shown") | ||
| } | ||
| } | ||
|
|
||
| /** BugC shows a suffix " (and 2 more)" since 5 - 3 = 2 entities are omitted. */ | ||
| class TestBugCSuffix extends Test, Case { | ||
| override predicate run(Qnit test) { | ||
| if | ||
| forall(BugField f, string msg | | ||
| Results::hasLimitedResult(any(Bug b | b = TBugC()), f, msg) | ||
| | | ||
| msg.matches("% (and 2 more)") | ||
| ) | ||
| then test.pass("BugC: all shown results have ' (and 2 more)' suffix") | ||
| 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") | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete this .gitignore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleted in d58a5a0.