Merge pull request #16754 from github/tausbn/python-disregard-unused-imports-in-pytest-tests

Python: Disregard unused imports in `pytest` tests

Author: tausbn

python/ql/src/Imports/UnusedImport.ql

@@ -12,6 +12,29 @@
 
 import python
 import Variables.Definition
+import semmle.python.ApiGraphs
+
+private predicate is_pytest_fixture(Import imp, Variable name) {
+  exists(Alias a, API::Node pytest_fixture, API::Node decorator |
+    pytest_fixture = API::moduleImport("pytest").getMember("fixture") and
+    // The additional `.getReturn()` is to account for the difference between
+    // ```
+    // @pytest.fixture
+    // def foo():
+    //    ...
+    // ```
+    // and
+    // ```
+    // @pytest.fixture(some, args, here)
+    // def foo():
+    //    ...
+    // ```
+    decorator in [pytest_fixture, pytest_fixture.getReturn()] and
+    a = imp.getAName() and
+    a.getAsname().(Name).getVariable() = name and
+    a.getValue() = decorator.getReturn().getAValueReachableFromSource().asExpr()
+  )
+}
 
 predicate global_name_used(Module m, string name) {
   exists(Name u, GlobalVariable v |
@@ -117,6 +140,7 @@ predicate unused_import(Import imp, Variable name) {
   not all_not_understood(imp.getEnclosingModule()) and
   not imported_module_used_in_doctest(imp) and
   not imported_alias_used_in_typehint(imp, name) and
+  not is_pytest_fixture(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

@@ -5,3 +5,4 @@
 | imports_test.py:10:1:10:22 | Import | Import of 'top_level_cycle' is not used. |
 | imports_test.py:27:1:27:25 | Import | Import of 'func2' is not used. |
 | imports_test.py:34:1:34:14 | Import | Import of 'module2' is not used. |
+| imports_test.py:116:1:116:41 | Import | Import of 'not_a_fixture' is not used. |

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

@@ -111,3 +111,8 @@ def bar(x: Optional['subexpression_parameter_annotation']):
 
 def baz() -> Optional['subexpression_return_type']:
     pass
+
+
+from pytest_fixtures import not_a_fixture  # BAD
+from pytest_fixtures import fixture, wrapped_fixture  # GOOD (pytest fixtures are used implicitly by pytest)
+from pytest_fixtures import session_fixture, wrapped_autouse_fixture  # GOOD (pytest fixtures are used implicitly by pytest)

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

@@ -0,0 +1,34 @@
+import pytest
+
+
+@pytest.fixture
+def fixture():
+    pass
+
+def fixture_wrapper():
+    @pytest.fixture
+    def delegate():
+        pass
+    return delegate
+
+@fixture_wrapper
+def wrapped_fixture():
+    pass
+
+
+@pytest.fixture(scope='session')
+def session_fixture():
+    pass
+
+def not_a_fixture():
+    pass
+
+def another_fixture_wrapper():
+    @pytest.fixture(autouse=True)
+    def delegate():
+        pass
+    return delegate
+
+@another_fixture_wrapper
+def wrapped_autouse_fixture():
+    pass