diff --git a/AUTHORS.rst b/AUTHORS.rst index 9f408c53..c669f9dd 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -13,3 +13,4 @@ Authors * Nathan McDougall - https://github.com/nathanjmcdougall * Oleksandr Zaiats - https://github.com/z4y4ts * Nikhil Dabas - https://github.com/ndabas +* Pierre-Yves Le Borgne - https://github.com/pylaterreur diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8ee3b38a..61d6af78 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,6 +7,8 @@ latest * Fix missing macOS wheels for regular (non-freethreaded) Python 3.14+ (https://github.com/python-grimp/grimp/issues/317). +* Fix imports in the ``else`` (or non-type checking ``elif``) branch of an ``if TYPE_CHECKING:`` + statement being excluded by ``exclude_type_checking_imports``, even though they run at runtime. 3.17 (2026-09-04) ----------------- diff --git a/docs/usage.rst b/docs/usage.rst index 6419c03b..e658df63 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -76,10 +76,11 @@ Building the graph *Note: external packages are only analysed as modules that are imported; any imports they make themselves will not be included in the graph.* :param bool, optional exclude_type_checking_imports: Whether to exclude imports made in type checking guards. If this is ``True``, - any import made under an ``if TYPE_CHECKING:`` statement will not be added to the graph. + any import made under an ``if TYPE_CHECKING:`` statement will not be added to the graph. Imports in its ``else`` + branch are still added, as that is the branch that runs at runtime. See the `typing module documentation`_ for reference. (The type checking guard is detected purely by looking for - a statement in the form ``if TYPE_CHECKING`` or ``if {some_alias}.TYPE_CHECKING``. It does not check whether - ``TYPE_CHECKING`` is actually the attribute from the ``typing`` module.) + a statement in the form ``if TYPE_CHECKING`` or ``if {some_alias}.TYPE_CHECKING``, or the ``elif`` equivalents. + It does not check whether ``TYPE_CHECKING`` is actually the attribute from the ``typing`` module.) :param str, optional cache_dir: The directory to use for caching the graph. Defaults to ``.grimp_cache``. To disable caching, pass ``None``. See :doc:`caching`. :return: An import graph that you can use to analyse the package. diff --git a/rust/src/import_parsing.rs b/rust/src/import_parsing.rs index 2d2fe0ac..fe11fce6 100644 --- a/rust/src/import_parsing.rs +++ b/rust/src/import_parsing.rs @@ -122,25 +122,19 @@ impl<'a> StatementVisitor<'a> for Visitor<'a> { } walk_stmt(self, stmt); } - Stmt::If(if_stmt) => match if_stmt.test.as_ref() { - Expr::Name(expr) => { - if expr.id == "TYPE_CHECKING" { - self.typechecking_only = true; - walk_stmt(self, stmt); - self.typechecking_only = false; - } else { - walk_stmt(self, stmt); - } + Stmt::If(if_stmt) => { + // Only the branches guarded by `TYPE_CHECKING` are type checking only. The other + // branches (e.g. `else`) run when `TYPE_CHECKING` is false, that is, at runtime. + let outer_typechecking_only = self.typechecking_only; + self.typechecking_only = outer_typechecking_only || is_type_checking(&if_stmt.test); + self.visit_body(&if_stmt.body); + for clause in &if_stmt.elif_else_clauses { + self.typechecking_only = outer_typechecking_only + || clause.test.as_ref().is_some_and(is_type_checking); + self.visit_elif_else_clause(clause); } - Expr::Attribute(expr) if expr.attr.id == "TYPE_CHECKING" => { - self.typechecking_only = true; - walk_stmt(self, stmt); - self.typechecking_only = false; - } - _ => { - walk_stmt(self, stmt); - } - }, + self.typechecking_only = outer_typechecking_only; + } _ => { walk_stmt(self, stmt); } @@ -148,6 +142,15 @@ impl<'a> StatementVisitor<'a> for Visitor<'a> { } } +/// Whether the expression is `TYPE_CHECKING` or `.TYPE_CHECKING`. +fn is_type_checking(expr: &Expr) -> bool { + match expr { + Expr::Name(name) => name.id == "TYPE_CHECKING", + Expr::Attribute(attribute) => attribute.attr.id == "TYPE_CHECKING", + _ => false, + } +} + #[cfg(test)] mod tests { use super::parse_imports_from_code; diff --git a/tests/unit/application/test_scanning.py b/tests/unit/application/test_scanning.py index a510e0b3..7f209442 100644 --- a/tests/unit/application/test_scanning.py +++ b/tests/unit/application/test_scanning.py @@ -1079,6 +1079,87 @@ def test_exclude_type_checking_imports( assert {module_foo_one_file: expected_result} == result +@pytest.mark.parametrize( + "code, expected_imported_modules", + ( + pytest.param( + """ + if TYPE_CHECKING: + import foo.two + else: + import foo.three + """, + {"foo.three"}, + id="else", + ), + pytest.param( + """ + if typing.TYPE_CHECKING: + import foo.two + elif sys.version_info >= (3, 12): + import foo.three + else: + import foo.four + """, + {"foo.three", "foo.four"}, + id="elif-and-else", + ), + pytest.param( + """ + if sys.version_info >= (3, 12): + import foo.two + elif TYPE_CHECKING: + import foo.three + else: + import foo.four + """, + {"foo.two", "foo.four"}, + id="elif-type-checking", + ), + pytest.param( + """ + if TYPE_CHECKING: + if sys.version_info >= (3, 12): + import foo.two + else: + import foo.three + if TYPE_CHECKING: + import foo.four + import foo.five + """, + set(), + id="nested", + ), + ), +) +def test_exclude_type_checking_imports_only_excludes_type_checking_branches( + code, expected_imported_modules +): + all_modules = {Module(f"foo.{name}") for name in ("one", "two", "three", "four", "five")} + module_foo_one_file = _module_to_module_file(Module("foo.one")) + file_system = rust.FakeBasicFileSystem(content_map={"/path/to/foo/one.py": code}) + found_packages = { + FoundPackage( + name="foo", + directory="/path/to/foo", + module_files=_modules_to_module_files(all_modules), + ) + } + + with override_settings(FILE_SYSTEM=file_system): + result = scanning.scan_imports( + {module_foo_one_file}, + found_packages=found_packages, + include_external_packages=False, + exclude_type_checking_imports=True, + ) + + imported_modules = { + direct_import.imported.name for direct_import in result[module_foo_one_file] + } + assert imported_modules == expected_imported_modules + + def test_t_string_syntax(): module_file_to_scan = _module_to_module_file(Module("foo.one"))