Merge pull request #3211 from RasmusWL/python-unused-import-small-fix

Python: Fix FN in unused import

Author: tausbn

python/ql/src/Imports/UnusedImport.ql

@@ -62,7 +62,7 @@ private string doctest_in_scope(Scope scope) {
 }
 
 pragma[noinline]
-private string typehint_annotation_in_file(File file) {
+private string typehint_annotation_in_module(Module module_scope) {
     exists(StrConst annotation |
         annotation = any(Arguments a).getAnAnnotation().getASubExpression*()
         or
@@ -71,7 +71,7 @@ private string typehint_annotation_in_file(File file) {
         annotation = any(FunctionExpr f).getReturns().getASubExpression*()
     |
         annotation.pointsTo(Value::forString(result)) and
-        file = annotation.getLocation().getFile()
+        annotation.getEnclosingModule() = module_scope
     )
 }
 
@@ -84,17 +84,19 @@ private string typehint_comment_in_file(File file) {
     )
 }
 
-predicate imported_module_used_in_typehint(Import imp) {
-    exists(string modname, File file |
-        imp.getAName().getAsname().(Name).getId() = modname and
-        file = imp.getScope().(Module).getFile()
+/** Holds if the imported alias `name` from `imp` is used in a typehint (in the same file as `imp`) */
+predicate imported_alias_used_in_typehint(Import imp, Variable name) {
+    imp.getAName().getAsname().(Name).getVariable() = name and
+    exists(File file, Module module_scope |
+        module_scope = imp.getEnclosingModule() and
+        file = module_scope.getFile()
     |
         // Look for type hints containing the patterns:
         // # type: …name…
-        typehint_comment_in_file(file).regexpMatch("# type:.*" + modname + ".*")
+        typehint_comment_in_file(file).regexpMatch("# type:.*" + name.getId() + ".*")
         or
         // Type hint is inside a string annotation, as needed for forward references
-        typehint_annotation_in_file(file).regexpMatch(".*\\b" + modname + "\\b.*")
+        typehint_annotation_in_module(module_scope).regexpMatch(".*\\b" + name.getId() + "\\b.*")
     )
 }
 
@@ -114,7 +116,7 @@ predicate unused_import(Import imp, Variable name) {
     // 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 imported_alias_used_in_typehint(imp, name) 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(_)

python/ql/test/query-tests/Imports/unused/UnusedImport.expected

@@ -1,3 +1,5 @@
+| import_structure_1.py:5:1:5:28 | Import | Import of 'bar' is not used. |
+| import_structure_2.py:6:1:6:23 | Import | Import of 'bar' is not used. |
 | imports_test.py:2:1:2:23 | Import | Import of 'module2' is not used. |
 | imports_test.py:6:1:6:12 | Import | Import of 'cycle' is not used. |
 | imports_test.py:10:1:10:22 | Import | Import of 'top_level_cycle' is not used. |

python/ql/test/query-tests/Imports/unused/import_structure_1.py

@@ -0,0 +1,8 @@
+# there should be no difference whether you import 2 things on 1 line, or use 2
+# lines
+from typing import Optional
+
+from unknown import foo, bar
+
+
+var: Optional['foo'] = None

python/ql/test/query-tests/Imports/unused/import_structure_2.py

@@ -0,0 +1,8 @@
+# there should be no difference whether you import 2 things on 1 line, or use 2
+# lines
+from typing import Optional
+
+from unknown import foo
+from unknown import bar
+
+var: Optional['foo'] = None