From c1b801c5468672a6020f52abdfe991e4abb3af3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= Date: Tue, 7 Jul 2026 22:07:34 +0200 Subject: [PATCH] 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> --- .../utils/test/InlineExpectationsTestQuery.ql | 9 ++ .../utils/test/InlineExpectationsTestQuery.ql | 8 ++ .../utils/test/InlineExpectationsTestQuery.ql | 8 ++ .../utils/test/InlineExpectationsTestQuery.ql | 8 ++ .../utils/test/InlineExpectationsTestQuery.ql | 8 ++ .../utils/test/InlineExpectationsTestQuery.ql | 8 ++ .../utils/test/InlineExpectationsTestQuery.ql | 8 ++ .../utils/test/InlineExpectationsTestQuery.ql | 8 ++ .../utils/test/InlineExpectationsTestQuery.ql | 8 ++ .../util/test/InlineExpectationsTest.qll | 122 ++++++++++++++++++ .../utils/test/InlineExpectationsTestQuery.ql | 7 + .../utils/test/InlineExpectationsTestQuery.ql | 10 ++ 12 files changed, 212 insertions(+) diff --git a/cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql index 8e6977ba5321..1325e5e3fd12 100644 --- a/cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql +++ b/cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql @@ -18,4 +18,13 @@ private module Input implements T::TestPostProcessing::InputSig { f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn ) } + + bindingset[relativePath] + string getStartCommentMarker(string relativePath) { + // C/C++ databases can also contain XML (e.g. `.xml`, `.props`), whose block-comment + // syntax is not yet supported, so we only render for C/C++/Objective-C sources. + relativePath + .regexpMatch(".*\\.(c|cc|cpp|cxx|cp|c\\+\\+|h|hh|hpp|hxx|h\\+\\+|inl|tcc|ipp|tpp|cu|cuh|m|mm)") and + result = "//" + } } diff --git a/csharp/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/csharp/ql/lib/utils/test/InlineExpectationsTestQuery.ql index 35901ee64012..1609ca867e80 100644 --- a/csharp/ql/lib/utils/test/InlineExpectationsTestQuery.ql +++ b/csharp/ql/lib/utils/test/InlineExpectationsTestQuery.ql @@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig { f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn ) } + + bindingset[relativePath] + string getStartCommentMarker(string relativePath) { + // C# databases can also contain XML (e.g. `.csproj`, `.config`) and Razor markup, whose + // comment syntaxes are not yet supported, so we only render for C# sources. + relativePath.regexpMatch(".*\\.(cs|csx)") and + result = "//" + } } diff --git a/go/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/go/ql/lib/utils/test/InlineExpectationsTestQuery.ql index 1cf2f5ea1d9b..9a180ed8a173 100644 --- a/go/ql/lib/utils/test/InlineExpectationsTestQuery.ql +++ b/go/ql/lib/utils/test/InlineExpectationsTestQuery.ql @@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig { f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn ) } + + bindingset[relativePath] + string getStartCommentMarker(string relativePath) { + // Go databases can also contain XML, whose block-comment syntax is not yet supported, so + // we only render for Go sources. + relativePath.matches("%.go") and + result = "//" + } } diff --git a/java/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/java/ql/lib/utils/test/InlineExpectationsTestQuery.ql index b0360dfecd8d..ad330a3d1f2c 100644 --- a/java/ql/lib/utils/test/InlineExpectationsTestQuery.ql +++ b/java/ql/lib/utils/test/InlineExpectationsTestQuery.ql @@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig { f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn ) } + + bindingset[relativePath] + string getStartCommentMarker(string relativePath) { + // Java databases can also contain XML; those files use a different (block) comment + // syntax that is not yet supported, so we only render for Java and Kotlin sources. + (relativePath.matches("%.java") or relativePath.matches("%.kt")) and + result = "//" + } } diff --git a/javascript/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/javascript/ql/lib/utils/test/InlineExpectationsTestQuery.ql index 55892be75d79..c1151457a0a2 100644 --- a/javascript/ql/lib/utils/test/InlineExpectationsTestQuery.ql +++ b/javascript/ql/lib/utils/test/InlineExpectationsTestQuery.ql @@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig { f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn ) } + + bindingset[relativePath] + string getStartCommentMarker(string relativePath) { + // JavaScript databases can also contain HTML, whose (block) comment syntax is not yet + // supported, so we only render for the line-comment source files. + relativePath.regexpMatch(".*\\.(js|cjs|mjs|jsx|ts|cts|mts|tsx)") and + result = "//" + } } diff --git a/python/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/python/ql/lib/utils/test/InlineExpectationsTestQuery.ql index 9ce5fdf326ca..d849809f6d9d 100644 --- a/python/ql/lib/utils/test/InlineExpectationsTestQuery.ql +++ b/python/ql/lib/utils/test/InlineExpectationsTestQuery.ql @@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig { f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn ) } + + bindingset[relativePath] + string getStartCommentMarker(string relativePath) { + // Python databases can also contain XML, whose block-comment syntax is not yet supported, + // so we only render for Python sources. + relativePath.regexpMatch(".*\\.(py|pyi)") and + result = "#" + } } diff --git a/ql/ql/src/utils/test/InlineExpectationsTestQuery.ql b/ql/ql/src/utils/test/InlineExpectationsTestQuery.ql index 979839480e1d..89d8a2788e99 100644 --- a/ql/ql/src/utils/test/InlineExpectationsTestQuery.ql +++ b/ql/ql/src/utils/test/InlineExpectationsTestQuery.ql @@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig { f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn ) } + + bindingset[relativePath] + string getStartCommentMarker(string relativePath) { + // The QL extractor can also extract YAML (e.g. `qlpack.yml`), whose `#` comment syntax + // differs, so we only render for QL sources and dbscheme files. + relativePath.regexpMatch(".*\\.(ql|qll|dbscheme)") and + result = "//" + } } diff --git a/ruby/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/ruby/ql/lib/utils/test/InlineExpectationsTestQuery.ql index 1cbc37a7fe85..08e60a966a15 100644 --- a/ruby/ql/lib/utils/test/InlineExpectationsTestQuery.ql +++ b/ruby/ql/lib/utils/test/InlineExpectationsTestQuery.ql @@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig { f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn ) } + + bindingset[relativePath] + string getStartCommentMarker(string relativePath) { + // Ruby databases can also contain ERB, whose comment syntax is not yet supported, so we + // only render for plain Ruby sources. + relativePath.matches("%.rb") and + result = "#" + } } diff --git a/rust/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/rust/ql/lib/utils/test/InlineExpectationsTestQuery.ql index e5821ba4f50c..b20a70450dce 100644 --- a/rust/ql/lib/utils/test/InlineExpectationsTestQuery.ql +++ b/rust/ql/lib/utils/test/InlineExpectationsTestQuery.ql @@ -18,4 +18,12 @@ private module Input implements T::TestPostProcessing::InputSig { f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn ) } + + bindingset[relativePath] + string getStartCommentMarker(string relativePath) { + // Rust databases can also contain YAML, whose `#` comment syntax differs, so we only + // render for Rust sources. + relativePath.matches("%.rs") and + result = "//" + } } diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll index 4e0b2f678449..3834495b7907 100644 --- a/shared/util/codeql/util/test/InlineExpectationsTest.qll +++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll @@ -635,6 +635,28 @@ module TestPostProcessing { signature module InputSig { string getRelativeUrl(Input::Location location); + + /** + * Gets the marker that starts a line comment (for example `"//"` or `"#"`) in the source + * file with the given `relativePath`, provided that `codeql test run --learn` is able to + * render inline expectations for that file. Files for which this has no result are left + * untouched by `--learn`. + * + * This is keyed on the file rather than on the analysed language because a single database + * may contain source files in several languages with different comment syntaxes (for + * example Java together with XML). `relativePath` is the path reported by `getRelativeUrl`. + */ + bindingset[relativePath] + string getStartCommentMarker(string relativePath); + + /** + * Gets the marker that ends a comment (for example `"-->"`) in the source file with the + * given `relativePath`. Defaults to the empty string, which is correct for languages whose + * inline expectations use line comments; block-comment languages can override it so that + * `--learn` renders a closing marker. + */ + bindingset[relativePath] + default string getEndCommentMarker(string relativePath) { result = "" } } module Make Input2> { @@ -1011,5 +1033,105 @@ module TestPostProcessing { Test::testFailures(_, _) and relation = "testFailures" } + + /** + * Gets the fully rendered inline expectation comment (including the comment markers) that + * `--learn` should insert for a new `tag` expectation in the file with the given + * `relativePath`, or has no result if that file's comment syntax is not supported. + * + * The leading space separates the comment from any existing content on the line. + */ + bindingset[relativePath, tag] + private string renderExpectationComment(string relativePath, string tag) { + exists(string startMarker, string endMarker, string endSuffix | + startMarker = Input2::getStartCommentMarker(relativePath) and + endMarker = Input2::getEndCommentMarker(relativePath) and + ( + endMarker = "" and endSuffix = "" + or + endMarker != "" and endSuffix = " " + endMarker + ) and + result = " " + startMarker + " $ " + tag + endSuffix + ) + } + + /** + * Holds if `codeql test run --learn` should edit the source file `file` so that its inline + * expectations match the current query results. Each row asks the test runner to change + * `line`, where `operation` is either: + * + * - `"append"`: add `text` (a fully rendered comment) at the end of the line; `startColumn` + * and `endColumn` are both 0. + * - `"replace"`: replace the 1-based inclusive column range `[startColumn, endColumn]` with + * `text` (the empty string deletes the range). + * + * Only a deliberately reliable subset is emitted so far, restricted to the plain `Alert` + * tag with no value or query-id annotation: + * + * - an actual result with no matching expectation gets a new `// $ Alert` comment appended + * (an *unexpected result*), and + * - a plain `// $ Alert` comment whose result is now missing, and which is the only + * expectation on that comment, is removed (a *missing result*). + * + * Cases that need sub-comment column information (promoting `MISSING:`/clearing `SPURIOUS:`, + * or editing one tag among several on a line) are intentionally left for a follow-up. + */ + query predicate learnEdits( + string file, int line, string operation, int startColumn, int endColumn, string text + ) { + // Unexpected result: append a new `// $ Alert` comment on the alert's line. The comment + // must go on the result's *end* line, because an expectation matches a result when the + // expectation's start line equals the result's end line (see `onSameLine`). For most + // languages a result spans a single line, but some (e.g. Rust) include leading trivia in + // the location, so the start and end lines differ. + exists(Test::ActualTestResult actualResult, string relativePath, int el, string comment | + actualResult.getTag() = "Alert" and + actualResult.getValue() = "" and + not actualResult.isOptional() and + not exists( + Test::getAMatchingExpectation(actualResult.getLocation(), actualResult.toString(), + actualResult.getTag(), actualResult.getValue(), false) + ) and + parseLocationString(actualResult.getLocation().getRelativeUrl(), relativePath, _, _, el, _) and + comment = renderExpectationComment(relativePath, "Alert") and + file = relativePath and + line = el and + operation = "append" and + startColumn = 0 and + endColumn = 0 and + text = comment + ) + or + // Missing result: remove a plain `// $ Alert` comment that is the comment's sole + // expectation and no longer matches any actual result. + exists(Test::GoodTestExpectation expectation, string relativePath, int sl, int sc | + expectation.getTag() = "Alert" and + expectation.getValue() = "" and + not expectation = Test::getAMatchingExpectation(_, _, _, _, _) and + // the comment carries exactly this one expectation, so removing it wholesale is safe + count(Test::FailureLocatable other | + other.getLocation() = expectation.getLocation() and + ( + other instanceof Test::GoodTestExpectation or + other instanceof Test::FalsePositiveTestExpectation or + other instanceof Test::FalseNegativeTestExpectation + ) + ) = 1 and + not exists(Test::InvalidTestExpectation invalid | + invalid.getLocation() = expectation.getLocation() + ) and + parseLocationString(expectation.getLocation().getRelativeUrl(), relativePath, sl, sc, _, _) and + file = relativePath and + line = sl and + operation = "replace" and + startColumn = sc and + // A trailing inline expectation comment always runs to the end of its line, so delete from + // the comment marker to the end of the line. `endColumn = 0` is the engine's "to end of + // line" convention; using it 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). + endColumn = 0 and + text = "" + ) + } } } diff --git a/swift/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/swift/ql/lib/utils/test/InlineExpectationsTestQuery.ql index a7c112bc00e0..8f7e397d0afe 100644 --- a/swift/ql/lib/utils/test/InlineExpectationsTestQuery.ql +++ b/swift/ql/lib/utils/test/InlineExpectationsTestQuery.ql @@ -18,4 +18,11 @@ private module Input implements T::TestPostProcessing::InputSig { f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn ) } + + bindingset[relativePath] + string getStartCommentMarker(string relativePath) { + // The Swift extractor only ingests Swift sources (no XML/YAML/HTML in its dbscheme), so a + // constant marker is safe; revisit if Swift ever gains extraction of another file type. + exists(relativePath) and result = "//" + } } diff --git a/unified/ql/lib/utils/test/InlineExpectationsTestQuery.ql b/unified/ql/lib/utils/test/InlineExpectationsTestQuery.ql index 039194bc2e38..950898fb12be 100644 --- a/unified/ql/lib/utils/test/InlineExpectationsTestQuery.ql +++ b/unified/ql/lib/utils/test/InlineExpectationsTestQuery.ql @@ -18,4 +18,14 @@ private module Input implements T::TestPostProcessing::InputSig { f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn ) } + + bindingset[relativePath] + string getStartCommentMarker(string relativePath) { + // The unified extractor is a new tree-sitter-based extractor that currently ingests only + // Swift sources (see `file_types` in its `codeql-extractor.yml`), which use `//`. Gating on + // the extension keeps this correct if it later gains a language with a different comment + // syntax. + relativePath.regexpMatch(".*\\.(swift|swiftinterface)") and + result = "//" + } }