Skip to content

Conversation

@taus-semmle
Copy link
Contributor

Fixes one of the false positives reported in #2014 (in two different ways).

Firstly, we now consider occurrences of a module name inside a string annotation of a parameter to be a "use" of that module.

Secondly, we only report unused modules arising from import statements that are likely to be executed, using the existence of points-to information for the module name as a proxy for reachability. This should be fairly safe -- even if we don't correctly identify what the module actually points to, we should infer that it points to some unknown module. On its own, this would also prevent the reported false positive from occurring, since the import in question is guarded by a check of typing.TYPE_CHECKING, which is False except during type checking.

This still needs careful testing to see how it affects the already existing alerts on LGTM.com.

@tausbn tausbn requested a review from RasmusWL November 4, 2019 10:07
@tausbn tausbn marked this pull request as ready for review November 4, 2019 10:07
Copy link
Member

@RasmusWL RasmusWL left a comment

Choose a reason for hiding this comment

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

LGTM.

Caught me a bit off guard that only_used_in_parameter_annotation.py did not exist, but I see that for the purposes of this test, it's not a problem.

@RasmusWL RasmusWL merged commit ca22ec6 into github:master Nov 4, 2019
@tausbn tausbn deleted the python-fix-unused-import-fps branch February 12, 2021 18:05
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