Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

java inline expectations proof-of-concept with tests #16911

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ginsbach
Copy link
Contributor

@ginsbach ginsbach commented Jul 5, 2024

This is a proof-of-concept inline expectations query that I developed alongside the CLI implementation of test postprocessing to verify that everything works and the interface makes sense. It is not meant to be merged, but should serve as inspiration for a proper version.

Note that java/ql/test/query-tests/Postprocessing/passing/SuspiciousRegexpRange.java is an identical copy of java/ql/test/query-tests/security/CWE-020/SuspiciousRegexpRange.java, with the other added (non-empty) .java files being slight variations.

@ginsbach ginsbach added the WIP This is a work-in-progress, do not merge yet! label Jul 5, 2024
@github-actions github-actions bot added the Java label Jul 5, 2024
@ginsbach ginsbach force-pushed the ginsbach/InlineExpectationsPrototype branch 3 times, most recently from dc5f876 to 6c957f9 Compare July 5, 2024 09:55
@ginsbach ginsbach force-pushed the ginsbach/InlineExpectationsPrototype branch from 6c957f9 to 65655db Compare July 5, 2024 10:21
@ginsbach ginsbach changed the title java inline expectations prototype with tests java inline expectations proof-of-concept with tests Jul 5, 2024
Comment on lines 39 to 48
s = actualLines() and
posString = s.substring(s.indexOf("|", 0, 0) + 1, s.indexOf("|", 1, 0)).trim() and
filename = posString.substring(0, posString.indexOf(":", 0, 0)) and
lineString = posString.substring(posString.indexOf(":", 0, 0) + 1, posString.indexOf(":", 1, 0)) and
lineString = posString.substring(posString.indexOf(":", 2, 0) + 1, posString.indexOf(":", 3, 0)) and
colStart =
posString.substring(posString.indexOf(":", 1, 0) + 1, posString.indexOf(":", 2, 0)).toInt() and
colEnd = posString.substring(posString.indexOf(":", 3, 0) + 1, posString.length()).toInt() and
line = lineString.toInt() and
content = s.substring(s.indexOf("|", 2, 0) + 1, s.indexOf("|", 3, 0)).trim()
Copy link
Contributor

@jbj jbj Jul 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this meant as an example of best practice? If not, I'd like to see such an example. That will help us confirm we have the right interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean whether my proof-of-concept inline expectations query is meant as best practice? I certainly wouldn't claim so, I just tried to have something that works well enough to verify the CLI part.

There is a test case below that gives an example of this code working in practice:

| SuspiciousRegexpRange.java:7:49:7:51 | unexpected alert | Suspicious character range that overlaps with A-Z in the same character class, and is equivalent to [A-Z\\[\\\\\\]^_`a-z]. |

Perhaps I misunderstand what you mean, though?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is that the QL code works by concatenating all the columns and then splitting by | to take them apart again. I hope that's unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies, I see what you mean now.

This concatenation is completely unnecessary and merely an artifact of me having gone through multiple different interfaces while working on this issue and not properly cleaning up the QL code after the most recent change.

I have pushed a third commit to remove actualLines and with it the concatenation.

@ginsbach ginsbach force-pushed the ginsbach/InlineExpectationsPrototype branch from abf9a0a to 402f835 Compare July 5, 2024 16:19
@ginsbach ginsbach force-pushed the ginsbach/InlineExpectationsPrototype branch from 402f835 to 7370a2e Compare July 8, 2024 12:48
@ginsbach ginsbach force-pushed the ginsbach/InlineExpectationsPrototype branch from 7078c40 to c86e008 Compare July 8, 2024 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java WIP This is a work-in-progress, do not merge yet!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants