Skip to content

Python: Fix false positives in py/unused-import. #2042

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.

Sign up for GitHub

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
RasmusWL merged 3 commits into from
Nov 4, 2019
Merged

Python: Fix false positives in py/unused-import. #2042

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
25985e9
Python: Remove a few false positives from `py/unused-import`.
taus-semmle Sep 26, 2019
4341e88
Python: Clean up comments in preparation for autoformat.
taus-semmle Sep 27, 2019
9878e4f
Python: Apply four-space autoformat.
taus-semmle Sep 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 45 additions & 48 deletions python/ql/src/Imports/UnusedImport.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ import python
import Variables.Definition

predicate global_name_used(Module m, Variable name) {
exists (Name u, GlobalVariable v |
exists(Name u, GlobalVariable v |
u.uses(v) and
v.getId() = name.getId() and
u.getEnclosingModule() = m
)
or
/* A use of an undefined class local variable, will use the global variable */
// A use of an undefined class local variable, will use the global variable
exists(Name u, LocalVariable v |
u.uses(v) and
v.getId() = name.getId() and
Expand All @@ -31,24 +31,21 @@ predicate global_name_used(Module m, Variable name) {

/** Holds if a module has `__all__` but we don't understand it */
predicate all_not_understood(Module m) {
exists(GlobalVariable a |
a.getId() = "__all__" and a.getScope() = m |
/* __all__ is not defined as a simple list */
exists(GlobalVariable a | a.getId() = "__all__" and a.getScope() = m |
// `__all__` is not defined as a simple list
not m.declaredInAll(_)
or
/* __all__ is modified */
// `__all__` is modified
exists(Call c | c.getFunc().(Attribute).getObject() = a.getALoad())
)
}

predicate imported_module_used_in_doctest(Import imp) {
exists(string modname |
imp.getAName().getAsname().(Name).getId() = modname
and
/* Look for doctests containing the patterns:
* >>> …name…
* ... …name…
*/
imp.getAName().getAsname().(Name).getId() = modname and
// Look for doctests containing the patterns:
// >>> …name…
// ... …name…
exists(StrConst doc |
doc.getEnclosingModule() = imp.getScope() and
doc.isDocString() and
Expand All @@ -58,52 +55,52 @@ predicate imported_module_used_in_doctest(Import imp) {
}

predicate imported_module_used_in_typehint(Import imp) {
exists(string modname |
imp.getAName().getAsname().(Name).getId() = modname
and
/* Look for typehints containing the patterns:
* # type: …name…
*/
exists(string modname, Location loc |
imp.getAName().getAsname().(Name).getId() = modname and
loc.getFile() = imp.getScope().(Module).getFile()
|
// Look for type hints containing the patterns:
// # type: …name…
exists(Comment typehint |
typehint.getLocation().getFile() = imp.getScope().(Module).getFile() and
loc = typehint.getLocation() and
typehint.getText().regexpMatch("# type:.*" + modname + ".*")
)
or
// Type hint is inside a string annotation, as needed for forward references
exists(string typehint, Expr annotation |
annotation = any(Arguments a).getAnAnnotation()
or
annotation = any(AnnAssign a).getAnnotation()
|
annotation.pointsTo(Value::forString(typehint)) and
loc = annotation.getLocation() and
typehint.regexpMatch(".*\\b" + modname + "\\b.*")
)
)
}


predicate unused_import(Import imp, Variable name) {
((Name)imp.getAName().getAsname()).getVariable() = name
and
not imp.getAnImportedModuleName() = "__future__"
and
not imp.getEnclosingModule().declaredInAll(name.getId())
and
imp.getScope() = imp.getEnclosingModule()
and
not global_name_used(imp.getScope(), name)
and
/* Imports in __init__.py are used to force module loading */
not imp.getEnclosingModule().isPackageInit()
and
/* Name may be imported for use in epytext documentation */
not exists(Comment cmt |
cmt.getText().matches("%L{" + name.getId() + "}%") |
imp.getAName().getAsname().(Name).getVariable() = name and
not imp.getAnImportedModuleName() = "__future__" and
not imp.getEnclosingModule().declaredInAll(name.getId()) and
imp.getScope() = imp.getEnclosingModule() and
not global_name_used(imp.getScope(), name) and
// Imports in `__init__.py` are used to force module loading
not imp.getEnclosingModule().isPackageInit() and
// Name may be imported for use in epytext documentation
not exists(Comment cmt | cmt.getText().matches("%L{" + name.getId() + "}%") |
cmt.getLocation().getFile() = imp.getLocation().getFile()
)
and
not name_acceptable_for_unused_variable(name)
and
/* Assume that opaque `__all__` includes imported module */
not all_not_understood(imp.getEnclosingModule())
and
not imported_module_used_in_doctest(imp)
and
not imported_module_used_in_typehint(imp)
) and
not name_acceptable_for_unused_variable(name) and
// Assume that opaque `__all__` includes imported module
not all_not_understood(imp.getEnclosingModule()) and
not imported_module_used_in_doctest(imp) and
not imported_module_used_in_typehint(imp) and
// Only consider import statements that actually point-to something (possibly an unknown module).
// If this is not the case, it's likely that the import statement never gets executed.
imp.getAName().getValue().pointsTo(_)
}


from Stmt s, Variable name
where unused_import(s, name)
select s, "Import of '" + name.getId() + "' is not used."

14 changes: 14 additions & 0 deletions python/ql/test/query-tests/Imports/unused/imports_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,17 @@ def f():

foo = None # type: typing.Optional[int]


# OK since the import statement is never executed
if False:
import never_imported

# OK since the imported module is used in a forward-referenced type annotation.
import only_used_in_parameter_annotation

def func(x: 'Optional[only_used_in_parameter_annotation]'):
pass

import only_used_in_annotated_assignment

var : 'Optional[only_used_in_annotated_assignment]' = 5