[Java]: Add precondition support for testing library asserts #8493
Conversation
| or | ||
| checkTrue = false and m.hasName("assumeFalse") | ||
| ) | ||
| or | ||
| exists(Parameter p, IfStmt ifstmt, Expr cond | |
There was a problem hiding this comment.
Might as well generalise this at the same time
| @@ -11,6 +11,8 @@ import java | |||
| * is equal to `checkTrue` and throws otherwise. | |||
| */ | |||
| predicate conditionCheckMethod(Method m, boolean checkTrue) { | |||
| conditionCheckMethod(m, 0, checkTrue) | |||
There was a problem hiding this comment.
Rather than have two mutually recursive functions, just merge these into one method with an argument parameter and set it to zero where required
There was a problem hiding this comment.
I was thinking about doing that, but because this was a public API I didn't want to beak any downstream consumers. That being said, this is in a package called internal so I'm happy to make that change.
There was a problem hiding this comment.
Good point, keep the two-arg overload to be non-breaking (actually consider giving this a different name; that's usually better than overloading for clarity's sake), but just let that one be a convenience method with a one-line definition rather than a mutually recursive pair.
There was a problem hiding this comment.
actually consider giving this a different name;
Got any suggestions here?
There was a problem hiding this comment.
conditionCheckMethodArgument?
| } | ||
|
|
||
| void test9() { | ||
| r(true); |
There was a problem hiding this comment.
Test the wrapper-function case using a logical-not operator too
There was a problem hiding this comment.
I believe I've done this, but let me know if I misunderstood the request here.
Co-authored-by: intrigus-lgtm <60750685+intrigus-lgtm@users.noreply.github.com>
|
Anything else that this PR is missing? |
|
If you're deprecating |
|
(apologies for the delay, didn't notice you'd done anything here; as a rule I ignore emails telling me about pushed commits and only attend a PR again when I see a comment) |
Last I checked, I had done this, let me know if you spot any cases I missed. 😃 |
| } | ||
|
|
||
| /** | ||
| * Holds if `m` is a non-overridable testing framework methopd that checks that its first argument |
There was a problem hiding this comment.
| * Holds if `m` is a non-overridable testing framework methopd that checks that its first argument | |
| * Holds if `m` is a non-overridable testing framework method that checks that its first argument |
Adds support for
assertTrueandassertFalseas guard preconditions.Not having this was causing false positives in tests where
assertTruewas guarding a bit of logic, but CodeQL didn't know thatassertTruewas a valid guard.