-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Python support ListElement in MaD
#21134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for the ListElement token in Models as Data (MaD) for Python, enabling users to specify list element access in data flow models. The implementation maps ListElement to the existing subscript functionality, reusing infrastructure already in place for list element content.
Key changes:
- Enabled
ListElementtoken in MaD access paths by treating it the same asDictionaryElementAny - Updated test case to verify list element source detection works correctly
- Added
ListElementto the list of valid tokens for identifying access paths
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
python/ql/test/library-tests/frameworks/data/test.ext.yml |
Uncommented test case for ListElement to validate the new functionality |
python/ql/test/library-tests/frameworks/data/test.expected |
Added expected test output showing that list element sources are now detected |
python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModelsSpecific.qll |
Added ListElement support to token handling predicates, mapping it to subscript access |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TODO comment should be updated to reflect that list support has now been added. Consider changing it to "TODO: Add support for tuples" since only tuple support remains unimplemented.
@tausbn
tausbn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks very reasonable, modulo a small issue of naming that I just want to make sure we've thought about. 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like we may have had this discussion before, but should this instead be ListElementAny? Or is it safe to overload ListElement if we want to have more fine-grained control over the index later on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did consider this and initially paused. I then noticed that no other language is using ListElement, so there is no conflicting semantics, at least.
We have several choices for how to take this, including adding ListElementAny or supporting ListElement[any] to be the same as just ListElement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, let's just go with the current implementation, then. No need to complicate things.
It should be easy enough to ad tuple element (although these should take an index) and set element. List element was added in response to a user question. The change is trivial as it simply maps to the already supported subscript.