From b33a03b9c96f0212045a4f9485ec460aea26bf97 Mon Sep 17 00:00:00 2001 From: Knut Wannheden Date: Mon, 23 Feb 2026 11:05:40 +0100 Subject: [PATCH 1/6] Add type attribution tests and fix method declaration/type hint types - Add type attribution tests to 13 existing parser test files covering method invocations, binary ops, type hints, collection literals, imports, field access, class instances, method declarations, async defs, for loops, unary ops, ternaries, and lambdas - Add `method_declaration_type()` to type_mapping.py to build JavaType.Method for function declarations using ty-types descriptor data with annotation fallback - Add type attribution to ParameterizedType nodes in type hint expressions - Fix pre-existing assign_test.py bug (simple_name access, FQN startswith) - Add typing.Text test to test_type_attribution.py - Bump ty-types dependency to >=0.0.19.dev20260223093555 --- rewrite-python/rewrite/pyproject.toml | 2 +- .../src/rewrite/python/_parser_visitor.py | 4 +- .../src/rewrite/python/type_mapping.py | 81 ++++++++++ .../tests/python/all/tree/assign_test.py | 14 +- .../tests/python/all/tree/binary_test.py | 101 +++++++++++++ .../tests/python/all/tree/class_test.py | 49 ++++++ .../all/tree/collection_literal_test.py | 75 ++++++++++ .../rewrite/tests/python/all/tree/def_test.py | 49 ++++++ .../python/all/tree/field_access_test.py | 43 ++++++ .../rewrite/tests/python/all/tree/for_test.py | 43 ++++++ .../tests/python/all/tree/import_test.py | 97 ++++++++++++ .../tests/python/all/tree/lambda_test.py | 40 +++++ .../all/tree/method_declaration_test.py | 64 ++++++++ .../python/all/tree/method_invocation_test.py | 123 ++++++++++++++++ .../tests/python/all/tree/ternary_test.py | 41 ++++++ .../tests/python/all/tree/type_hint_test.py | 139 ++++++++++++++++++ .../tests/python/all/tree/unary_test.py | 41 ++++++ .../tests/python/test_type_attribution.py | 21 +++ 18 files changed, 1017 insertions(+), 10 deletions(-) diff --git a/rewrite-python/rewrite/pyproject.toml b/rewrite-python/rewrite/pyproject.toml index 2b56638e84c..1d0bc3a97a6 100644 --- a/rewrite-python/rewrite/pyproject.toml +++ b/rewrite-python/rewrite/pyproject.toml @@ -24,7 +24,7 @@ requires-python = ">=3.10" dependencies = [ "cbor2>=5.6.5", "more_itertools>=10.0.0", - "ty-types>=0.0.18.dev0", # Type inference CLI for Python type attribution + "ty-types>=0.0.19.dev20260223093555", # Type inference CLI for Python type attribution "parso>=0.7.1,<0.8", # Python 2/3 parser with CST support (0.8+ dropped Python 2.7 grammar) ] diff --git a/rewrite-python/rewrite/src/rewrite/python/_parser_visitor.py b/rewrite-python/rewrite/src/rewrite/python/_parser_visitor.py index a8e3f31c4c1..2520c50f8a8 100644 --- a/rewrite-python/rewrite/src/rewrite/python/_parser_visitor.py +++ b/rewrite-python/rewrite/src/rewrite/python/_parser_visitor.py @@ -2182,7 +2182,7 @@ def visit_FunctionDef(self, node: ast.FunctionDef) -> j.MethodDeclaration: None, body, None, - self.__as_method_type(self._type_mapping.type(node)), + self._type_mapping.method_declaration_type(node), ) def __map_decorator(self, decorator) -> j.Annotation: @@ -2871,7 +2871,7 @@ def __convert_type_mapper(self, node) -> Optional[TypeTree]: enumerate(slices)], Markers.EMPTY ), - None + self._type_mapping.type(node) ) elif isinstance(node, ast.BinOp): # Type unions using `|` was added in Python 3.10 diff --git a/rewrite-python/rewrite/src/rewrite/python/type_mapping.py b/rewrite-python/rewrite/src/rewrite/python/type_mapping.py index fcc55378ff6..102ef1f17ca 100644 --- a/rewrite-python/rewrite/src/rewrite/python/type_mapping.py +++ b/rewrite-python/rewrite/src/rewrite/python/type_mapping.py @@ -618,6 +618,87 @@ def attribute_type_info(self, node: ast.Attribute, return expr_type, JavaType.Variable(_name=node.attr, _type=expr_type, _owner=receiver_type) return expr_type, None + def method_declaration_type(self, node: ast.FunctionDef) -> Optional[JavaType.Method]: + """Get the method type for a function/method declaration. + + Builds a JavaType.Method from the function's type descriptor when + available (parameters + returnType fields), falling back to resolving + parameter annotations and return annotation individually via ty-types. + + Args: + node: The ast.FunctionDef or ast.AsyncFunctionDef node. + + Returns: + A JavaType.Method, or None if types cannot be determined. + """ + # First try: use structured data from the function descriptor + type_id = self._lookup_type_id(node) + if type_id is not None: + descriptor = self._type_registry.get(type_id) + if descriptor and descriptor.get('kind') in ('function', 'boundMethod'): + # If the descriptor has parameters/returnType, use them directly + params = descriptor.get('parameters') + ret_id = descriptor.get('returnType') + if params is not None or ret_id is not None: + return self._method_from_function_descriptor( + descriptor, node.name) + + # Fallback: build from individual parameter/return annotation types + param_names: List[str] = [] + param_types: List[JavaType] = [] + for arg in node.args.args: + if arg.arg in ('self', 'cls'): + continue + param_names.append(arg.arg) + if arg.annotation is not None: + t = self.type(arg.annotation) + param_types.append(t if t is not None else _UNKNOWN) + else: + param_types.append(_UNKNOWN) + + return_type = None + if node.returns is not None: + return_type = self.type(node.returns) + + if not param_names and return_type is None: + return None + + return JavaType.Method( + _flags_bit_map=0, + _declaring_type=None, + _name=node.name, + _return_type=return_type, + _parameter_names=param_names if param_names else None, + _parameter_types=param_types if param_types else None, + ) + + def _method_from_function_descriptor( + self, descriptor: Dict[str, Any], name: str + ) -> JavaType.Method: + """Build a JavaType.Method from a function descriptor with parameters/returnType.""" + param_names: List[str] = [] + param_types: List[JavaType] = [] + for param in descriptor.get('parameters', []): + p_name = param.get('name', '') + if p_name in ('self', 'cls'): + continue + param_names.append(p_name) + param_types.append(self._resolve_param_type(param)) + + return_type = None + ret_id = descriptor.get('returnType') + if ret_id is not None: + return_type = self._resolve_type(ret_id) + + return JavaType.Method( + _flags_bit_map=0, + _declaring_type=None, + _name=name, + _return_type=return_type, + _parameter_names=param_names if param_names else None, + _parameter_types=param_types if param_types else None, + ) + def method_invocation_type(self, node: ast.Call) -> Optional[JavaType.Method]: """Get the method type for a function/method call. diff --git a/rewrite-python/rewrite/tests/python/all/tree/assign_test.py b/rewrite-python/rewrite/tests/python/all/tree/assign_test.py index 9acfa223daf..7945aa70d1a 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/assign_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/assign_test.py @@ -169,8 +169,8 @@ def visit_assignment(self, assignment, p): if not assignment.type._type_parameters: errors.append("Parameterized._type_parameters is empty") elif isinstance(assignment.type, JavaType.Class): - if assignment.type._fully_qualified_name != 'list': - errors.append(f"Assignment.type fqn is '{assignment.type._fully_qualified_name}', expected 'list'") + if not assignment.type._fully_qualified_name.startswith('list'): + errors.append(f"Assignment.type fqn is '{assignment.type._fully_qualified_name}', expected to start with 'list'") else: errors.append(f"Assignment.type is {type(assignment.type).__name__}, expected Parameterized or Class") return assignment @@ -178,7 +178,7 @@ def visit_assignment(self, assignment, p): def visit_method_invocation(self, method, p): if not isinstance(method, MethodInvocation): return method - if method.simple_name != 'split': + if method.name.simple_name != 'split': return method # method_type should be populated if method.method_type is None: @@ -194,11 +194,11 @@ def visit_method_invocation(self, method, p): if method.method_type is not None and method.method_type._return_type is not None: rt = method.method_type._return_type if isinstance(rt, Parameterized): - if rt._type._fully_qualified_name != 'list': - errors.append(f"return_type fqn is '{rt._type._fully_qualified_name}', expected 'list'") + if not rt._type._fully_qualified_name.startswith('list'): + errors.append(f"return_type fqn is '{rt._type._fully_qualified_name}', expected to start with 'list'") elif isinstance(rt, JavaType.Class): - if rt._fully_qualified_name != 'list': - errors.append(f"return_type fqn is '{rt._fully_qualified_name}', expected 'list'") + if not rt._fully_qualified_name.startswith('list'): + errors.append(f"return_type fqn is '{rt._fully_qualified_name}', expected to start with 'list'") else: errors.append(f"return_type is {type(rt).__name__}, expected Parameterized or Class") return method diff --git a/rewrite-python/rewrite/tests/python/all/tree/binary_test.py b/rewrite-python/rewrite/tests/python/all/tree/binary_test.py index d51db70d394..d7a17e06639 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/binary_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/binary_test.py @@ -1,7 +1,18 @@ +import shutil + import pytest +from rewrite.java.support_types import JavaType +from rewrite.java.tree import Binary +from rewrite.python.tree import CompilationUnit +from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python +requires_ty_cli = pytest.mark.skipif( + shutil.which('ty-types') is None, + reason="ty-types CLI is not installed" +) + def test_bool_ops(): # language=python @@ -95,3 +106,93 @@ def test_multiline_tuple_comparison(): , False) is not None """, ),) + + +@requires_ty_cli +def test_arithmetic_type_attribution(): + """Verify that 1 + 2 has type Int.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_binary(self, binary, p): + if not isinstance(binary, Binary): + return binary + if binary.operator != Binary.Type.Addition: + return binary + if binary.type is None: + errors.append("Binary(Addition).type is None") + elif binary.type != JavaType.Primitive.Int: + errors.append(f"Binary(Addition).type is {binary.type}, expected Primitive.Int") + return binary + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + "x = 1 + 2", + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) + + +@requires_ty_cli +def test_comparison_type_attribution(): + """Verify that 1 < 2 has type Boolean.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_binary(self, binary, p): + if not isinstance(binary, Binary): + return binary + if binary.operator != Binary.Type.LessThan: + return binary + if binary.type is None: + errors.append("Binary(LessThan).type is None") + elif binary.type != JavaType.Primitive.Boolean: + errors.append(f"Binary(LessThan).type is {binary.type}, expected Primitive.Boolean") + return binary + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + "x = 1 < 2", + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) + + +@requires_ty_cli +def test_boolean_op_type_attribution(): + """Verify that True and False has type Boolean.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_binary(self, binary, p): + if not isinstance(binary, Binary): + return binary + if binary.operator != Binary.Type.And: + return binary + if binary.type is None: + errors.append("Binary(And).type is None") + elif binary.type != JavaType.Primitive.Boolean: + errors.append(f"Binary(And).type is {binary.type}, expected Primitive.Boolean") + return binary + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + "x = True and False", + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) diff --git a/rewrite-python/rewrite/tests/python/all/tree/class_test.py b/rewrite-python/rewrite/tests/python/all/tree/class_test.py index 3a625db712e..99afd6caa1e 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/class_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/class_test.py @@ -1,5 +1,18 @@ +import shutil + +import pytest + +from rewrite.java.support_types import JavaType +from rewrite.java.tree import Assignment +from rewrite.python.tree import CompilationUnit +from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python +requires_ty_cli = pytest.mark.skipif( + shutil.which('ty-types') is None, + reason="ty-types CLI is not installed" +) + def test_empty(): # language=python @@ -110,3 +123,39 @@ class C3(Generic[T], metaclass=type, *[str]): ... """ )) + + +@requires_ty_cli +def test_class_instance_type_attribution(): + """Verify that x = Foo() assigns a type with fqn 'Foo'.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_assignment(self, assignment, p): + if not isinstance(assignment, Assignment): + return assignment + if assignment.type is None: + errors.append("Assignment.type is None for Foo()") + elif isinstance(assignment.type, JavaType.Class): + if assignment.type._fully_qualified_name != 'Foo': + errors.append(f"Assignment.type fqn is '{assignment.type._fully_qualified_name}', expected 'Foo'") + else: + # Accept any non-None type + pass + return assignment + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + class Foo: + pass + x = Foo() + """, + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) diff --git a/rewrite-python/rewrite/tests/python/all/tree/collection_literal_test.py b/rewrite-python/rewrite/tests/python/all/tree/collection_literal_test.py index 06baebf2d56..17a06eb8148 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/collection_literal_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/collection_literal_test.py @@ -1,11 +1,22 @@ +import shutil from typing import cast import pytest from rewrite.java import MethodDeclaration, Return +from rewrite.java.support_types import JavaType +from rewrite.java.tree import Assignment from rewrite.python import CollectionLiteral, CompilationUnit +from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python +Parameterized = JavaType.Parameterized + +requires_ty_cli = pytest.mark.skipif( + shutil.which('ty-types') is None, + reason="ty-types CLI is not installed" +) + def test_empty_tuple(): # language=python @@ -94,3 +105,67 @@ def test_list_of_tuples_with_double_parens(): "a": 0 }), ((set(), {}))] """)) + + +@requires_ty_cli +def test_list_literal_type_attribution(): + """Verify that [1, 2, 3] has type list.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_assignment(self, assignment, p): + if not isinstance(assignment, Assignment): + return assignment + if assignment.type is None: + errors.append("Assignment.type is None for list literal") + elif isinstance(assignment.type, Parameterized): + if not assignment.type._type._fully_qualified_name.startswith('list'): + errors.append(f"Parameterized base fqn is '{assignment.type._type._fully_qualified_name}', expected to start with 'list'") + elif isinstance(assignment.type, JavaType.Class): + if not assignment.type._fully_qualified_name.startswith('list'): + errors.append(f"Class fqn is '{assignment.type._fully_qualified_name}', expected to start with 'list'") + return assignment + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + "x = [1, 2, 3]", + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) + + +@requires_ty_cli +def test_dict_literal_type_attribution(): + """Verify that {"a": 1} has type dict.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_assignment(self, assignment, p): + if not isinstance(assignment, Assignment): + return assignment + if assignment.type is None: + errors.append("Assignment.type is None for dict literal") + elif isinstance(assignment.type, Parameterized): + if not assignment.type._type._fully_qualified_name.startswith('dict'): + errors.append(f"Parameterized base fqn is '{assignment.type._type._fully_qualified_name}', expected to start with 'dict'") + elif isinstance(assignment.type, JavaType.Class): + if not assignment.type._fully_qualified_name.startswith('dict'): + errors.append(f"Class fqn is '{assignment.type._fully_qualified_name}', expected to start with 'dict'") + return assignment + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + 'x = {"a": 1}', + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) diff --git a/rewrite-python/rewrite/tests/python/all/tree/def_test.py b/rewrite-python/rewrite/tests/python/all/tree/def_test.py index b1378302121..2c7c82a809a 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/def_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/def_test.py @@ -1,7 +1,18 @@ +import shutil + import pytest +from rewrite.java.support_types import JavaType +from rewrite.java.tree import MethodDeclaration +from rewrite.python.tree import CompilationUnit +from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python +requires_ty_cli = pytest.mark.skipif( + shutil.which('ty-types') is None, + reason="ty-types CLI is not installed" +) + def test_async_def(): # language=python @@ -11,3 +22,41 @@ async def main(): pass """ )) + + +@requires_ty_cli +def test_async_def_type_attribution(): + """Verify that async def main() -> int has a method_type with a return_type. + + Note: ty-types correctly reports the return type as CoroutineType (the actual + runtime return type of an async function), not int (the annotated return type). + """ + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_method_declaration(self, method, p): + if not isinstance(method, MethodDeclaration): + return method + if method.name.simple_name != 'main': + return method + if method.method_type is None: + errors.append("MethodDeclaration.method_type is None for async def main()") + else: + if method.method_type._return_type is None: + errors.append("method_type.return_type is None") + return method + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + async def main() -> int: + return 0 + """, + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) diff --git a/rewrite-python/rewrite/tests/python/all/tree/field_access_test.py b/rewrite-python/rewrite/tests/python/all/tree/field_access_test.py index 9a0cb275dd8..edaa2406f0f 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/field_access_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/field_access_test.py @@ -1,5 +1,17 @@ +import shutil + +import pytest + +from rewrite.java.tree import FieldAccess +from rewrite.python.tree import CompilationUnit +from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python +requires_ty_cli = pytest.mark.skipif( + shutil.which('ty-types') is None, + reason="ty-types CLI is not installed" +) + # noinspection PyUnresolvedReferences def test_attribute(): @@ -11,3 +23,34 @@ def test_attribute(): def test_nested_attribute(): # language=python RecipeSpec().rewrite_run(python("a = foo.bar.baz")) + + +@requires_ty_cli +def test_field_access_type_attribution(): + """Verify that os.path has a non-None FieldAccess.type.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_field_access(self, field_access, p): + if not isinstance(field_access, FieldAccess): + return field_access + if field_access.name.simple_name != 'path': + return field_access + if field_access.type is None: + errors.append("FieldAccess.type is None for os.path") + return field_access + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + import os + x = os.path + """, + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) diff --git a/rewrite-python/rewrite/tests/python/all/tree/for_test.py b/rewrite-python/rewrite/tests/python/all/tree/for_test.py index aad12c0569e..b4357746e71 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/for_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/for_test.py @@ -1,5 +1,17 @@ +import shutil + +import pytest + +from rewrite.java.tree import ForEachLoop, Identifier +from rewrite.python.tree import CompilationUnit +from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python +requires_ty_cli = pytest.mark.skipif( + shutil.which('ty-types') is None, + reason="ty-types CLI is not installed" +) + def test_for(): # language=python @@ -61,3 +73,34 @@ def test_async(): pass """ )) + + +@requires_ty_cli +def test_for_loop_variable_type_attribution(): + """Verify that the loop control variable in 'for x in [1, 2, 3]' has a type.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_for_each_loop(self, for_each, p): + if not isinstance(for_each, ForEachLoop): + return for_each + variable = for_each.control.variable + if isinstance(variable, Identifier): + if variable.type is None: + errors.append("ForEachLoop control variable Identifier.type is None") + return for_each + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + for x in [1, 2, 3]: + pass + """, + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) diff --git a/rewrite-python/rewrite/tests/python/all/tree/import_test.py b/rewrite-python/rewrite/tests/python/all/tree/import_test.py index d3c2623b99a..eb4e5086b98 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/import_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/import_test.py @@ -1,7 +1,19 @@ +import shutil + +import pytest + from rewrite.java import Import +from rewrite.java.support_types import JavaType +from rewrite.java.tree import MethodInvocation from rewrite.python import CompilationUnit +from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python +requires_ty_cli = pytest.mark.skipif( + shutil.which('ty-types') is None, + reason="ty-types CLI is not installed" +) + def _assert_single_j_import(cu: CompilationUnit) -> None: assert len(cu.statements) == 1 @@ -85,3 +97,88 @@ def test_crlf(): import bar """.replace('\n', '\r\n') )) + + +def _check_getcwd_declaring_type(source_file, errors): + """Shared checker: verify getcwd() method_type.declaring_type.fqn == 'os'.""" + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_method_invocation(self, method, p): + if not isinstance(method, MethodInvocation): + return method + if method.name.simple_name != 'getcwd': + return method + if method.method_type is None: + errors.append("MethodInvocation.method_type is None for getcwd()") + else: + if method.method_type.declaring_type is None: + errors.append("method_type.declaring_type is None") + elif method.method_type.declaring_type._fully_qualified_name != 'os': + errors.append( + f"declaring_type fqn is '{method.method_type.declaring_type._fully_qualified_name}', expected 'os'" + ) + return method + + TypeChecker().visit(source_file, None) + + +@requires_ty_cli +def test_qualified_import_type_attribution(): + """import os; os.getcwd() → declaring_type.fqn == 'os'.""" + errors = [] + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + import os + x = os.getcwd() + """, + after_recipe=lambda sf: _check_getcwd_declaring_type(sf, errors), + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) + + +@requires_ty_cli +@pytest.mark.xfail(reason="from-import direct calls do not yet populate declaring_type") +def test_from_import_type_attribution(): + """from os import getcwd; getcwd() → declaring_type.fqn == 'os'.""" + errors = [] + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + from os import getcwd + x = getcwd() + """, + after_recipe=lambda sf: _check_getcwd_declaring_type(sf, errors), + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) + + +@requires_ty_cli +def test_aliased_import_type_attribution(): + """import os as o; o.getcwd() → declaring_type.fqn == 'os'.""" + errors = [] + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + import os as o + x = o.getcwd() + """, + after_recipe=lambda sf: _check_getcwd_declaring_type(sf, errors), + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) + + +@requires_ty_cli +def test_aliased_from_import_type_attribution(): + """from os import getcwd as gwd; gwd() → declaring_type.fqn == 'os'.""" + errors = [] + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + from os import getcwd as gwd + x = gwd() + """, + after_recipe=lambda sf: _check_getcwd_declaring_type(sf, errors), + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) diff --git a/rewrite-python/rewrite/tests/python/all/tree/lambda_test.py b/rewrite-python/rewrite/tests/python/all/tree/lambda_test.py index 2432c5b685e..143949cc1e0 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/lambda_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/lambda_test.py @@ -1,5 +1,17 @@ +import shutil + +import pytest + +from rewrite.java.tree import Assignment, Lambda +from rewrite.python.tree import CompilationUnit +from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python +requires_ty_cli = pytest.mark.skipif( + shutil.which('ty-types') is None, + reason="ty-types CLI is not installed" +) + def test_no_parameters(): # language=python @@ -57,3 +69,31 @@ def test_multiple_complex(): lambda a, b=20, /, c=30: 1 lambda a, b, /, c, *, d, e: 0 ''')) + + +@requires_ty_cli +def test_lambda_type_attribution(): + """Verify that lambda x: x + 1 has a non-None type.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_assignment(self, assignment, p): + if not isinstance(assignment, Assignment): + return assignment + # Check the RHS is a Lambda with a type + if isinstance(assignment.assignment, Lambda): + if assignment.assignment.type is None: + errors.append("Lambda.type is None") + return assignment + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + "l = lambda x: x + 1", + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) diff --git a/rewrite-python/rewrite/tests/python/all/tree/method_declaration_test.py b/rewrite-python/rewrite/tests/python/all/tree/method_declaration_test.py index 818849ad184..4deba301559 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/method_declaration_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/method_declaration_test.py @@ -1,5 +1,18 @@ +import shutil + +import pytest + +from rewrite.java.support_types import JavaType +from rewrite.java.tree import MethodDeclaration +from rewrite.python.tree import CompilationUnit +from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python +requires_ty_cli = pytest.mark.skipif( + shutil.which('ty-types') is None, + reason="ty-types CLI is not installed" +) + def test_whitespace_before_colon(): # language=python @@ -152,3 +165,54 @@ def f( return x """ )) + + +@requires_ty_cli +def test_method_declaration_type_attribution(): + """Verify method_type on a function with typed parameters and return type.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_method_declaration(self, method, p): + if not isinstance(method, MethodDeclaration): + return method + if method.name.simple_name != 'foo': + return method + if method.method_type is None: + errors.append("MethodDeclaration.method_type is None") + else: + mt = method.method_type + if mt._return_type is None: + errors.append("method_type.return_type is None") + elif mt._return_type != JavaType.Primitive.Boolean: + errors.append(f"method_type.return_type is {mt._return_type}, expected Primitive.Boolean") + if mt._parameter_types is None: + errors.append("method_type.parameter_types is None") + elif len(mt._parameter_types) < 2: + errors.append(f"method_type.parameter_types has {len(mt._parameter_types)} elements, expected at least 2") + else: + if mt._parameter_types[0] != JavaType.Primitive.Int: + errors.append(f"parameter_types[0] is {mt._parameter_types[0]}, expected Primitive.Int") + if mt._parameter_types[1] != JavaType.Primitive.String: + errors.append(f"parameter_types[1] is {mt._parameter_types[1]}, expected Primitive.String") + if mt._parameter_names is not None and len(mt._parameter_names) >= 2: + if mt._parameter_names[0] != 'a': + errors.append(f"parameter_names[0] is '{mt._parameter_names[0]}', expected 'a'") + if mt._parameter_names[1] != 'b': + errors.append(f"parameter_names[1] is '{mt._parameter_names[1]}', expected 'b'") + return method + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + def foo(a: int, b: str) -> bool: + return True + """, + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) diff --git a/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py b/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py index a7e9066f16e..690eebf10fb 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py @@ -1,7 +1,18 @@ +import shutil + import pytest +from rewrite.java.support_types import JavaType +from rewrite.java.tree import MethodInvocation +from rewrite.python.tree import CompilationUnit +from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python +requires_ty_cli = pytest.mark.skipif( + shutil.which('ty-types') is None, + reason="ty-types CLI is not installed" +) + def test_no_select(): # language=python @@ -84,3 +95,115 @@ def test_keyword_argument(): def test_no_name(): # language=python RecipeSpec().rewrite_run(python("v = (a)()")) + + +@requires_ty_cli +def test_builtin_function_type_attribution(): + """Verify type attribution on a builtin function call like len().""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_method_invocation(self, method, p): + if not isinstance(method, MethodInvocation): + return method + if method.name.simple_name != 'len': + return method + if method.method_type is None: + errors.append("MethodInvocation.method_type is None for len()") + else: + if method.method_type.name != 'len': + errors.append(f"method_type.name is '{method.method_type.name}', expected 'len'") + if method.method_type._return_type is None: + errors.append("method_type.return_type is None") + elif method.method_type._return_type != JavaType.Primitive.Int: + errors.append(f"method_type.return_type is {method.method_type._return_type}, expected Primitive.Int") + return method + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + 'x = len("hello")', + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) + + +@requires_ty_cli +def test_string_method_type_attribution(): + """Verify type attribution on a string method call like str.upper().""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_method_invocation(self, method, p): + if not isinstance(method, MethodInvocation): + return method + if method.name.simple_name != 'upper': + return method + if method.method_type is None: + errors.append("MethodInvocation.method_type is None for upper()") + else: + if method.method_type.declaring_type is not None: + dt = method.method_type.declaring_type + if dt._fully_qualified_name != 'str': + errors.append(f"declaring_type fqn is '{dt._fully_qualified_name}', expected 'str'") + if method.method_type._return_type is None: + errors.append("method_type.return_type is None") + elif method.method_type._return_type != JavaType.Primitive.String: + errors.append(f"method_type.return_type is {method.method_type._return_type}, expected Primitive.String") + return method + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + 'x = "hello".upper()', + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) + + +@requires_ty_cli +def test_stdlib_function_type_attribution(): + """Verify type attribution on a stdlib function call like os.getcwd().""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_method_invocation(self, method, p): + if not isinstance(method, MethodInvocation): + return method + if method.name.simple_name != 'getcwd': + return method + if method.method_type is None: + errors.append("MethodInvocation.method_type is None for os.getcwd()") + else: + if method.method_type.declaring_type is not None: + dt = method.method_type.declaring_type + if dt._fully_qualified_name != 'os': + errors.append(f"declaring_type fqn is '{dt._fully_qualified_name}', expected 'os'") + if method.method_type._return_type is None: + errors.append("method_type.return_type is None") + elif method.method_type._return_type != JavaType.Primitive.String: + errors.append(f"method_type.return_type is {method.method_type._return_type}, expected Primitive.String") + return method + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + import os + x = os.getcwd() + """, + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) diff --git a/rewrite-python/rewrite/tests/python/all/tree/ternary_test.py b/rewrite-python/rewrite/tests/python/all/tree/ternary_test.py index db7bd77b8be..1a1b33dd65c 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/ternary_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/ternary_test.py @@ -1,6 +1,47 @@ +import shutil + +import pytest + +from rewrite.java.support_types import JavaType +from rewrite.java.tree import Ternary +from rewrite.python.tree import CompilationUnit +from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python +requires_ty_cli = pytest.mark.skipif( + shutil.which('ty-types') is None, + reason="ty-types CLI is not installed" +) + def test_simple(): # language=python RecipeSpec().rewrite_run(python("assert True if True else False")) + + +@requires_ty_cli +def test_ternary_type_attribution(): + """Verify that '1 if True else 2' has type Int.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_ternary(self, ternary, p): + if not isinstance(ternary, Ternary): + return ternary + if ternary.type is None: + errors.append("Ternary.type is None") + elif ternary.type != JavaType.Primitive.Int: + errors.append(f"Ternary.type is {ternary.type}, expected Primitive.Int") + return ternary + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + "x = 1 if True else 2", + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) diff --git a/rewrite-python/rewrite/tests/python/all/tree/type_hint_test.py b/rewrite-python/rewrite/tests/python/all/tree/type_hint_test.py index a75255c332d..4cdd939dd03 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/type_hint_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/type_hint_test.py @@ -1,5 +1,20 @@ +import shutil + +import pytest + +from rewrite.java.support_types import JavaType +from rewrite.java.tree import VariableDeclarations +from rewrite.python.tree import CompilationUnit +from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python +Parameterized = JavaType.Parameterized + +requires_ty_cli = pytest.mark.skipif( + shutil.which('ty-types') is None, + reason="ty-types CLI is not installed" +) + def test_primitive_type_hint(): # language=python @@ -173,3 +188,127 @@ def f(x: "Union[str]") -> None: ... ''' )) + + +@requires_ty_cli +def test_list_int_param_type_attribution(): + """Verify List[int] parameter type is Parameterized with base list and type param Int.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_variable_declarations(self, var_decls, p): + if not isinstance(var_decls, VariableDeclarations): + return var_decls + # Look for the parameter 'n' with type hint List[int] + for v in var_decls.variables: + if v.name.simple_name != 'n': + continue + vt = var_decls.type_expression + if vt is None: + continue + t = vt.type if hasattr(vt, 'type') else None + if t is None: + errors.append("List[int] parameter type is None") + elif isinstance(t, Parameterized): + if not t._type._fully_qualified_name.startswith('list'): + errors.append(f"Parameterized base fqn is '{t._type._fully_qualified_name}', expected to start with 'list'") + elif isinstance(t, JavaType.Class): + if not t._fully_qualified_name.startswith('list'): + errors.append(f"Class fqn is '{t._fully_qualified_name}', expected to start with 'list'") + return var_decls + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + from typing import List + + def test(n: List[int]): + return n[0] + 1 + """, + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) + + +@requires_ty_cli +def test_dict_str_int_type_attribution(): + """Verify Dict[str, int] variable type is Parameterized with base dict.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_variable_declarations(self, var_decls, p): + if not isinstance(var_decls, VariableDeclarations): + return var_decls + for v in var_decls.variables: + if v.name.simple_name != 'foo': + continue + vt = var_decls.type_expression + if vt is None: + continue + t = vt.type if hasattr(vt, 'type') else None + if t is None: + errors.append("Dict[str, int] variable type is None") + elif isinstance(t, Parameterized): + if t._type._fully_qualified_name != 'dict': + errors.append(f"Parameterized base fqn is '{t._type._fully_qualified_name}', expected 'dict'") + elif isinstance(t, JavaType.Class): + if t._fully_qualified_name != 'dict': + errors.append(f"Class fqn is '{t._fully_qualified_name}', expected 'dict'") + return var_decls + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + from typing import Dict + foo: Dict[str, int] = {} + """, + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) + + +@requires_ty_cli +def test_optional_str_type_attribution(): + """Verify Optional[str] variable type resolves to str or a union containing str.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_variable_declarations(self, var_decls, p): + if not isinstance(var_decls, VariableDeclarations): + return var_decls + for v in var_decls.variables: + if v.name.simple_name != 'foo': + continue + vt = var_decls.type_expression + if vt is None: + continue + t = vt.type if hasattr(vt, 'type') else None + if t is None: + errors.append("Optional[str] variable type is None") + # Accept any non-None type (could be str, union, etc.) + return var_decls + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + from typing import Optional + foo: Optional[str] = None + """, + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) diff --git a/rewrite-python/rewrite/tests/python/all/tree/unary_test.py b/rewrite-python/rewrite/tests/python/all/tree/unary_test.py index 8b5da7efe0d..3cd818b0afb 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/unary_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/unary_test.py @@ -1,7 +1,18 @@ +import shutil + import pytest +from rewrite.java.support_types import JavaType +from rewrite.java.tree import Unary +from rewrite.python.tree import CompilationUnit +from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python +requires_ty_cli = pytest.mark.skipif( + shutil.which('ty-types') is None, + reason="ty-types CLI is not installed" +) + def test_bool_ops(): # language=python @@ -15,3 +26,33 @@ def test_arithmetic_ops(): RecipeSpec().rewrite_run(python("assert -1")) # language=python RecipeSpec().rewrite_run(python("assert ~1")) + + +@requires_ty_cli +def test_not_type_attribution(): + """Verify that 'not True' has type Boolean.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_unary(self, unary, p): + if not isinstance(unary, Unary): + return unary + if unary.operator != Unary.Type.Not: + return unary + if unary.type is None: + errors.append("Unary(Not).type is None") + elif unary.type != JavaType.Primitive.Boolean: + errors.append(f"Unary(Not).type is {unary.type}, expected Primitive.Boolean") + return unary + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + "x = not True", + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) diff --git a/rewrite-python/rewrite/tests/python/test_type_attribution.py b/rewrite-python/rewrite/tests/python/test_type_attribution.py index 88cb542c56f..3db15a340cd 100644 --- a/rewrite-python/rewrite/tests/python/test_type_attribution.py +++ b/rewrite-python/rewrite/tests/python/test_type_attribution.py @@ -1137,6 +1137,27 @@ def test_default_param_still_listed(self): _cleanup_mapping(mapping, tmpdir, client) +@requires_ty_types_cli +class TestTypingAliases: + """Tests for typing module type aliases (e.g. typing.Text → str).""" + + def test_typing_text_resolves_to_str(self): + """typing.Text (deprecated alias for str) should resolve to Primitive.String. + + ty-types resolves typing.Text to str at the type level, so our type_mapping + receives className='str' and maps it to JavaType.Primitive.String automatically. + """ + source = 'from typing import Text\nx: Text = "hello"\nx\n' + mapping, tree, tmpdir, client = _make_mapping(source) + try: + name_node = tree.body[2].value # bare 'x' expression + result = mapping.type(name_node) + assert result == JavaType.Primitive.String, \ + f"typing.Text should resolve to Primitive.String, got {result}" + finally: + _cleanup_mapping(mapping, tmpdir, client) + + class TestCyclicTypeResolution: """Tests that cyclic type references don't cause infinite recursion.""" From 4019cb1d7aa4527a8b2175df7f97a7360568ca8a Mon Sep 17 00:00:00 2001 From: Knut Wannheden Date: Mon, 23 Feb 2026 11:50:28 +0100 Subject: [PATCH 2/6] Add parameter field_type attribution and call-site type arguments - Add param_type_info() to PythonTypeMapping for function parameter Identifier nodes to get JavaType.Variable field_type - Update __convert_name() and map_arg() to flow field_type through to J.Identifier and NamedVariable - Use call signature returnTypeId for call-site-specific return types (e.g. int for identity(42) instead of generic T) - Populate _declared_formal_type_names on method invocation types from function descriptor type parameters - Bump ty-types to 0.0.19.dev20260223102528 for callSignature typeArguments/returnTypeId support --- rewrite-python/rewrite/pyproject.toml | 2 +- .../src/rewrite/python/_parser_visitor.py | 12 +-- .../src/rewrite/python/type_mapping.py | 75 ++++++++++++++++- .../tests/python/all/tree/class_test.py | 51 +++++++++++- .../all/tree/method_declaration_test.py | 81 ++++++++++++++++++- .../python/all/tree/method_invocation_test.py | 42 ++++++++++ 6 files changed, 252 insertions(+), 11 deletions(-) diff --git a/rewrite-python/rewrite/pyproject.toml b/rewrite-python/rewrite/pyproject.toml index 1d0bc3a97a6..3261de4185e 100644 --- a/rewrite-python/rewrite/pyproject.toml +++ b/rewrite-python/rewrite/pyproject.toml @@ -24,7 +24,7 @@ requires-python = ">=3.10" dependencies = [ "cbor2>=5.6.5", "more_itertools>=10.0.0", - "ty-types>=0.0.19.dev20260223093555", # Type inference CLI for Python type attribution + "ty-types>=0.0.19.dev20260223102528", # Type inference CLI for Python type attribution "parso>=0.7.1,<0.8", # Python 2/3 parser with CST support (0.8+ dropped Python 2.7 grammar) ] diff --git a/rewrite-python/rewrite/src/rewrite/python/_parser_visitor.py b/rewrite-python/rewrite/src/rewrite/python/_parser_visitor.py index 2520c50f8a8..4b7efd6b28f 100644 --- a/rewrite-python/rewrite/src/rewrite/python/_parser_visitor.py +++ b/rewrite-python/rewrite/src/rewrite/python/_parser_visitor.py @@ -338,7 +338,8 @@ def visit_arguments(self, node, with_close_paren: bool = True) -> List[JRightPad def map_arg(self, node, default=None, vararg=False, kwarg=False): prefix = self.__source_before('**') if kwarg else self.__whitespace() vararg_prefix = self.__source_before('*') if vararg else None - name = self.__convert_name(node.arg, self._type_mapping.type(node)) + expr_type, field_type = self._type_mapping.param_type_info(node) + name = self.__convert_name(node.arg, expr_type, field_type) after_name = self.__source_before(':') if node.annotation else Space.EMPTY type_expression = self.__convert_type(node.annotation) if node.annotation else None initializer = self.__pad_left(self.__source_before('='), self.__convert(default)) if default else None @@ -359,7 +360,7 @@ def map_arg(self, node, default=None, vararg=False, kwarg=False): cast(j.Identifier, name), [], initializer, - self.__as_variable_type(self._type_mapping.type(node)) + field_type ), after_name)], ) @@ -3039,12 +3040,13 @@ def __as_method_type(t: Optional[JavaType]) -> Optional[JavaType]: return t return None - def __convert_name(self, name: str, name_type: Optional[JavaType] = None) -> NameTree: + def __convert_name(self, name: str, name_type: Optional[JavaType] = None, + field_type: Optional[JavaType.Variable] = None) -> NameTree: def ident_or_field(parts: List[str]) -> NameTree: if len(parts) == 1: space, actual_name = self.__consume_identifier(parts[-1]) return j.Identifier(random_id(), space, Markers.EMPTY, [], actual_name, - name_type, None) + name_type, field_type) else: return j.FieldAccess( random_id(), @@ -3055,7 +3057,7 @@ def ident_or_field(parts: List[str]) -> NameTree: self.__source_before('.'), (lambda s, n: j.Identifier(random_id(), s, Markers.EMPTY, [], n, name_type, - None))(*self.__consume_identifier(parts[-1])), + field_type))(*self.__consume_identifier(parts[-1])), ), name_type ) diff --git a/rewrite-python/rewrite/src/rewrite/python/type_mapping.py b/rewrite-python/rewrite/src/rewrite/python/type_mapping.py index 102ef1f17ca..d5c30db986f 100644 --- a/rewrite-python/rewrite/src/rewrite/python/type_mapping.py +++ b/rewrite-python/rewrite/src/rewrite/python/type_mapping.py @@ -477,6 +477,17 @@ def _descriptor_to_java_type(self, descriptor: Dict[str, Any]) -> Optional[JavaT if interfaces: class_type._interfaces = interfaces + # Populate type parameters from typeVar descriptors + type_params = descriptor.get('typeParameters', []) + if type_params and getattr(class_type, '_type_parameters', None) is None: + resolved_type_params = [] + for tp_id in type_params: + tp_type = self._resolve_type(tp_id) + if tp_type is not None: + resolved_type_params.append(tp_type) + if resolved_type_params: + class_type._type_parameters = resolved_type_params + # Populate methods from function/boundMethod members members = descriptor.get('members', []) if members and getattr(class_type, '_methods', None) is None: @@ -533,6 +544,10 @@ def _descriptor_to_java_type(self, descriptor: Dict[str, Any]) -> Optional[JavaT elif kind == 'property': return _UNKNOWN + elif kind == 'typeVar': + name = descriptor.get('name', '') + return self._create_class_type(name) if name else _UNKNOWN + else: return _UNKNOWN @@ -601,6 +616,16 @@ def name_type_info(self, node: ast.Name) -> Tuple[Optional[JavaType], Optional[J return expr_type, JavaType.Variable(_name=node.id, _type=expr_type) return expr_type, None + def param_type_info(self, node: ast.arg) -> Tuple[Optional[JavaType], Optional[JavaType.Variable]]: + """Get expression type and variable type for a function parameter. + + Returns (expression_type, variable_field_type). + """ + expr_type = self.type(node) + if expr_type is None: + return None, None + return expr_type, JavaType.Variable(_name=node.arg, _type=expr_type) + def attribute_type_info(self, node: ast.Attribute, receiver_type: Optional[JavaType] = None ) -> Tuple[Optional[JavaType], Optional[JavaType.Variable]]: @@ -660,7 +685,13 @@ def method_declaration_type(self, node: ast.FunctionDef) -> Optional[JavaType.Me if node.returns is not None: return_type = self.type(node.returns) - if not param_names and return_type is None: + # Extract type parameter names from Python 3.12+ type_params + type_param_names: List[str] = [] + for tp in getattr(node, 'type_params', []) or []: + if hasattr(tp, 'name'): + type_param_names.append(tp.name) + + if not param_names and return_type is None and not type_param_names: return None return JavaType.Method( @@ -670,6 +701,7 @@ def method_declaration_type(self, node: ast.FunctionDef) -> Optional[JavaType.Me _return_type=return_type, _parameter_names=param_names if param_names else None, _parameter_types=param_types if param_types else None, + _declared_formal_type_names=type_param_names if type_param_names else None, ) def _method_from_function_descriptor( @@ -690,6 +722,8 @@ def _method_from_function_descriptor( if ret_id is not None: return_type = self._resolve_type(ret_id) + type_param_names = self._extract_type_param_names(descriptor) + return JavaType.Method( _flags_bit_map=0, _declaring_type=None, @@ -697,6 +731,7 @@ def _method_from_function_descriptor( _return_type=return_type, _parameter_names=param_names if param_names else None, _parameter_types=param_types if param_types else None, + _declared_formal_type_names=type_param_names if type_param_names else None, ) def method_invocation_type(self, node: ast.Call) -> Optional[JavaType.Method]: @@ -729,6 +764,14 @@ def method_invocation_type(self, node: ast.Call) -> Optional[JavaType.Method]: # Get return type return_type = self._get_return_type(node) + # Extract type parameter names from function descriptor + type_param_names: List[str] = [] + func_type_id = self._lookup_func_type_id(node) + if func_type_id is not None: + func_desc = self._type_registry.get(func_type_id) + if func_desc: + type_param_names = self._extract_type_param_names(func_desc) + return JavaType.Method( _flags_bit_map=0, _declaring_type=declaring_type, @@ -736,6 +779,7 @@ def method_invocation_type(self, node: ast.Call) -> Optional[JavaType.Method]: _return_type=return_type, _parameter_names=param_names if param_names else None, _parameter_types=param_types if param_types else None, + _declared_formal_type_names=type_param_names if type_param_names else None, ) def _extract_method_name(self, node: ast.Call) -> Optional[str]: @@ -1037,9 +1081,20 @@ def _get_parameter_types(self, node: ast.Call) -> Optional[List[JavaType]]: def _get_return_type(self, node: ast.Call) -> Optional[JavaType]: """Get the return type of a method call. - First tries the ExprCall node type (which IS the return type), - then falls back to the function descriptor's returnType field. + Prefers the call-site-specific returnTypeId from the call signature + (which gives resolved types like int instead of generic T), + then tries the ExprCall node type, then falls back to the function + descriptor's returnType field. """ + # Prefer call-site-specific return type from call signature + sig = self._lookup_call_signature(node) + if sig: + ret_id = sig.get('returnTypeId') + if ret_id is not None: + result = self._resolve_type(ret_id) + if result is not None: + return result + # The type of an ExprCall node in ty-types IS the return type type_id = self._lookup_type_id(node) if type_id is not None: @@ -1083,6 +1138,8 @@ def _create_method_from_descriptor(self, descriptor: Dict[str, Any], else: param_types.append(_UNKNOWN) + type_param_names = self._extract_type_param_names(descriptor) + return JavaType.Method( _flags_bit_map=0, _declaring_type=declaring_type, @@ -1090,8 +1147,20 @@ def _create_method_from_descriptor(self, descriptor: Dict[str, Any], _return_type=return_type, _parameter_names=param_names if param_names else None, _parameter_types=param_types if param_types else None, + _declared_formal_type_names=type_param_names if type_param_names else None, ) + def _extract_type_param_names(self, descriptor: Dict[str, Any]) -> List[str]: + """Extract type parameter names from a descriptor's typeParameters list.""" + names: List[str] = [] + for tp_id in descriptor.get('typeParameters', []): + tp_desc = self._type_registry.get(tp_id) + if tp_desc and tp_desc.get('kind') == 'typeVar': + name = tp_desc.get('name', '') + if name: + names.append(name) + return names + def _create_class_type(self, fqn: str) -> JavaType.Class: """Create a JavaType.Class from a fully qualified name.""" if fqn in self._type_cache: diff --git a/rewrite-python/rewrite/tests/python/all/tree/class_test.py b/rewrite-python/rewrite/tests/python/all/tree/class_test.py index 99afd6caa1e..655081daf00 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/class_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/class_test.py @@ -3,7 +3,7 @@ import pytest from rewrite.java.support_types import JavaType -from rewrite.java.tree import Assignment +from rewrite.java.tree import Assignment, Identifier from rewrite.python.tree import CompilationUnit from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python @@ -125,6 +125,55 @@ class C3(Generic[T], metaclass=type, *[str]): )) +@requires_ty_cli +def test_generic_class_type_params(): + """Verify type parameters on a generic class like class Box[T].""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_assignment(self, assignment, p): + if not isinstance(assignment, Assignment): + return assignment + # Only check the `x = Box(42)` assignment + if not isinstance(assignment.variable, Identifier) or assignment.variable.simple_name != 'x': + return assignment + if assignment.type is None: + errors.append("Assignment.type is None for x = Box(42)") + elif isinstance(assignment.type, JavaType.Class): + if assignment.type._fully_qualified_name != 'Box': + errors.append(f"Assignment.type fqn is '{assignment.type._fully_qualified_name}', expected 'Box'") + type_params = getattr(assignment.type, '_type_parameters', None) + if type_params is None: + errors.append("Box class type has no _type_parameters") + elif len(type_params) != 1: + errors.append(f"Box class type has {len(type_params)} type params, expected 1") + else: + tp = type_params[0] + if isinstance(tp, JavaType.Class): + if tp._fully_qualified_name != 'T': + errors.append(f"type_parameter fqn is '{tp._fully_qualified_name}', expected 'T'") + else: + errors.append(f"type_parameter is {type(tp).__name__}, expected Class") + return assignment + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + class Box[T]: + def __init__(self, value: T) -> None: + self.value = value + x = Box(42) + """, + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) + + @requires_ty_cli def test_class_instance_type_attribution(): """Verify that x = Foo() assigns a type with fqn 'Foo'.""" diff --git a/rewrite-python/rewrite/tests/python/all/tree/method_declaration_test.py b/rewrite-python/rewrite/tests/python/all/tree/method_declaration_test.py index 4deba301559..7de1841b90c 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/method_declaration_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/method_declaration_test.py @@ -3,7 +3,7 @@ import pytest from rewrite.java.support_types import JavaType -from rewrite.java.tree import MethodDeclaration +from rewrite.java.tree import MethodDeclaration, VariableDeclarations from rewrite.python.tree import CompilationUnit from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python @@ -167,6 +167,45 @@ def f( )) +@requires_ty_cli +def test_generic_function_type_params(): + """Verify method_type.declared_formal_type_names for def identity[T](x: T) -> T.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_method_declaration(self, method, p): + if not isinstance(method, MethodDeclaration): + return method + if method.name.simple_name != 'identity': + return method + if method.method_type is None: + errors.append("MethodDeclaration.method_type is None") + else: + mt = method.method_type + if mt._declared_formal_type_names is None: + errors.append("method_type._declared_formal_type_names is None") + elif mt._declared_formal_type_names != ['T']: + errors.append(f"method_type._declared_formal_type_names is {mt._declared_formal_type_names}, expected ['T']") + if mt._parameter_names is not None and 'x' not in mt._parameter_names: + errors.append(f"parameter_names {mt._parameter_names} does not contain 'x'") + return method + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + def identity[T](x: T) -> T: + return x + """, + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) + + @requires_ty_cli def test_method_declaration_type_attribution(): """Verify method_type on a function with typed parameters and return type.""" @@ -216,3 +255,43 @@ def foo(a: int, b: str) -> bool: after_recipe=check_types, )) assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) + + +@requires_ty_cli +def test_param_identifier_field_type(): + """Verify J.Identifier.field_type is JavaType.Variable for typed parameters.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_variable_declarations(self, vd, p): + if not isinstance(vd, VariableDeclarations): + return vd + for named_var in vd.variables: + ident = named_var.name + if ident.simple_name not in ('x', 'y'): + continue + if ident.field_type is None: + errors.append(f"Identifier '{ident.simple_name}' has field_type=None") + elif not isinstance(ident.field_type, JavaType.Variable): + errors.append(f"Identifier '{ident.simple_name}' field_type is {type(ident.field_type)}, expected JavaType.Variable") + else: + if ident.field_type._name != ident.simple_name: + errors.append(f"field_type._name is '{ident.field_type._name}', expected '{ident.simple_name}'") + if ident.field_type._type is None: + errors.append(f"field_type._type is None for '{ident.simple_name}'") + return vd + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + def greet(x: int, y: str) -> bool: + return True + """, + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) diff --git a/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py b/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py index 690eebf10fb..54aa1fcf903 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py @@ -207,3 +207,45 @@ def visit_method_invocation(self, method, p): after_recipe=check_types, )) assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) + + +@requires_ty_cli +def test_generic_call_site_return_type(): + """Verify method_invocation_type returns call-site-specific return type for generic functions.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_method_invocation(self, method, p): + if not isinstance(method, MethodInvocation): + return method + if method.name.simple_name != 'identity': + return method + if method.method_type is None: + errors.append("MethodInvocation.method_type is None for identity()") + else: + mt = method.method_type + if mt._return_type is None: + errors.append("method_type.return_type is None") + elif mt._return_type != JavaType.Primitive.Int: + errors.append(f"method_type.return_type is {mt._return_type}, expected Primitive.Int") + if mt._declared_formal_type_names is None: + errors.append("method_type._declared_formal_type_names is None") + elif 'T' not in mt._declared_formal_type_names: + errors.append(f"method_type._declared_formal_type_names is {mt._declared_formal_type_names}, expected to contain 'T'") + return method + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + def identity[T](x: T) -> T: + return x + result = identity(42) + """, + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) From d3114779c012e8a25ac0b49fbdf40e915f154022 Mon Sep 17 00:00:00 2001 From: Knut Wannheden Date: Mon, 23 Feb 2026 13:34:07 +0100 Subject: [PATCH 3/6] Use moduleName from ty-types for module-qualified FQNs classLiteral types now use moduleName to build proper FQNs (e.g. mymodule.MyClass instead of bare MyClass), and bare function calls get a declaring type from their module. Bumps ty-types to 0.0.19.dev20260223122104. --- rewrite-python/rewrite/pyproject.toml | 2 +- .../src/rewrite/python/type_mapping.py | 11 ++- .../tests/python/all/tree/class_test.py | 64 +++++++++++++++++- .../tests/python/all/tree/import_test.py | 1 - .../python/all/tree/method_invocation_test.py | 67 +++++++++++++++++++ 5 files changed, 141 insertions(+), 4 deletions(-) diff --git a/rewrite-python/rewrite/pyproject.toml b/rewrite-python/rewrite/pyproject.toml index 3261de4185e..674cfb39185 100644 --- a/rewrite-python/rewrite/pyproject.toml +++ b/rewrite-python/rewrite/pyproject.toml @@ -24,7 +24,7 @@ requires-python = ">=3.10" dependencies = [ "cbor2>=5.6.5", "more_itertools>=10.0.0", - "ty-types>=0.0.19.dev20260223102528", # Type inference CLI for Python type attribution + "ty-types>=0.0.19.dev20260223122104", # Type inference CLI for Python type attribution "parso>=0.7.1,<0.8", # Python 2/3 parser with CST support (0.8+ dropped Python 2.7 grammar) ] diff --git a/rewrite-python/rewrite/src/rewrite/python/type_mapping.py b/rewrite-python/rewrite/src/rewrite/python/type_mapping.py index d5c30db986f..250b23a8638 100644 --- a/rewrite-python/rewrite/src/rewrite/python/type_mapping.py +++ b/rewrite-python/rewrite/src/rewrite/python/type_mapping.py @@ -446,7 +446,12 @@ def _descriptor_to_java_type(self, descriptor: Dict[str, Any]) -> Optional[JavaT elif kind == 'classLiteral': class_name = descriptor.get('className', '') - class_type = self._create_class_type(class_name) + module_name = descriptor.get('moduleName') + if module_name and module_name != 'builtins': + fqn = f"{module_name}.{class_name}" + else: + fqn = class_name + class_type = self._create_class_type(fqn) # Infer Kind from supertypes before resolving them supertypes = descriptor.get('supertypes', []) @@ -881,6 +886,10 @@ def _get_declaring_type(self, node: ast.Call) -> Optional[JavaType.FullyQualifie kind = descriptor.get('kind') if kind == 'module': return self._create_class_type(descriptor.get('moduleName', '')) + elif kind in ('function', 'boundMethod'): + module_name = descriptor.get('moduleName') + if module_name and module_name != 'builtins': + return self._create_class_type(module_name) return self._infer_declaring_type_from_ast(node) diff --git a/rewrite-python/rewrite/tests/python/all/tree/class_test.py b/rewrite-python/rewrite/tests/python/all/tree/class_test.py index 655081daf00..d355b463536 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/class_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/class_test.py @@ -1,9 +1,12 @@ +import json import shutil +import subprocess +import tempfile import pytest from rewrite.java.support_types import JavaType -from rewrite.java.tree import Assignment, Identifier +from rewrite.java.tree import Assignment, ClassDeclaration, Identifier from rewrite.python.tree import CompilationUnit from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python @@ -14,6 +17,30 @@ ) +def _ty_types_has_module_name() -> bool: + """Check if the installed ty-types CLI provides moduleName on classLiteral descriptors.""" + if shutil.which('ty-types') is None: + return False + try: + with tempfile.NamedTemporaryFile(mode='w', suffix='.py', delete=False) as f: + f.write('class _C:\n pass\n') + fname = f.name + result = subprocess.run(['ty-types', fname], capture_output=True, text=True, timeout=30) + data = json.loads(result.stdout) + return any( + d.get('kind') == 'classLiteral' and 'moduleName' in d + for d in data.get('types', {}).values() + ) + except Exception: + return False + + +requires_module_name = pytest.mark.skipif( + not _ty_types_has_module_name(), + reason="ty-types CLI does not provide moduleName on classLiteral descriptors" +) + + def test_empty(): # language=python RecipeSpec().rewrite_run(python( @@ -174,6 +201,41 @@ def __init__(self, value: T) -> None: assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) +@requires_module_name +def test_class_literal_module_qualified_fqn(): + """Verify that a class defined in a module gets a module-qualified FQN on its classLiteral type.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_class_declaration(self, class_decl, p): + if not isinstance(class_decl, ClassDeclaration): + return class_decl + cd_type = class_decl.type + if cd_type is None: + errors.append("ClassDeclaration.type is None for MyClass") + elif isinstance(cd_type, JavaType.Class): + fqn = cd_type._fully_qualified_name + # The FQN should contain a module prefix (not just bare 'MyClass') + if '.' not in fqn: + errors.append(f"ClassDeclaration.type fqn '{fqn}' has no module prefix, expected '.MyClass'") + return class_decl + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + class MyClass: + pass + """, + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) + + @requires_ty_cli def test_class_instance_type_attribution(): """Verify that x = Foo() assigns a type with fqn 'Foo'.""" diff --git a/rewrite-python/rewrite/tests/python/all/tree/import_test.py b/rewrite-python/rewrite/tests/python/all/tree/import_test.py index eb4e5086b98..998bc6c4b36 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/import_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/import_test.py @@ -139,7 +139,6 @@ def test_qualified_import_type_attribution(): @requires_ty_cli -@pytest.mark.xfail(reason="from-import direct calls do not yet populate declaring_type") def test_from_import_type_attribution(): """from os import getcwd; getcwd() → declaring_type.fqn == 'os'.""" errors = [] diff --git a/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py b/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py index 54aa1fcf903..414c36e4b55 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py @@ -1,4 +1,7 @@ +import json import shutil +import subprocess +import tempfile import pytest @@ -14,6 +17,30 @@ ) +def _ty_types_has_module_name() -> bool: + """Check if the installed ty-types CLI provides moduleName on function descriptors.""" + if shutil.which('ty-types') is None: + return False + try: + with tempfile.NamedTemporaryFile(mode='w', suffix='.py', delete=False) as f: + f.write('from os.path import join\njoin("a", "b")\n') + fname = f.name + result = subprocess.run(['ty-types', fname], capture_output=True, text=True, timeout=30) + data = json.loads(result.stdout) + return any( + d.get('kind') == 'function' and 'moduleName' in d + for d in data.get('types', {}).values() + ) + except Exception: + return False + + +requires_module_name = pytest.mark.skipif( + not _ty_types_has_module_name(), + reason="ty-types CLI does not provide moduleName on function descriptors" +) + + def test_no_select(): # language=python RecipeSpec().rewrite_run(python("assert len('a')")) @@ -249,3 +276,43 @@ def identity[T](x: T) -> T: after_recipe=check_types, )) assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) + + +@requires_module_name +def test_bare_function_declaring_type_has_module(): + """Verify that a bare function call imported from a module gets a declaring type with the module name.""" + errors = [] + + def check_types(source_file): + assert isinstance(source_file, CompilationUnit) + + class TypeChecker(PythonVisitor): + def visit_method_invocation(self, method, p): + if not isinstance(method, MethodInvocation): + return method + if method.name.simple_name != 'join': + return method + if method.method_type is None: + errors.append("MethodInvocation.method_type is None for join()") + else: + dt = method.method_type.declaring_type + if dt is None: + errors.append("method_type.declaring_type is None for join()") + elif not hasattr(dt, '_fully_qualified_name') or 'posixpath' not in dt._fully_qualified_name: + errors.append( + f"declaring_type fqn is '{getattr(dt, '_fully_qualified_name', '?')}', " + f"expected to contain 'posixpath' (os.path module)" + ) + return method + + TypeChecker().visit(source_file, None) + + # language=python + RecipeSpec(type_attribution=True).rewrite_run(python( + """\ + from os.path import join + x = join("a", "b") + """, + after_recipe=check_types, + )) + assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) From 3ba8fd9f1fb71c12f8723cf3dc714bd6028f1b36 Mon Sep 17 00:00:00 2001 From: Knut Wannheden Date: Mon, 23 Feb 2026 14:11:18 +0100 Subject: [PATCH 4/6] Address review feedback on type attribution tests - Extract _ty_types_has_module_name() into shared _markers.py module to eliminate duplication and reduce subprocess overhead - Fix temp file leak by adding os.unlink() in finally block - Fix test_class_instance_type_attribution to accept module-qualified FQNs (endsWith 'Foo' instead of exact match) - Gate from-import tests with @requires_module_name since bare function calls need moduleName for declaring type resolution - Accept both posixpath and ntpath in os.path test for Windows compat --- .../rewrite/tests/python/all/tree/_markers.py | 39 +++++++++++++++++++ .../tests/python/all/tree/class_test.py | 37 ++++-------------- .../tests/python/all/tree/import_test.py | 6 ++- .../python/all/tree/method_invocation_test.py | 36 ++++------------- 4 files changed, 57 insertions(+), 61 deletions(-) create mode 100644 rewrite-python/rewrite/tests/python/all/tree/_markers.py diff --git a/rewrite-python/rewrite/tests/python/all/tree/_markers.py b/rewrite-python/rewrite/tests/python/all/tree/_markers.py new file mode 100644 index 00000000000..00b49b88949 --- /dev/null +++ b/rewrite-python/rewrite/tests/python/all/tree/_markers.py @@ -0,0 +1,39 @@ +"""Shared pytest markers for type attribution tests.""" +import json +import os +import shutil +import subprocess +import tempfile + +import pytest + + +def _ty_types_has_module_name() -> bool: + """Check if the installed ty-types CLI provides moduleName on descriptors.""" + if shutil.which('ty-types') is None: + return False + fname = None + try: + with tempfile.NamedTemporaryFile(mode='w', suffix='.py', delete=False) as f: + f.write('class _C:\n pass\n') + fname = f.name + result = subprocess.run(['ty-types', fname], capture_output=True, text=True, timeout=30) + data = json.loads(result.stdout) + return any( + 'moduleName' in d + for d in data.get('types', {}).values() + ) + except Exception: + return False + finally: + if fname is not None: + try: + os.unlink(fname) + except OSError: + pass + + +requires_module_name = pytest.mark.skipif( + not _ty_types_has_module_name(), + reason="ty-types CLI does not provide moduleName on descriptors" +) diff --git a/rewrite-python/rewrite/tests/python/all/tree/class_test.py b/rewrite-python/rewrite/tests/python/all/tree/class_test.py index d355b463536..c7f16d5d63c 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/class_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/class_test.py @@ -1,7 +1,4 @@ -import json import shutil -import subprocess -import tempfile import pytest @@ -11,36 +8,14 @@ from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python +from ._markers import requires_module_name + requires_ty_cli = pytest.mark.skipif( shutil.which('ty-types') is None, reason="ty-types CLI is not installed" ) -def _ty_types_has_module_name() -> bool: - """Check if the installed ty-types CLI provides moduleName on classLiteral descriptors.""" - if shutil.which('ty-types') is None: - return False - try: - with tempfile.NamedTemporaryFile(mode='w', suffix='.py', delete=False) as f: - f.write('class _C:\n pass\n') - fname = f.name - result = subprocess.run(['ty-types', fname], capture_output=True, text=True, timeout=30) - data = json.loads(result.stdout) - return any( - d.get('kind') == 'classLiteral' and 'moduleName' in d - for d in data.get('types', {}).values() - ) - except Exception: - return False - - -requires_module_name = pytest.mark.skipif( - not _ty_types_has_module_name(), - reason="ty-types CLI does not provide moduleName on classLiteral descriptors" -) - - def test_empty(): # language=python RecipeSpec().rewrite_run(python( @@ -238,7 +213,7 @@ class MyClass: @requires_ty_cli def test_class_instance_type_attribution(): - """Verify that x = Foo() assigns a type with fqn 'Foo'.""" + """Verify that x = Foo() assigns a type with fqn ending in 'Foo'.""" errors = [] def check_types(source_file): @@ -251,8 +226,10 @@ def visit_assignment(self, assignment, p): if assignment.type is None: errors.append("Assignment.type is None for Foo()") elif isinstance(assignment.type, JavaType.Class): - if assignment.type._fully_qualified_name != 'Foo': - errors.append(f"Assignment.type fqn is '{assignment.type._fully_qualified_name}', expected 'Foo'") + fqn = assignment.type._fully_qualified_name + # Accept 'Foo' or '.Foo' (module-qualified with newer ty-types) + if not fqn.endswith('Foo'): + errors.append(f"Assignment.type fqn is '{fqn}', expected to end with 'Foo'") else: # Accept any non-None type pass diff --git a/rewrite-python/rewrite/tests/python/all/tree/import_test.py b/rewrite-python/rewrite/tests/python/all/tree/import_test.py index 998bc6c4b36..77c7cee3597 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/import_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/import_test.py @@ -9,6 +9,8 @@ from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python +from ._markers import requires_module_name + requires_ty_cli = pytest.mark.skipif( shutil.which('ty-types') is None, reason="ty-types CLI is not installed" @@ -138,7 +140,7 @@ def test_qualified_import_type_attribution(): assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_ty_cli +@requires_module_name def test_from_import_type_attribution(): """from os import getcwd; getcwd() → declaring_type.fqn == 'os'.""" errors = [] @@ -168,7 +170,7 @@ def test_aliased_import_type_attribution(): assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_ty_cli +@requires_module_name def test_aliased_from_import_type_attribution(): """from os import getcwd as gwd; gwd() → declaring_type.fqn == 'os'.""" errors = [] diff --git a/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py b/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py index 414c36e4b55..c23b9273482 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py @@ -1,7 +1,4 @@ -import json import shutil -import subprocess -import tempfile import pytest @@ -11,36 +8,14 @@ from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python +from ._markers import requires_module_name + requires_ty_cli = pytest.mark.skipif( shutil.which('ty-types') is None, reason="ty-types CLI is not installed" ) -def _ty_types_has_module_name() -> bool: - """Check if the installed ty-types CLI provides moduleName on function descriptors.""" - if shutil.which('ty-types') is None: - return False - try: - with tempfile.NamedTemporaryFile(mode='w', suffix='.py', delete=False) as f: - f.write('from os.path import join\njoin("a", "b")\n') - fname = f.name - result = subprocess.run(['ty-types', fname], capture_output=True, text=True, timeout=30) - data = json.loads(result.stdout) - return any( - d.get('kind') == 'function' and 'moduleName' in d - for d in data.get('types', {}).values() - ) - except Exception: - return False - - -requires_module_name = pytest.mark.skipif( - not _ty_types_has_module_name(), - reason="ty-types CLI does not provide moduleName on function descriptors" -) - - def test_no_select(): # language=python RecipeSpec().rewrite_run(python("assert len('a')")) @@ -298,10 +273,13 @@ def visit_method_invocation(self, method, p): dt = method.method_type.declaring_type if dt is None: errors.append("method_type.declaring_type is None for join()") - elif not hasattr(dt, '_fully_qualified_name') or 'posixpath' not in dt._fully_qualified_name: + elif not hasattr(dt, '_fully_qualified_name') or ( + 'posixpath' not in dt._fully_qualified_name and + 'ntpath' not in dt._fully_qualified_name + ): errors.append( f"declaring_type fqn is '{getattr(dt, '_fully_qualified_name', '?')}', " - f"expected to contain 'posixpath' (os.path module)" + f"expected to contain 'posixpath' or 'ntpath' (os.path module)" ) return method From a89d2891b56a04cf90c261a69cf751b3f40fa9e5 Mon Sep 17 00:00:00 2001 From: Knut Wannheden Date: Mon, 23 Feb 2026 14:30:20 +0100 Subject: [PATCH 5/6] Remove skip markers and fix UUID temp filenames for ty-types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since ty-types minimum version is pinned in pyproject.toml, the @requires_ty_cli and @requires_module_name skip markers are unnecessary. Remove them and the shared _markers.py module. Also prefix UUID-based temp filenames with underscore to ensure they are valid Python module names — ty-types omits moduleName when the filename starts with a digit, causing non-deterministic test behavior. --- .../rewrite/src/rewrite/test/rewrite_test.py | 2 +- .../rewrite/tests/python/all/tree/_markers.py | 39 ------------------- .../tests/python/all/tree/assign_test.py | 11 ------ .../tests/python/all/tree/binary_test.py | 12 ------ .../tests/python/all/tree/class_test.py | 15 +------ .../all/tree/collection_literal_test.py | 10 ----- .../rewrite/tests/python/all/tree/def_test.py | 10 ----- .../python/all/tree/field_access_test.py | 10 ----- .../rewrite/tests/python/all/tree/for_test.py | 10 ----- .../tests/python/all/tree/import_test.py | 15 ------- .../tests/python/all/tree/lambda_test.py | 10 ----- .../all/tree/method_declaration_test.py | 12 ------ .../python/all/tree/method_invocation_test.py | 16 -------- .../tests/python/all/tree/ternary_test.py | 10 ----- .../tests/python/all/tree/type_hint_test.py | 12 ------ .../tests/python/all/tree/unary_test.py | 10 ----- .../tests/recipes/test_change_import.py | 9 ----- 17 files changed, 3 insertions(+), 210 deletions(-) delete mode 100644 rewrite-python/rewrite/tests/python/all/tree/_markers.py diff --git a/rewrite-python/rewrite/src/rewrite/test/rewrite_test.py b/rewrite-python/rewrite/src/rewrite/test/rewrite_test.py index 7eb88e5354d..b792f570e7b 100644 --- a/rewrite-python/rewrite/src/rewrite/test/rewrite_test.py +++ b/rewrite-python/rewrite/src/rewrite/test/rewrite_test.py @@ -227,7 +227,7 @@ def _parse(self, specs: List[SourceSpec]) -> List[Tuple[SourceSpec, SourceFile]] continue # Determine source path - source_path = spec.path or Path(f"{uuid4().hex}.{spec.ext}") + source_path = spec.path or Path(f"_{uuid4().hex}.{spec.ext}") # Parse the source source = dedent(spec.before) diff --git a/rewrite-python/rewrite/tests/python/all/tree/_markers.py b/rewrite-python/rewrite/tests/python/all/tree/_markers.py deleted file mode 100644 index 00b49b88949..00000000000 --- a/rewrite-python/rewrite/tests/python/all/tree/_markers.py +++ /dev/null @@ -1,39 +0,0 @@ -"""Shared pytest markers for type attribution tests.""" -import json -import os -import shutil -import subprocess -import tempfile - -import pytest - - -def _ty_types_has_module_name() -> bool: - """Check if the installed ty-types CLI provides moduleName on descriptors.""" - if shutil.which('ty-types') is None: - return False - fname = None - try: - with tempfile.NamedTemporaryFile(mode='w', suffix='.py', delete=False) as f: - f.write('class _C:\n pass\n') - fname = f.name - result = subprocess.run(['ty-types', fname], capture_output=True, text=True, timeout=30) - data = json.loads(result.stdout) - return any( - 'moduleName' in d - for d in data.get('types', {}).values() - ) - except Exception: - return False - finally: - if fname is not None: - try: - os.unlink(fname) - except OSError: - pass - - -requires_module_name = pytest.mark.skipif( - not _ty_types_has_module_name(), - reason="ty-types CLI does not provide moduleName on descriptors" -) diff --git a/rewrite-python/rewrite/tests/python/all/tree/assign_test.py b/rewrite-python/rewrite/tests/python/all/tree/assign_test.py index 7945aa70d1a..8f4d52c6da9 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/assign_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/assign_test.py @@ -1,7 +1,3 @@ -import shutil - -import pytest - from rewrite.java.support_types import JavaType from rewrite.java.tree import Assignment, Identifier, MethodInvocation from rewrite.python.tree import CompilationUnit @@ -10,11 +6,6 @@ Parameterized = JavaType.Parameterized -requires_ty_cli = pytest.mark.skipif( - shutil.which('ty-types') is None, - reason="ty-types CLI is not installed" -) - def test_assign(): # language=python @@ -112,7 +103,6 @@ def test_assign_op(): RecipeSpec().rewrite_run(python("a @= 1")) -@requires_ty_cli def test_assign_type_attribution(): """Verify that type attribution is populated on assignment AST nodes.""" errors = [] @@ -146,7 +136,6 @@ def visit_assignment(self, assignment, p): assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_ty_cli def test_assign_method_call_type_attribution(): """Verify type attribution on an assignment from a method call like str.split().""" errors = [] diff --git a/rewrite-python/rewrite/tests/python/all/tree/binary_test.py b/rewrite-python/rewrite/tests/python/all/tree/binary_test.py index d7a17e06639..9fccfd18c7b 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/binary_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/binary_test.py @@ -1,18 +1,9 @@ -import shutil - -import pytest - from rewrite.java.support_types import JavaType from rewrite.java.tree import Binary from rewrite.python.tree import CompilationUnit from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python -requires_ty_cli = pytest.mark.skipif( - shutil.which('ty-types') is None, - reason="ty-types CLI is not installed" -) - def test_bool_ops(): # language=python @@ -108,7 +99,6 @@ def test_multiline_tuple_comparison(): ),) -@requires_ty_cli def test_arithmetic_type_attribution(): """Verify that 1 + 2 has type Int.""" errors = [] @@ -138,7 +128,6 @@ def visit_binary(self, binary, p): assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_ty_cli def test_comparison_type_attribution(): """Verify that 1 < 2 has type Boolean.""" errors = [] @@ -168,7 +157,6 @@ def visit_binary(self, binary, p): assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_ty_cli def test_boolean_op_type_attribution(): """Verify that True and False has type Boolean.""" errors = [] diff --git a/rewrite-python/rewrite/tests/python/all/tree/class_test.py b/rewrite-python/rewrite/tests/python/all/tree/class_test.py index c7f16d5d63c..c1fc89597b5 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/class_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/class_test.py @@ -1,6 +1,4 @@ -import shutil - -import pytest +from pathlib import Path from rewrite.java.support_types import JavaType from rewrite.java.tree import Assignment, ClassDeclaration, Identifier @@ -8,13 +6,6 @@ from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python -from ._markers import requires_module_name - -requires_ty_cli = pytest.mark.skipif( - shutil.which('ty-types') is None, - reason="ty-types CLI is not installed" -) - def test_empty(): # language=python @@ -127,7 +118,6 @@ class C3(Generic[T], metaclass=type, *[str]): )) -@requires_ty_cli def test_generic_class_type_params(): """Verify type parameters on a generic class like class Box[T].""" errors = [] @@ -176,7 +166,6 @@ def __init__(self, value: T) -> None: assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_module_name def test_class_literal_module_qualified_fqn(): """Verify that a class defined in a module gets a module-qualified FQN on its classLiteral type.""" errors = [] @@ -206,12 +195,12 @@ def visit_class_declaration(self, class_decl, p): class MyClass: pass """, + path=Path("a.py"), after_recipe=check_types, )) assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_ty_cli def test_class_instance_type_attribution(): """Verify that x = Foo() assigns a type with fqn ending in 'Foo'.""" errors = [] diff --git a/rewrite-python/rewrite/tests/python/all/tree/collection_literal_test.py b/rewrite-python/rewrite/tests/python/all/tree/collection_literal_test.py index 17a06eb8148..5ba084cc0c2 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/collection_literal_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/collection_literal_test.py @@ -1,8 +1,5 @@ -import shutil from typing import cast -import pytest - from rewrite.java import MethodDeclaration, Return from rewrite.java.support_types import JavaType from rewrite.java.tree import Assignment @@ -12,11 +9,6 @@ Parameterized = JavaType.Parameterized -requires_ty_cli = pytest.mark.skipif( - shutil.which('ty-types') is None, - reason="ty-types CLI is not installed" -) - def test_empty_tuple(): # language=python @@ -107,7 +99,6 @@ def test_list_of_tuples_with_double_parens(): """)) -@requires_ty_cli def test_list_literal_type_attribution(): """Verify that [1, 2, 3] has type list.""" errors = [] @@ -139,7 +130,6 @@ def visit_assignment(self, assignment, p): assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_ty_cli def test_dict_literal_type_attribution(): """Verify that {"a": 1} has type dict.""" errors = [] diff --git a/rewrite-python/rewrite/tests/python/all/tree/def_test.py b/rewrite-python/rewrite/tests/python/all/tree/def_test.py index 2c7c82a809a..ab8c11b9e80 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/def_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/def_test.py @@ -1,18 +1,9 @@ -import shutil - -import pytest - from rewrite.java.support_types import JavaType from rewrite.java.tree import MethodDeclaration from rewrite.python.tree import CompilationUnit from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python -requires_ty_cli = pytest.mark.skipif( - shutil.which('ty-types') is None, - reason="ty-types CLI is not installed" -) - def test_async_def(): # language=python @@ -24,7 +15,6 @@ async def main(): )) -@requires_ty_cli def test_async_def_type_attribution(): """Verify that async def main() -> int has a method_type with a return_type. diff --git a/rewrite-python/rewrite/tests/python/all/tree/field_access_test.py b/rewrite-python/rewrite/tests/python/all/tree/field_access_test.py index edaa2406f0f..bbb1f2f1460 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/field_access_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/field_access_test.py @@ -1,17 +1,8 @@ -import shutil - -import pytest - from rewrite.java.tree import FieldAccess from rewrite.python.tree import CompilationUnit from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python -requires_ty_cli = pytest.mark.skipif( - shutil.which('ty-types') is None, - reason="ty-types CLI is not installed" -) - # noinspection PyUnresolvedReferences def test_attribute(): @@ -25,7 +16,6 @@ def test_nested_attribute(): RecipeSpec().rewrite_run(python("a = foo.bar.baz")) -@requires_ty_cli def test_field_access_type_attribution(): """Verify that os.path has a non-None FieldAccess.type.""" errors = [] diff --git a/rewrite-python/rewrite/tests/python/all/tree/for_test.py b/rewrite-python/rewrite/tests/python/all/tree/for_test.py index b4357746e71..6fc158857ff 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/for_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/for_test.py @@ -1,17 +1,8 @@ -import shutil - -import pytest - from rewrite.java.tree import ForEachLoop, Identifier from rewrite.python.tree import CompilationUnit from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python -requires_ty_cli = pytest.mark.skipif( - shutil.which('ty-types') is None, - reason="ty-types CLI is not installed" -) - def test_for(): # language=python @@ -75,7 +66,6 @@ def test_async(): )) -@requires_ty_cli def test_for_loop_variable_type_attribution(): """Verify that the loop control variable in 'for x in [1, 2, 3]' has a type.""" errors = [] diff --git a/rewrite-python/rewrite/tests/python/all/tree/import_test.py b/rewrite-python/rewrite/tests/python/all/tree/import_test.py index 77c7cee3597..ce62292a61a 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/import_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/import_test.py @@ -1,7 +1,3 @@ -import shutil - -import pytest - from rewrite.java import Import from rewrite.java.support_types import JavaType from rewrite.java.tree import MethodInvocation @@ -9,13 +5,6 @@ from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python -from ._markers import requires_module_name - -requires_ty_cli = pytest.mark.skipif( - shutil.which('ty-types') is None, - reason="ty-types CLI is not installed" -) - def _assert_single_j_import(cu: CompilationUnit) -> None: assert len(cu.statements) == 1 @@ -125,7 +114,6 @@ def visit_method_invocation(self, method, p): TypeChecker().visit(source_file, None) -@requires_ty_cli def test_qualified_import_type_attribution(): """import os; os.getcwd() → declaring_type.fqn == 'os'.""" errors = [] @@ -140,7 +128,6 @@ def test_qualified_import_type_attribution(): assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_module_name def test_from_import_type_attribution(): """from os import getcwd; getcwd() → declaring_type.fqn == 'os'.""" errors = [] @@ -155,7 +142,6 @@ def test_from_import_type_attribution(): assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_ty_cli def test_aliased_import_type_attribution(): """import os as o; o.getcwd() → declaring_type.fqn == 'os'.""" errors = [] @@ -170,7 +156,6 @@ def test_aliased_import_type_attribution(): assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_module_name def test_aliased_from_import_type_attribution(): """from os import getcwd as gwd; gwd() → declaring_type.fqn == 'os'.""" errors = [] diff --git a/rewrite-python/rewrite/tests/python/all/tree/lambda_test.py b/rewrite-python/rewrite/tests/python/all/tree/lambda_test.py index 143949cc1e0..e2c80ecc383 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/lambda_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/lambda_test.py @@ -1,17 +1,8 @@ -import shutil - -import pytest - from rewrite.java.tree import Assignment, Lambda from rewrite.python.tree import CompilationUnit from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python -requires_ty_cli = pytest.mark.skipif( - shutil.which('ty-types') is None, - reason="ty-types CLI is not installed" -) - def test_no_parameters(): # language=python @@ -71,7 +62,6 @@ def test_multiple_complex(): ''')) -@requires_ty_cli def test_lambda_type_attribution(): """Verify that lambda x: x + 1 has a non-None type.""" errors = [] diff --git a/rewrite-python/rewrite/tests/python/all/tree/method_declaration_test.py b/rewrite-python/rewrite/tests/python/all/tree/method_declaration_test.py index 7de1841b90c..03112e31a79 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/method_declaration_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/method_declaration_test.py @@ -1,18 +1,9 @@ -import shutil - -import pytest - from rewrite.java.support_types import JavaType from rewrite.java.tree import MethodDeclaration, VariableDeclarations from rewrite.python.tree import CompilationUnit from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python -requires_ty_cli = pytest.mark.skipif( - shutil.which('ty-types') is None, - reason="ty-types CLI is not installed" -) - def test_whitespace_before_colon(): # language=python @@ -167,7 +158,6 @@ def f( )) -@requires_ty_cli def test_generic_function_type_params(): """Verify method_type.declared_formal_type_names for def identity[T](x: T) -> T.""" errors = [] @@ -206,7 +196,6 @@ def identity[T](x: T) -> T: assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_ty_cli def test_method_declaration_type_attribution(): """Verify method_type on a function with typed parameters and return type.""" errors = [] @@ -257,7 +246,6 @@ def foo(a: int, b: str) -> bool: assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_ty_cli def test_param_identifier_field_type(): """Verify J.Identifier.field_type is JavaType.Variable for typed parameters.""" errors = [] diff --git a/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py b/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py index c23b9273482..98d77ccba38 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/method_invocation_test.py @@ -1,20 +1,9 @@ -import shutil - -import pytest - from rewrite.java.support_types import JavaType from rewrite.java.tree import MethodInvocation from rewrite.python.tree import CompilationUnit from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python -from ._markers import requires_module_name - -requires_ty_cli = pytest.mark.skipif( - shutil.which('ty-types') is None, - reason="ty-types CLI is not installed" -) - def test_no_select(): # language=python @@ -99,7 +88,6 @@ def test_no_name(): RecipeSpec().rewrite_run(python("v = (a)()")) -@requires_ty_cli def test_builtin_function_type_attribution(): """Verify type attribution on a builtin function call like len().""" errors = [] @@ -134,7 +122,6 @@ def visit_method_invocation(self, method, p): assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_ty_cli def test_string_method_type_attribution(): """Verify type attribution on a string method call like str.upper().""" errors = [] @@ -171,7 +158,6 @@ def visit_method_invocation(self, method, p): assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_ty_cli def test_stdlib_function_type_attribution(): """Verify type attribution on a stdlib function call like os.getcwd().""" errors = [] @@ -211,7 +197,6 @@ def visit_method_invocation(self, method, p): assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_ty_cli def test_generic_call_site_return_type(): """Verify method_invocation_type returns call-site-specific return type for generic functions.""" errors = [] @@ -253,7 +238,6 @@ def identity[T](x: T) -> T: assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_module_name def test_bare_function_declaring_type_has_module(): """Verify that a bare function call imported from a module gets a declaring type with the module name.""" errors = [] diff --git a/rewrite-python/rewrite/tests/python/all/tree/ternary_test.py b/rewrite-python/rewrite/tests/python/all/tree/ternary_test.py index 1a1b33dd65c..8c8de8de7c4 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/ternary_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/ternary_test.py @@ -1,25 +1,15 @@ -import shutil - -import pytest - from rewrite.java.support_types import JavaType from rewrite.java.tree import Ternary from rewrite.python.tree import CompilationUnit from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python -requires_ty_cli = pytest.mark.skipif( - shutil.which('ty-types') is None, - reason="ty-types CLI is not installed" -) - def test_simple(): # language=python RecipeSpec().rewrite_run(python("assert True if True else False")) -@requires_ty_cli def test_ternary_type_attribution(): """Verify that '1 if True else 2' has type Int.""" errors = [] diff --git a/rewrite-python/rewrite/tests/python/all/tree/type_hint_test.py b/rewrite-python/rewrite/tests/python/all/tree/type_hint_test.py index 4cdd939dd03..8fe18f37cfc 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/type_hint_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/type_hint_test.py @@ -1,7 +1,3 @@ -import shutil - -import pytest - from rewrite.java.support_types import JavaType from rewrite.java.tree import VariableDeclarations from rewrite.python.tree import CompilationUnit @@ -10,11 +6,6 @@ Parameterized = JavaType.Parameterized -requires_ty_cli = pytest.mark.skipif( - shutil.which('ty-types') is None, - reason="ty-types CLI is not installed" -) - def test_primitive_type_hint(): # language=python @@ -190,7 +181,6 @@ def f(x: "Union[str]") -> None: )) -@requires_ty_cli def test_list_int_param_type_attribution(): """Verify List[int] parameter type is Parameterized with base list and type param Int.""" errors = [] @@ -235,7 +225,6 @@ def test(n: List[int]): assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_ty_cli def test_dict_str_int_type_attribution(): """Verify Dict[str, int] variable type is Parameterized with base dict.""" errors = [] @@ -277,7 +266,6 @@ def visit_variable_declarations(self, var_decls, p): assert not errors, "Type attribution errors:\n" + "\n".join(f" - {e}" for e in errors) -@requires_ty_cli def test_optional_str_type_attribution(): """Verify Optional[str] variable type resolves to str or a union containing str.""" errors = [] diff --git a/rewrite-python/rewrite/tests/python/all/tree/unary_test.py b/rewrite-python/rewrite/tests/python/all/tree/unary_test.py index 3cd818b0afb..55bb0b32053 100644 --- a/rewrite-python/rewrite/tests/python/all/tree/unary_test.py +++ b/rewrite-python/rewrite/tests/python/all/tree/unary_test.py @@ -1,18 +1,9 @@ -import shutil - -import pytest - from rewrite.java.support_types import JavaType from rewrite.java.tree import Unary from rewrite.python.tree import CompilationUnit from rewrite.python.visitor import PythonVisitor from rewrite.test import RecipeSpec, python -requires_ty_cli = pytest.mark.skipif( - shutil.which('ty-types') is None, - reason="ty-types CLI is not installed" -) - def test_bool_ops(): # language=python @@ -28,7 +19,6 @@ def test_arithmetic_ops(): RecipeSpec().rewrite_run(python("assert ~1")) -@requires_ty_cli def test_not_type_attribution(): """Verify that 'not True' has type Boolean.""" errors = [] diff --git a/rewrite-python/rewrite/tests/recipes/test_change_import.py b/rewrite-python/rewrite/tests/recipes/test_change_import.py index 1613f55d4ab..07b527e5b3f 100644 --- a/rewrite-python/rewrite/tests/recipes/test_change_import.py +++ b/rewrite-python/rewrite/tests/recipes/test_change_import.py @@ -14,15 +14,7 @@ """Tests for ChangeImport recipe.""" -import shutil - -import pytest from rewrite.java.support_types import JavaType - -requires_ty_cli = pytest.mark.skipif( - shutil.which('ty-types') is None, - reason="ty-types CLI is not installed" -) from rewrite.java.tree import FieldAccess, Identifier, MethodInvocation from rewrite.python.recipes.change_import import ChangeImport from rewrite.python.tree import CompilationUnit @@ -417,7 +409,6 @@ def test_no_rename_bare_ref_in_unrelated_code(self): ) ) - @requires_ty_cli def test_no_rename_shadowed_in_function_scope(self): """Don't rename a local variable in function scope that shadows the imported name.""" spec = RecipeSpec(recipe=ChangeImport( From b151dc26d50e6b55eceb4784fbf9993db6b228f0 Mon Sep 17 00:00:00 2001 From: Knut Wannheden Date: Mon, 23 Feb 2026 14:43:51 +0100 Subject: [PATCH 6/6] Use type attribution in RemoveImport to detect shadowed locals RemoveImport._is_referenced now skips identifiers inside function scopes that have field_type set, as these are local variables shadowing the imported name rather than actual uses of the import. This prevents keeping unused imports when a local variable happens to share the same name. Also fix test_type_attribution assertions to accept module-qualified FQNs (e.g. 'test.Greeter' instead of bare 'Greeter') since the moduleName improvements now produce module-prefixed names. --- .../src/rewrite/python/remove_import.py | 7 +- .../tests/python/test_remove_import.py | 83 +++++++++++++++++++ .../tests/python/test_type_attribution.py | 14 ++-- 3 files changed, 96 insertions(+), 8 deletions(-) diff --git a/rewrite-python/rewrite/src/rewrite/python/remove_import.py b/rewrite-python/rewrite/src/rewrite/python/remove_import.py index d076b958117..78d04021aa8 100644 --- a/rewrite-python/rewrite/src/rewrite/python/remove_import.py +++ b/rewrite-python/rewrite/src/rewrite/python/remove_import.py @@ -20,7 +20,7 @@ from rewrite.java import J from rewrite.java.support_types import JContainer, JRightPadded -from rewrite.java.tree import Empty, FieldAccess, Identifier, Import, Space +from rewrite.java.tree import Empty, FieldAccess, Identifier, Import, MethodDeclaration, Space from rewrite.markers import Markers from rewrite.python.tree import CompilationUnit, MultiImport from rewrite.python.visitor import PythonVisitor @@ -129,6 +129,11 @@ def visit_multi_import(self, multi: MultiImport, p) -> J: def visit_identifier(self, ident: Identifier, p) -> J: if not self.in_import: + # Inside a function scope, identifiers with field_type are + # local variables (shadowing the import), not actual uses. + if self.cursor.first_enclosing(MethodDeclaration) is not None: + if ident.field_type is not None: + return ident used.add(ident.simple_name) return ident diff --git a/rewrite-python/rewrite/tests/python/test_remove_import.py b/rewrite-python/rewrite/tests/python/test_remove_import.py index 52fbaea5400..143f7cd2a73 100644 --- a/rewrite-python/rewrite/tests/python/test_remove_import.py +++ b/rewrite-python/rewrite/tests/python/test_remove_import.py @@ -118,6 +118,89 @@ def visit_compilation_unit(self, cu: CompilationUnit, p: ExecutionContext) -> J: ) ) + def test_remove_import_when_only_shadowed_in_function(self): + """Remove import when the name is only used as a local variable shadowing it.""" + class RemoveJoinVisitor(PythonVisitor[ExecutionContext]): + def visit_compilation_unit(self, cu: CompilationUnit, p: ExecutionContext) -> J: + maybe_remove_import(self, RemoveImportOptions( + module='os.path', + name='join', + only_if_unused=True + )) + return super().visit_compilation_unit(cu, p) + + spec = RecipeSpec(recipe=from_visitor(RemoveJoinVisitor()), type_attribution=True) + spec.rewrite_run( + python( + """\ + from os.path import join + + def foo(): + join = "not a function" + return join + """, + """\ + def foo(): + join = "not a function" + return join + """, + ) + ) + + def test_keep_import_when_used_at_module_level(self): + """Keep import when it's used at module level even if also shadowed in a function.""" + class RemoveJoinVisitor(PythonVisitor[ExecutionContext]): + def visit_compilation_unit(self, cu: CompilationUnit, p: ExecutionContext) -> J: + maybe_remove_import(self, RemoveImportOptions( + module='os.path', + name='join', + only_if_unused=True + )) + return super().visit_compilation_unit(cu, p) + + spec = RecipeSpec(recipe=from_visitor(RemoveJoinVisitor()), type_attribution=True) + spec.rewrite_run( + python( + """\ + from os.path import join + + x = join("a", "b") + + def foo(): + join = "shadowed" + return join + """, + ) + ) + + def test_remove_direct_import_when_only_shadowed(self): + """Remove 'import os' when 'os' is only used as a local variable name.""" + class RemoveOsVisitor(PythonVisitor[ExecutionContext]): + def visit_compilation_unit(self, cu: CompilationUnit, p: ExecutionContext) -> J: + maybe_remove_import(self, RemoveImportOptions( + module='os', + only_if_unused=True + )) + return super().visit_compilation_unit(cu, p) + + spec = RecipeSpec(recipe=from_visitor(RemoveOsVisitor()), type_attribution=True) + spec.rewrite_run( + python( + """\ + import os + + def foo(): + os = "not a module" + return os + """, + """\ + def foo(): + os = "not a module" + return os + """, + ) + ) + def test_no_change_when_import_not_present(self): """No change when the import to remove doesn't exist.""" class RemoveJoinVisitor(PythonVisitor[ExecutionContext]): diff --git a/rewrite-python/rewrite/tests/python/test_type_attribution.py b/rewrite-python/rewrite/tests/python/test_type_attribution.py index 3db15a340cd..7e109b4dc0e 100644 --- a/rewrite-python/rewrite/tests/python/test_type_attribution.py +++ b/rewrite-python/rewrite/tests/python/test_type_attribution.py @@ -828,7 +828,7 @@ def greet(self, name: str) -> str: assert result is not None assert result._name == 'greet' assert result._declaring_type is not None - assert result._declaring_type._fully_qualified_name == 'Greeter' + assert result._declaring_type._fully_qualified_name.endswith('Greeter') finally: _cleanup_mapping(mapping, tmpdir, client) @@ -1261,7 +1261,7 @@ class Color(Enum): result = mapping.type(name_node) assert result is not None assert isinstance(result, JavaType.Class) - assert result._fully_qualified_name == 'Color' + assert result._fully_qualified_name.endswith('Color') assert result._kind == JavaType.FullyQualified.Kind.Enum finally: _cleanup_mapping(mapping, tmpdir, client) @@ -1281,7 +1281,7 @@ def greet(self, name: str) -> str: ... result = mapping.type(name_node) assert result is not None assert isinstance(result, JavaType.Class) - assert result._fully_qualified_name == 'Greeter' + assert result._fully_qualified_name.endswith('Greeter') assert result._kind == JavaType.FullyQualified.Kind.Interface finally: _cleanup_mapping(mapping, tmpdir, client) @@ -1299,7 +1299,7 @@ def test_regular_class_has_class_kind(self): result = mapping.type(name_node) assert result is not None assert isinstance(result, JavaType.Class) - assert result._fully_qualified_name == 'MyClass' + assert result._fully_qualified_name.endswith('MyClass') assert result._kind == JavaType.FullyQualified.Kind.Class finally: _cleanup_mapping(mapping, tmpdir, client) @@ -1325,17 +1325,17 @@ class Multi(Base, Mixin): result = mapping.type(name_node) assert result is not None assert isinstance(result, JavaType.Class) - assert result._fully_qualified_name == 'Multi' + assert result._fully_qualified_name.endswith('Multi') # First supertype → _supertype assert getattr(result, '_supertype', None) is not None, \ "Multi should have a supertype" - assert result._supertype._fully_qualified_name == 'Base' + assert result._supertype._fully_qualified_name.endswith('Base') # Remaining supertypes → _interfaces interfaces = getattr(result, '_interfaces', None) assert interfaces is not None, \ "Multi should have interfaces for additional supertypes" assert len(interfaces) >= 1 iface_names = [i._fully_qualified_name for i in interfaces] - assert 'Mixin' in iface_names + assert any(n.endswith('Mixin') for n in iface_names) finally: _cleanup_mapping(mapping, tmpdir, client)