Skip to content

Commit c1b801c

Browse files
d10cCopilot
andcommitted
Support --learn for inline expectations via learnEdits
Add a `learnEdits` query predicate to `TestPostProcessing::Make` so that `codeql test run --learn` can update inline `// $ Alert` expectation comments in source files, instead of only rewriting `.expected`. The test runner consumes this predicate and applies the edits; here we only compute them. The predicate emits a deliberately reliable MVP subset, restricted to the plain `Alert` tag with no value or query-id annotation: - an actual result with no matching expectation -> append a new `// $ Alert` comment on the result's line ("append" operation), and - a plain `// $ Alert` comment that is the sole expectation on its line and no longer matches any result -> remove the comment ("replace" with ""). The comment is appended on, and removed from, the result's *end* line: an expectation matches a result when the expectation's start line equals the result's end line (see `onSameLine`). Most results span a single line, but some extractors include leading trivia in a location (e.g. Rust), so start and end lines can differ. Removal deletes from the comment marker to the end of the line ("replace" with an end column of 0, the engine's "to end of line" convention); this avoids depending on how each extractor reports a line comment's end column (e.g. Swift reports it as ending at column 1 of the next line). Cases needing sub-comment column surgery (promoting `MISSING:`, clearing `SPURIOUS:`, or editing one tag among several) are left for a follow-up. To render a new comment, `InputSig` gains `getStartCommentMarker(relativePath)` and a defaulted `getEndCommentMarker(relativePath)`. These are keyed on the source file's relative path rather than the analysed language, because one database can mix languages with different comment syntaxes (e.g. Java + XML); each per-language Input module returns a marker only for the file types whose comment syntax it supports. Languages whose extractor can ingest other file types (e.g. C# also extracts XML, JavaScript also extracts HTML) gate on the file extension so those files are skipped; extractors that only ingest a single line-comment language (e.g. Swift) can return a constant. Both predicates are `bindingset[relativePath]` so they need not be materialised for all files. The end marker is defaulted to "" so existing modules need only implement the start marker; this leaves the door open for block-comment (XML/YAML) languages in a later PR without another signature break. No committed QL test accompanies this change: the predicate is only observable through the engine's `--learn` handling (a released engine rejects the reserved `learnEdits` name outright), so it cannot be pinned via a `.expected` file. It is instead covered by engine-side end-to-end tests, one per language. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 11be079 commit c1b801c

12 files changed

Lines changed: 212 additions & 0 deletions

File tree

cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,13 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
1818
f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
1919
)
2020
}
21+
22+
bindingset[relativePath]
23+
string getStartCommentMarker(string relativePath) {
24+
// C/C++ databases can also contain XML (e.g. `.xml`, `.props`), whose block-comment
25+
// syntax is not yet supported, so we only render for C/C++/Objective-C sources.
26+
relativePath
27+
.regexpMatch(".*\\.(c|cc|cpp|cxx|cp|c\\+\\+|h|hh|hpp|hxx|h\\+\\+|inl|tcc|ipp|tpp|cu|cuh|m|mm)") and
28+
result = "//"
29+
}
2130
}

csharp/ql/lib/utils/test/InlineExpectationsTestQuery.ql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
1818
f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
1919
)
2020
}
21+
22+
bindingset[relativePath]
23+
string getStartCommentMarker(string relativePath) {
24+
// C# databases can also contain XML (e.g. `.csproj`, `.config`) and Razor markup, whose
25+
// comment syntaxes are not yet supported, so we only render for C# sources.
26+
relativePath.regexpMatch(".*\\.(cs|csx)") and
27+
result = "//"
28+
}
2129
}

go/ql/lib/utils/test/InlineExpectationsTestQuery.ql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
1818
f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
1919
)
2020
}
21+
22+
bindingset[relativePath]
23+
string getStartCommentMarker(string relativePath) {
24+
// Go databases can also contain XML, whose block-comment syntax is not yet supported, so
25+
// we only render for Go sources.
26+
relativePath.matches("%.go") and
27+
result = "//"
28+
}
2129
}

java/ql/lib/utils/test/InlineExpectationsTestQuery.ql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
1818
f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
1919
)
2020
}
21+
22+
bindingset[relativePath]
23+
string getStartCommentMarker(string relativePath) {
24+
// Java databases can also contain XML; those files use a different (block) comment
25+
// syntax that is not yet supported, so we only render for Java and Kotlin sources.
26+
(relativePath.matches("%.java") or relativePath.matches("%.kt")) and
27+
result = "//"
28+
}
2129
}

