Title: Python: simplify decorator-detection predicates to pure AST match by yoff · Pull Request #21926 · github/codeql · GitHub
Open Graph Title: Python: simplify decorator-detection predicates to pure AST match by yoff · Pull Request #21926 · github/codeql
X Title: Python: simplify decorator-detection predicates to pure AST match by yoff · Pull Request #21926 · github/codeql
Description: Factor-out from the Python shared-CFG dataflow migration stack (#21919/#21920/#21921/#21923/#21925). The internal predicates isStaticmethod, isClassmethod, and hasPropertyDecorator (in DataFlowDispatch.qll) previously required the decorator's NameNode to satisfy isGlobal() — i.e. no SSA definition reaches the name use. That filter was correct but unnecessarily indirect: staticmethod, classmethod, and property are builtins. Even when a class body redefines one (staticmethod = ... inside the class), the class body has not started executing yet at the decorator position, so Python uses the builtin at that point. This PR matches the decorator's AST Name directly instead, dropping the CFG/SSA detour. Semantic change There is a small behavioural difference. The previous isGlobal() check rejected module-level shadowing of these builtins, e.g.: def staticmethod(x): ... # shadows the builtin in this module class C: @staticmethod def foo(): ... # at the decorator position, `staticmethod` is the local one With the previous code, foo was not classified as a staticmethod. With the new code, it is. Shadowing these three names at module scope is essentially never seen in practice, so this is a negligible false-positive risk in exchange for much cleaner code. Documented in a change note. Why not hasContextmanagerDecorator / hasOverloadDecorator too? Those predicates also use the NameNode.isGlobal() pattern, but their target names (contextmanager from contextlib, overload from typing) are imported, not builtin. The isGlobal() check legitimately filters out cases where these names refer to a local variable rather than the imported one, which is a more realistic shadowing scenario. So they stay as-is. Verification All 361 python/ql/{src,lib} queries compile. All 88 tests in library-tests/dataflow, library-tests/PointsTo/general, and query-tests/Functions pass. This PR has no dependencies on the rest of the migration stack and can land independently. After it merges, the corresponding predicate changes in yoff/python-shared-cfg-dataflow-flip (#21925) collapse to a noop (just the Cfg:: qualification on the two remaining contextmanager/overload predicates).
Open Graph Description: Factor-out from the Python shared-CFG dataflow migration stack (#21919/#21920/#21921/#21923/#21925). The internal predicates isStaticmethod, isClassmethod, and hasPropertyDecorator (in DataFlowDisp...
X Description: Factor-out from the Python shared-CFG dataflow migration stack (#21919/#21920/#21921/#21923/#21925). The internal predicates isStaticmethod, isClassmethod, and hasPropertyDecorator (in DataFlowDisp...
Opengraph URL: https://github.com/github/codeql/pull/21926
X: @github
Domain: github.com
| route-pattern | /:user_id/:repository/pull/:id/files(.:format) |
| route-controller | pull_requests |
| route-action | files |
| fetch-nonce | v2:3f347204-8a94-b74e-074b-9c01dac91c78 |
| current-catalog-service-hash | ae870bc5e265a340912cde392f23dad3671a0a881730ffdadd82f2f57d81641b |
| request-id | 8D32:34DA5A:5CE0DDB:831CB48:6A5024E9 |
| html-safe-nonce | 1c68e2b0427343c2b0f9261fd07b8dcde02ff053b860d1af45ddf12d88bade5b |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiI4RDMyOjM0REE1QTo1Q0UwRERCOjgzMUNCNDg6NkE1MDI0RTkiLCJ2aXNpdG9yX2lkIjoiNjkxNDU2NDAzNzQwNjYzMTE0NSIsInJlZ2lvbl9lZGdlIjoiaWFkIiwicmVnaW9uX3JlbmRlciI6ImlhZCJ9 |
| visitor-hmac | d36ab6c01c84f37eccfaecf92d162fec58392b9a5f3dd442a626f7a882e3487c |
| hovercard-subject-tag | pull_request:3781469027 |
| github-keyboard-shortcuts | repository,pull-request-list,pull-request-conversation,pull-request-files-changed,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/github/codeql/pull/21926/files |
| twitter:image | https://avatars.githubusercontent.com/u/647347?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/647347?s=400&v=4 |
| og:image:alt | Factor-out from the Python shared-CFG dataflow migration stack (#21919/#21920/#21921/#21923/#21925). The internal predicates isStaticmethod, isClassmethod, and hasPropertyDecorator (in DataFlowDisp... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | a08040246389072795e3b4e7b33c6d9a0c564eee02e829a809326d67c418b069 |
| turbo-cache-control | no-preview |
| diff-view | unified |
| go-import | github.com/github/codeql git https://github.com/github/codeql.git |
| octolytics-dimension-user_id | 9919 |
| octolytics-dimension-user_login | github |
| octolytics-dimension-repository_id | 143040428 |
| octolytics-dimension-repository_nwo | github/codeql |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 143040428 |
| octolytics-dimension-repository_network_root_nwo | github/codeql |
| turbo-body-classes | logged-out env-production page-responsive full-width |
| disable-turbo | true |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | d1b91b18e7330fc458dfd824c61eaed7daa830d6 |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width