Skip to content

Conversation

RasmusWL
Copy link
Member

@RasmusWL RasmusWL commented Feb 9, 2021

In general, if there is some decorator on a function, it might not be safe to track content out of it (since the decorator could do anything), but in this case, we can see what the decorator does, so we should be able to handle it (but we don't right now).

By my understanding of how type-tracking works, if we track content through my_decorator, then we would also track content to the result of unrelated_func(), which I wanted to make sure our tests would catch.

I found out the core of the problem seems to come from our lack of being able to track to the inner scope, and added an explicit test for that.

In general, if there is _some_ decorator on a function, it might not be safe to
track content out of it (since the decorator could do anything), but in this
case, we can see what the decorator does, so we should be able to handle it (but
we don't right now).

By my understanding of how type-tracking works, if we track content through
`my_decorator`, then we would also track content to the result of
`unrelated_func()`, which I wanted to make sure our tests would catch.

I found out the core of the problem seems to come from our lack of being able to
track to the inner scope, and added an explicit test for that.
@RasmusWL RasmusWL requested a review from a team as a code owner February 9, 2021 12:44
@github-actions github-actions bot added the Python label Feb 9, 2021
@RasmusWL
Copy link
Member Author

RasmusWL commented Feb 9, 2021

After looking through some more tests, I see we have tests on the dataflow level for this in https://github.com/github/codeql/blob/main/python/ql/test/experimental/dataflow/variable-capture/in.py

Copy link
Contributor

@yoff yoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice clear example :-)

@yoff yoff merged commit 9930d59 into github:main Feb 10, 2021
@RasmusWL RasmusWL deleted the typetracking-with-decorator branch February 10, 2021 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants