-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Python: Add API graph support for parameter annotations #18112
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
Merged
yoff
merged 1 commit into
main
from
tausbn/add-api-graph-support-for-parameter-annotations
Dec 5, 2024
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
5 changes: 5 additions & 0 deletions
5
python/ql/lib/change-notes/2024-11-26-parameter-annotation-api-graph-support.md
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
category: feature | ||
--- | ||
|
||
- Added support for parameter annotations in API graphs. This means that in a function definition such as `def foo(x: Bar): ...`, you can now use the `getInstanceFromAnnotation()` method to step from `Bar` to `x`. In addition to this, the `getAnInstance` method now also includes instances arising from parameter annotations. |
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
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
25 changes: 25 additions & 0 deletions
25
python/ql/test/library-tests/ApiGraphs/py3/test_annotations.py
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
from types import AssignmentAnnotation, ParameterAnnotation | ||
|
||
def test_annotated_assignment(): | ||
local_x : AssignmentAnnotation = create_x() #$ MISSING: use=moduleImport("types").getMember("AssignmentAnnotation") | ||
local_x #$ MISSING: use=moduleImport("types").getMember("AssignmentAnnotation").getAnnotatedInstance() | ||
|
||
global_x : AssignmentAnnotation #$ use=moduleImport("types").getMember("AssignmentAnnotation") | ||
global_x #$ MISSING: use=moduleImport("types").getMember("AssignmentAnnotation").getAnnotatedInstance() | ||
|
||
def test_parameter_annotation(parameter_y: ParameterAnnotation): #$ use=moduleImport("types").getMember("ParameterAnnotation") | ||
parameter_y #$ use=moduleImport("types").getMember("ParameterAnnotation").getAnnotatedInstance() | ||
|
||
type Alias = AssignmentAnnotation | ||
|
||
global_z : Alias #$ MISSING: use=moduleImport("types").getMember("AssignmentAnnotation") | ||
global_z #$ MISSING: use=moduleImport("types").getMember("AssignmentAnnotation").getAnnotatedInstance() | ||
|
||
def test_parameter_alias(parameter_z: Alias): #$ MISSING: use=moduleImport("types").getMember("AssignmentAnnotation") | ||
parameter_z #$ MISSING: use=moduleImport("types").getMember("AssignmentAnnotation").getAnnotatedInstance() | ||
|
||
# local type aliases | ||
def test_local_type_alias(): | ||
type LocalAlias = AssignmentAnnotation | ||
local_alias : LocalAlias = create_value() #$ MISSING: use=moduleImport("types").getMember("AssignmentAnnotation") | ||
local_alias #$ MISSING: use=moduleImport("types").getMember("AssignmentAnnotation").getAnnotatedInstance() |
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.
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.
Why is this missing? Is it because there is no assignment on the line above, so that
global_x
is not ingetTarget
(which is presumably empty)?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.
Hmm... This is a very salient question. If I quick-eval the
annotatedInstance
predicate, I get four results:ControlFlowNode for ImportMember, ControlFlowNode for global_x
ControlFlowNode for ImportMember, ControlFlowNode for parameter_y
ControlFlowNode for Alias, ControlFlowNode for parameter_z
ControlFlowNode for Alias, ControlFlowNode for global_z
So, we are picking up the instancing from
from ... import AssignmentAnnotation
toglobal_x : AssignmentAnnotation
, but we're not picking up that the annotation is a use of that same identifier as in theimport
statement. What's curious, then, is thatglobal_x
isn't seen as an instance ofAssignmentAnnotation
. Forglobal_z
it makes sense, since we don't understand the simple type aliasing that's taking place on line 13.Looking at
getTarget
it does exist for the type ascription ofglobal_x
, but it seems that we do not track the flow between the two occurrences ofglobal_x
. I'm trying to figure out why now.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.
A thought occurred to me after writing that message. Could it be that we're observing that
global_x
gets overwritten here (because it's the target of an assignment), but then when we go to see what value was assigned we don't find it (because it's just a type ascription)? That would explain the weird behaviour.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.
Yes that could be it. I wonder if
global_x
inglobal_x : AssignmentAnnotation
should actually be considered a use rather than a def..