Skip to content

Support --learn for inline expectations via learnEdits#22140

Draft
d10c wants to merge 2 commits into
github:mainfrom
d10c:d10c/learn-inline-expectations
Draft

Support --learn for inline expectations via learnEdits#22140
d10c wants to merge 2 commits into
github:mainfrom
d10c:d10c/learn-inline-expectations

Conversation

@d10c

@d10c d10c commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Overview

Adds 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 files. The test
runner consumes this predicate and applies the edits; this PR only computes
them.

Scope

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, and
  • a plain // $ Alert comment that is the sole expectation on its line and no
    longer matches any result → remove it.

The comment is added on / removed from 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). Removal deletes from the comment marker to
the end of the line, which 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.

Rendering comments per language

To render a new comment, InputSig gains getStartCommentMarker(relativePath)
plus a defaulted getEndCommentMarker(relativePath). These are keyed on the
source file's relative path rather than the analysed language, because a single
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: extractors that can ingest other file types (e.g.
C# also extracts XML, JavaScript also extracts HTML) gate on the file extension,
while single-line-comment-only extractors can return a constant. Both predicates
are bindingset[relativePath] so they need not be materialised for all files.
The defaulted getEndCommentMarker leaves the door open for block-comment
(XML/YAML) languages later without another signature break.

Testing

The predicate is only observable through the engine's --learn handling, so it
cannot be pinned via a .expected file. It is covered by engine-side
end-to-end tests, one per language.

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>
Comment thread shared/util/codeql/util/test/InlineExpectationsTest.qll Fixed
Comment thread cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql Outdated
The C/C++ start-comment marker regex listed `.m`/`.mm` (Objective-C and
Objective-C++), but the cpp extractor does not extract Objective-C, so
those files never appear in a database and the entries were dead. Drop
them and reword the comment to say we only render for C/C++ sources, per
review feedback.

Also fix the US spelling "analyzed" in the shared getStartCommentMarker
doc comment, flagged by the misspelling check.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants