From df249a9b16ee88e463d8dd86db4b02bc70239d7e Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Tue, 17 Oct 2023 16:58:17 +0530 Subject: [PATCH 01/10] add a test for forward references --- tests/test_cmdline.py | 48 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/test_cmdline.py b/tests/test_cmdline.py index 8080f817..82271a4b 100644 --- a/tests/test_cmdline.py +++ b/tests/test_cmdline.py @@ -764,3 +764,51 @@ def test_tidy_imports_sorting(): sympy """).strip().format(f=f) assert result == expected + + +def test_tidy_imports_forward_references(): + with tempfile.TemporaryDirectory() as temp_dir: + foo = os.path.join(temp_dir, "foo.py") + with open(foo, "w") as foo_fp: + foo_fp.write(dedent(""" + from __future__ import annotations + + + class A: + param1: str + param2: B + + + class B: + param1: str + """).lstrip()) + foo_fp.flush() + + dot_pyflyby = os.path.join(temp_dir, ".pyflyby") + with open(dot_pyflyby, "w") as dot_pyflyby_fp: + dot_pyflyby_fp.write(dedent(""" + from foo import A, B + """).lstrip()) + dot_pyflyby_fp.flush() + + os.chdir(temp_dir) + result = pipe([ + BIN_DIR+"/tidy-imports", foo_fp.name + ], env={ + "PYFLYBY_PATH": dot_pyflyby + }) + + expected = dedent(f""" + [PYFLYBY] {foo}: added 'from foo import B' + from __future__ import annotations + from foo import B + + class A: + param1: str + param2: B + + + class B: + param1: str + """).strip().format() + assert result == expected From b563ce0b8872b91ecd039405200a980e837cdfc3 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Tue, 17 Oct 2023 17:41:24 +0530 Subject: [PATCH 02/10] remove missing import even if its inside class --- lib/python/pyflyby/_autoimp.py | 4 ++-- tests/test_autoimp.py | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/python/pyflyby/_autoimp.py b/lib/python/pyflyby/_autoimp.py index e2f5a93f..5689bf16 100644 --- a/lib/python/pyflyby/_autoimp.py +++ b/lib/python/pyflyby/_autoimp.py @@ -878,13 +878,13 @@ def _visit_Store(self, fullname, value=None): def _remove_from_missing_imports(self, fullname): for missing_import in self.missing_imports: # If it was defined inside a class method, then it wouldn't have been added to - # the missing imports anyways. + # the missing imports anyways (except in that case of annotations) # See the following tests: # - tests.test_autoimp.test_method_reference_current_class # - tests.test_autoimp.test_find_missing_imports_class_name_1 # - tests.test_autoimp.test_scan_for_import_issues_class_defined_after_use inside_class = missing_import[1].scope_info.get('_in_class_def') - if missing_import[1].startswith(fullname) and not inside_class: + if missing_import[1].startswith(fullname): self.missing_imports.remove(missing_import) def _get_scope_info(self): diff --git a/tests/test_autoimp.py b/tests/test_autoimp.py index 9fe0cf55..45f6c458 100644 --- a/tests/test_autoimp.py +++ b/tests/test_autoimp.py @@ -521,7 +521,6 @@ def foo(): assert unused == [] - def test_find_missing_imports_class_name_1(): code = dedent( """ From 688bdd268b1893894f361cc5514ac2e7d2bbd6f0 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Tue, 24 Oct 2023 18:42:03 +0530 Subject: [PATCH 03/10] update failing test to pass --- tests/test_cmdline.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_cmdline.py b/tests/test_cmdline.py index 82271a4b..11630e6d 100644 --- a/tests/test_cmdline.py +++ b/tests/test_cmdline.py @@ -799,9 +799,7 @@ class B: }) expected = dedent(f""" - [PYFLYBY] {foo}: added 'from foo import B' from __future__ import annotations - from foo import B class A: param1: str From 1fb9dbb9ba38ebe500debcdb520e1c9e0e278484 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Tue, 31 Oct 2023 16:25:41 +0530 Subject: [PATCH 04/10] add annotation test --- tests/test_autoimp.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_autoimp.py b/tests/test_autoimp.py index 45f6c458..7c41a18b 100644 --- a/tests/test_autoimp.py +++ b/tests/test_autoimp.py @@ -521,6 +521,23 @@ def foo(): assert unused == [] +def test_annotation_inside_class(): + code = dedent( + """ + class B: + param1: str + + + class A: + param1: str + param2: B + """ + ) + missing, unused = scan_for_import_issues(code, [{}]) + assert missing == [] + assert unused == [] + + def test_find_missing_imports_class_name_1(): code = dedent( """ From a6a217bed02887d3929362ba894ed80df41d2866 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Tue, 31 Oct 2023 17:20:03 +0530 Subject: [PATCH 05/10] fix remaining tests --- lib/python/pyflyby/_autoimp.py | 5 ++++- tests/test_autoimp.py | 7 +++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/python/pyflyby/_autoimp.py b/lib/python/pyflyby/_autoimp.py index 5689bf16..82bb57a1 100644 --- a/lib/python/pyflyby/_autoimp.py +++ b/lib/python/pyflyby/_autoimp.py @@ -883,9 +883,12 @@ def _remove_from_missing_imports(self, fullname): # - tests.test_autoimp.test_method_reference_current_class # - tests.test_autoimp.test_find_missing_imports_class_name_1 # - tests.test_autoimp.test_scan_for_import_issues_class_defined_after_use + scopestack = missing_import[1].scope_info['scopestack'] + in_class_scope = isinstance(scopestack[-1], _ClassScope) inside_class = missing_import[1].scope_info.get('_in_class_def') if missing_import[1].startswith(fullname): - self.missing_imports.remove(missing_import) + if in_class_scope or not inside_class: + self.missing_imports.remove(missing_import) def _get_scope_info(self): return { diff --git a/tests/test_autoimp.py b/tests/test_autoimp.py index 7c41a18b..417ba225 100644 --- a/tests/test_autoimp.py +++ b/tests/test_autoimp.py @@ -524,13 +524,12 @@ def foo(): def test_annotation_inside_class(): code = dedent( """ - class B: - param1: str - - class A: param1: str param2: B + + class B: + param1: str """ ) missing, unused = scan_for_import_issues(code, [{}]) From 0824ca7d665ccc8aa51257be3e1df8da7f3de43c Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Tue, 31 Oct 2023 17:23:12 +0530 Subject: [PATCH 06/10] remove object subclass --- tests/test_autoimp.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_autoimp.py b/tests/test_autoimp.py index 417ba225..f3554f3c 100644 --- a/tests/test_autoimp.py +++ b/tests/test_autoimp.py @@ -540,10 +540,10 @@ class B: def test_find_missing_imports_class_name_1(): code = dedent( """ - class Corinne(object): + class Corinne: pass - class Bobtail(object): - class Chippewa(object): + class Bobtail: + class Chippewa: Bobtail Rockton = Passall, Corinne, Chippewa """) From e28c88b46a4552c4fec5dcab5cc6e028fd23da85 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Tue, 31 Oct 2023 17:35:08 +0530 Subject: [PATCH 07/10] fix lint issues --- tests/test_cmdline.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cmdline.py b/tests/test_cmdline.py index 11630e6d..61635360 100644 --- a/tests/test_cmdline.py +++ b/tests/test_cmdline.py @@ -808,5 +808,5 @@ class A: class B: param1: str - """).strip().format() + """).strip() assert result == expected From 877f4a125223d73ecf8f859b23a7eb96d023050e Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Tue, 14 Nov 2023 17:40:43 +0100 Subject: [PATCH 08/10] minor fixes --- tests/test_autoimp.py | 3 ++- tests/test_cmdline.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_autoimp.py b/tests/test_autoimp.py index f3554f3c..4a9b87f4 100644 --- a/tests/test_autoimp.py +++ b/tests/test_autoimp.py @@ -544,8 +544,9 @@ class Corinne: pass class Bobtail: class Chippewa: - Bobtail + Bobtail # will be name error at runtime Rockton = Passall, Corinne, Chippewa + # ^error, ^ok , ^ok """) result = find_missing_imports(code, [{}]) result = _dilist2strlist(result) diff --git a/tests/test_cmdline.py b/tests/test_cmdline.py index 61635360..cc46eb8d 100644 --- a/tests/test_cmdline.py +++ b/tests/test_cmdline.py @@ -798,7 +798,7 @@ class B: "PYFLYBY_PATH": dot_pyflyby }) - expected = dedent(f""" + expected = dedent(""" from __future__ import annotations class A: From a0d4bd7b9cddf8a59b01e105b33cf22c9e061749 Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Wed, 22 Nov 2023 10:50:51 +0100 Subject: [PATCH 09/10] mark xfail --- tests/test_autoimp.py | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/tests/test_autoimp.py b/tests/test_autoimp.py index 4a9b87f4..448d078e 100644 --- a/tests/test_autoimp.py +++ b/tests/test_autoimp.py @@ -537,6 +537,9 @@ class B: assert unused == [] +@pytest.mark.xfail( + reason="Had to deactivate as part of https://github.com/deshaw/pyflyby/pull/269/files conflicting requirements" +) def test_find_missing_imports_class_name_1(): code = dedent( """ @@ -547,10 +550,29 @@ class Chippewa: Bobtail # will be name error at runtime Rockton = Passall, Corinne, Chippewa # ^error, ^ok , ^ok - """) - result = find_missing_imports(code, [{}]) - result = _dilist2strlist(result) - expected = ['Bobtail', 'Passall'] + """ + ) + result = find_missing_imports(code, [{}]) + result = _dilist2strlist(result) + expected = ["Bobtail", "Passall"] + assert expected == result + + +def test_find_missing_imports_class_name_1b(): + code = dedent( + """ + class Corinne: + pass + class Bobtail: + class Chippewa: + Bobtail # will be name error at runtime + Rockton = Passall, Corinne, Chippewa + # ^error, ^ok , ^ok + """ + ) + result = find_missing_imports(code, [{}]) + result = _dilist2strlist(result) + expected = ["Passall"] assert expected == result From dbd1a2f9c4380ad6616872ac9a44e534c3164f46 Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Tue, 5 Dec 2023 09:34:34 +0100 Subject: [PATCH 10/10] fix test --- tests/test_cmdline.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/test_cmdline.py b/tests/test_cmdline.py index cc46eb8d..74271047 100644 --- a/tests/test_cmdline.py +++ b/tests/test_cmdline.py @@ -13,7 +13,7 @@ import tempfile from textwrap import dedent -from pyflyby._util import EnvVarCtx +from pyflyby._util import EnvVarCtx, CwdCtx import pytest @@ -790,23 +790,23 @@ class B: from foo import A, B """).lstrip()) dot_pyflyby_fp.flush() + with CwdCtx(temp_dir): + result = pipe( + [BIN_DIR + "/tidy-imports", foo_fp.name], + env={"PYFLYBY_PATH": dot_pyflyby}, + ) + + expected = dedent( + """ + from __future__ import annotations - os.chdir(temp_dir) - result = pipe([ - BIN_DIR+"/tidy-imports", foo_fp.name - ], env={ - "PYFLYBY_PATH": dot_pyflyby - }) - - expected = dedent(""" - from __future__ import annotations - - class A: - param1: str - param2: B + class A: + param1: str + param2: B - class B: - param1: str - """).strip() + class B: + param1: str + """ + ).strip() assert result == expected