Mix learned tags with plain comments and re-wrap trailing notes by marker

`codeql test run --learn` could already keep a trailing regular note when
rewriting an expectation comment (`// $ Alert // note`), but only for
`//`-comment languages: the note was carried verbatim *including* its `//`
marker, so deleting the last expectation from a `#`-comment file would have
produced an invalid `// note` line rather than `# note`, and a plain comment
carrying no expectation at all was never a merge target.

Generalise trailing-note handling so it is marker-aware and works for every
line-comment language:

- Rename `getTrailingComment` to `getTrailingNote` and return the note's
  *content*, with its own comment marker and surrounding whitespace stripped.
  It now also matches a plain comment that carries no expectation of its own
  (`// note`, `# note`), whose whole content is the note.
- When re-rendering a comment, the note is re-wrapped with the inner `//`
  delimiter the framework recognises regardless of the outer marker, so a
  `#`-comment file renders `# $ Alert // note`.
- When the last expectation is removed but a note survives, re-wrap the note in
  the file's own start marker (`# note`, not the invalid `// note`).
- Broaden `isRewritableComment` to accept a plain comment carrying only a note,
  so an unexpected result on that line merges `Alert` into the comment
  (`// note` -> `// $ Alert // note`) instead of appending a second comment.

`isRewritableComment` is only consulted on the `--learn` path, so ordinary
`codeql test run` output is unchanged.

Author: d10c

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

