Skip to content

Commit e1a7cb4

Browse files
d10cCopilot
andcommitted
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
1 parent a68e198 commit e1a7cb4

8 files changed

Lines changed: 246 additions & 13 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package p
2+
3+
func sharedTarget() {} // $ hit
4+
5+
func valuedTarget() {} // $ mark[go/test/q1]=q1 mark[go/test/q2]=q2
6+
7+
func q1Target() {} // $ one[go/test/q1]
8+
9+
func q2Target() {} // $ two[go/test/q2]
10+
11+
func foreignTarget() {} // $ hit hit[go/test/other]
12+
13+
func missingTarget() {} // $ MISSING: absent[go/test/q1] absent[go/test/q2]
14+
15+
func spuriousTarget() {} // $ SPURIOUS: spurious[go/test/q1]
16+
17+
func noteTarget() {} // $ hit // trailing note
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
hit
2+
| code.go:3:6:3:17 | sharedTarget |
3+
| code.go:11:6:11:18 | foreignTarget |
4+
| code.go:17:6:17:15 | noteTarget |
5+
mark
6+
| code.go:5:6:5:17 | valuedTarget | q1 |
7+
one
8+
| code.go:7:6:7:13 | q1Target |
9+
spurious
10+
| code.go:15:6:15:19 | spuriousTarget |
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* @id go/test/q1
3+
*/
4+
5+
private import go
6+
7+
private predicate isFixtureFunction(Function function, string name) { function.getName() = name }
8+
9+
query predicate hit(Function function) {
10+
isFixtureFunction(function, ["sharedTarget", "foreignTarget", "noteTarget"])
11+
}
12+
13+
query predicate mark(Function function, string value) {
14+
isFixtureFunction(function, "valuedTarget") and
15+
value = "q1"
16+
}
17+
18+
query predicate one(Function function) { isFixtureFunction(function, "q1Target") }
19+
20+
query predicate spurious(Function function) { isFixtureFunction(function, "spuriousTarget") }
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
query: q1.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
hit
2+
| code.go:3:6:3:17 | sharedTarget |
3+
| code.go:11:6:11:18 | foreignTarget |
4+
| code.go:17:6:17:15 | noteTarget |
5+
mark
6+
| code.go:5:6:5:17 | valuedTarget | q2 |
7+
two
8+
| code.go:9:6:9:13 | q2Target |
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/**
2+
* @id go/test/q2
3+
*/
4+
5+
private import go
6+
7+
private predicate isFixtureFunction(Function function, string name) { function.getName() = name }
8+
9+
query predicate hit(Function function) {
10+
isFixtureFunction(function, ["sharedTarget", "foreignTarget", "noteTarget"])
11+
}
12+
13+
query predicate mark(Function function, string value) {
14+
isFixtureFunction(function, "valuedTarget") and
15+
value = "q2"
16+
}
17+
18+
query predicate two(Function function) { isFixtureFunction(function, "q2Target") }
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
query: q2.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql

‎shared/util/codeql/util/test/InlineExpectationsTest.qll‎

