Skip to content

Support --learn for inline expectation tests - #22228

Open
d10c wants to merge 4 commits into
github:mainfrom
d10c:d10c/learn-inline-expectations-lib
Open

Support --learn for inline expectation tests#22228
d10c wants to merge 4 commits into
github:mainfrom
d10c:d10c/learn-inline-expectations-lib

Conversation

@d10c

@d10c d10c commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Teaches the shared inline-expectations test library to drive codeql test run --learn for inline // $ Alert expectations: when a postprocess test query is run under --learn, the library now emits a learnEdits relation describing how to rewrite the annotated source so the comments match the actual query results.

Requires the matching CLI support for consuming learnEdits; the library change is incompatible CLIs older than that. The CI on this PR should pass only once a supported CLI gets released to nightly.

What it does

For each result/expectation mismatch on the standard // $ tag1 tag2=value tag3[query] // rest of comment…-style tags, learnEdits records a structured edit (append a new comment, remove or rewrite an existing one). The CLI applies these edits to the source file, so --learn updates the inline expectations directly rather than writing a failure grid into .expected. Without --learn the relation is invisible and behaviour is unchanged.

API

  • TestPostProcessing::InputSig gains getStartCommentMarker(...) (and a defaulted getEndCommentMarker), keyed on the source file of a location so a single database can mix languages with different comment syntax. Each per-language utils/test/InlineExpectationsTestQuery.ql supplies its marker (//, #, ...); block-comment languages are left for a follow-up but the end-marker hook is already in place.

Follow-ups not implemented in this PR

  • Block-comment languages (XML/YAML/HTML/ERB/Razor …)
  • Multi-query reconciliation (// $ Alert[query1] Alert[query2])
  • Splitting the tag1,tag2=value shorthand where only one tag needs to be updated.
  • Support for non-qlref/non-postprocessor inline expectations tests.

Comment thread shared/util/codeql/util/test/InlineExpectationsTest.qll Fixed
@d10c
d10c force-pushed the d10c/learn-inline-expectations-lib branch 3 times, most recently from 63edc17 to 52f2ac6 Compare July 28, 2026 16:03
@d10c
d10c requested a review from cklin July 28, 2026 16:17
@d10c
d10c marked this pull request as ready for review July 28, 2026 16:17
Copilot AI review requested due to automatic review settings July 28, 2026 16:17
@d10c
d10c requested review from a team as code owners July 28, 2026 16:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds inline --learn support so test expectations can be rewritten directly in source files.

Changes:

  • Adds structured learnEdits generation and expectation-preserving rewrite logic.
  • Introduces per-file comment-marker APIs.
  • Configures supported source extensions for each language.
Show a summary per file
File Description
shared/util/codeql/util/test/InlineExpectationsTest.qll Implements parsing and learned edits.
cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql Enables C/C++ line comments.
csharp/ql/lib/utils/test/InlineExpectationsTestQuery.ql Enables C# line comments.
go/ql/lib/utils/test/InlineExpectationsTestQuery.ql Enables Go line comments.
java/ql/lib/utils/test/InlineExpectationsTestQuery.ql Enables Java/Kotlin line comments.
javascript/ql/lib/utils/test/InlineExpectationsTestQuery.ql Enables JavaScript/TypeScript line comments.
python/ql/lib/utils/test/InlineExpectationsTestQuery.ql Enables Python line comments.
ql/ql/src/utils/test/InlineExpectationsTestQuery.ql Enables QL/dbscheme line comments.
ruby/ql/lib/utils/test/InlineExpectationsTestQuery.ql Enables Ruby line comments.
rust/ql/lib/utils/test/InlineExpectationsTestQuery.ql Enables Rust line comments.
swift/ql/lib/utils/test/InlineExpectationsTestQuery.ql Enables Swift line comments.
unified/ql/lib/utils/test/InlineExpectationsTestQuery.ql Enables unified Swift line comments.

Review details

  • Files reviewed: 12/12 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread shared/util/codeql/util/test/InlineExpectationsTest.qll Outdated
@d10c d10c added the no-change-note-required This PR does not need a change note label Jul 28, 2026
Comment thread shared/util/codeql/util/test/InlineExpectationsTest.qll Outdated
Comment thread shared/util/codeql/util/test/InlineExpectationsTest.qll Outdated
@d10c
d10c force-pushed the d10c/learn-inline-expectations-lib branch from 52f2ac6 to 62a0475 Compare July 30, 2026 17:05
@d10c
d10c marked this pull request as draft August 13, 2026 16:08
@d10c
d10c force-pushed the d10c/learn-inline-expectations-lib branch from 62a0475 to 4b9a47d Compare August 13, 2026 16:09
d10c and others added 4 commits August 13, 2026 18:39
`getAnExpectation` -- the low-level parse of a `// $ ...` expectation
comment -- lived as a private predicate inside `Make<Impl>`, reachable
only by that module's expectation classes. The upcoming `--learn`
postprocessing needs the same parse (to see every expectation on a
comment, including ones the running test ignores, so it can preserve
them), but it lives in `TestPostProcessing`, outside `Make`.

Move the predicate into a new top-level `private module
ExpectationParser<Impl>` that both consumers `private import`, so neither
has to expose the parse as part of its API nor reach into the other's
internals. This is a pure refactor: the predicate body and its callers'
behavior are unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1adfa307-c48e-43f5-94e3-8286e4a8c083
The `--learn` support that follows needs to render inline-expectation
comments in the syntax of the file being edited, and a single CodeQL
database can mix source languages (e.g. Java plus XML), so the marker
must be chosen per file rather than per database.

Extend `TestPostProcessing::InputSig` with `getStartCommentMarker(relativePath)`
(no result -> that file is left untouched by `--learn`) and a defaulted
`getEndCommentMarker(relativePath)` (`none()` by default: line-comment
languages render no closing marker; block-comment languages can override
it later). Each language's `InlineExpectationsTestQuery.ql` implements
`getStartCommentMarker`, gated to that language's own source extensions so
that other file types (XML/YAML/HTML/ERB/Razor) extracted into the same
database are not rewritten with the wrong syntax.

This commit only wires the marker API through the signature and the
per-language inputs; nothing consumes it yet, so behavior is unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1adfa307-c48e-43f5-94e3-8286e4a8c083
`codeql test run --learn` could previously only rewrite `.expected`
files; inline expectations (the `// $ Alert` comments checked by
`InlineExpectationsTest`) had to be fixed by hand. This teaches the
shared test library to compute those source edits so the test runner
can apply them.

The `test-postprocess` query now exposes a `learnEdits` relation
(`file, line, operation, startColumn, endColumn, text`) describing the
minimal source rewrite that would make the inline expectations match the
actual query results. It covers:

  - appending a fresh comment carrying every tag learned for a line that
    has an unexpected result and no existing comment to merge into;
  - rewriting an existing expectation comment as a whole so it matches
    the current results: dropping fixed-spurious tags, promoting a
    `MISSING:` expectation that now fires, clearing a stale `SPURIOUS:`
    annotation, and merging in freshly learned tags, re-rendering the
    remaining expectations (or deleting the comment when none remain);
  - preserving expectations this test does not own -- e.g. a tag
    annotated with a different query's id that shares the source file --
    and any trailing regular note (`// $ Alert // note`);
  - recording any unexpected result, not just `Alert`.

The comment syntax comes from the `getStartCommentMarker` /
`getEndCommentMarker` markers added to `InputSig` in the previous commit,
so `LearnEditsImpl` renders each edit in the target file's own syntax and
stays language-agnostic. Edits are emitted as a query predicate rather
than applied here: the engine consumes `learnEdits` only under `--learn`
and ignores it otherwise, so ordinary `test run` output is unchanged. The
supporting predicates are wrapped in a `private module LearnEditsImpl` so
only the `query predicate learnEdits` is re-exposed.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1adfa307-c48e-43f5-94e3-8286e4a8c083
`getAnExpectation` is a relation that holds between an expectation comment
and the parsed parts it carries, not a function returning a single value,
so the `get` prefix is misleading and trips the CodeQL predicate-naming
style check. Rename it to `hasExpectation`, matching the `has`-prefixed
convention for such predicates (and the sibling `hasExpectationWithValue`).

Pure rename with no behavior change; reformatted with `codeql query format`.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1adfa307-c48e-43f5-94e3-8286e4a8c083
@d10c
d10c force-pushed the d10c/learn-inline-expectations-lib branch from 4b9a47d to f8bdb41 Compare August 13, 2026 16:46
@d10c
d10c marked this pull request as ready for review August 13, 2026 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C# C++ Go Java JS no-change-note-required This PR does not need a change note Python QL-for-QL Ruby Rust Pull requests that update Rust code Swift

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants