177 lines
5.8 KiB
Markdown
177 lines
5.8 KiB
Markdown
# Unused Import Investigation Rule
|
|
|
|
**Rule ID**: Rule 24
|
|
**Status**: ACTIVE
|
|
**Created**: 2025-12-13
|
|
**Category**: Code Quality / Refactoring Safety
|
|
|
|
## Core Principle
|
|
|
|
**Unused imports are signals that require investigation, not automatic deletion.**
|
|
|
|
Before removing an unused import, you MUST investigate whether it indicates:
|
|
1. An incomplete implementation
|
|
2. A recently removed feature that should be restored
|
|
3. A pattern mismatch requiring broader cleanup
|
|
4. Genuinely unused cruft that can be safely removed
|
|
|
|
## Investigation Checklist
|
|
|
|
| Step | Check | How to Verify | Action if Found |
|
|
|------|-------|---------------|-----------------|
|
|
| 1 | **Recent removal** | `git log -p --all -S 'ImportName' -- file.py` | Consider if removal was intentional |
|
|
| 2 | **TODO/FIXME comments** | Search file for TODO, FIXME, XXX near the import | Complete the TODO before removing |
|
|
| 3 | **Pattern mismatch** | Check if file mixes old (`Optional[X]`) and new (`X \| None`) syntax | Standardize entire file first |
|
|
| 4 | **Incomplete feature** | Look for stub functions, empty implementations, `pass` statements | Complete or deliberately remove feature |
|
|
| 5 | **Conditional usage** | Check for `TYPE_CHECKING` blocks, feature flags, platform checks | Verify import isn't needed conditionally |
|
|
| 6 | **Documentation reference** | Check docstrings mentioning the import | Update docs if removing |
|
|
|
|
## When Removal IS Appropriate
|
|
|
|
After investigation confirms:
|
|
|
|
- **Legacy cruft**: File consistently uses modern syntax, import is leftover from Python 3.9 era
|
|
- Example: `Optional` import when file uses `X | None` syntax throughout (60+ occurrences)
|
|
- **Refactored code**: Import was part of deleted/refactored functionality
|
|
- **Template artifact**: Import was copy-pasted from template but never used
|
|
- **Dead code cleanup**: Associated code was intentionally removed
|
|
|
|
## When Removal is NOT Appropriate
|
|
|
|
- **TODO comment references the import**: Implementation is planned
|
|
- **Partial implementation exists**: Import supports incomplete feature
|
|
- **Feature flag dependency**: Import used in conditional code paths
|
|
- **TYPE_CHECKING block**: Import needed for type hints only at static analysis time
|
|
- **Platform-specific code**: Import used on certain platforms only
|
|
|
|
## Examples
|
|
|
|
### Example 1: Safe Removal (Legacy Cruft)
|
|
|
|
```python
|
|
# File: multi_embedding_retriever.py
|
|
from typing import Optional # ← UNUSED
|
|
|
|
# Investigation results:
|
|
# 1. git log: No recent changes involving Optional
|
|
# 2. No TODO comments
|
|
# 3. File uses `X | None` syntax consistently (60+ occurrences)
|
|
# 4. No incomplete implementations
|
|
#
|
|
# Verdict: Safe to remove - legacy cruft from Python 3.9 migration
|
|
```
|
|
|
|
### Example 2: DO NOT Remove (Incomplete Implementation)
|
|
|
|
```python
|
|
# File: data_processor.py
|
|
from dataclasses import dataclass # ← Appears unused
|
|
from typing import Protocol # ← Appears unused
|
|
|
|
# TODO: Implement DataProcessor protocol pattern
|
|
class DataProcessor:
|
|
def process(self, data):
|
|
pass # Stub implementation
|
|
|
|
# Investigation results:
|
|
# 1. TODO comment indicates planned work
|
|
# 2. `pass` stub suggests incomplete implementation
|
|
#
|
|
# Verdict: DO NOT remove - imports support planned feature
|
|
```
|
|
|
|
### Example 3: DO NOT Remove (TYPE_CHECKING Block)
|
|
|
|
```python
|
|
# File: hybrid_retriever.py
|
|
from typing import TYPE_CHECKING
|
|
|
|
if TYPE_CHECKING:
|
|
from qdrant_client import QdrantClient # Only for type hints
|
|
from typedb.driver import TypeDBDriver
|
|
|
|
# Investigation results:
|
|
# 1. Imports are inside TYPE_CHECKING block
|
|
# 2. Used for type annotations, not runtime
|
|
#
|
|
# Verdict: DO NOT remove - needed for static type checking
|
|
```
|
|
|
|
### Example 4: Safe Removal (Refactored Code)
|
|
|
|
```python
|
|
# File: old_parser.py
|
|
import xml.etree.ElementTree as ET # ← UNUSED
|
|
|
|
# Investigation results:
|
|
# 1. git log shows XML parsing was removed in commit abc123
|
|
# 2. Commit message: "Migrate from XML to JSON configuration"
|
|
# 3. No TODO or plans to restore XML support
|
|
#
|
|
# Verdict: Safe to remove - associated code was intentionally removed
|
|
```
|
|
|
|
## Implementation Workflow
|
|
|
|
```
|
|
1. Linter/IDE flags unused import
|
|
↓
|
|
2. STOP - Do not auto-remove
|
|
↓
|
|
3. Run investigation checklist (all 6 steps)
|
|
↓
|
|
4. Document findings in commit message
|
|
↓
|
|
5. Either:
|
|
a) Remove with clear rationale, OR
|
|
b) Keep and add TODO explaining why, OR
|
|
c) Complete the incomplete implementation
|
|
```
|
|
|
|
## Git Commands for Investigation
|
|
|
|
```bash
|
|
# Check git history for the import
|
|
git log -p --all -S 'Optional' -- src/glam_extractor/api/multi_embedding_retriever.py
|
|
|
|
# Find when import was added
|
|
git log --diff-filter=A -p -- file.py | grep -A5 -B5 'from typing import'
|
|
|
|
# Check if import was recently used and removed
|
|
git log -p --since="2 weeks ago" -- file.py | grep -B5 -A5 'Optional'
|
|
```
|
|
|
|
## Rationale
|
|
|
|
Unused imports often indicate one of several situations:
|
|
|
|
1. **Incomplete work**: Developer started implementing a feature but didn't finish
|
|
2. **Accidental removal**: Code using the import was deleted by mistake
|
|
3. **Planned refactoring**: Import is placeholder for upcoming changes
|
|
4. **Pattern evolution**: Codebase is migrating between patterns (e.g., `Optional[X]` → `X | None`)
|
|
|
|
Automatically removing unused imports without investigation can:
|
|
- Delete evidence of planned features
|
|
- Make it harder to complete partial implementations
|
|
- Create inconsistent patterns across the codebase
|
|
- Remove imports needed for type checking only
|
|
|
|
## Related Rules
|
|
|
|
- **Rule 5**: NEVER Delete Enriched Data - Additive Only (similar principle: investigate before removing)
|
|
- **Rule 21**: Data Fabrication is Strictly Prohibited (thoroughness principle)
|
|
|
|
## Verification
|
|
|
|
After removing an unused import, verify:
|
|
|
|
```bash
|
|
# Module still imports successfully
|
|
python -c "from module_name import *"
|
|
|
|
# Type checking passes
|
|
mypy src/path/to/file.py
|
|
|
|
# Tests still pass
|
|
pytest tests/path/to/test_file.py -v
|
|
```
|