Skip to content

Commit

Permalink
Improve detection of TYPE_CHECKING blocks imported from `typing_ext…
Browse files Browse the repository at this point in the history
…ensions` or `_typeshed` (#8429)

~Improves detection of types imported from `typing_extensions`. Removes
the hard-coded list of supported types in `typing_extensions`; instead
assuming all types could be imported from `typing`, `_typeshed`, or
`typing_extensions`.~

~The typing extensions package appears to re-export types even if they
do not need modification.~


Adds detection of `if typing_extensions.TYPE_CHECKING` blocks. Avoids
inserting a new `if TYPE_CHECKING` block and `from typing import
TYPE_CHECKING` if `typing_extensions.TYPE_CHECKING` is used (closes
#8427)

---------

Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
  • Loading branch information
zanieb and charliermarsh committed Nov 9, 2023
1 parent 9d167a1 commit 565ddeb
Show file tree
Hide file tree
Showing 16 changed files with 194 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -172,3 +172,14 @@ def f():
from module import Member

x: Member = 1


def f():
from typing_extensions import TYPE_CHECKING

from pandas import y

if TYPE_CHECKING:
_type = x
elif True:
_type = y
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from __future__ import annotations

from typing_extensions import TYPE_CHECKING

if TYPE_CHECKING:
from pandas import DataFrame


def example() -> DataFrame:
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from __future__ import annotations

from typing_extensions import TYPE_CHECKING

if TYPE_CHECKING:
from pandas import DataFrame


def example() -> DataFrame:
x = DataFrame()
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,9 @@ class Test:

if 0:
x: List


from typing_extensions import TYPE_CHECKING

if TYPE_CHECKING:
pass # TCH005
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from __future__ import annotations

from typing_extensions import Self


def func():
from pandas import DataFrame

df: DataFrame
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from __future__ import annotations

import typing_extensions


def func():
from pandas import DataFrame

df: DataFrame
30 changes: 25 additions & 5 deletions crates/ruff_linter/src/importer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,7 @@ impl<'a> Importer<'a> {
)?;

// Import the `TYPE_CHECKING` symbol from the typing module.
let (type_checking_edit, type_checking) = self.get_or_import_symbol(
&ImportRequest::import_from("typing", "TYPE_CHECKING"),
at,
semantic,
)?;
let (type_checking_edit, type_checking) = self.get_or_import_type_checking(at, semantic)?;

// Add the import to a `TYPE_CHECKING` block.
let add_import_edit = if let Some(block) = self.preceding_type_checking_block(at) {
Expand All @@ -161,6 +157,30 @@ impl<'a> Importer<'a> {
})
}

/// Generate an [`Edit`] to reference `typing.TYPE_CHECKING`. Returns the [`Edit`] necessary to
/// make the symbol available in the current scope along with the bound name of the symbol.
fn get_or_import_type_checking(
&self,
at: TextSize,
semantic: &SemanticModel,
) -> Result<(Edit, String), ResolutionError> {
for module in semantic.typing_modules() {
if let Some((edit, name)) = self.get_symbol(
&ImportRequest::import_from(module, "TYPE_CHECKING"),
at,
semantic,
)? {
return Ok((edit, name));
}
}

self.import_symbol(
&ImportRequest::import_from("typing", "TYPE_CHECKING"),
at,
semantic,
)
}

/// Generate an [`Edit`] to reference the given symbol. Returns the [`Edit`] necessary to make
/// the symbol available in the current scope along with the bound name of the symbol.
///
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ mod tests {
#[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_13.py"))]
#[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_14.pyi"))]
#[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_15.py"))]
#[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_16.py"))]
#[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_17.py"))]
#[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_2.py"))]
#[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_3.py"))]
#[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_4.py"))]
Expand All @@ -36,6 +38,8 @@ mod tests {
#[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("snapshot.py"))]
#[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("TCH002.py"))]
#[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("strict.py"))]
#[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("typing_modules_1.py"))]
#[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("typing_modules_2.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,19 @@ TCH005.py:22:9: TCH005 [*] Found empty type-checking block
24 22 |
25 23 |

TCH005.py:45:5: TCH005 [*] Found empty type-checking block
|
44 | if TYPE_CHECKING:
45 | pass # TCH005
| ^^^^ TCH005
|
= help: Delete empty type-checking block

Safe fix
41 41 |
42 42 | from typing_extensions import TYPE_CHECKING
43 43 |
44 |-if TYPE_CHECKING:
45 |- pass # TCH005


Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
---

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
---
TCH004_17.py:6:24: TCH004 [*] Move import `pandas.DataFrame` out of type-checking block. Import is used for more than type hinting.
|
5 | if TYPE_CHECKING:
6 | from pandas import DataFrame
| ^^^^^^^^^ TCH004
|
= help: Move out of type-checking block

Unsafe fix
1 1 | from __future__ import annotations
2 2 |
3 3 | from typing_extensions import TYPE_CHECKING
4 |+from pandas import DataFrame
4 5 |
5 6 | if TYPE_CHECKING:
6 |- from pandas import DataFrame
7 |+ pass
7 8 |
8 9 |
9 10 | def example() -> DataFrame:


Original file line number Diff line number Diff line change
Expand Up @@ -248,5 +248,6 @@ TCH002.py:172:24: TCH002 [*] Move third-party import `module.Member` into a type
172 |- from module import Member
173 176 |
174 177 | x: Member = 1
175 178 |


Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
---
typing_modules_1.py:7:24: TCH002 [*] Move third-party import `pandas.DataFrame` into a type-checking block
|
6 | def func():
7 | from pandas import DataFrame
| ^^^^^^^^^ TCH002
8 |
9 | df: DataFrame
|
= help: Move into type-checking block

Unsafe fix
1 1 | from __future__ import annotations
2 2 |
3 3 | from typing_extensions import Self
4 |+from typing import TYPE_CHECKING
5 |+
6 |+if TYPE_CHECKING:
7 |+ from pandas import DataFrame
4 8 |
5 9 |
6 10 | def func():
7 |- from pandas import DataFrame
8 11 |
9 12 | df: DataFrame


Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
---
typing_modules_2.py:7:24: TCH002 [*] Move third-party import `pandas.DataFrame` into a type-checking block
|
6 | def func():
7 | from pandas import DataFrame
| ^^^^^^^^^ TCH002
8 |
9 | df: DataFrame
|
= help: Move into type-checking block

Unsafe fix
2 2 |
3 3 | import typing_extensions
4 4 |
5 |+if typing_extensions.TYPE_CHECKING:
6 |+ from pandas import DataFrame
7 |+
5 8 |
6 9 | def func():
7 |- from pandas import DataFrame
8 10 |
9 11 | df: DataFrame


5 changes: 1 addition & 4 deletions crates/ruff_python_semantic/src/analyze/typing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,10 +315,7 @@ pub fn is_type_checking_block(stmt: &ast::StmtIf, semantic: &SemanticModel) -> b
}

// Ex) `if typing.TYPE_CHECKING:`
if semantic
.resolve_call_path(test)
.is_some_and(|call_path| matches!(call_path.as_slice(), ["typing", "TYPE_CHECKING"]))
{
if semantic.match_typing_expr(test, "TYPE_CHECKING") {
return true;
}

Expand Down
8 changes: 8 additions & 0 deletions crates/ruff_python_semantic/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,14 @@ impl<'a> SemanticModel<'a> {
false
}

/// Return an iterator over the set of `typing` modules allowed in the semantic model.
pub fn typing_modules(&self) -> impl Iterator<Item = &str> {
["typing", "_typeshed", "typing_extensions"]
.iter()
.copied()
.chain(self.typing_modules.iter().map(String::as_str))
}

/// Create a new [`Binding`] for a builtin.
pub fn push_builtin(&mut self) -> BindingId {
self.bindings.push(Binding {
Expand Down

0 comments on commit 565ddeb

Please sign in to comment.