@@ -374,12 +374,14 @@ module Make<InlineExpectationsTestSig Impl> {
     }
 
     /**
-     * Holds if `location` is the location of an inline expectation comment that
+     * Holds if `location` is the location of a comment that
      * `codeql test run --learn` may rewrite as a whole.
      *
-     * A comment is rewritable when it carries at least one parseable expectation, none of its
-     * expectations is unparseable, and none mixes (in a single comma-separated group) a tag this
-     * test understands with a tag it ignores (for example a query ID that does not match the
+     * A comment is rewritable when it either carries at least one parseable expectation, or is a
+     * plain comment carrying only a trailing note (see `getTrailingNote`) that a freshly learned
+     * tag can be merged into (for example `// note` -> `// $ Alert // note`); and in addition none
+     * of its expectations is unparseable, and none mixes (in a single comma-separated group) a tag
+     * this test understands with a tag it ignores (for example a query ID that does not match the
      * current query). The last condition keeps the rewrite from having to take apart a group such
      * as `Alert,Source[other-query]` whose parts this test treats differently; such comments are
      * left untouched.
@@ -391,7 +393,14 @@ module Make<InlineExpectationsTestSig Impl> {
      */
     predicate isRewritableComment(Impl::Location location) {
       exists(Impl::ExpectationComment comment | comment.getLocation() = location |
-        getAnExpectation(comment, _, _, _, _) and
+        (
+          getAnExpectation(comment, _, _, _, _)
+          or
+          // A plain comment carrying only a note (no expectation of its own) is rewritable too, so
+          // that a freshly learned tag can be merged into it rather than appended as a second
+          // comment (for example `// note` -> `// $ Alert // note`).
+          getTrailingNote(location, _) and not getAnExpectation(comment, _, _, _, _)
+        ) and
         not exists(InvalidTestExpectation invalid | invalid.getLocation() = location) and
         not exists(string tags, string owned, string ignored |
           getAnExpectation(comment, _, _, tags, _) and
@@ -423,22 +432,32 @@ module Make<InlineExpectationsTestSig Impl> {
     }
 
     /**
-     * Holds if the expectation comment at `location` ends with a trailing regular (non-interpreted)
-     * comment `text`, that is a `//` sequence after the expectations whose remainder the framework
-     * treats as an ordinary comment (see `expectationCommentPattern`). `text` includes the trailing
-     * comment's own `//` marker, for example `// note`.
+     * Holds if the comment at `location` carries a trailing regular (non-interpreted) note `note`,
+     * with the note's own comment marker and surrounding whitespace stripped. This is either:
      *
-     * `codeql test run --learn` keeps this trailing comment when it rewrites or deletes the
-     * expectation comment, so an explanatory note written next to an expectation is not lost. Only
-     * `//` starts such a trailing comment, mirroring `expectationCommentPattern`'s
-     * `(?:[^/]|/[^/])*` expectation region, which ends only at `//`; a `#` never does, so this is in
-     * practice a feature of `//`-comment languages.
+     * - the text after a `//` that follows the expectations in an expectation comment (for example
+     *   `note` in `// $ Alert // note` or `# $ Alert // note`), which the framework treats as an
+     *   ordinary comment (see `expectationCommentPattern`); or
+     * - the whole content of a plain comment that carries no expectation at all (for example `note`
+     *   in `// note` or `# note`), into which `--learn` may merge a freshly learned tag.
+     *
+     * `codeql test run --learn` keeps this note when it rewrites, deletes, or merges into the
+     * comment, re-wrapping it with the appropriate markers (`<marker> $ ... // note` when
+     * expectations remain, or `<marker> note` when none do), so an explanatory note written next to
+     * code is never lost. Only `//` delimits such a note within an expectation comment, mirroring
+     * `expectationCommentPattern`'s `(?:[^/]|/[^/])*` expectation region, which ends only at `//`; a
+     * `#` never does, so `# $ Alert # note` reads `note` as a tag rather than a note.
      */
-    predicate getTrailingComment(Impl::Location location, string text) {
-      exists(Impl::ExpectationComment comment |
-        comment.getLocation() = location and
-        text = comment.getContents().regexpCapture("\\s*\\$ (?:[^/]|/[^/])*(//.*)", 1)
-      )
+    predicate getTrailingNote(Impl::Location location, string note) {
+      exists(Impl::ExpectationComment comment | comment.getLocation() = location |
+        note = comment.getContents().regexpCapture("\\s*\\$ (?:[^/]|/[^/])*//(.*)", 1).trim()
+        or
+        // A plain comment with no expectation of its own: its whole content is the note.
+        not getAnExpectation(comment, _, _, _, _) and
+        not exists(InvalidTestExpectation invalid | invalid.getLocation() = location) and
+        note = comment.getContents().trim()
+      ) and
+      note != ""
     }
 
     query predicate testFailures(FailureLocatable element, string message) {
@@ -1307,14 +1326,16 @@ module TestPostProcessing {
           or
           endMarker != "" and endSuffix = " " + endMarker
         ) and
-        // Preserve a trailing regular comment (e.g. `// $ Alert // note`) that sits after the
-        // expectations, so rewriting the expectations never drops an explanatory note.
+        // Preserve a trailing regular note (e.g. the `note` in `// $ Alert // note`) that sits
+        // after the expectations, so rewriting the expectations never drops an explanatory note.
+        // The inner delimiter is always `//`, which the framework recognises regardless of the
+        // outer comment marker (so a `#`-comment file renders `# $ Alert // note`).
         (
-          exists(string trailing |
-            Test::getTrailingComment(commentLoc, trailing) and trailingSuffix = " " + trailing
+          exists(string note |
+            Test::getTrailingNote(commentLoc, note) and trailingSuffix = " // " + note
           )
           or
-          not Test::getTrailingComment(commentLoc, _) and trailingSuffix = ""
+          not Test::getTrailingNote(commentLoc, _) and trailingSuffix = ""
         ) and
         body =
           concat(string column |
@@ -1352,10 +1373,10 @@ module TestPostProcessing {
      * `SPURIOUS:`, and `MISSING:` columns. Expectations this test ignores (for example a tag
      * annotated with a different query's ID) are preserved verbatim, so the comment keeps any
      * expectation belonging to a different query that shares the same source file; see
-     * `isRewritableComment` and `Test::getAForeignExpectation`. A trailing regular comment after
-     * the expectations (for example the ` // note` in `// $ Alert // note`) is likewise preserved;
-     * see `Test::getTrailingComment`. Only `//` starts such a trailing comment, so this is in
-     * practice a feature of `//`-comment languages.
+     * `isRewritableComment` and `Test::getAForeignExpectation`. A trailing regular note after the
+     * expectations (for example the `note` in `// $ Alert // note` or `# $ Alert // note`) is
+     * likewise preserved, and a freshly learned tag can be merged into a plain comment that carries
+     * only such a note (`// note` -> `// $ Alert // note`); see `Test::getTrailingNote`.
      */
     query predicate learnEdits(
       string file, int line, string operation, int startColumn, int endColumn, string text
@@ -1413,12 +1434,17 @@ module TestPostProcessing {
           text = renderLearnedComment(relativePath, commentLoc)
           or
           not exists(string column, string t | desiredExpectation(commentLoc, column, t)) and
-          // No expectation survives, so drop the `$ ...` comment. If it carried a trailing regular
-          // comment, keep that in place of the whole comment; otherwise delete to the end of line.
+          // No expectation survives, so drop the `$ ...` part of the comment. If it carried a
+          // trailing regular note, keep that note re-wrapped in the file's own comment marker (so a
+          // `#`-comment file yields `# note`, not an invalid `// note`); otherwise delete to the
+          // end of line.
           (
-            Test::getTrailingComment(commentLoc, text)
+            exists(string note |
+              Test::getTrailingNote(commentLoc, note) and
+              text = Input2::getStartCommentMarker(relativePath) + " " + note
+            )
             or
-            not Test::getTrailingComment(commentLoc, _) and text = ""
+            not Test::getTrailingNote(commentLoc, _) and text = ""
           )
         )
       )