Inline expectation tests should always have space before and after $#21405
Open
owen-mc wants to merge 18 commits intogithub:mainfrom
Open
Inline expectation tests should always have space before and after $#21405owen-mc wants to merge 18 commits intogithub:mainfrom
$#21405owen-mc wants to merge 18 commits intogithub:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR standardizes inline expectation comment formatting in Java and Go test sources by ensuring there is always a space after the $ marker (for example, // $ hasTaintFlow instead of // $hasTaintFlow). This aligns the tests with the format emitted by the Java flow test case generator and improves consistency for inline-expectations parsing.
Changes:
- Updated many Java inline expectation comments to include a space after
$(and normalized some nearby whitespace). - Updated Go inline expectation comments similarly.
- Added Micronaut-related Java library test files with inline expectations using the standardized
$format.
Reviewed changes
Copilot reviewed 3 out of 92 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| java/ql/test/query-tests/security/CWE-927/SensitiveResultReceiver.java | Normalize $ inline expectation spacing. |
| java/ql/test/query-tests/security/CWE-749/UnsafeAndroidBroadcastReceiver.java | Normalize $ inline expectation spacing. |
| java/ql/test/query-tests/security/CWE-749/UnsafeAndroidAccess.java | Normalize $ inline expectation spacing. |
| java/ql/test/query-tests/security/CWE-749/UnsafeActivity3.java | Normalize $ inline expectation spacing. |
| java/ql/test/query-tests/security/CWE-749/UnsafeActivity2.java | Normalize $ inline expectation spacing. |
| java/ql/test/query-tests/security/CWE-749/UnsafeActivity1.java | Normalize $ inline expectation spacing. |
| java/ql/test/query-tests/security/CWE-312/android/CleartextStorage/CleartextStorageSharedPrefsTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/query-tests/security/CWE-297/InsecureSimpleEmailTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/query-tests/security/CWE-297/InsecureJavaMailTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/query-tests/security/CWE-297/InsecureJakartaMailTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/query-tests/security/CWE-295/ImproperWebVeiwCertificateValidation/Test.java | Normalize $ inline expectation spacing and trim trailing whitespace. |
| java/ql/test/query-tests/security/CWE-295/AndroidMissingCertificatePinning/Test4/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/query-tests/security/CWE-295/AndroidMissingCertificatePinning/Test3/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/query-tests/security/CWE-295/AndroidMissingCertificatePinning/Test2/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/query-tests/security/CWE-295/AndroidMissingCertificatePinning/Test1/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/query-tests/security/CWE-287/InsecureLocalAuth/Test2.java | Normalize $ inline expectation spacing and remove blank trailing whitespace. |
| java/ql/test/query-tests/security/CWE-287/InsecureLocalAuth/Test.java | Normalize $ inline expectation spacing and remove blank trailing whitespace. |
| java/ql/test/query-tests/security/CWE-287/InsecureKeys/Test1/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/query-tests/security/CWE-200/semmle/tests/SensitiveTextView/Test.java | Normalize $ inline expectation spacing and trim trailing whitespace. |
| java/ql/test/query-tests/security/CWE-200/semmle/tests/SensitiveNotification/Test.java | Normalize $ inline expectation spacing and normalize whitespace. |
| java/ql/test/query-tests/security/CWE-1204/StaticInitializationVector.java | Normalize $ inline expectation spacing and trim trailing whitespace. |
| java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java | Normalize $ inline expectation spacing and trim blank lines. |
| java/ql/test/query-tests/security/CWE-078/TaintedEnvironment.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/optional/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/optional/FunctionalTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/spring/websocket/Test.java | Normalize $ inline expectation spacing and normalize annotation whitespace. |
| java/ql/test/library-tests/frameworks/spring/webmultipart/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/spring/validation/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/spring/http/TestHttp.java | Normalize $ inline expectation spacing and trim whitespace. |
| java/ql/test/library-tests/frameworks/spring/controller/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/spring/context/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/spring/cache/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/ratpack/resources/PairTest.java | Normalize $ inline expectation spacing and trim blank line. |
| java/ql/test/library-tests/frameworks/rabbitmq/Test.java | Normalize $ inline expectation spacing and minor whitespace normalization. |
| java/ql/test/library-tests/frameworks/netty/manual/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/micronaut/MicronautWebSocketTest.java | Add Micronaut websocket source test with standardized $ expectations. |
| java/ql/test/library-tests/frameworks/lastaflute/Test.java | Normalize $ inline expectation spacing and remove trailing blank lines. |
| java/ql/test/library-tests/frameworks/jms/MessageListenerImpl.java | Normalize $ inline expectation spacing for sources/tainted sinks. |
| java/ql/test/library-tests/frameworks/guava/handwritten/TestIO.java | Normalize $ inline expectation spacing (including numbered tags). |
| java/ql/test/library-tests/frameworks/guava/handwritten/TestCollect.java | Normalize $ inline expectation spacing and trim whitespace. |
| java/ql/test/library-tests/frameworks/guava/handwritten/TestBase.java | Normalize $ inline expectation spacing and trim whitespace. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/WordUtilsTextTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/WordUtilsTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/TripleTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/ToStringBuilderTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/StringTokenizerTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/StringSubstitutorTextTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/StringLookupTextTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/StringEscapeUtilsTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/StrTokenizerTextTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/StrTokenizerTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/StrSubstitutorTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/StrLookupTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/StrBuilderTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/RegExUtilsTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/PairTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/ObjectUtilsTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/MutableTest.java | Normalize $ inline expectation spacing (including SPURIOUS). |
| java/ql/test/library-tests/frameworks/apache-commons-lang3/ArrayUtilsTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/android/taint-database/FlowSteps.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/frameworks/android/slice/TestSources.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/dataflow/taintsources/SpringSavedRequest.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/dataflow/taintsources/SpringMultiPart.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/dataflow/taintsources/RmiFlowImpl.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/dataflow/taintsources/MicronautConfig.java | Add Micronaut config taint source tests with standardized $ expectations. |
| java/ql/test/library-tests/dataflow/taintsources/IntentSourcesActivity.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/dataflow/taintsources/Hudson.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/dataflow/taintsources/AndroidExposedObject.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/dataflow/taintsources/A.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/dataflow/fluent-methods/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/library-tests/dataflow/entrypoint-types/EntryPointTypesTest.java | Normalize $ inline expectation spacing. |
| java/ql/test/ext/TestModels/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/experimental/query-tests/quantum/examples/WeakOrUnknownSymmetricCipher/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/experimental/query-tests/quantum/examples/WeakOrUnknownKDFKeySize/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/experimental/query-tests/quantum/examples/WeakOrUnknownKDFIterationCount/Test.java | Normalize $ inline expectation spacing and trim whitespace. |
| java/ql/test/experimental/query-tests/quantum/examples/WeakOrUnknownHash/WeakHashing.java | Normalize $ inline expectation spacing and trim whitespace. |
| java/ql/test/experimental/query-tests/quantum/examples/WeakOrUnknownBlockMode/Test.java | Normalize $ inline expectation spacing. |
| java/ql/test/experimental/query-tests/quantum/examples/WeakOrUnknownAsymmetricKeySize/InsufficientAsymmetricKeySize.java | Normalize $ inline expectation spacing. |
| java/ql/test/experimental/query-tests/quantum/examples/InsecureOrUnknownNonceSource/InsecureIVorNonceSource.java | Normalize $ inline expectation spacing and whitespace. |
| java/ql/test/experimental/query-tests/quantum/examples/BadMacUse/BadMacUse.java | Normalize $ inline expectation spacing and trim whitespace. |
| java/ql/src/Security/CWE/CWE-295/ImproperWebViewCertificateValidation.java | Normalize $ inline expectation spacing and trim whitespace. |
| go/ql/test/library-tests/semmle/go/frameworks/Macaron/sources.go | Normalize $ inline expectation spacing in Go tests. |
| go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/stdin/test.go | Normalize $ inline expectation spacing in Go tests. |
| go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/file/test.go | Normalize $ inline expectation spacing in Go tests. |
|
|
||
| Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5PADDING"); | ||
| cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $Alert[java/quantum/examples/insecure-iv-or-nonce]] | ||
| cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $ Alert[java/quantum/examples/insecure-iv-or-nonce]] |
There was a problem hiding this comment.
Inline expectation comment has an extra closing bracket (]]) at the end of the query id. This likely prevents the inline expectation from being parsed/matched correctly; please change it to a single ].
Suggested change
| cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $ Alert[java/quantum/examples/insecure-iv-or-nonce]] | |
| cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $ Alert[java/quantum/examples/insecure-iv-or-nonce] |
f1f4065 to
3a089a9
Compare
Contributor
|
Why not do this for all languages? |
Contributor
Author
|
@MathiasVP I started, and then I was confused by the syntax |
3a089a9 to
1a08fcd
Compare
Contributor
|
I'm pretty sure C++ has a few more of those. For example |
$
This was a regex-find-replace from `// \$(?! )` (using a negative lookahead) to `// $ `.
This was a regex-find-replace from `// \$(?! )` (using a negative lookahead) to `// $ `.
This was a regex-find-replace from `// \$(?! )` (using a negative lookahead) to `// $ `.
This was a regex-find-replace from `// \$(?! )` (using a negative lookahead) to `// $ `.
This was a regex-find-replace from `// \$(?! )` (using a negative lookahead) to `// $ `.
This was a regex-find-replace from `# \$(?! )` (using a negative lookahead) to `# $ `.
This was a regex-find-replace from `# \$(?! )` (using a negative lookahead) to `# $ `.
90ddd89 to
7a22923
Compare
7a22923 to
78b86ca
Compare
geoffw0
reviewed
Mar 4, 2026
Contributor
geoffw0
left a comment
There was a problem hiding this comment.
Given this is such a common mistake, is there a good reason we don't want to make this syntax correct - i.e. make the space optional?
swift/ql/test/library-tests/dataflow/taint/libraries/nsstring.swift
Outdated
Show resolved
Hide resolved
We cannot easily require a space before $ because some languages, like C#, strip whitespace from the beginning of the comment text.
78b86ca to
501485b
Compare
Contributor
Author
|
@geoffw0 I think it better to have one consistent style. If you are used to one style and then you try to read another then it is harder. It also makes it slightly easier to do a simple text search for inline expectations if the style is stricter. I think that multiple styles took root initially because the parsing library wasn't strict and there weren't many examples to copy from. Now that it is widespread, if we make it more consistent stylistically then I think people will stick with that. |
jketema
previously approved these changes
Mar 4, 2026
Contributor
jketema
left a comment
There was a problem hiding this comment.
C++ and Swift 👍
Not for this PR: I wonder if there's someway in which we can enforce this?
Contributor
Author
|
@jketema That also occured to me overnight, so today I added a commit updating the shared library to enforce a space after the |
tausbn
approved these changes
Mar 4, 2026
Contributor
Author
|
As far as I can tell the CI failures aren't related to the changes in this PR... |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This was mostly a regex-find-replace from
// \$(?! )(using a negative lookahead) to// $.