diff --git a/python/ql/src/Imports/UnusedImport.ql b/python/ql/src/Imports/UnusedImport.ql index 6d927550c0c7..3ac04e2e4d2f 100644 --- a/python/ql/src/Imports/UnusedImport.ql +++ b/python/ql/src/Imports/UnusedImport.ql @@ -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 @@ -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 @@ -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." - diff --git a/python/ql/test/query-tests/Imports/unused/imports_test.py b/python/ql/test/query-tests/Imports/unused/imports_test.py index a1b457f04228..cf2024ac1833 100644 --- a/python/ql/test/query-tests/Imports/unused/imports_test.py +++ b/python/ql/test/query-tests/Imports/unused/imports_test.py @@ -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