hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-06-02 20:30:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

Python: simplify decorator-detection predicates to pure AST match

Browse Source 
The internal predicates that identify `@staticmethod`, `@classmethod` and
`@property` decorators previously required the decorator's `NameNode` to
satisfy `isGlobal()` (i.e. no SSA def reaches the decorator's name use).
That filter was correct but unnecessarily indirect: these three names
are builtins, and even when a class body redefines one, the class body
has not started executing at the decorator position, so Python uses the
builtin.

Match the decorator's AST `Name` directly instead, dropping the CFG/SSA
detour. The slight semantic change — `isGlobal()` would have rejected
module-level shadowing of these builtins — is negligible in practice
and explicitly documented in the change note.

`hasContextmanagerDecorator` and `hasOverloadDecorator` keep the
`NameNode.isGlobal()` check because their target names (`contextmanager`,
`overload`) are imported, not builtin, and local shadowing is a real
concern.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
yoff
2026-06-01 14:04:43 +00:00
parent ab4a575243
commit 5fb75ac987
2 changed files with 15 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
python/ql/lib/change-notes/2026-06-01-decorator-predicate-simplification.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Simplified the internal predicates that detect `@staticmethod`, `@classmethod` and `@property` decorators to match the decorator's AST `Name` directly, rather than going through the CFG and requiring the name to resolve globally. Code that shadows these three builtin decorators at the module-scope will now be classified by the decorator name alone; in practice, shadowing these names is extremely rare and the call-graph results are unchanged.

20
python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatch.qll
View File

@@ -256,9 +256,12 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
*/
overlay[local]
predicate isStaticmethod(Function func) {
exists(NameNode id | id.getId() = "staticmethod" and id.isGlobal() |
func.getADecorator() = id.getNode()
)
// The decorator is *syntactically* a `Name` "staticmethod" — we don't
// care which variable it resolves to. `staticmethod` is a builtin and
// is almost never shadowed in a module-level scope; even if a class
// redefines `staticmethod` in its body, the class body has not started
// executing yet at the decorator position, so Python uses the builtin.
func.getADecorator().(Name).getId() = "staticmethod"
}
/**
@@ -268,9 +271,9 @@ predicate isStaticmethod(Function func) {
*/
overlay[local]
predicate isClassmethod(Function func) {
exists(NameNode id | id.getId() = "classmethod" and id.isGlobal() |
func.getADecorator() = id.getNode()
)
// See `isStaticmethod` for the rationale for matching on the AST `Name`
// rather than going via the CFG and `isGlobal()`.
func.getADecorator().(Name).getId() = "classmethod"
or
exists(Class cls |
cls.getAMethod() = func and
@@ -285,9 +288,8 @@ predicate isClassmethod(Function func) {
/** Holds if the function `func` has a `property` decorator. */
overlay[local]
predicate hasPropertyDecorator(Function func) {
exists(NameNode id | id.getId() = "property" and id.isGlobal() |
func.getADecorator() = id.getNode()
)
// See `isStaticmethod` for the rationale for matching on the AST `Name`.
func.getADecorator().(Name).getId() = "property"
}
/**