Java: Consolidate Assertions.qll and Preconditions.qll.#20377
Java: Consolidate Assertions.qll and Preconditions.qll.#20377aschackmull merged 3 commits intogithub:mainfrom
Conversation
aschackmull
force-pushed
the
java/preconditions
branch
3 times, most recently
from
63fae98 to
39578c6
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR consolidates two similar libraries (Assertions.qll and Preconditions.qll) that provide overlapping functionality for modeling assertion and precondition checks. The consolidation centralizes this functionality into the enhanced Preconditions.qll library and updates all references accordingly.
- Deprecates the existing
Assertions.qlllibrary - Expands
Preconditions.qllwith comprehensive assertion and precondition modeling - Updates all import statements and method calls to use the consolidated library
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| DeadLocals.qll | Updates import from Assertions to Preconditions and inlines assertion logic |
| Assertions.qll | Marks library as deprecated |
| Nullness.qll | Switches import from Assertions to Preconditions and simplifies null checking logic |
| Preconditions.qll | Consolidates functionality from both libraries with new predicates and deprecation notices |
| Guards.qll | Updates method calls to use new consolidated API |
| ControlFlowGraph.qll | Updates method calls to use new consolidated API |
aschackmull
force-pushed
the
java/preconditions
branch
3 times, most recently
from
7b62eb8 to
2145a88
Compare
aschackmull
force-pushed
the
java/preconditions
branch
from
2145a88 to
b5c7bc1
Compare
|
Finally got a dca run that I'm happy with. There are a number of small result changes, so I've added a change note to cover that. |
| | | ||
| ma.getAnArgument().(MethodCall).getMethod().getName() = "notNullValue" | ||
| ) | ||
| private ControlFlowNode ensureNotNull(SsaVariable v) { result = varDereference(v, _) } |
There was a problem hiding this comment.
Does the QlDoc needs to be changed as the assertion related logic has been removed?
There was a problem hiding this comment.
Right, that needs a slight cleanup. Do you mind if I postpone that to #20367? (Which I'm going to rebase once this is merged)
| */ | ||
| overlay[local?] | ||
| module; | ||
| deprecated module; |
There was a problem hiding this comment.
Very nice that it is possible to deprecate the entire module!
2 participants
The two libraries do very similar things and overlap somewhat with neither being a subset of the other. This PR consolidates the models of both libraries into a single library and ensures that it is properly recognized by the Guards library.