javascript/ql/lib/utils/test/InlineExpectationsTestQuery.ql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
1818
f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
1919
)
2020
}
21+
22+
bindingset[relativePath]
23+
string getStartCommentMarker(string relativePath) {
24+
// JavaScript databases can also contain HTML, whose (block) comment syntax is not yet
25+
// supported, so we only render for the line-comment source files.
26+
relativePath.regexpMatch(".*\\.(js|cjs|mjs|jsx|ts|cts|mts|tsx)") and
27+
result = "//"
28+
}
2129
}

python/ql/lib/utils/test/InlineExpectationsTestQuery.ql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
1818
f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
1919
)
2020
}
21+
22+
bindingset[relativePath]
23+
string getStartCommentMarker(string relativePath) {
24+
// Python databases can also contain XML, whose block-comment syntax is not yet supported,
25+
// so we only render for Python sources.
26+
relativePath.regexpMatch(".*\\.(py|pyi)") and
27+
result = "#"
28+
}
2129
}

ql/ql/src/utils/test/InlineExpectationsTestQuery.ql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
1818
f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
1919
)
2020
}
21+
22+
bindingset[relativePath]
23+
string getStartCommentMarker(string relativePath) {
24+
// The QL extractor can also extract YAML (e.g. `qlpack.yml`), whose `#` comment syntax
25+
// differs, so we only render for QL sources and dbscheme files.
26+
relativePath.regexpMatch(".*\\.(ql|qll|dbscheme)") and
27+
result = "//"
28+
}
2129
}

ruby/ql/lib/utils/test/InlineExpectationsTestQuery.ql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
1818
f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
1919
)
2020
}
21+
22+
bindingset[relativePath]
23+
string getStartCommentMarker(string relativePath) {
24+
// Ruby databases can also contain ERB, whose comment syntax is not yet supported, so we
25+
// only render for plain Ruby sources.
26+
relativePath.matches("%.rb") and
27+
result = "#"
28+
}
2129
}

