Uh oh!
There was an error while loading. Please reload this page.
Python: Port simple points-to queries without DCA alert changes - #21519
Merged
Conversation
Also extends the list of known built-ins slightly, to add some that were missing.
Uses a (perhaps) slightly coarser approximation of what modules are imported, but it's probably fine.
This module (which for convenience currently resides inside `DataFlowDispatch`, but this may change later) contains convenience predicates for bridging the gap between the data-flow layer and the old points-to analysis.
Approximates the behaviour of `Types::isNewStyle` but without depending on points-to
These could arguably be moved to `Class` itself, but for now I'm choosing to limit the changes to the `DuckTyping` module (until we decide on a proper API).
Only trivial test changes.
Only trivial test changes.
No test changes.
Only trivial test changes.
Adds `overridesMethod` and `isPropertyAccessor`.
Primarily used to filter out false positives in cases where our MRO approximation may be wrong.
Only trivial test changes.
tausbnforce-pushed
the
tausbn/python-port-no-alert-change
branch
from
March 20, 2026 13:30
7a255dc to
434b397Comparetausbn
marked this pull request as ready for review
March 23, 2026 13:28
Contributor
There was a problem hiding this comment.
Pull request overview
Ports a set of Python QL queries away from the legacy points-to library (while aiming to avoid DCA-observed alert changes), updating shared libraries and test expectations to match the new resolution approach.
Changes:
- Replace
LegacyPointsTousage in several Python queries withsemmle.python.ApiGraphsand/orDataFlowDispatch.DuckTyping. - Introduce a
DuckTypinghelper module inDataFlowDispatch.qllfor structural class/property checks. - Update query test
.expectedfiles and add a change note for the analysis change.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| python/ql/src/Variables/SuspiciousUnusedLoopIterationVariable.ql | Switches range/list(range) detection from points-to to API graph call matching. |
| python/ql/src/Statements/UnnecessaryDelete.ql | Replaces points-to detection of sys.exc_info calls with API graph matching. |
| python/ql/src/Statements/TopLevelPrint.ql | Removes points-to dependency and adds a local predicate to detect “used as module”. |
| python/ql/src/Statements/SideEffectInAssert.ql | Replaces points-to matching of subprocess.* calls with API graph matching. |
| python/ql/src/Statements/DocStrings.ql | Replaces points-to override/property checks with DuckTyping-based checks. |
| python/ql/src/Imports/DeprecatedModule.ql | Replaces points-to check for ImportError with API graph value reachability. |
| python/ql/src/Functions/DeprecatedSliceMethod.ql | Uses DuckTyping to exclude overrides/unresolved bases instead of points-to method modeling. |
| python/ql/src/Expressions/UseofApply.ql | Matches apply(...) via API graphs for Python 2 instead of points-to. |
| python/ql/src/Classes/UselessClass.ql | Replaces inheritance checks based on points-to class values with class hierarchy predicates. |
| python/ql/src/Classes/SuperInOldStyleClass.ql | Uses DuckTyping::isNewStyle instead of points-to-derived class object inference. |
| python/ql/src/Classes/SlotsInOldStyleClass.ql | Uses DuckTyping attribute declaration checks instead of points-to class object inference. |
| python/ql/src/Classes/ShouldBeContextManager.ql | Uses DuckTyping context manager/method checks instead of points-to class values. |
| python/ql/src/Classes/PropertyInOldStyleClass.ql | Detects @property functions in old-style classes without points-to. |
| python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatch.qll | Adds DuckTyping module with structural predicates and MRO-related helpers. |
| python/ql/lib/semmle/python/dataflow/new/internal/Builtins.qll | Extends builtins list (notably adds exit/quit etc.) and includes apply for Py2. |
| python/ql/test/query-tests/Functions/general/DeprecatedSliceMethod.expected | Updates expected result rendering for the updated query entity selection. |
| python/ql/test/query-tests/Classes/should-be-context-manager/ShouldBeContextManager.expected | Updates expected result rendering for the updated query entity selection. |
| python/ql/test/2/query-tests/Classes/new-style/SlotsInOldStyleClass.expected | Updates expected result rendering for the updated query entity selection. |
| python/ql/test/2/query-tests/Classes/new-style/PropertyInOldStyleClass.expected | Updates expected result location/entity rendering for the updated query. |
| python/ql/src/change-notes/2026-03-13-port-simple-points-to-queries.md | Adds a change note describing the points-to removal work. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
yoff
previously approved these changes
Mar 23, 2026
yoff
left a comment
Contributor
There was a problem hiding this comment.
Looks good. It is great to get started on this!
Very nice presentation, this was super easy to review :-)
in PropertyInOldStyleClass. This matches the previous behaviour more closely.
This one also allows easy access to the method being overridden and the class on which it resides. This let's us simplify DocStrings.ql accordingly.
For module-level metaclass declarations, we now also check that the right hand side in a `__metaclass__ = type` assignment is in fact the built-in `type`.
By limiting the results to the class that actually defines the `__del__` method, we eliminate a bunch of FPs where a _subclass_ of such a class would also get flagged.
tausbn
commented
Mar 26, 2026
ContributorAuthor
DCA looks nice. Main alert changes are in queries that are not part of any standard suite. Also, overall analysis time is roughly cut in half! |
owen-mc
approved these changes
Mar 27, 2026
owen-mc
left a comment
Contributor
There was a problem hiding this comment.
Proxying previous approval.
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
A spin-off of #21350, containing only those changes that did not result in alert changes according to DCA. This should hopefully make it easier to review.