From 59116856a2ce31cc3bd75feaf82a8913b61e30d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= Date: Wed, 9 Sep 2026 18:19:04 +0200 Subject: [PATCH 1/3] Reconcile inline expectations across query cohorts Read cohort membership through query metadata and derive producer sets from comments re-extracted between serialized query passes. Render full-cohort atoms bare, qualify partial atoms, and normalize existing qualified sets while preserving foreign and named-column expectations. Add a focused Go-pack fixture for kind-less qualified matching and preservation behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6e9cf9f-9b2e-487b-b28d-a79029d09ea8 --- .../inline-expectations/multi-query/code.go | 17 ++ .../multi-query/q1.expected | 10 + .../inline-expectations/multi-query/q1.ql | 20 ++ .../inline-expectations/multi-query/q1.qlref | 2 + .../multi-query/q2.expected | 8 + .../inline-expectations/multi-query/q2.ql | 18 ++ .../inline-expectations/multi-query/q2.qlref | 2 + .../util/test/InlineExpectationsTest.qll | 182 ++++++++++++++++-- 8 files changed, 246 insertions(+), 13 deletions(-) create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query/code.go create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query/q1.expected create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query/q1.ql create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query/q1.qlref create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query/q2.expected create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query/q2.ql create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query/q2.qlref diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/code.go b/go/ql/test/library-tests/inline-expectations/multi-query/code.go new file mode 100644 index 000000000000..bd62c6bbaf5a --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query/code.go @@ -0,0 +1,17 @@ +package p + +func sharedTarget() {} // $ hit + +func valuedTarget() {} // $ mark[go/test/q1]=q1 mark[go/test/q2]=q2 + +func q1Target() {} // $ one[go/test/q1] + +func q2Target() {} // $ two[go/test/q2] + +func foreignTarget() {} // $ hit hit[go/test/other] + +func missingTarget() {} // $ MISSING: absent[go/test/q1] absent[go/test/q2] + +func spuriousTarget() {} // $ SPURIOUS: spurious[go/test/q1] + +func noteTarget() {} // $ hit // trailing note diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/q1.expected b/go/ql/test/library-tests/inline-expectations/multi-query/q1.expected new file mode 100644 index 000000000000..b13b2201cdd0 --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query/q1.expected @@ -0,0 +1,10 @@ +hit +| code.go:3:6:3:17 | sharedTarget | +| code.go:11:6:11:18 | foreignTarget | +| code.go:17:6:17:15 | noteTarget | +mark +| code.go:5:6:5:17 | valuedTarget | q1 | +one +| code.go:7:6:7:13 | q1Target | +spurious +| code.go:15:6:15:19 | spuriousTarget | diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/q1.ql b/go/ql/test/library-tests/inline-expectations/multi-query/q1.ql new file mode 100644 index 000000000000..fe69aefcead5 --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query/q1.ql @@ -0,0 +1,20 @@ +/** + * @id go/test/q1 + */ + +private import go + +private predicate isFixtureFunction(Function function, string name) { function.getName() = name } + +query predicate hit(Function function) { + isFixtureFunction(function, ["sharedTarget", "foreignTarget", "noteTarget"]) +} + +query predicate mark(Function function, string value) { + isFixtureFunction(function, "valuedTarget") and + value = "q1" +} + +query predicate one(Function function) { isFixtureFunction(function, "q1Target") } + +query predicate spurious(Function function) { isFixtureFunction(function, "spuriousTarget") } diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/q1.qlref b/go/ql/test/library-tests/inline-expectations/multi-query/q1.qlref new file mode 100644 index 000000000000..bd0315a1c73e --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query/q1.qlref @@ -0,0 +1,2 @@ +query: q1.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/q2.expected b/go/ql/test/library-tests/inline-expectations/multi-query/q2.expected new file mode 100644 index 000000000000..0fe120abc193 --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query/q2.expected @@ -0,0 +1,8 @@ +hit +| code.go:3:6:3:17 | sharedTarget | +| code.go:11:6:11:18 | foreignTarget | +| code.go:17:6:17:15 | noteTarget | +mark +| code.go:5:6:5:17 | valuedTarget | q2 | +two +| code.go:9:6:9:13 | q2Target | diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/q2.ql b/go/ql/test/library-tests/inline-expectations/multi-query/q2.ql new file mode 100644 index 000000000000..5db691ecffc3 --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query/q2.ql @@ -0,0 +1,18 @@ +/** + * @id go/test/q2 + */ + +private import go + +private predicate isFixtureFunction(Function function, string name) { function.getName() = name } + +query predicate hit(Function function) { + isFixtureFunction(function, ["sharedTarget", "foreignTarget", "noteTarget"]) +} + +query predicate mark(Function function, string value) { + isFixtureFunction(function, "valuedTarget") and + value = "q2" +} + +query predicate two(Function function) { isFixtureFunction(function, "q2Target") } diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/q2.qlref b/go/ql/test/library-tests/inline-expectations/multi-query/q2.qlref new file mode 100644 index 000000000000..0bdc72578b78 --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query/q2.qlref @@ -0,0 +1,2 @@ +query: q2.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll index 7cee282a58fa..a9fb80ca9d6f 100644 --- a/shared/util/codeql/util/test/InlineExpectationsTest.qll +++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll @@ -685,6 +685,27 @@ module TestPostProcessing { private string getQueryKind() { queryMetadata("kind", result) } + /** + * Holds if `queryId` is the `@id` of a query in the shared-source cohort for this `--learn` run. + * Empty for an ordinary single-query run. + */ + private predicate cohortMember(string queryId) { queryMetadata("cohortMember", queryId) } + + /** + * Parses `full` into a bare tag and an optional query ID qualifier. For example, + * `mark[ql/test/q1]` becomes `mark` and `ql/test/q1`, while `mark` becomes `mark` and `""`. + */ + bindingset[full] + private predicate parseQualifiedTag(string full, string base, string queryId) { + if full.regexpMatch("[^\\[\\]]+\\[[^\\[\\]]*\\]") + then + base = full.regexpCapture("([^\\[\\]]+)\\[([^\\[\\]]*)\\]", 1) and + queryId = full.regexpCapture("([^\\[\\]]+)\\[([^\\[\\]]*)\\]", 2) + else ( + base = full and queryId = "" + ) + } + module Make { private import InlineExpectationsTest as InlineExpectationsTest @@ -921,8 +942,14 @@ module TestPostProcessing { predicate tagMatches(string expectedTag, string actualTag) { PathProblemSourceTestInput::tagMatches(expectedTag, actualTag) or + // Kind-less query predicates use the predicate name as their tag. A qualifier restricts + // that expectation to the query with the corresponding `@id`. not exists(getQueryKind()) and - expectedTag = actualTag + exists(string base, string queryId | + parseQualifiedTag(expectedTag, base, queryId) and + base = actualTag and + (queryId = "" or queryId = getQueryId()) + ) } bindingset[expectedTag] @@ -938,6 +965,15 @@ module TestPostProcessing { queryId != getQueryId() ) ) + or + // A qualified kind-less tag for another query is foreign to this query and must survive + // any rewrite it performs. + not exists(getQueryKind()) and + exists(string queryId | + parseQualifiedTag(expectedTag, _, queryId) and + queryId != "" and + queryId != getQueryId() + ) } private predicate hasPathProblemSource = PathProblemSourceTestInput::hasPathProblemSource/5; @@ -1169,10 +1205,21 @@ module TestPostProcessing { private predicate foreignExpectation( TestImpl2::ExpectationComment comment, string column, string text ) { - exists(TColumn col, string tags | - hasExpectation(comment, col, text, tags, _) and + exists(TColumn col, string tags, string value | + hasExpectation(comment, col, text, tags, value) and column = getColumnString(col) and - forall(string tag | tag = tags.splitAt(",") | TestInput::tagIsIgnored(tag)) + forall(string tag | tag = tags.splitAt(",") | TestInput::tagIsIgnored(tag)) and + // Once this pass collapses an atom to its bare form, an earlier cohort member's + // qualified default-column tag is redundant. Named-column expectations and genuinely + // foreign query IDs remain untouched. + not exists(string relativePath, int line, string base, string queryId | + col = TDefaultColumn() and + not tags.matches("%,%") and + parseQualifiedTag(tags, base, queryId) and + cohortMember(queryId) and + parseLocationString(comment.getLocation().getRelativeUrl(), relativePath, line, _, _, _) and + isCollapsedAtom(relativePath, line, base, value) + ) ) } @@ -1207,6 +1254,112 @@ module TestPostProcessing { result != "" } + /** Holds if this is a shared-source cohort pass rather than an ordinary single-query run. */ + private predicate inCohortMode() { exists(string queryId | cohortMember(queryId)) } + + /** + * Holds if `queryId` is recorded on disk as a producer of `(tag, value)` on the given line. + * A bare tag denotes the full cohort, while a qualified tag denotes its named cohort member. + */ + private predicate onDiskProducer( + string relativePath, int line, string tag, string value, string queryId + ) { + exists(TestImpl2::ExpectationComment comment, string full, string base, string qualifier | + parseLocationString(comment.getLocation().getRelativeUrl(), relativePath, line, _, _, _) and + hasExpectation(comment, TDefaultColumn(), _, full, value) and + not full.matches("%,%") and + parseQualifiedTag(full, base, qualifier) and + base = tag and + ( + qualifier = "" and cohortMember(queryId) + or + qualifier != "" and queryId = qualifier and cohortMember(qualifier) + ) + ) + } + + /** Holds if the current query produces `(tag, value)` on the given result end line. */ + private predicate selfProducesAtom(string relativePath, int endLine, string tag, string value) { + exists(Test::ActualTestResult actualResult | + not actualResult.isOptional() and + actualResult.getTag() != "RelatedLocation" and + parseLocationString(actualResult.getLocation().getRelativeUrl(), relativePath, _, _, + endLine, _) and + tag = actualResult.getTag() and + value = actualResult.getValue() + ) + } + + /** + * Holds if `queryId` produces the atom after reconciling the current query's on-disk state + * with its actual results. Other cohort members come from disk; the current member comes only + * from its current results, so a stale self-qualified tag does not count as a producer. + */ + private predicate reconciledProducer( + string relativePath, int line, string tag, string value, string queryId + ) { + queryId = getQueryId() and selfProducesAtom(relativePath, line, tag, value) + or + queryId != getQueryId() and onDiskProducer(relativePath, line, tag, value, queryId) + } + + /** Holds if the reconciled producer set for the atom is the whole cohort. */ + private predicate isCollapsedAtom(string relativePath, int line, string tag, string value) { + inCohortMode() and + ( + selfProducesAtom(relativePath, line, tag, value) + or + onDiskProducer(relativePath, line, tag, value, _) + ) and + forall(string queryId | cohortMember(queryId) | + reconciledProducer(relativePath, line, tag, value, queryId) + ) + } + + /** Renders a bare or qualified tag with its optional value. */ + bindingset[tag, value] + private string renderAtom(string tag, string value) { + if value = "" then result = tag else result = tag + "=" + value + } + + /** + * Renders a current-query atom in its canonical cohort form: bare when every member produces + * it, otherwise qualified with the current query ID. + */ + bindingset[relativePath, line, tag, value] + private string renderCohortAtom(string relativePath, int line, string tag, string value) { + ( + not inCohortMode() + or + isCollapsedAtom(relativePath, line, tag, value) + ) and + result = renderAtom(tag, value) + or + inCohortMode() and + not isCollapsedAtom(relativePath, line, tag, value) and + result = renderAtom(tag + "[" + getQueryId() + "]", value) + } + + /** + * Renders a matching default-column expectation for the current query. This also normalizes + * pre-existing cohort comments: a complete qualified set collapses to bare, while a bare atom + * no longer produced by the whole cohort becomes current-query-qualified. + */ + private string renderSurvivingDefaultExpectation(Test::FailureLocatable expectation) { + exists(string relativePath, int line, string base, string queryId | + parseQualifiedTag(expectation.getTag(), base, queryId) and + parseLocationString(expectation.getLocation().getRelativeUrl(), relativePath, line, _, _, + _) and + ( + inCohortMode() and + (queryId = "" or queryId = getQueryId()) and + result = renderCohortAtom(relativePath, line, base, expectation.getValue()) + or + not inCohortMode() and result = expectation.getExpectationText() + ) + ) + } + /** * Gets the exclusive end column for replacing `comment`. Same-line CodeQL locations use * inclusive end columns, so their end boundary is one column later. A comment whose location @@ -1237,28 +1390,30 @@ module TestPostProcessing { private predicate survivingExpectation( TestImpl2::ExpectationComment comment, string column, string text ) { - exists(Test::FailureLocatable e | - e.getLocation() = comment.getLocation() and text = e.getExpectationText() - | + exists(Test::FailureLocatable e | e.getLocation() = comment.getLocation() | // a default expectation that still matches a result is kept e instanceof Test::GoodTestExpectation and e = Test::getAMatchingExpectation(_, _, _, _, _) and - column = "" + column = "" and + text = renderSurvivingDefaultExpectation(e) or // a `SPURIOUS:` expectation whose result still fires is kept e instanceof Test::FalsePositiveTestExpectation and e = Test::getAMatchingExpectation(_, _, _, _, _) and - column = "SPURIOUS" + column = "SPURIOUS" and + text = e.getExpectationText() or // a `MISSING:` expectation whose result now fires is promoted to the default column e instanceof Test::FalseNegativeTestExpectation and e = Test::getAMatchingExpectation(_, _, _, _, _) and - column = "" + column = "" and + text = renderSurvivingDefaultExpectation(e) or // a `MISSING:` expectation whose result is still absent is kept e instanceof Test::FalseNegativeTestExpectation and not e = Test::getAMatchingExpectation(_, _, _, _, _) and - column = "MISSING" + column = "MISSING" and + text = e.getExpectationText() ) } @@ -1292,9 +1447,10 @@ module TestPostProcessing { Test::getAMatchingExpectation(actualResult.getLocation(), actualResult.toString(), actualResult.getTag(), actualResult.getValue(), false) ) and - text = actualResult.getExpectationText() and parseLocationString(actualResult.getLocation().getRelativeUrl(), relativePath, endLine, _, - endLine, _) + endLine, _) and + text = + renderCohortAtom(relativePath, endLine, actualResult.getTag(), actualResult.getValue()) ) } From d8270c10582bc9e56d1f061a34b23a85cd0f57cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= Date: Tue, 3 Nov 2026 16:08:23 +0100 Subject: [PATCH 2/3] Preserve cohort producer provenance When a bare atom becomes partial, render the surviving sibling producers with query-ID qualifiers instead of deleting their expectation. Carry forward only tags the current query understands, accept only non-empty single-ID qualifiers as cohort syntax, and leave default-column comma groups or whitespace-bearing qualifiers untouched because they cannot express one producer unambiguously. Treat a complete qualified producer set as equivalent to its collapsed bare atom when deciding whether a comment needs an edit. This preserves useful query provenance on otherwise-correct comments; comments requiring another repair still use canonical bare rendering. Extend the Go fixtures with bare-to-partial, ignored-tag, complete-qualified-set, and unsupported multi-ID mutation cases, then regenerate their expected output. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6e9cf9f-9b2e-487b-b28d-a79029d09ea8 --- .../multi-query-invalid-qualifier/code.go | 3 + .../code.go.invalid | 3 + .../multi-query-invalid-qualifier/q1.expected | 1 + .../multi-query-invalid-qualifier/q1.ql | 7 + .../multi-query-invalid-qualifier/q1.qlref | 2 + .../multi-query-invalid-qualifier/q2.expected | 1 + .../multi-query-invalid-qualifier/q2.ql | 7 + .../multi-query-invalid-qualifier/q2.qlref | 2 + .../multi-query-problem/code.go | 3 + .../multi-query-problem/q1.expected | 1 + .../multi-query-problem/q1.ql | 10 ++ .../multi-query-problem/q1.qlref | 2 + .../multi-query-problem/q2.expected | 1 + .../multi-query-problem/q2.ql | 10 ++ .../multi-query-problem/q2.qlref | 2 + .../inline-expectations/multi-query/code.go | 4 + .../multi-query/q1.expected | 11 +- .../inline-expectations/multi-query/q1.ql | 2 +- .../multi-query/q2.expected | 10 +- .../inline-expectations/multi-query/q2.ql | 6 +- .../util/test/InlineExpectationsTest.qll | 141 +++++++++++++++--- 21 files changed, 196 insertions(+), 33 deletions(-) create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/code.go create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/code.go.invalid create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.expected create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.ql create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.qlref create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.expected create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.ql create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.qlref create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query-problem/code.go create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query-problem/q1.expected create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query-problem/q1.ql create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query-problem/q1.qlref create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query-problem/q2.expected create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query-problem/q2.ql create mode 100644 go/ql/test/library-tests/inline-expectations/multi-query-problem/q2.qlref diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/code.go b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/code.go new file mode 100644 index 000000000000..1b4d5af91eb3 --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/code.go @@ -0,0 +1,3 @@ +package p + +func sharedTarget() {} // $ hit[go/test/q1] hit[go/test/q2] diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/code.go.invalid b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/code.go.invalid new file mode 100644 index 000000000000..e2ce3dcd4a80 --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/code.go.invalid @@ -0,0 +1,3 @@ +package p + +func sharedTarget() {} // $ hit[go/test/q1 go/test/q2] diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.expected b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.expected new file mode 100644 index 000000000000..16571ecc74c3 --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.expected @@ -0,0 +1 @@ +| code.go:3:6:3:17 | sharedTarget | diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.ql b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.ql new file mode 100644 index 000000000000..dc01fc45c027 --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.ql @@ -0,0 +1,7 @@ +/** + * @id go/test/q1 + */ + +private import go + +query predicate hit(Function function) { function.getName() = "sharedTarget" } diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.qlref b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.qlref new file mode 100644 index 000000000000..bd0315a1c73e --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.qlref @@ -0,0 +1,2 @@ +query: q1.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.expected b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.expected new file mode 100644 index 000000000000..16571ecc74c3 --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.expected @@ -0,0 +1 @@ +| code.go:3:6:3:17 | sharedTarget | diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.ql b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.ql new file mode 100644 index 000000000000..ebdcd9f95f7a --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.ql @@ -0,0 +1,7 @@ +/** + * @id go/test/q2 + */ + +private import go + +query predicate hit(Function function) { function.getName() = "sharedTarget" } diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.qlref b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.qlref new file mode 100644 index 000000000000..0bdc72578b78 --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.qlref @@ -0,0 +1,2 @@ +query: q2.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-problem/code.go b/go/ql/test/library-tests/inline-expectations/multi-query-problem/code.go new file mode 100644 index 000000000000..012216543834 --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query-problem/code.go @@ -0,0 +1,3 @@ +package p + +func sharedTarget() {} // $ Alert unrelated=value diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-problem/q1.expected b/go/ql/test/library-tests/inline-expectations/multi-query-problem/q1.expected new file mode 100644 index 000000000000..e947a88cc5c9 --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query-problem/q1.expected @@ -0,0 +1 @@ +| code.go:3:6:3:17 | sharedTarget | problem | diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-problem/q1.ql b/go/ql/test/library-tests/inline-expectations/multi-query-problem/q1.ql new file mode 100644 index 000000000000..9a71b77b8b88 --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query-problem/q1.ql @@ -0,0 +1,10 @@ +/** + * @kind problem + * @id go/test/problem-q1 + */ + +import go + +from Function function +where function.getName() = "sharedTarget" +select function, "problem" diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-problem/q1.qlref b/go/ql/test/library-tests/inline-expectations/multi-query-problem/q1.qlref new file mode 100644 index 000000000000..bd0315a1c73e --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query-problem/q1.qlref @@ -0,0 +1,2 @@ +query: q1.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-problem/q2.expected b/go/ql/test/library-tests/inline-expectations/multi-query-problem/q2.expected new file mode 100644 index 000000000000..e947a88cc5c9 --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query-problem/q2.expected @@ -0,0 +1 @@ +| code.go:3:6:3:17 | sharedTarget | problem | diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-problem/q2.ql b/go/ql/test/library-tests/inline-expectations/multi-query-problem/q2.ql new file mode 100644 index 000000000000..13a23379401f --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query-problem/q2.ql @@ -0,0 +1,10 @@ +/** + * @kind problem + * @id go/test/problem-q2 + */ + +import go + +from Function function +where function.getName() = "sharedTarget" +select function, "problem" diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-problem/q2.qlref b/go/ql/test/library-tests/inline-expectations/multi-query-problem/q2.qlref new file mode 100644 index 000000000000..0bdc72578b78 --- /dev/null +++ b/go/ql/test/library-tests/inline-expectations/multi-query-problem/q2.qlref @@ -0,0 +1,2 @@ +query: q2.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/code.go b/go/ql/test/library-tests/inline-expectations/multi-query/code.go index bd62c6bbaf5a..38feb2ba3b21 100644 --- a/go/ql/test/library-tests/inline-expectations/multi-query/code.go +++ b/go/ql/test/library-tests/inline-expectations/multi-query/code.go @@ -2,12 +2,16 @@ package p func sharedTarget() {} // $ hit +func qualifiedTarget() {} // $ hit[go/test/q1] hit[go/test/q2] + func valuedTarget() {} // $ mark[go/test/q1]=q1 mark[go/test/q2]=q2 func q1Target() {} // $ one[go/test/q1] func q2Target() {} // $ two[go/test/q2] +func survivingSiblingTarget() {} // $ two[go/test/q2] + func foreignTarget() {} // $ hit hit[go/test/other] func missingTarget() {} // $ MISSING: absent[go/test/q1] absent[go/test/q2] diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/q1.expected b/go/ql/test/library-tests/inline-expectations/multi-query/q1.expected index b13b2201cdd0..3e630ec88391 100644 --- a/go/ql/test/library-tests/inline-expectations/multi-query/q1.expected +++ b/go/ql/test/library-tests/inline-expectations/multi-query/q1.expected @@ -1,10 +1,11 @@ hit | code.go:3:6:3:17 | sharedTarget | -| code.go:11:6:11:18 | foreignTarget | -| code.go:17:6:17:15 | noteTarget | +| code.go:5:6:5:20 | qualifiedTarget | +| code.go:15:6:15:18 | foreignTarget | +| code.go:21:6:21:15 | noteTarget | mark -| code.go:5:6:5:17 | valuedTarget | q1 | +| code.go:7:6:7:17 | valuedTarget | q1 | one -| code.go:7:6:7:13 | q1Target | +| code.go:9:6:9:13 | q1Target | spurious -| code.go:15:6:15:19 | spuriousTarget | +| code.go:19:6:19:19 | spuriousTarget | diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/q1.ql b/go/ql/test/library-tests/inline-expectations/multi-query/q1.ql index fe69aefcead5..52706e7340bc 100644 --- a/go/ql/test/library-tests/inline-expectations/multi-query/q1.ql +++ b/go/ql/test/library-tests/inline-expectations/multi-query/q1.ql @@ -7,7 +7,7 @@ private import go private predicate isFixtureFunction(Function function, string name) { function.getName() = name } query predicate hit(Function function) { - isFixtureFunction(function, ["sharedTarget", "foreignTarget", "noteTarget"]) + isFixtureFunction(function, ["sharedTarget", "qualifiedTarget", "foreignTarget", "noteTarget"]) } query predicate mark(Function function, string value) { diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/q2.expected b/go/ql/test/library-tests/inline-expectations/multi-query/q2.expected index 0fe120abc193..9c2a2780d0e5 100644 --- a/go/ql/test/library-tests/inline-expectations/multi-query/q2.expected +++ b/go/ql/test/library-tests/inline-expectations/multi-query/q2.expected @@ -1,8 +1,10 @@ hit | code.go:3:6:3:17 | sharedTarget | -| code.go:11:6:11:18 | foreignTarget | -| code.go:17:6:17:15 | noteTarget | +| code.go:5:6:5:20 | qualifiedTarget | +| code.go:15:6:15:18 | foreignTarget | +| code.go:21:6:21:15 | noteTarget | mark -| code.go:5:6:5:17 | valuedTarget | q2 | +| code.go:7:6:7:17 | valuedTarget | q2 | two -| code.go:9:6:9:13 | q2Target | +| code.go:11:6:11:13 | q2Target | +| code.go:13:6:13:27 | survivingSiblingTarget | diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/q2.ql b/go/ql/test/library-tests/inline-expectations/multi-query/q2.ql index 5db691ecffc3..57df5390f5c4 100644 --- a/go/ql/test/library-tests/inline-expectations/multi-query/q2.ql +++ b/go/ql/test/library-tests/inline-expectations/multi-query/q2.ql @@ -7,7 +7,7 @@ private import go private predicate isFixtureFunction(Function function, string name) { function.getName() = name } query predicate hit(Function function) { - isFixtureFunction(function, ["sharedTarget", "foreignTarget", "noteTarget"]) + isFixtureFunction(function, ["sharedTarget", "qualifiedTarget", "foreignTarget", "noteTarget"]) } query predicate mark(Function function, string value) { @@ -15,4 +15,6 @@ query predicate mark(Function function, string value) { value = "q2" } -query predicate two(Function function) { isFixtureFunction(function, "q2Target") } +query predicate two(Function function) { + isFixtureFunction(function, ["q2Target", "survivingSiblingTarget"]) +} diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll index a9fb80ca9d6f..a8e391dc6515 100644 --- a/shared/util/codeql/util/test/InlineExpectationsTest.qll +++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll @@ -694,16 +694,17 @@ module TestPostProcessing { /** * Parses `full` into a bare tag and an optional query ID qualifier. For example, * `mark[ql/test/q1]` becomes `mark` and `ql/test/q1`, while `mark` becomes `mark` and `""`. + * Empty qualifiers and tags containing unmatched brackets have no result. */ bindingset[full] private predicate parseQualifiedTag(string full, string base, string queryId) { - if full.regexpMatch("[^\\[\\]]+\\[[^\\[\\]]*\\]") - then - base = full.regexpCapture("([^\\[\\]]+)\\[([^\\[\\]]*)\\]", 1) and - queryId = full.regexpCapture("([^\\[\\]]+)\\[([^\\[\\]]*)\\]", 2) - else ( - base = full and queryId = "" - ) + full.regexpMatch("[^\\[\\]]+\\[[^\\[\\]]+\\]") and + base = full.regexpCapture("([^\\[\\]]+)\\[([^\\[\\]]+)\\]", 1) and + queryId = full.regexpCapture("([^\\[\\]]+)\\[([^\\[\\]]+)\\]", 2) + or + full.regexpMatch("[^\\[\\]]+") and + base = full and + queryId = "" } module Make { @@ -1172,7 +1173,9 @@ module TestPostProcessing { * group) a tag this test understands with a tag it ignores (for example a query ID that does * not match the current query). The latter keeps the rewrite from having to take apart a group * such as `Alert,Source[other-query]` whose parts this test treats differently; such comments - * are left untouched. + * are left untouched. A qualifier containing whitespace is also left untouched in cohort mode: + * it may be domain-specific bracket syntax or an unsupported attempt to name several producer + * queries, neither of which can be reconciled as one query ID. * * A rewritable comment may still carry whole expectations this test ignores (for example * `// $ Alert[other-query]`). Those are preserved verbatim by `foreignExpectation` when the @@ -1189,6 +1192,20 @@ module TestPostProcessing { hasExpectation(comment, _, _, tags, _) and not TestInput::tagIsIgnored(tags.splitAt(",")) and TestInput::tagIsIgnored(tags.splitAt(",")) + ) and + // A comma group cannot express per-query ownership without changing its grammar. Leave the + // whole comment untouched in cohort mode so verification reports the unsupported syntax. + not exists(string tags | + inCohortMode() and + hasExpectation(comment, TDefaultColumn(), _, tags, _) and + tags.matches("%,%") + ) and + not exists(string tags, string queryId | + inCohortMode() and + hasExpectation(comment, TDefaultColumn(), _, tags, _) and + not tags.matches("%,%") and + parseQualifiedTag(tags, _, queryId) and + queryId.regexpMatch(".*\\s+.*") ) } @@ -1255,7 +1272,7 @@ module TestPostProcessing { } /** Holds if this is a shared-source cohort pass rather than an ordinary single-query run. */ - private predicate inCohortMode() { exists(string queryId | cohortMember(queryId)) } + private predicate inCohortMode() { cohortMember(_) } /** * Holds if `queryId` is recorded on disk as a producer of `(tag, value)` on the given line. @@ -1306,11 +1323,7 @@ module TestPostProcessing { /** Holds if the reconciled producer set for the atom is the whole cohort. */ private predicate isCollapsedAtom(string relativePath, int line, string tag, string value) { inCohortMode() and - ( - selfProducesAtom(relativePath, line, tag, value) - or - onDiskProducer(relativePath, line, tag, value, _) - ) and + reconciledProducer(relativePath, line, tag, value, _) and forall(string queryId | cohortMember(queryId) | reconciledProducer(relativePath, line, tag, value, queryId) ) @@ -1341,9 +1354,35 @@ module TestPostProcessing { } /** - * Renders a matching default-column expectation for the current query. This also normalizes - * pre-existing cohort comments: a complete qualified set collapses to bare, while a bare atom - * no longer produced by the whole cohort becomes current-query-qualified. + * Carries forward a sibling producer when a bare on-disk atom has a partial reconciled producer + * set. The current query is taken from its actual result, and each surviving sibling is + * rendered explicitly. + */ + private predicate carriedSiblingExpectation( + TestImpl2::ExpectationComment comment, string column, string text + ) { + exists( + string relativePath, int line, string full, string base, string qualifier, string value, + string queryId + | + inCohortMode() and + hasExpectation(comment, TDefaultColumn(), _, full, value) and + not full.matches("%,%") and + parseQualifiedTag(full, base, qualifier) and + qualifier = "" and + not TestInput::tagIsIgnored(full) and + parseLocationString(comment.getLocation().getRelativeUrl(), relativePath, line, _, _, _) and + not isCollapsedAtom(relativePath, line, base, value) and + queryId != getQueryId() and + reconciledProducer(relativePath, line, base, value, queryId) and + column = "" and + text = renderAtom(base + "[" + queryId + "]", value) + ) + } + + /** + * Renders a matching default-column expectation for the current query. A complete qualified + * producer set collapses to bare, while a partial set is rendered with explicit query IDs. */ private string renderSurvivingDefaultExpectation(Test::FailureLocatable expectation) { exists(string relativePath, int line, string base, string queryId | @@ -1478,8 +1517,9 @@ module TestPostProcessing { * This combines the surviving expectations this test understands (see `survivingExpectation`) * with the expectations it ignores (see `foreignExpectation`), which are preserved * verbatim so that rewriting a comment for one query never drops another query's expectation on - * the same line, and with any freshly learned expectations merged into the comment (see - * `mergedNewExpectation`). + * the same line, sibling producers carried forward from a formerly bare atom (see + * `carriedSiblingExpectation`), and any freshly learned expectations merged into the comment + * (see `mergedNewExpectation`). */ private predicate desiredExpectation( TestImpl2::ExpectationComment comment, string column, string text @@ -1488,6 +1528,8 @@ module TestPostProcessing { or foreignExpectation(comment, column, text) or + carriedSiblingExpectation(comment, column, text) + or mergedNewExpectation(comment, column, text) } @@ -1512,16 +1554,73 @@ module TestPostProcessing { foreignExpectation(comment, column, text) } + /** + * Holds if `comment` uses a complete qualified representation of a collapsed default-column + * atom. This representation is semantically equivalent to the bare desired atom, but retains + * useful query provenance. + */ + private predicate hasCompleteQualifiedRepresentation( + TestImpl2::ExpectationComment comment, string tag, string value + ) { + exists(string full, string queryId | + hasExpectation(comment, TDefaultColumn(), _, full, value) and + parseQualifiedTag(full, tag, queryId) and + cohortMember(queryId) + ) and + not hasExpectation(comment, TDefaultColumn(), _, tag, value) and + forall(string queryId | cohortMember(queryId) | + exists(string full | + hasExpectation(comment, TDefaultColumn(), _, full, value) and + parseQualifiedTag(full, tag, queryId) + ) + ) + } + + /** Holds if a desired bare atom is already represented by a complete qualified set. */ + private predicate desiredExpectationIsRepresented( + TestImpl2::ExpectationComment comment, string column, string text + ) { + currentExpectation(comment, column, text) + or + exists(string relativePath, int line, string tag, string value | + column = "" and + parseLocationString(comment.getLocation().getRelativeUrl(), relativePath, line, _, _, _) and + isCollapsedAtom(relativePath, line, tag, value) and + hasCompleteQualifiedRepresentation(comment, tag, value) and + text = renderAtom(tag, value) + ) + } + + /** Holds if a current qualified atom is covered by the desired collapsed bare atom. */ + private predicate currentExpectationIsRepresented( + TestImpl2::ExpectationComment comment, string column, string text + ) { + desiredExpectation(comment, column, text) + or + exists( + string relativePath, int line, string full, string tag, string queryId, string value + | + column = "" and + text = renderAtom(full, value) and + hasExpectation(comment, TDefaultColumn(), _, full, value) and + parseQualifiedTag(full, tag, queryId) and + cohortMember(queryId) and + parseLocationString(comment.getLocation().getRelativeUrl(), relativePath, line, _, _, _) and + isCollapsedAtom(relativePath, line, tag, value) and + hasCompleteQualifiedRepresentation(comment, tag, value) + ) + } + /** Holds if `--learn` should change the set of expectations carried by `comment`. */ private predicate commentNeedsRewrite(TestImpl2::ExpectationComment comment) { exists(string column, string text | desiredExpectation(comment, column, text) and - not currentExpectation(comment, column, text) + not desiredExpectationIsRepresented(comment, column, text) ) or exists(string column, string text | currentExpectation(comment, column, text) and - not desiredExpectation(comment, column, text) + not currentExpectationIsRepresented(comment, column, text) ) } From 3baabf9e287dbdffe0b1d8ff64b5bf9d3478e4d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= Date: Wed, 4 Nov 2026 14:05:04 +0100 Subject: [PATCH 3/3] Preserve cohort ownership for unsupported atoms Leave unsupported qualified syntax untouched, collapse sibling qualifications only when the rewritten comment emits the covering bare atom, and carry sibling ownership in the original expectation column. Cover invalid qualifier forms and default, MISSING, and SPURIOUS reconciliation through the Go inline-expectation fixtures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1adfa307-c48e-43f5-94e3-8286e4a8c083 --- .../multi-query-invalid-qualifier/code.go | 4 +- .../code.go.invalid | 4 +- .../multi-query-invalid-qualifier/q1.expected | 3 +- .../multi-query-invalid-qualifier/q1.ql | 4 +- .../multi-query-invalid-qualifier/q2.expected | 3 +- .../multi-query-invalid-qualifier/q2.ql | 4 +- .../inline-expectations/multi-query/code.go | 6 ++ .../multi-query/q1.expected | 4 +- .../inline-expectations/multi-query/q1.ql | 6 +- .../multi-query/q2.expected | 4 +- .../inline-expectations/multi-query/q2.ql | 6 +- .../util/test/InlineExpectationsTest.qll | 56 ++++++++++++------- 12 files changed, 74 insertions(+), 30 deletions(-) diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/code.go b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/code.go index 1b4d5af91eb3..f6a63c5e6cc5 100644 --- a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/code.go +++ b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/code.go @@ -1,3 +1,5 @@ package p -func sharedTarget() {} // $ hit[go/test/q1] hit[go/test/q2] +func emptyQualifierTarget() {} // $ hit[go/test/q1] hit[go/test/q2] + +func repeatedQualifierTarget() {} // $ hit[go/test/q1] hit[go/test/q2] diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/code.go.invalid b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/code.go.invalid index e2ce3dcd4a80..1ef782c0fabb 100644 --- a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/code.go.invalid +++ b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/code.go.invalid @@ -1,3 +1,5 @@ package p -func sharedTarget() {} // $ hit[go/test/q1 go/test/q2] +func emptyQualifierTarget() {} // $ hit[] + +func repeatedQualifierTarget() {} // $ hit[go/test/q1][go/test/q2] diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.expected b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.expected index 16571ecc74c3..f56aad90d2cc 100644 --- a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.expected +++ b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.expected @@ -1 +1,2 @@ -| code.go:3:6:3:17 | sharedTarget | +| code.go:3:6:3:25 | emptyQualifierTarget | +| code.go:5:6:5:28 | repeatedQualifierTarget | diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.ql b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.ql index dc01fc45c027..5becbcef4448 100644 --- a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.ql +++ b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q1.ql @@ -4,4 +4,6 @@ private import go -query predicate hit(Function function) { function.getName() = "sharedTarget" } +query predicate hit(Function function) { + function.getName() = ["emptyQualifierTarget", "repeatedQualifierTarget"] +} diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.expected b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.expected index 16571ecc74c3..f56aad90d2cc 100644 --- a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.expected +++ b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.expected @@ -1 +1,2 @@ -| code.go:3:6:3:17 | sharedTarget | +| code.go:3:6:3:25 | emptyQualifierTarget | +| code.go:5:6:5:28 | repeatedQualifierTarget | diff --git a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.ql b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.ql index ebdcd9f95f7a..78aea3fbef45 100644 --- a/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.ql +++ b/go/ql/test/library-tests/inline-expectations/multi-query-invalid-qualifier/q2.ql @@ -4,4 +4,6 @@ private import go -query predicate hit(Function function) { function.getName() = "sharedTarget" } +query predicate hit(Function function) { + function.getName() = ["emptyQualifierTarget", "repeatedQualifierTarget"] +} diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/code.go b/go/ql/test/library-tests/inline-expectations/multi-query/code.go index 38feb2ba3b21..d618a702b2bf 100644 --- a/go/ql/test/library-tests/inline-expectations/multi-query/code.go +++ b/go/ql/test/library-tests/inline-expectations/multi-query/code.go @@ -18,4 +18,10 @@ func missingTarget() {} // $ MISSING: absent[go/test/q1] absent[go/test/q2] func spuriousTarget() {} // $ SPURIOUS: spurious[go/test/q1] +func suppressedSiblingTarget() {} // $ hit[go/test/q2] SPURIOUS: hit[go/test/q1] + +func namedMissingTarget() {} // $ hit[go/test/q1] MISSING: hit[go/test/q2] + +func namedSpuriousTarget() {} // $ SPURIOUS: hit[go/test/q2] + func noteTarget() {} // $ hit // trailing note diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/q1.expected b/go/ql/test/library-tests/inline-expectations/multi-query/q1.expected index 3e630ec88391..a89ad9bc8add 100644 --- a/go/ql/test/library-tests/inline-expectations/multi-query/q1.expected +++ b/go/ql/test/library-tests/inline-expectations/multi-query/q1.expected @@ -2,7 +2,9 @@ hit | code.go:3:6:3:17 | sharedTarget | | code.go:5:6:5:20 | qualifiedTarget | | code.go:15:6:15:18 | foreignTarget | -| code.go:21:6:21:15 | noteTarget | +| code.go:21:6:21:28 | suppressedSiblingTarget | +| code.go:23:6:23:23 | namedMissingTarget | +| code.go:27:6:27:15 | noteTarget | mark | code.go:7:6:7:17 | valuedTarget | q1 | one diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/q1.ql b/go/ql/test/library-tests/inline-expectations/multi-query/q1.ql index 52706e7340bc..31765e54dc10 100644 --- a/go/ql/test/library-tests/inline-expectations/multi-query/q1.ql +++ b/go/ql/test/library-tests/inline-expectations/multi-query/q1.ql @@ -7,7 +7,11 @@ private import go private predicate isFixtureFunction(Function function, string name) { function.getName() = name } query predicate hit(Function function) { - isFixtureFunction(function, ["sharedTarget", "qualifiedTarget", "foreignTarget", "noteTarget"]) + isFixtureFunction(function, + [ + "sharedTarget", "qualifiedTarget", "foreignTarget", "suppressedSiblingTarget", + "namedMissingTarget", "noteTarget" + ]) } query predicate mark(Function function, string value) { diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/q2.expected b/go/ql/test/library-tests/inline-expectations/multi-query/q2.expected index 9c2a2780d0e5..6fd2d34110c2 100644 --- a/go/ql/test/library-tests/inline-expectations/multi-query/q2.expected +++ b/go/ql/test/library-tests/inline-expectations/multi-query/q2.expected @@ -2,7 +2,9 @@ hit | code.go:3:6:3:17 | sharedTarget | | code.go:5:6:5:20 | qualifiedTarget | | code.go:15:6:15:18 | foreignTarget | -| code.go:21:6:21:15 | noteTarget | +| code.go:21:6:21:28 | suppressedSiblingTarget | +| code.go:25:6:25:24 | namedSpuriousTarget | +| code.go:27:6:27:15 | noteTarget | mark | code.go:7:6:7:17 | valuedTarget | q2 | two diff --git a/go/ql/test/library-tests/inline-expectations/multi-query/q2.ql b/go/ql/test/library-tests/inline-expectations/multi-query/q2.ql index 57df5390f5c4..99b6049f8f1a 100644 --- a/go/ql/test/library-tests/inline-expectations/multi-query/q2.ql +++ b/go/ql/test/library-tests/inline-expectations/multi-query/q2.ql @@ -7,7 +7,11 @@ private import go private predicate isFixtureFunction(Function function, string name) { function.getName() = name } query predicate hit(Function function) { - isFixtureFunction(function, ["sharedTarget", "qualifiedTarget", "foreignTarget", "noteTarget"]) + isFixtureFunction(function, + [ + "sharedTarget", "qualifiedTarget", "foreignTarget", "suppressedSiblingTarget", + "namedSpuriousTarget", "noteTarget" + ]) } query predicate mark(Function function, string value) { diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll index a8e391dc6515..789cc6133bb6 100644 --- a/shared/util/codeql/util/test/InlineExpectationsTest.qll +++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll @@ -1173,9 +1173,9 @@ module TestPostProcessing { * group) a tag this test understands with a tag it ignores (for example a query ID that does * not match the current query). The latter keeps the rewrite from having to take apart a group * such as `Alert,Source[other-query]` whose parts this test treats differently; such comments - * are left untouched. A qualifier containing whitespace is also left untouched in cohort mode: - * it may be domain-specific bracket syntax or an unsupported attempt to name several producer - * queries, neither of which can be reconciled as one query ID. + * are left untouched. An unsupported qualifier is also left untouched in cohort mode: it may + * be domain-specific bracket syntax or an unsupported attempt to name producer queries, neither + * of which can be reconciled as one query ID. * * A rewritable comment may still carry whole expectations this test ignores (for example * `// $ Alert[other-query]`). Those are preserved verbatim by `foreignExpectation` when the @@ -1200,12 +1200,17 @@ module TestPostProcessing { hasExpectation(comment, TDefaultColumn(), _, tags, _) and tags.matches("%,%") ) and - not exists(string tags, string queryId | + not exists(string tags | inCohortMode() and - hasExpectation(comment, TDefaultColumn(), _, tags, _) and + hasExpectation(comment, _, _, tags, _) and not tags.matches("%,%") and - parseQualifiedTag(tags, _, queryId) and - queryId.regexpMatch(".*\\s+.*") + ( + not parseQualifiedTag(tags, _, _) + or + exists(string queryId | + parseQualifiedTag(tags, _, queryId) and queryId.regexpMatch(".*\\s+.*") + ) + ) ) } @@ -1229,13 +1234,12 @@ module TestPostProcessing { // Once this pass collapses an atom to its bare form, an earlier cohort member's // qualified default-column tag is redundant. Named-column expectations and genuinely // foreign query IDs remain untouched. - not exists(string relativePath, int line, string base, string queryId | + not exists(string base, string queryId | col = TDefaultColumn() and not tags.matches("%,%") and parseQualifiedTag(tags, base, queryId) and cohortMember(queryId) and - parseLocationString(comment.getLocation().getRelativeUrl(), relativePath, line, _, _, _) and - isCollapsedAtom(relativePath, line, base, value) + emittedBareExpectation(comment, "", base, value) ) ) } @@ -1354,28 +1358,25 @@ module TestPostProcessing { } /** - * Carries forward a sibling producer when a bare on-disk atom has a partial reconciled producer - * set. The current query is taken from its actual result, and each surviving sibling is - * rendered explicitly. + * Carries forward sibling ownership when a bare on-disk atom is no longer emitted in its + * original column. Each sibling is rendered explicitly in that column. */ private predicate carriedSiblingExpectation( TestImpl2::ExpectationComment comment, string column, string text ) { exists( - string relativePath, int line, string full, string base, string qualifier, string value, - string queryId + TColumn col, string full, string base, string qualifier, string value, string queryId | inCohortMode() and - hasExpectation(comment, TDefaultColumn(), _, full, value) and + hasExpectation(comment, col, _, full, value) and not full.matches("%,%") and parseQualifiedTag(full, base, qualifier) and qualifier = "" and not TestInput::tagIsIgnored(full) and - parseLocationString(comment.getLocation().getRelativeUrl(), relativePath, line, _, _, _) and - not isCollapsedAtom(relativePath, line, base, value) and + column = getColumnString(col) and + not emittedBareExpectation(comment, column, base, value) and queryId != getQueryId() and - reconciledProducer(relativePath, line, base, value, queryId) and - column = "" and + cohortMember(queryId) and text = renderAtom(base + "[" + queryId + "]", value) ) } @@ -1510,6 +1511,21 @@ module TestPostProcessing { ) } + /** Holds if the rewritten comment emits the bare atom in `column`. */ + bindingset[comment, column, tag, value] + private predicate emittedBareExpectation( + TestImpl2::ExpectationComment comment, string column, string tag, string value + ) { + exists(string text | + text = renderAtom(tag, value) and + ( + survivingExpectation(comment, column, text) + or + mergedNewExpectation(comment, column, text) + ) + ) + } + /** * Holds if, after `--learn`, `comment` should carry the expectation `text` in `column` (`""` * for the default column, or a named column such as `"SPURIOUS"` / `"MISSING"`).