rust/ql/lib/utils/test/InlineExpectationsTestQuery.ql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig<Impl> {
1818
f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
1919
)
2020
}
21+
22+
bindingset[relativePath]
23+
string getStartCommentMarker(string relativePath) {
24+
// Rust databases can also contain YAML, whose `#` comment syntax differs, so we only
25+
// render for Rust sources.
26+
relativePath.matches("%.rs") and
27+
result = "//"
28+
}
2129
}

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

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,28 @@ module TestPostProcessing {
635635

636636
signature module InputSig<InlineExpectationsTestSig Input> {
637637
string getRelativeUrl(Input::Location location);
638+
639+
/**
640+
* Gets the marker that starts a line comment (for example `"//"` or `"#"`) in the source
641+
* file with the given `relativePath`, provided that `codeql test run --learn` is able to
642+
* render inline expectations for that file. Files for which this has no result are left
643+
* untouched by `--learn`.
644+
*
645+
* This is keyed on the file rather than on the analysed language because a single database
646+
* may contain source files in several languages with different comment syntaxes (for
647+
* example Java together with XML). `relativePath` is the path reported by `getRelativeUrl`.
648+
*/
649+
bindingset[relativePath]
650+
string getStartCommentMarker(string relativePath);
651+
652+
/**
653+
* Gets the marker that ends a comment (for example `"-->"`) in the source file with the
654+
* given `relativePath`. Defaults to the empty string, which is correct for languages whose
655+
* inline expectations use line comments; block-comment languages can override it so that
656+
* `--learn` renders a closing marker.
657+
*/
658+
bindingset[relativePath]
659+
default string getEndCommentMarker(string relativePath) { result = "" }
638660
}
639661

640662
module Make<InlineExpectationsTestSig Input, InputSig<Input> Input2> {
@@ -1011,5 +1033,105 @@ module TestPostProcessing {
10111033
Test::testFailures(_, _) and
10121034
relation = "testFailures"
10131035
}
1036+
1037+
/**
1038+
* Gets the fully rendered inline expectation comment (including the comment markers) that
1039+
* `--learn` should insert for a new `tag` expectation in the file with the given
1040+
* `relativePath`, or has no result if that file's comment syntax is not supported.
1041+
*
1042+
* The leading space separates the comment from any existing content on the line.
1043+
*/
1044+
bindingset[relativePath, tag]
1045+
private string renderExpectationComment(string relativePath, string tag) {
1046+
exists(string startMarker, string endMarker, string endSuffix |
1047+
startMarker = Input2::getStartCommentMarker(relativePath) and
1048+
endMarker = Input2::getEndCommentMarker(relativePath) and
1049+
(
1050+
endMarker = "" and endSuffix = ""
1051+
or
1052+
endMarker != "" and endSuffix = " " + endMarker
1053+
) and
1054+
result = " " + startMarker + " $ " + tag + endSuffix
1055+
)
1056+
}
1057+
1058+
/**
1059+
* Holds if `codeql test run --learn` should edit the source file `file` so that its inline
1060+
* expectations match the current query results. Each row asks the test runner to change
1061+
* `line`, where `operation` is either:
1062+
*
1063+
* - `"append"`: add `text` (a fully rendered comment) at the end of the line; `startColumn`
1064+
* and `endColumn` are both 0.
1065+
* - `"replace"`: replace the 1-based inclusive column range `[startColumn, endColumn]` with
1066+
* `text` (the empty string deletes the range).
1067+
*
1068+
* Only a deliberately reliable subset is emitted so far, restricted to the plain `Alert`
1069+
* tag with no value or query-id annotation:
1070+
*
1071+
* - an actual result with no matching expectation gets a new `// $ Alert` comment appended
1072+
* (an *unexpected result*), and
1073+
* - a plain `// $ Alert` comment whose result is now missing, and which is the only
1074+
* expectation on that comment, is removed (a *missing result*).
1075+
*
1076+
* Cases that need sub-comment column information (promoting `MISSING:`/clearing `SPURIOUS:`,
1077+
* or editing one tag among several on a line) are intentionally left for a follow-up.
1078+
*/
1079+
query predicate learnEdits(
1080+
string file, int line, string operation, int startColumn, int endColumn, string text
1081+
) {
1082+
// Unexpected result: append a new `// $ Alert` comment on the alert's line. The comment
1083+
// must go on the result's *end* line, because an expectation matches a result when the
1084+
// expectation's start line equals the result's end line (see `onSameLine`). For most
1085+
// languages a result spans a single line, but some (e.g. Rust) include leading trivia in
1086+
// the location, so the start and end lines differ.
1087+
exists(Test::ActualTestResult actualResult, string relativePath, int el, string comment |
1088+
actualResult.getTag() = "Alert" and
1089+
actualResult.getValue() = "" and
1090+
not actualResult.isOptional() and
1091+
not exists(
1092+
Test::getAMatchingExpectation(actualResult.getLocation(), actualResult.toString(),
1093+
actualResult.getTag(), actualResult.getValue(), false)
1094+
) and
1095+
parseLocationString(actualResult.getLocation().getRelativeUrl(), relativePath, _, _, el, _) and
1096+
comment = renderExpectationComment(relativePath, "Alert") and
1097+
file = relativePath and
1098+
line = el and
1099+
operation = "append" and
1100+
startColumn = 0 and
1101+
endColumn = 0 and
1102+
text = comment
1103+
)
1104+
or
1105+
// Missing result: remove a plain `// $ Alert` comment that is the comment's sole
1106+
// expectation and no longer matches any actual result.
1107+
exists(Test::GoodTestExpectation expectation, string relativePath, int sl, int sc |
1108+
expectation.getTag() = "Alert" and
1109+
expectation.getValue() = "" and
1110+
not expectation = Test::getAMatchingExpectation(_, _, _, _, _) and
1111+
// the comment carries exactly this one expectation, so removing it wholesale is safe
1112+
count(Test::FailureLocatable other |
1113+
other.getLocation() = expectation.getLocation() and
1114+
(
1115+
other instanceof Test::GoodTestExpectation or
1116+
other instanceof Test::FalsePositiveTestExpectation or
1117+
other instanceof Test::FalseNegativeTestExpectation
1118+
)
1119+
) = 1 and
1120+
not exists(Test::InvalidTestExpectation invalid |
1121+
invalid.getLocation() = expectation.getLocation()
1122+
) and
1123+
parseLocationString(expectation.getLocation().getRelativeUrl(), relativePath, sl, sc, _, _) and
1124+
file = relativePath and
1125+
line = sl and
1126+
operation = "replace" and
1127+
startColumn = sc and
1128+
// A trailing inline expectation comment always runs to the end of its line, so delete from
1129+
// the comment marker to the end of the line. `endColumn = 0` is the engine's "to end of
1130+
// line" convention; using it avoids depending on how each extractor reports a line
1131+
// comment's end column (e.g. Swift reports it as ending at column 1 of the next line).
1132+
endColumn = 0 and
1133+
text = ""
1134+
)
1135+
}
10141136
}
10151137
}

0 commit comments

Comments
 (0)