Add ImportAttribute node to restrict Attribute.value in import module contexts
- Dominant language
- Python
- Stars
- 1.9k
- Forks
- 229
- PR merge metrics
- No merged PRs in 30d
Description
## Problem
`ImportFrom.module` is typed as `Attribute | Name | None` and `ImportAlias.name` is typed as `Attribute | Name`, which correctly reflects Python's grammar. However, `Attribute.value` is typed as `BaseExpression`, which is overly broad for the import context.
Python's grammar restricts module paths in import statements to dotted names only (e.g., `from a.b.c import d`, `import a.b.c`). This means that within `ImportFrom.module` and `ImportAlias.name`, any `Attribute` node's value can only ever be `Name` or another `Attribute` ? never `Call`, `Subscript`, or any other `BaseExpression`.
LibCST's own `scope_provider.py` acknowledges this with a `while isinstance(name_node, cst.Attribute)` loop that assumes only `Name` or `Attribute` will appear, raising an exception if anything else is found.
This creates a typing gap: code that processes `ImportFrom.module` recursively must add unreachable fallback branches to handle the impossible cases, which can never be covered by tests.
## Example
```
def module_to_str(module: cst.Attribute | cst.Name | None) -> str:
if module is None:
return ""
if isinstance(module, cst.Name):
return module.value
if isinstance(module, cst.Attribute):
# module.value is BaseExpression here ? mypy forces us to recurse with BaseExpression
return f"{module_to_str(module.value)}.{module.attr.value}"
return "" # unreachable, untestable, but required by mypy
```
## Proposed Solution
Define `ImportAttribute` at module level using `if TYPE_CHECKING:`. Under the type checker it is a subclass of `Attribute` with `value` narrowed to `Name | Self`; at runtime it is simply `Attribute`, so no behaviour changes whatsoever.
```
if TYPE_CHECKING:
@add_slots
@dataclass(frozen=True)
class ImportAttribute(Attribute):
"""An Attribute narrowed to import module paths.
value is restricted to Name or ImportAttribute.
"""
value: Union[Name, Self]
else:
ImportAttribute = Attribute
```
Then update both affected fields:
```
# ImportAlias
name: Union[ImportAttribute, Name]
# ImportFrom
module: Optional[Union[ImportAttribute, Name]]
```
Applied to both:
- `ImportAlias.name` (`import a.b.c` and `from x import y`)
- `ImportFrom.module` (`from a.b.c import ...`)
## Why This Works
- Zero runtime impact `ImportAttribute = Attribute` in the `else` branch; existing code is entirely unaffected
- No `__name__`/`__qualname__` hacks needed ? visitor dispatch is unaffected
- `Self` correctly expresses the recursive dotted-name constraint
- Consumers processing import nodes recursively can now write type-safe code without unreachable fallback branches
## Verification
With this change, the example from the Problem section becomes:
```
def module_to_str(module: cst.ImportAttribute | cst.Name | None) -> str:
if module is None:
return ""
if isinstance(module, cst.Name):
return module.value
return f"{module_to_str(module.value)}.{module.attr.value}"
# module.value is ImportAttribute | Name ? no unreachable branch needed
```
Contributor guide
Assessment
This issue has not been assessed yet.