Add Variable type attribution for Python identifiers (#6772)

* ChangeImport: rename bare references when from-import name changes

When `from X import old_name` is rewritten to `from X import new_name`,
bare references to `old_name` in the code (e.g., function calls like
`clock()`) are now renamed to `new_name` (e.g., `perf_counter()`).

This is done via a new `visit_identifier` method in ChangeImportVisitor
that renames matching identifiers outside of import statements. An xfail
test documents that scope analysis for shadowed locals is not yet
implemented.

* Add Variable type attribution for Python identifiers

Populate field_type with JavaType.Variable on Identifiers for local
variables, module-level constants, and class fields (self.x). Uses a
single ty hover query per name/attribute to avoid double lookups.

- Implement JavaType.Variable as a full dataclass with name, type, owner
- Add name_type_info() and attribute_type_info() to PythonTypeMapping
- Wire visit_Name and visit_Attribute to use the new combined methods
- Add RPC sender/receiver support for Variable serialization
- Add tests for variable hover detection, Variable creation, and
  integration tests with ty for name/attribute type info

* ChangeImport: skip renaming local variables that shadow imports

Use field_type from Variable type attribution to distinguish local
variable references from imported symbol references. Also exclude
Unknown hover results from being treated as variables.

* Address code review feedback

- Fix frozen mismatch: remove frozen=True from Variable .pyi stub
- Use constructor kwargs in _make_variable instead of post-mutation
- Remove duplicate Variable receiver code in python_receiver.py
- Make remove_import.py visit_import/visit_multi_import exception-safe
  with try/finally
- Document limitations: field_type shadow detection needs ty,
  visit_method_invocation only handles simple Identifier selects,
  attribute_type_info column offset for multiline expressions
- Add test for both from-import and direct import simultaneously

* ChangeImport: scope shadow check to function bodies only

Module-level bare references to an imported name were incorrectly
skipped when type attribution populated field_type. Limit the
field_type shadow check to identifiers inside MethodDeclaration
scopes so module-level references are always renamed.

Also add default initializers to visitor fields, expand inline
documentation, and remove redundant truthiness guards in the RPC
variable receiver.

* Skip shadow detection test when ty CLI is not available

Author: knutwannheden

rewrite-python/rewrite/src/rewrite/java/support_types.py

@@ -298,8 +298,33 @@ def default_value(self) -> Optional[List[str]]:
         def declared_formal_type_names(self) -> Optional[List[str]]:
             return self._declared_formal_type_names
 
+    @dataclass
     class Variable:
-        pass
+        _flags_bit_map: int = field(default=0)
+        _name: str = field(default="")
+        _owner: Optional[JavaType] = field(default=None)
+        _type: Optional[JavaType] = field(default=None)
+        _annotations: Optional[List[JavaType.FullyQualified]] = field(default=None)
+
+        @property
+        def flags_bit_map(self) -> int:
+            return self._flags_bit_map
+
+        @property
+        def name(self) -> str:
+            return self._name
+
+        @property
+        def owner(self) -> Optional[JavaType]:
+            return self._owner
+
+        @property
+        def type(self) -> Optional[JavaType]:
+            return self._type
+
+        @property
+        def annotations(self) -> Optional[List[JavaType.FullyQualified]]:
+            return self._annotations
 
     class Array:
         pass

rewrite-python/rewrite/src/rewrite/java/support_types.pyi

@@ -13,7 +13,7 @@ J2 = TypeVar('J2', bound=J)
 J3 = TypeVar('J3', bound=J)
 
 from abc import abstractmethod, ABC
-from enum import Enum, auto
+from enum import Enum
 from rewrite import Markers
 from rewrite import Tree, SourceFile, TreeVisitor
 from rewrite.utils import replace_if_changed
@@ -144,8 +144,24 @@ class JavaType(ABC):
         @property
         def declared_formal_type_names(self) -> Optional[List[str]]: ...
 
+    @dataclass
     class Variable:
-        pass
+        _flags_bit_map: int = ...
+        _name: str = ...
+        _owner: Optional[JavaType] = ...
+        _type: Optional[JavaType] = ...
+        _annotations: Optional[List[JavaType.FullyQualified]] = ...
+
+        @property
+        def flags_bit_map(self) -> int: ...
+        @property
+        def name(self) -> str: ...
+        @property
+        def owner(self) -> Optional[JavaType]: ...
+        @property
+        def type(self) -> Optional[JavaType]: ...
+        @property
+        def annotations(self) -> Optional[List[JavaType.FullyQualified]]: ...
 
     class Array:
         pass

rewrite-python/rewrite/src/rewrite/python/_parser_visitor.py

@@ -1,4 +1,5 @@
 import ast
+import dataclasses
 import sys
 import token
 from argparse import ArgumentError
@@ -1050,13 +1051,24 @@ def visit_TypeIgnore(self, node):
         raise NotImplementedError("Implement visit_TypeIgnore!")
 
     def visit_Attribute(self, node):
+        prefix = self.__whitespace()
+        target = self.__convert(node.value)
+        receiver_type = getattr(target, 'type', None) if hasattr(target, 'type') else None
+        dot_space = self.__source_before('.')
+        name_ident = self.__convert_name(node.attr)
+
+        expr_type, field_type = self._type_mapping.attribute_type_info(node, receiver_type)
+
+        if isinstance(name_ident, j.Identifier):
+            name_ident = dataclasses.replace(name_ident, _type=expr_type, _field_type=field_type)
+
         return j.FieldAccess(
             random_id(),
-            self.__whitespace(),
+            prefix,
             Markers.EMPTY,
-            self.__convert(node.value),
-            self.__pad_left(self.__source_before('.'), self.__convert_name(node.attr)),
-            self._type_mapping.type(node),
+            target,
+            self.__pad_left(dot_space, name_ident),
+            expr_type,
         )
 
     def visit_Del(self, node):
@@ -2548,14 +2560,15 @@ def visit_Module(self, node: ast.Module) -> py.CompilationUnit:
 
     def visit_Name(self, node):
         space, actual_name = self.__consume_identifier(node.id)
+        expr_type, field_type = self._type_mapping.name_type_info(node)
         return j.Identifier(
             random_id(),
             space,
             Markers.EMPTY,
             [],
             actual_name,
-            self._type_mapping.type(node),
-            None
+            expr_type,
+            field_type
         )
 
     def visit_NamedExpr(self, node):

rewrite-python/rewrite/src/rewrite/python/recipes/change_import.py

@@ -24,7 +24,7 @@
 from rewrite.recipe import option
 from rewrite.java import J
 from rewrite.java.support_types import JavaType
-from rewrite.java.tree import Empty, FieldAccess, Identifier, Import, MethodInvocation
+from rewrite.java.tree import Empty, FieldAccess, Identifier, Import, MethodDeclaration, MethodInvocation
 from rewrite.python.tree import CompilationUnit, MultiImport
 from rewrite.python.visitor import PythonVisitor
 from rewrite.python.add_import import AddImportOptions, maybe_add_import
@@ -35,7 +35,11 @@
 
 
 def _create_module_type(fqn: str) -> JavaType.Class:
-    """Create a JavaType.Class for a module from its fully qualified name."""
+    """Create a JavaType.Class for a module from its fully qualified name.
+
+    JavaType.Class is not a dataclass, so fields are set directly after
+    construction.  This matches the pattern used elsewhere in the codebase.
+    """
     class_type = JavaType.Class()
     class_type._flags_bit_map = 0
     class_type._fully_qualified_name = fqn
@@ -131,12 +135,12 @@ def editor(self) -> TreeVisitor[Any, ExecutionContext]:
         new_alias = self.new_alias
 
         class ChangeImportVisitor(PythonVisitor[ExecutionContext]):
-            has_old_import: bool
-            old_alias: Optional[str]
-            has_direct_module_import: bool
-            module_alias: Optional[str]
-            rewrote_qualified_refs: bool
-            new_module_type: Optional[JavaType.Class]
+            has_old_import: bool = False
+            old_alias: Optional[str] = None
+            has_direct_module_import: bool = False
+            module_alias: Optional[str] = None
+            rewrote_qualified_refs: bool = False
+            new_module_type: Optional[JavaType.Class] = None
 
             def visit_compilation_unit(self, cu: CompilationUnit, p: ExecutionContext) -> J:
                 self.has_old_import = False
@@ -237,18 +241,51 @@ def visit_multi_import(self, multi: MultiImport, p: ExecutionContext) -> Optiona
                     # import X - remove entire import
                     return self._remove_module_from_import(multi, old_module)
 
+            def visit_identifier(self, ident: Identifier, p: ExecutionContext) -> J:
+                ident = super().visit_identifier(ident, p)
+                if not isinstance(ident, Identifier):
+                    return ident
+                if not old_name or not new_name or not self.has_old_import:
+                    return ident
+                old_ref_name = self.old_alias or old_name
+                new_ref_name = new_alias or self.old_alias or new_name
+                if old_ref_name == new_ref_name:
+                    return ident
+                if ident.simple_name != old_ref_name:
+                    return ident
+                # Skip identifiers inside import statements
+                if self.cursor.first_enclosing(Import):
+                    return ident
+                # Skip local variables that shadow the imported name.
+                # Only check field_type inside function scopes — at module level,
+                # bare references to the imported name always need renaming.
+                # When ty is unavailable, field_type is None for all identifiers
+                # and shadowed locals may be incorrectly renamed.
+                if self.cursor.first_enclosing(MethodDeclaration) is not None:
+                    if ident.field_type is not None:
+                        return ident
+                return ident.replace(_simple_name=new_ref_name)
+
             def visit_method_invocation(self, method: MethodInvocation, p: ExecutionContext) -> J:
                 method = super().visit_method_invocation(method, p)
-                if not old_name or not self.has_direct_module_import:
-                    return method
                 if not isinstance(method, MethodInvocation):
                     return method
+                if not old_name or not self.has_direct_module_import:
+                    return method
+                # Only matches simple module.func() calls where the select is an
+                # Identifier. Nested attribute chains like pkg.module.func()
+                # (where select is a FieldAccess) are not currently handled.
                 if not isinstance(method.select, Identifier):
                     return method
                 if not isinstance(method.name, Identifier):
                     return method
 
                 select_name = method.select.simple_name
+                # For dotted modules without aliases (e.g. `import os.path`),
+                # `old_module` is a dotted string like "os.path" which will
+                # never match a simple Identifier name — but those cases are
+                # already excluded by the `isinstance(method.select, Identifier)`
+                # guard above (the select would be a FieldAccess instead).
                 expected_name = self.module_alias or old_module
                 if select_name != expected_name:
                     return method

rewrite-python/rewrite/src/rewrite/python/remove_import.py

@@ -114,16 +114,18 @@ def __init__(self):
             def visit_import(self, import_: Import, p) -> J:
                 # Don't collect identifiers from standalone import statements
                 self.in_import = True
-                result = super().visit_import(import_, p)
-                self.in_import = False
-                return result
+                try:
+                    return super().visit_import(import_, p)
+                finally:
+                    self.in_import = False
 
             def visit_multi_import(self, multi: MultiImport, p) -> J:
                 # Don't collect identifiers from import statements
                 self.in_import = True
-                result = super().visit_multi_import(multi, p)
-                self.in_import = False
-                return result
+                try:
+                    return super().visit_multi_import(multi, p)
+                finally:
+                    self.in_import = False
 
             def visit_identifier(self, ident: Identifier, p) -> J:
                 if not self.in_import:

rewrite-python/rewrite/src/rewrite/python/type_mapping.py

@@ -266,6 +266,54 @@ def _constant_type(self, node: ast.Constant) -> Optional[JavaType]:
             return JavaType.Primitive.None_
         return None
 
+    def _is_variable_hover(self, hover: str) -> bool:
+        """Check if hover text describes a variable (not a function, class, or module).
+
+        ty hover patterns:
+        - Variable: just the type, e.g. ``int``, ``Literal[5]``, ``str``
+        - Function: ``def func(args) -> return_type``
+        - Class definition: ``class MyClass``
+        - Module: ``<module 'name'>``
+        """
+        if not hover:
+            return False
+        clean = self._strip_markdown(hover)
+        if not clean:
+            return False
+        if clean.startswith('def ') or clean.startswith('class ') or clean.startswith('<module'):
+            return False
+        if clean == 'Unknown':
+            return False
+        return True
+
+    def _make_variable(self, name: str, var_type: Optional[JavaType],
+                       owner: Optional[JavaType] = None) -> JavaType.Variable:
+        """Create a JavaType.Variable instance."""
+        return JavaType.Variable(_name=name, _type=var_type, _owner=owner)
+
+    def name_type_info(self, node: ast.Name) -> Tuple[Optional[JavaType], Optional[JavaType.Variable]]:
+        """Get expression type and variable type for a name reference.
+
+        Returns (expression_type, variable_field_type) from a single hover query.
+        """
+        if self._ty_client is None:
+            return None, None
+
+        hover = self._ty_client.get_hover(
+            self._uri,
+            node.lineno - 1,  # LSP uses 0-based lines
+            node.col_offset
+        )
+        if not hover:
+            return None, None
+
+        expr_type = self._parse_hover_type(hover)
+
+        if self._is_variable_hover(hover):
+            return expr_type, self._make_variable(node.id, expr_type)
+
+        return expr_type, None
+
     def _name_type(self, node: ast.Name) -> Optional[JavaType]:
         """Get the type for a name reference."""
         if self._ty_client is None:
@@ -281,6 +329,40 @@ def _name_type(self, node: ast.Name) -> Optional[JavaType]:
             return self._parse_hover_type(hover)
         return None
 
+    def attribute_type_info(self, node: ast.Attribute,
+                            receiver_type: Optional[JavaType] = None
+                            ) -> Tuple[Optional[JavaType], Optional[JavaType.Variable]]:
+        """Get expression type and variable type for an attribute access.
+
+        Args:
+            node: The ast.Attribute node.
+            receiver_type: The type of the receiver (e.g., type of 'self').
+
+        Returns (expression_type, variable_field_type).
+
+        Note:
+            The hover column is computed as ``node.col_offset + len(receiver_text) + 1``.
+            This may be inaccurate for multiline or parenthesized receiver expressions
+            where ``_get_node_text`` returns a substring that differs from the full
+            source span.
+        """
+        if self._ty_client is None:
+            return None, None
+
+        hover = self._ty_client.get_hover(
+            self._uri, node.lineno - 1,
+            node.col_offset + len(self._get_node_text(node.value)) + 1
+        )
+        if not hover:
+            return None, None
+
+        expr_type = self._parse_hover_type(hover)
+
+        if self._is_variable_hover(hover):
+            return expr_type, self._make_variable(node.attr, expr_type, owner=receiver_type)
+
+        return expr_type, None
+
     def _attribute_type(self, node: ast.Attribute) -> Optional[JavaType]:
         """Get the type for an attribute access."""
         if self._ty_client is None: