Convert selected Python qlref tests to inline expectations

[Copilot]

This updates the Python qlref-to-inline-expectation migration by converting a focused set of standard problem and path-problem tests that can be represented cleanly without changing test semantics. The converted cases cover exception queries, overriding-signature queries, and the local threat-model SQL injection test.

• Converted standard Python query tests

  • Added postprocess: utils/test/InlineExpectationsTestQuery.ql to:
    • python/ql/test/query-tests/Exceptions/general/IllegalExceptionHandlerType.qlref
    • python/ql/test/query-tests/Exceptions/general/IllegalRaise.qlref
    • python/ql/test/query-tests/Exceptions/general/NotImplementedIsNotAnException.qlref
    • python/ql/test/query-tests/Functions/overriding/IncorrectlyOverriddenMethod.qlref
    • python/ql/test/query-tests/Functions/overriding/IncorrectlySpecifiedOverriddenMethod.qlref
    • python/ql/test/query-tests/Functions/overriding/WrongNameForArgumentInCall.qlref
    • python/ql/test/query-tests/Functions/overriding/WrongNumberArgumentsInCall.qlref
  • Extended python/ql/test/query-tests/Security/CWE-089-SqlInjection-local-threat-model/SqlInjection.qlref by preserving PrettyPrintModels.ql and appending InlineExpectationsTestQuery.ql.

• Annotated source at result locations

  • Added inline # $ Alert[...] markers in:
    • python/ql/test/query-tests/Exceptions/general/exceptions_test.py
    • python/ql/test/query-tests/Exceptions/general/pypy_test.py
    • python/ql/test/query-tests/Functions/overriding/test.py
    • python/ql/test/query-tests/Security/CWE-089-SqlInjection-local-threat-model/test.py
  • Used query-id-qualified tags where multiple queries share the same source file and would otherwise compete for the same comment lines.

• Preserved existing test ownership boundaries

  • Left .expected files untouched.
  • Kept existing postprocessors intact and only switched to list form where an additional postprocessor had to be appended.

• Excluded behavior-sensitive cases from this slice

  • Did not convert tests where inline comments would suppress or alter the query result on the annotated line (for example, shared exception tests where adding a comment changes whether the query reports the clause at all).

Example of the migrated style:

def illegal_raise_value2():
    raise NotException2()  # $ Alert[py/illegal-raise]

conn.execute(sys.argv[1])  # $ Alert

Original prompt

Please do the same for all other languages that CodeQL supports (i.e. other than go or java), using what you have learned. Do not make a PR. List any queries you do not convert and why. I think you will not have access to CodeQL to test your changes, so do your best without that feedback. I will provide a list of failing tests at a future point.

It seems like you have provided a context about the current date and some previous requests related to CodeQL files. If you need assistance with anything specific, such as formatting .qlref files for Java or addressing the test output failures, please let me know how I can help!

Created from VS Code.

[owen-mc]

Rerun has been triggered, but no failed runs found ⚠️

[github-advanced-security]

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

[owen-mc]

Rerun has been triggered: 5 restarted 🚀

• Swift Integration tests MacOS (all)
• Rust Integration tests MacOS (all)
• Rust Integration tests Windows
• Java Language Tests Linux
• Java Integration tests MacOS

[yoff]

It would be nice if the GitHUb platform would allow us toexpress that the diff here falls in two categories:

• adding query: and postprocess: utils/test/InlineExpectationsTestQuery.ql around the content of .qlref-files
• adding # $ Alert|Source|Sink lines to .expected-files.

Verifying this manually will immediately allow me to conclude that this is a safe PR to merge. Before I go through the remaining many files, though, I have a few questions:

• Why do we sometimes get Alert[query-name] and sometimes just Alert?
• Why do we sometimes not add the postprocessing directive?
• Why do we get QL alerts for "not using inline expectation tests"?

[owen-mc]

Note that I am currently working on undoing the changes in this PR which affects queries which import LegacyPointsTo, so as not to conflict with some ongoing work. No need to review till I next push, and then it should be a bit smaller.

1. If there are more than one inline expectations tests in one folder then you need to specify which query id an alert is for, otherwise both tests will think it's for them. Sometimes copilot puts the query id there even when there's only one inline expectations test, though I have instructed it not to.

2. We shouldn't add query: if we aren't also adding postprocess:. I will look into those. However, there are valid reasons for not converting a test to inline expectations. Below is an excerpt from the prompt that copilot developed after doing this for two languages:

Good Skip Reasons
Skip, or revert to non-inline, when conversion would make the test misleading, brittle, or noisy. Common reasons:

• The query is not @kind problem or @kind path-problem.
• The query is a telemetry, metrics, table, definitions, diagnostic, extractor-information, stub-generation, or tool-specific query.
• The test is an AST dump or similar structural output test, such as PrintAst.
• The selected result text is inside a string literal, raw string, heredoc, text block, XML literal, or multiline literal and adding a comment changes the selected text.
• The expected location is inside documentation syntax where adding a comment changes what the query reports, such as Javadoc tags.
• The .expected output is nonstandard enough that mechanical conversion is risky.
• Results are in shared stubs, generated copies, vendored files, or synthetic/test-project paths where inline comments would pollute shared fixtures.
• The alert, source, or sink locations are too dense or ambiguous to make the source understandable.
• The language’s inline expectation implementation does not support the source file or comment style needed.
• The query uses a special result kind or framework behavior, such as alert-suppression tests, that does not map cleanly to inline alert comments.

3. I looked at one, Statements/DocStrings.ql. It shouldn't be converted because one of the results has location DocStrings.py:0:0:0:0. However, we have erroneously put a query: in the .qlref file, so ql-for-ql is firing.

[BazookaMusic]

Wrong alerts?

[owen-mc]

Thanks for spotting that. It had put # Not OK # $ Alert instead of # $ Alert # Not OK.

[BazookaMusic]

Did you intend to include this?

[owen-mc]

Thanks for spotting that. I've deleted it now.

[BazookaMusic]

Looks good from what I could see. Reviewing all alerts manually is a bit unrealistic with a PR this size, but it looks good after spending reasonable time looking at it