From 07eb6487d566e8cda1e48eeb18cf37fb48dc1c93 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Sun, 28 May 2023 00:48:27 +0530 Subject: [PATCH 1/4] Handle dotted alias to check for implicit imports --- .../fixtures/flake8_type_checking/strict.py | 38 +++++++++++++++++++ .../rules/typing_only_runtime_import.rs | 17 +++++++-- ...__flake8_type_checking__tests__strict.snap | 36 ++++++++++++++++++ ...ing-only-third-party-import_strict.py.snap | 9 +++++ 4 files changed, 97 insertions(+), 3 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/flake8_type_checking/strict.py b/crates/ruff/resources/test/fixtures/flake8_type_checking/strict.py index d7f9bda9840ad..1762d3febc402 100644 --- a/crates/ruff/resources/test/fixtures/flake8_type_checking/strict.py +++ b/crates/ruff/resources/test/fixtures/flake8_type_checking/strict.py @@ -55,3 +55,41 @@ def f(): def test(value: A): return pkg.B() + + +def f(): + # In un-strict mode, this shouldn't rase an error, since `pkg.bar` is used at runtime. + import pkg + import pkg.bar as B + + def test(value: pkg.A): + return B() + + +def f(): + # In un-strict mode, this shouldn't rase an error, since `pkg.foo.bar` is used at runtime. + import pkg.foo as F + import pkg.foo.bar as B + + def test(value: F.Foo): + return B() + + +def f(): + # In un-strict mode, this shouldn't rase an error, since `pkg.foo.bar` is used at runtime. + import pkg + import pkg.foo.bar as B + + def test(value: pkg.A): + return B() + + +def f(): + # In un-strict mode, this _should_ rase an error, since `pkgfoo.bar` is used at runtime. + # Note that `pkg` is a prefix of `pkgfoo` which are both different modules. This is + # testing the implementation. + import pkg + import pkgfoo.bar as B + + def test(value: pkg.A): + return B() diff --git a/crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs b/crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs index df32a2dcbb352..e45b037fb117e 100644 --- a/crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs +++ b/crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs @@ -181,9 +181,20 @@ fn is_implicit_import(this: &Binding, that: &Binding) -> bool { } BindingKind::Importation(Importation { full_name: that_name, - .. - }) - | BindingKind::SubmoduleImportation(SubmoduleImportation { + name: that_alias, + }) => { + if that_name == that_alias { + // Ex) `pkg` vs. `pkg` + this_name == that_name + } else { + // Submodule importation with an alias: `import pkg.A as B` + this_name + .split('.') + .zip(that_name.split('.')) + .all(|(this_part, that_part)| this_part == that_part) + } + } + BindingKind::SubmoduleImportation(SubmoduleImportation { name: that_name, .. }) => { // Ex) `pkg.A` vs. `pkg.B` diff --git a/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__strict.snap b/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__strict.snap index 4ee56e39e1152..c4d7bca2329c0 100644 --- a/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__strict.snap +++ b/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__strict.snap @@ -31,4 +31,40 @@ strict.py:54:25: TCH002 Move third-party import `pkg.bar.A` into a type-checking 58 | def test(value: A): | +strict.py:62:12: TCH002 Move third-party import `pkg` into a type-checking block + | +62 | def f(): +63 | # In un-strict mode, this shouldn't rase an error, since `pkg.bar` is used at runtime. +64 | import pkg + | ^^^ TCH002 +65 | import pkg.bar as B + | + +strict.py:71:12: TCH002 Move third-party import `pkg.foo` into a type-checking block + | +71 | def f(): +72 | # In un-strict mode, this shouldn't rase an error, since `pkg.foo.bar` is used at runtime. +73 | import pkg.foo as F + | ^^^^^^^^^^^^ TCH002 +74 | import pkg.foo.bar as B + | + +strict.py:80:12: TCH002 Move third-party import `pkg` into a type-checking block + | +80 | def f(): +81 | # In un-strict mode, this shouldn't rase an error, since `pkg.foo.bar` is used at runtime. +82 | import pkg + | ^^^ TCH002 +83 | import pkg.foo.bar as B + | + +strict.py:91:12: TCH002 Move third-party import `pkg` into a type-checking block + | +91 | # Note that `pkg` is a prefix of `pkgfoo` which are both different modules. This is +92 | # testing the implementation. +93 | import pkg + | ^^^ TCH002 +94 | import pkgfoo.bar as B + | + diff --git a/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__typing-only-third-party-import_strict.py.snap b/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__typing-only-third-party-import_strict.py.snap index a26d1222d3d2b..b3044d4b63d0a 100644 --- a/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__typing-only-third-party-import_strict.py.snap +++ b/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__typing-only-third-party-import_strict.py.snap @@ -11,4 +11,13 @@ strict.py:54:25: TCH002 Move third-party import `pkg.bar.A` into a type-checking 58 | def test(value: A): | +strict.py:91:12: TCH002 Move third-party import `pkg` into a type-checking block + | +91 | # Note that `pkg` is a prefix of `pkgfoo` which are both different modules. This is +92 | # testing the implementation. +93 | import pkg + | ^^^ TCH002 +94 | import pkgfoo.bar as B + | + From 89cd6a754e5a4524b840bd6166bcf178d0967776 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Sun, 28 May 2023 09:09:57 +0530 Subject: [PATCH 2/4] Simplify and use root module to check implicit imports --- .../fixtures/flake8_type_checking/strict.py | 10 +++++++ .../rules/typing_only_runtime_import.rs | 29 +++++++++---------- ...__flake8_type_checking__tests__strict.snap | 10 +++++++ 3 files changed, 34 insertions(+), 15 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/flake8_type_checking/strict.py b/crates/ruff/resources/test/fixtures/flake8_type_checking/strict.py index 1762d3febc402..c9fd768a493a8 100644 --- a/crates/ruff/resources/test/fixtures/flake8_type_checking/strict.py +++ b/crates/ruff/resources/test/fixtures/flake8_type_checking/strict.py @@ -93,3 +93,13 @@ def f(): def test(value: pkg.A): return B() + + +def f(): + # Even in strict mode, this shouldn't rase an error, since `pkg.baz` is used at + # runtime, and implicitly imports `pkg.bar`. + import pkg.bar as B + import pkg.foo as F + + def test(value: F.Foo): + return B.Bar() diff --git a/crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs b/crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs index e45b037fb117e..8e05803c39ce0 100644 --- a/crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs +++ b/crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs @@ -181,25 +181,24 @@ fn is_implicit_import(this: &Binding, that: &Binding) -> bool { } BindingKind::Importation(Importation { full_name: that_name, - name: that_alias, + .. + }) + | BindingKind::SubmoduleImportation(SubmoduleImportation { + name: that_name, .. }) => { - if that_name == that_alias { + // Submodule importation with an alias (`import pkg.A as B`) + // are represented as `Importation`. + match (this_name.find('.'), that_name.find('.')) { + // Ex) `pkg.A` vs. `pkg.B` + (Some(i), Some(j)) => this_name[..i] == that_name[..j], + // Ex) `pkg.A` vs. `pkg` + (Some(i), None) => this_name[..i] == **that_name, + // Ex) `pkg` vs. `pkg.B` + (None, Some(j)) => **this_name == that_name[..j], // Ex) `pkg` vs. `pkg` - this_name == that_name - } else { - // Submodule importation with an alias: `import pkg.A as B` - this_name - .split('.') - .zip(that_name.split('.')) - .all(|(this_part, that_part)| this_part == that_part) + (None, None) => this_name == that_name, } } - BindingKind::SubmoduleImportation(SubmoduleImportation { - name: that_name, .. - }) => { - // Ex) `pkg.A` vs. `pkg.B` - this_name == that_name - } _ => false, }, BindingKind::FromImportation(FromImportation { diff --git a/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__strict.snap b/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__strict.snap index c4d7bca2329c0..badd402e64ff6 100644 --- a/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__strict.snap +++ b/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__strict.snap @@ -67,4 +67,14 @@ strict.py:91:12: TCH002 Move third-party import `pkg` into a type-checking block 94 | import pkgfoo.bar as B | +strict.py:102:12: TCH002 Move third-party import `pkg.foo` into a type-checking block + | +102 | # runtime, and implicitly imports `pkg.bar`. +103 | import pkg.bar as B +104 | import pkg.foo as F + | ^^^^^^^^^^^^ TCH002 +105 | +106 | def test(value: F.Foo): + | + From d9b625fedee245cc7847856a93bb6c889ecfe47c Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Sun, 28 May 2023 09:11:32 +0530 Subject: [PATCH 3/4] Fix doc --- .../resources/test/fixtures/flake8_type_checking/strict.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/flake8_type_checking/strict.py b/crates/ruff/resources/test/fixtures/flake8_type_checking/strict.py index c9fd768a493a8..409ca96ff8235 100644 --- a/crates/ruff/resources/test/fixtures/flake8_type_checking/strict.py +++ b/crates/ruff/resources/test/fixtures/flake8_type_checking/strict.py @@ -96,8 +96,7 @@ def test(value: pkg.A): def f(): - # Even in strict mode, this shouldn't rase an error, since `pkg.baz` is used at - # runtime, and implicitly imports `pkg.bar`. + # In un-strict mode, this shouldn't raise an error, since `pkg.bar` is used at runtime. import pkg.bar as B import pkg.foo as F From 15d3a60bb37e2f400ef100cc86dcf043875ddb15 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Sun, 28 May 2023 09:20:34 +0530 Subject: [PATCH 4/4] Update snapshot --- ...__rules__flake8_type_checking__tests__strict.snap | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__strict.snap b/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__strict.snap index badd402e64ff6..e110f3c9c3859 100644 --- a/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__strict.snap +++ b/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__strict.snap @@ -67,14 +67,14 @@ strict.py:91:12: TCH002 Move third-party import `pkg` into a type-checking block 94 | import pkgfoo.bar as B | -strict.py:102:12: TCH002 Move third-party import `pkg.foo` into a type-checking block +strict.py:101:12: TCH002 Move third-party import `pkg.foo` into a type-checking block | -102 | # runtime, and implicitly imports `pkg.bar`. -103 | import pkg.bar as B -104 | import pkg.foo as F +101 | # In un-strict mode, this shouldn't raise an error, since `pkg.bar` is used at runtime. +102 | import pkg.bar as B +103 | import pkg.foo as F | ^^^^^^^^^^^^ TCH002 -105 | -106 | def test(value: F.Foo): +104 | +105 | def test(value: F.Foo): |