Fix subpackage import incorrectly removed when sibling subpackage is used (#180) (#322)

* Fix subpackage import incorrectly removed when sibling subpackage is used (#180)

`import a.b` binds `a` in the namespace, so any `a.x.y` reference
relies on that binding. `is_match_sub_packages` was comparing the
import's root package against the full name string instead of just
its first component, causing attribute usages like `urllib.parse.urlparse`
to not match `import urllib.request`.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Prefer more specific import over sub-package match

When multiple imports share the same root package (e.g. import urllib.request
and import urllib.parse), is_match_sub_packages was matching both to any
urllib.* name usage, causing false negatives. Added _has_more_specific_import
to check if another import provides a more direct path match for the name,
so the sub-package fallback only applies when no specific import exists.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Apply black formatting and add issue fix checklist to CLAUDE.md

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add docs for subpackage imports and deferred execution behaviors

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: hakancelikdev

CLAUDE.md

@@ -103,3 +103,18 @@ Tests in `tests/cases/` use a **three-directory convention**:
 refactoring. To add a new test case, create matching files in all three directories.
 
 The `# unimport: skip_file` comment in source files tells unimport to skip analysis.
+
+## Issue Fix Checklist
+
+When fixing a bug or implementing a feature from a GitHub issue, follow these steps in
+order:
+
+1. **Understand the issue** — read the issue, reproduce the problem, identify root cause
+2. **Create a branch** — `git checkout -b fix/<short-description>`
+3. **Implement the fix** — make the minimal code change needed
+4. **Write test cases** — create matching files in all three test directories
+   (`source/`, `analyzer/`, `refactor/`). Cover edge cases
+5. **Run tests** — `pytest tests -x -v --disable-warnings`
+6. **Run pre-commit** — `pre-commit run --all-files` (black, isort, mypy, docformatter)
+7. **Fix any linting issues** — re-run tests after fixes
+8. **Commit and push** — commit with descriptive message, push to remote

docs/tutorial/supported-behaviors.md

@@ -188,6 +188,47 @@ explicit import, star imports won't produce a duplicate for that name.
 
 ---
 
+## Subpackage Imports
+
+`import a.b` binds `a` in the namespace, so any reference to `a.*` relies on that
+import. Unimport recognizes this and won't remove a subpackage import when its root
+package is used in the code.
+
+```python
+import urllib.request
+
+def parse_url(url):
+    return urllib.parse.urlparse(url)
+```
+
+In this example, `import urllib.request` is kept because removing it would remove
+`urllib` from the namespace, breaking the `urllib.parse.urlparse()` call.
+
+However, when a more specific import exists, unimport correctly identifies the redundant
+one:
+
+**input**
+
+```python
+import urllib.request
+import urllib.parse
+
+urllib.parse.urlparse('http://example.com')
+```
+
+**output**
+
+```python
+import urllib.parse
+
+urllib.parse.urlparse('http://example.com')
+```
+
+Here `import urllib.parse` directly provides the needed namespace, so
+`import urllib.request` is correctly flagged as unused.
+
+---
+
 ## Scope
 
 Unimport tries to better understand whether the import is unused by performing scope
@@ -250,3 +291,35 @@ class Klass:
 
         x
 ```
+
+### Deferred Execution
+
+Function and async function bodies are deferred — they only execute when called. So a
+module-level import that appears textually after a function definition is still
+available when the function runs. Unimport understands this and won't remove such
+imports.
+
+```python
+def bob():
+    print(sys.path)
+
+import sys  # kept: sys is available when bob() is called
+```
+
+This applies to regular functions, async functions, nested functions, and methods:
+
+```python
+async def fetch():
+    return aiohttp.get(url)
+
+import aiohttp  # kept
+```
+
+Class bodies execute immediately (not deferred), so the lineno check still applies:
+
+```python
+class Foo:
+    x = sys.platform
+
+import sys  # removed: class body runs before this import
+```

src/unimport/statement.py

@@ -23,7 +23,7 @@ def __len__(self) -> int:
         return len(self.name.split("."))
 
     def is_match_sub_packages(self, name_name: str) -> bool:
-        return self.name.split(".")[0] == name_name
+        return self.name.split(".")[0] == name_name.split(".")[0]
 
     @property
     def scope(self):
@@ -149,13 +149,25 @@ def _is_deferred_usage(self, imp: Import | ImportFrom) -> bool:
             scope = scope.parent
         return False
 
+    def _has_more_specific_import(self, imp: Import | ImportFrom) -> bool:
+        name_parts = self.name.split(".")
+        for other_imp in Import.imports:
+            if other_imp is imp:
+                continue
+            other_parts = other_imp.name.split(".")
+            if len(other_parts) <= len(name_parts) and name_parts[: len(other_parts)] == other_parts:
+                return True
+        return False
+
     def match_2(self, imp: Import | ImportFrom) -> bool:
         if self.is_all:
             is_match = self.name == imp.name
         elif self.is_attribute:
-            is_match = (imp.lineno < self.lineno or self._is_deferred_usage(imp)) and (
-                ".".join(self.name.split(".")[: len(imp)]) == imp.name or imp.is_match_sub_packages(self.name)
+            primary_match = ".".join(self.name.split(".")[: len(imp)]) == imp.name
+            sub_match = (
+                not primary_match and imp.is_match_sub_packages(self.name) and not self._has_more_specific_import(imp)
             )
+            is_match = (imp.lineno < self.lineno or self._is_deferred_usage(imp)) and (primary_match or sub_match)
         else:
             is_match = (imp.lineno < self.lineno or self._is_deferred_usage(imp)) and (
                 self.name == imp.name or imp.is_match_sub_packages(self.name)

tests/cases/analyzer/unused/subpackage_import_attribute_usage.py

@@ -0,0 +1,16 @@
+from typing import Union
+
+from unimport.statement import Import, ImportFrom, Name
+
+__all__ = ["NAMES", "IMPORTS", "UNUSED_IMPORTS"]
+
+
+NAMES: list[Name] = [
+    Name(lineno=4, name="urllib.parse.urlparse", is_all=False),
+    Name(lineno=4, name="urllib.parse", is_all=False),
+    Name(lineno=4, name="url", is_all=False),
+]
+IMPORTS: list[Union[Import, ImportFrom]] = [
+    Import(lineno=1, column=1, name="urllib.request", package="urllib.request"),
+]
+UNUSED_IMPORTS: list[Union[Import, ImportFrom]] = []

tests/cases/analyzer/unused/subpackage_import_same_branch.py

@@ -0,0 +1,16 @@
+from typing import Union
+
+from unimport.statement import Import, ImportFrom, Name
+
+__all__ = ["NAMES", "IMPORTS", "UNUSED_IMPORTS"]
+
+
+NAMES: list[Name] = [
+    Name(lineno=3, name="print", is_all=False),
+    Name(lineno=3, name="os.path.join", is_all=False),
+    Name(lineno=3, name="os.path", is_all=False),
+]
+IMPORTS: list[Union[Import, ImportFrom]] = [
+    Import(lineno=1, column=1, name="os.path", package="os.path"),
+]
+UNUSED_IMPORTS: list[Union[Import, ImportFrom]] = []

tests/cases/analyzer/unused/subpackage_import_simple_usage.py

@@ -0,0 +1,15 @@
+from typing import Union
+
+from unimport.statement import Import, ImportFrom, Name
+
+__all__ = ["NAMES", "IMPORTS", "UNUSED_IMPORTS"]
+
+
+NAMES: list[Name] = [
+    Name(lineno=3, name="print", is_all=False),
+    Name(lineno=3, name="os.getcwd", is_all=False),
+]
+IMPORTS: list[Union[Import, ImportFrom]] = [
+    Import(lineno=1, column=1, name="os.path", package="os.path"),
+]
+UNUSED_IMPORTS: list[Union[Import, ImportFrom]] = []

tests/cases/analyzer/unused/subpackage_import_unused.py

@@ -0,0 +1,16 @@
+from typing import Union
+
+from unimport.statement import Import, ImportFrom, Name
+
+__all__ = ["NAMES", "IMPORTS", "UNUSED_IMPORTS"]
+
+
+NAMES: list[Name] = [
+    Name(lineno=3, name="print", is_all=False),
+]
+IMPORTS: list[Union[Import, ImportFrom]] = [
+    Import(lineno=1, column=1, name="urllib.request", package="urllib.request"),
+]
+UNUSED_IMPORTS: list[Union[Import, ImportFrom]] = [
+    Import(lineno=1, column=1, name="urllib.request", package="urllib.request"),
+]

tests/cases/analyzer/unused/subpackage_import_with_specific_import.py

@@ -0,0 +1,18 @@
+from typing import Union
+
+from unimport.statement import Import, ImportFrom, Name
+
+__all__ = ["NAMES", "IMPORTS", "UNUSED_IMPORTS"]
+
+
+NAMES: list[Name] = [
+    Name(lineno=4, name="urllib.parse.urlparse", is_all=False),
+    Name(lineno=4, name="urllib.parse", is_all=False),
+]
+IMPORTS: list[Union[Import, ImportFrom]] = [
+    Import(lineno=1, column=1, name="urllib.request", package="urllib.request"),
+    Import(lineno=2, column=1, name="urllib.parse", package="urllib.parse"),
+]
+UNUSED_IMPORTS: list[Union[Import, ImportFrom]] = [
+    Import(lineno=1, column=1, name="urllib.request", package="urllib.request"),
+]

tests/cases/refactor/unused/subpackage_import_attribute_usage.py

@@ -0,0 +1,4 @@
+import urllib.request
+
+def parse_url(url):
+    return urllib.parse.urlparse(url)

tests/cases/refactor/unused/subpackage_import_same_branch.py

@@ -0,0 +1,3 @@
+import os.path
+
+print(os.path.join("a", "b"))