Skip to content

Commit ae24f4e

Browse files
committed
Decline unsafe multi-line expectation appends
Emit append edits only when an unexpected result starts and ends on the same source line. Multi-line locations need language-aware source anchors; leaving them unedited lets the tagged test fail safely instead of inserting a line comment inside a construct. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1adfa307-c48e-43f5-94e3-8286e4a8c083
1 parent d28ee5a commit ae24f4e

1 file changed

Lines changed: 12 additions & 11 deletions

File tree

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

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1269,11 +1269,12 @@ module TestPostProcessing {
12691269
* keyed on the result's own location (a `relativePath`/line pair) rather than on a comment
12701270
* location: there may be no comment on the line at all. Specifically it is keyed on the
12711271
* result's *end* line, because an expectation matches a result when the expectation's start
1272-
* line equals the result's end line (see `onSameLine`); for most languages a result occupies a
1273-
* single line, but some (e.g. Rust) fold leading trivia into the location so its start and end
1274-
* lines differ, and the expectation must land on the end line to match. Whether the new
1275-
* expectation is appended as a fresh comment or merged into an existing one is decided by the
1276-
* callers (see the append disjunct of `learnEdits` and `mergedNewExpectation`).
1272+
* line equals the result's end line (see `onSameLine`). Multi-line results are deliberately
1273+
* excluded: appending a line comment at either endpoint can put it inside a string literal or
1274+
* change the meaning of the construct, so finding a safe source anchor is left to the
1275+
* block-comment/source-anchor follow-up. Whether a new single-line expectation is appended as
1276+
* a fresh comment or merged into an existing one is decided by the callers (see the append
1277+
* disjunct of `learnEdits` and `mergedNewExpectation`).
12771278
*
12781279
* `RelatedLocation` results are excluded: they are only reported when an expectation on the
12791280
* line already references them (see `hasRelatedLocation`/`shouldReportRelatedLocations`), so
@@ -1288,7 +1289,7 @@ module TestPostProcessing {
12881289
actualResult.getTag(), actualResult.getValue(), false)
12891290
) and
12901291
text = actualResult.getExpectationText() and
1291-
parseLocationString(actualResult.getLocation().getRelativeUrl(), relativePath, _, _,
1292+
parseLocationString(actualResult.getLocation().getRelativeUrl(), relativePath, endLine, _,
12921293
endLine, _)
12931294
)
12941295
}
@@ -1465,11 +1466,11 @@ module TestPostProcessing {
14651466
// Unexpected result with no comment to merge into: append a fresh comment carrying every
14661467
// expectation learned for the result's line (see `unexpectedResultExpectation`). The comment must
14671468
// go on the result's *end* line, because an expectation matches a result when the
1468-
// expectation's start line equals the result's end line (see `onSameLine`). For most
1469-
// languages a result spans a single line, but some (e.g. Rust) include leading trivia in the
1470-
// location, so the start and end lines differ. If the line already has a rewritable comment,
1471-
// the new expectations are merged into it by the rewrite disjunct below (see
1472-
// `mergedNewExpectation`) rather than appended as a separate comment.
1469+
// expectation's start line equals the result's end line (see `onSameLine`).
1470+
// `unexpectedResultExpectation` deliberately has no result for a multi-line location because
1471+
// appending at either endpoint is not generally source-safe. If the line already has a
1472+
// rewritable comment, the new expectations are merged into it by the rewrite disjunct below
1473+
// (see `mergedNewExpectation`) rather than appended as a separate comment.
14731474
exists(string relativePath, int el, string comment |
14741475
unexpectedResultExpectation(relativePath, el, _) and
14751476
not exists(TestImpl2::ExpectationComment existing |

0 commit comments

Comments
 (0)