Lines changed: 169 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -685,6 +685,27 @@ module TestPostProcessing {
685685

686686
private string getQueryKind() { queryMetadata("kind", result) }
687687

688+
/**
689+
* Holds if `queryId` is the `@id` of a query in the shared-source cohort for this `--learn` run.
690+
* Empty for an ordinary single-query run.
691+
*/
692+
private predicate cohortMember(string queryId) { queryMetadata("cohortMember", queryId) }
693+
694+
/**
695+
* Parses `full` into a bare tag and an optional query ID qualifier. For example,
696+
* `mark[ql/test/q1]` becomes `mark` and `ql/test/q1`, while `mark` becomes `mark` and `""`.
697+
*/
698+
bindingset[full]
699+
private predicate parseQualifiedTag(string full, string base, string queryId) {
700+
if full.regexpMatch("[^\\[\\]]+\\[[^\\[\\]]*\\]")
701+
then
702+
base = full.regexpCapture("([^\\[\\]]+)\\[([^\\[\\]]*)\\]", 1) and
703+
queryId = full.regexpCapture("([^\\[\\]]+)\\[([^\\[\\]]*)\\]", 2)
704+
else (
705+
base = full and queryId = ""
706+
)
707+
}
708+
688709
module Make<InlineExpectationsTestSig Input> {
689710
private import InlineExpectationsTest as InlineExpectationsTest
690711

@@ -921,8 +942,14 @@ module TestPostProcessing {
921942
predicate tagMatches(string expectedTag, string actualTag) {
922943
PathProblemSourceTestInput::tagMatches(expectedTag, actualTag)
923944
or
945+
// Kind-less query predicates use the predicate name as their tag. A qualifier restricts
946+
// that expectation to the query with the corresponding `@id`.
924947
not exists(getQueryKind()) and
925-
expectedTag = actualTag
948+
exists(string base, string queryId |
949+
parseQualifiedTag(expectedTag, base, queryId) and
950+
base = actualTag and
951+
(queryId = "" or queryId = getQueryId())
952+
)
926953
}
927954

928955
bindingset[expectedTag]
@@ -938,6 +965,15 @@ module TestPostProcessing {
938965
queryId != getQueryId()
939966
)
940967
)
968+
or
969+
// A qualified kind-less tag for another query is foreign to this query and must survive
970+
// any rewrite it performs.
971+
not exists(getQueryKind()) and
972+
exists(string queryId |
973+
parseQualifiedTag(expectedTag, _, queryId) and
974+
queryId != "" and
975+
queryId != getQueryId()
976+
)
941977
}
942978

943979
private predicate hasPathProblemSource = PathProblemSourceTestInput::hasPathProblemSource/5;
@@ -1169,10 +1205,21 @@ module TestPostProcessing {
11691205
private predicate foreignExpectation(
11701206
TestImpl2::ExpectationComment comment, string column, string text
11711207
) {
1172-
exists(TColumn col, string tags |
1173-
hasExpectation(comment, col, text, tags, _) and
1208+
exists(TColumn col, string tags, string value |
1209+
hasExpectation(comment, col, text, tags, value) and
11741210
column = getColumnString(col) and
1175-
forall(string tag | tag = tags.splitAt(",") | TestInput::tagIsIgnored(tag))
1211+
forall(string tag | tag = tags.splitAt(",") | TestInput::tagIsIgnored(tag)) and
1212+
// Once this pass collapses an atom to its bare form, an earlier cohort member's
1213+
// qualified default-column tag is redundant. Named-column expectations and genuinely
1214+
// foreign query IDs remain untouched.
1215+
not exists(string relativePath, int line, string base, string queryId |
1216+
col = TDefaultColumn() and
1217+
not tags.matches("%,%") and
1218+
parseQualifiedTag(tags, base, queryId) and
1219+
cohortMember(queryId) and
1220+
parseLocationString(comment.getLocation().getRelativeUrl(), relativePath, line, _, _, _) and
1221+
isCollapsedAtom(relativePath, line, base, value)
1222+
)
11761223
)
11771224
}
11781225

@@ -1207,6 +1254,112 @@ module TestPostProcessing {
12071254
result != ""
12081255
}
12091256

1257+
/** Holds if this is a shared-source cohort pass rather than an ordinary single-query run. */
1258+
private predicate inCohortMode() { exists(string queryId | cohortMember(queryId)) }
1259+
1260+
/**
1261+
* Holds if `queryId` is recorded on disk as a producer of `(tag, value)` on the given line.
1262+
* A bare tag denotes the full cohort, while a qualified tag denotes its named cohort member.
1263+
*/
1264+
private predicate onDiskProducer(
1265+
string relativePath, int line, string tag, string value, string queryId
1266+
) {
1267+
exists(TestImpl2::ExpectationComment comment, string full, string base, string qualifier |
1268+
parseLocationString(comment.getLocation().getRelativeUrl(), relativePath, line, _, _, _) and
1269+
hasExpectation(comment, TDefaultColumn(), _, full, value) and
1270+
not full.matches("%,%") and
1271+
parseQualifiedTag(full, base, qualifier) and
1272+
base = tag and
1273+
(
1274+
qualifier = "" and cohortMember(queryId)
1275+
or
1276+
qualifier != "" and queryId = qualifier and cohortMember(qualifier)
1277+
)
1278+
)
1279+
}
1280+
1281+
/** Holds if the current query produces `(tag, value)` on the given result end line. */
1282+
private predicate selfProducesAtom(string relativePath, int endLine, string tag, string value) {
1283+
exists(Test::ActualTestResult actualResult |
1284+
not actualResult.isOptional() and
1285+
actualResult.getTag() != "RelatedLocation" and
1286+
parseLocationString(actualResult.getLocation().getRelativeUrl(), relativePath, _, _,
1287+
endLine, _) and
1288+
tag = actualResult.getTag() and
1289+
value = actualResult.getValue()
1290+
)
1291+
}
1292+
1293+
/**
1294+
* Holds if `queryId` produces the atom after reconciling the current query's on-disk state
1295+
* with its actual results. Other cohort members come from disk; the current member comes only
1296+
* from its current results, so a stale self-qualified tag does not count as a producer.
1297+
*/
1298+
private predicate reconciledProducer(
1299+
string relativePath, int line, string tag, string value, string queryId
1300+
) {
1301+
queryId = getQueryId() and selfProducesAtom(relativePath, line, tag, value)
1302+
or
1303+
queryId != getQueryId() and onDiskProducer(relativePath, line, tag, value, queryId)
1304+
}
1305+
1306+
/** Holds if the reconciled producer set for the atom is the whole cohort. */
1307+
private predicate isCollapsedAtom(string relativePath, int line, string tag, string value) {
1308+
inCohortMode() and
1309+
(
1310+
selfProducesAtom(relativePath, line, tag, value)
1311+
or
1312+
onDiskProducer(relativePath, line, tag, value, _)
1313+
) and
1314+
forall(string queryId | cohortMember(queryId) |
1315+
reconciledProducer(relativePath, line, tag, value, queryId)
1316+
)
1317+
}
1318+
1319+
/** Renders a bare or qualified tag with its optional value. */
1320+
bindingset[tag, value]
1321+
private string renderAtom(string tag, string value) {
1322+
if value = "" then result = tag else result = tag + "=" + value
1323+
}
1324+
1325+
/**
1326+
* Renders a current-query atom in its canonical cohort form: bare when every member produces
1327+
* it, otherwise qualified with the current query ID.
1328+
*/
1329+
bindingset[relativePath, line, tag, value]
1330+
private string renderCohortAtom(string relativePath, int line, string tag, string value) {
1331+
(
1332+
not inCohortMode()
1333+
or
1334+
isCollapsedAtom(relativePath, line, tag, value)
1335+
) and
1336+
result = renderAtom(tag, value)
1337+
or
1338+
inCohortMode() and
1339+
not isCollapsedAtom(relativePath, line, tag, value) and
1340+
result = renderAtom(tag + "[" + getQueryId() + "]", value)
1341+
}
1342+
1343+
/**
1344+
* Renders a matching default-column expectation for the current query. This also normalizes
1345+
* pre-existing cohort comments: a complete qualified set collapses to bare, while a bare atom
1346+
* no longer produced by the whole cohort becomes current-query-qualified.
1347+
*/
1348+
private string renderSurvivingDefaultExpectation(Test::FailureLocatable expectation) {
1349+
exists(string relativePath, int line, string base, string queryId |
1350+
parseQualifiedTag(expectation.getTag(), base, queryId) and
1351+
parseLocationString(expectation.getLocation().getRelativeUrl(), relativePath, line, _, _,
1352+
_) and
1353+
(
1354+
inCohortMode() and
1355+
(queryId = "" or queryId = getQueryId()) and
1356+
result = renderCohortAtom(relativePath, line, base, expectation.getValue())
1357+
or
1358+
not inCohortMode() and result = expectation.getExpectationText()
1359+
)
1360+
)
1361+
}
1362+
12101363
/**
12111364
* Gets the exclusive end column for replacing `comment`. Same-line CodeQL locations use
12121365
* inclusive end columns, so their end boundary is one column later. A comment whose location
@@ -1237,28 +1390,30 @@ module TestPostProcessing {
12371390
private predicate survivingExpectation(
12381391
TestImpl2::ExpectationComment comment, string column, string text
12391392
) {
1240-
exists(Test::FailureLocatable e |
1241-
e.getLocation() = comment.getLocation() and text = e.getExpectationText()
1242-
|
1393+
exists(Test::FailureLocatable e | e.getLocation() = comment.getLocation() |
12431394
// a default expectation that still matches a result is kept
12441395
e instanceof Test::GoodTestExpectation and
12451396
e = Test::getAMatchingExpectation(_, _, _, _, _) and
1246-
column = ""
1397+
column = "" and
1398+
text = renderSurvivingDefaultExpectation(e)
12471399
or
12481400
// a `SPURIOUS:` expectation whose result still fires is kept
12491401
e instanceof Test::FalsePositiveTestExpectation and
12501402
e = Test::getAMatchingExpectation(_, _, _, _, _) and
1251-
column = "SPURIOUS"
1403+
column = "SPURIOUS" and
1404+
text = e.getExpectationText()
12521405
or
12531406
// a `MISSING:` expectation whose result now fires is promoted to the default column
12541407
e instanceof Test::FalseNegativeTestExpectation and
12551408
e = Test::getAMatchingExpectation(_, _, _, _, _) and
1256-
column = ""
1409+
column = "" and
1410+
text = renderSurvivingDefaultExpectation(e)
12571411
or
12581412
// a `MISSING:` expectation whose result is still absent is kept
12591413
e instanceof Test::FalseNegativeTestExpectation and
12601414
not e = Test::getAMatchingExpectation(_, _, _, _, _) and
1261-
column = "MISSING"
1415+
column = "MISSING" and
1416+
text = e.getExpectationText()
12621417
)
12631418
}
12641419

@@ -1292,9 +1447,10 @@ module TestPostProcessing {
12921447
Test::getAMatchingExpectation(actualResult.getLocation(), actualResult.toString(),
12931448
actualResult.getTag(), actualResult.getValue(), false)
12941449
) and
1295-
text = actualResult.getExpectationText() and
12961450
parseLocationString(actualResult.getLocation().getRelativeUrl(), relativePath, endLine, _,
1297-
endLine, _)
1451+
endLine, _) and
1452+
text =
1453+
renderCohortAtom(relativePath, endLine, actualResult.getTag(), actualResult.getValue())
12981454
)
12991455
}
13001456

0 commit comments

Comments
 (0)