Skip to content

Commit

Permalink
fix(visitor): skip type checking blocks without future annotations (#662
Browse files Browse the repository at this point in the history
)
  • Loading branch information
mkniewallner authored Apr 5, 2024
1 parent 36a06f0 commit 1fa8170
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 55 deletions.
5 changes: 2 additions & 3 deletions docs/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,8 @@ Imports will be extracted regardless of where they are made in a file (top-level
conditions, ...).

The only exception is imports that are guarded
by [`TYPE_CHECKING`](https://docs.python.org/3/library/typing.html#typing.TYPE_CHECKING) when
using `from __future__ import annotations`, in accordance with [PEP 563](https://peps.python.org/pep-0563/). In this
specific case, _deptry_ will not extract those imports, as they are not considered problematic. For instance:
by [`TYPE_CHECKING`](https://docs.python.org/3/library/typing.html#typing.TYPE_CHECKING). In this specific case,
_deptry_ will not extract those imports, as they are not considered problematic. For instance:

```python
from __future__ import annotations
Expand Down
47 changes: 14 additions & 33 deletions src/visitor.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,17 @@
use ruff_python_ast::visitor::{walk_stmt, Visitor};
use ruff_python_ast::{self, Expr, ExprAttribute, ExprName, Stmt, StmtIf, StmtImportFrom};
use ruff_python_ast::{self, Expr, ExprAttribute, ExprName, Stmt, StmtIf};
use ruff_text_size::TextRange;
use std::collections::HashMap;

#[derive(Debug, Clone)]
pub struct ImportVisitor {
imports: HashMap<String, Vec<TextRange>>,
has_future_annotations: bool,
}

impl ImportVisitor {
pub fn new() -> Self {
Self {
imports: HashMap::new(),
has_future_annotations: false,
}
}

Expand All @@ -37,18 +35,14 @@ impl<'a> Visitor<'a> for ImportVisitor {
Stmt::ImportFrom(import_from_stmt) => {
if let Some(module) = &import_from_stmt.module {
if import_from_stmt.level == Some(0) {
if is_future_annotations_import(module.as_str(), import_from_stmt) {
self.has_future_annotations = true;
}

self.imports
.entry(get_top_level_module_name(module.as_str()))
.or_default()
.push(import_from_stmt.range);
}
}
}
Stmt::If(if_stmt) if is_typing_only_block(self.has_future_annotations, if_stmt) => {
Stmt::If(if_stmt) if is_guarded_by_type_checking(if_stmt) => {
// Avoid parsing imports that are only evaluated by type checkers.
}
_ => walk_stmt(self, stmt), // Delegate other statements to walk_stmt
Expand All @@ -66,35 +60,22 @@ fn get_top_level_module_name(module_name: &str) -> String {
.to_owned()
}

/// Checks if the import is a `from __future__ import annotations` one.
fn is_future_annotations_import(module: &str, import_from_stmt: &StmtImportFrom) -> bool {
return module == "__future__"
&& import_from_stmt
.names
.iter()
.any(|alias| alias.name.as_str() == "annotations");
}

/// Checks if we are in a block that will only be evaluated by type checkers, in accordance with
/// <https://peps.python.org/pep-0563/>. If no `__future__.annotations` import is made, a block using `TYPE_CHECKING`
/// will be evaluated at runtime, so we should not consider that this is a typing only block in that case.
fn is_typing_only_block(has_future_annotations: bool, if_stmt: &StmtIf) -> bool {
if has_future_annotations {
match &if_stmt.test.as_ref() {
Expr::Attribute(ExprAttribute { value, attr, .. }) => {
if let Expr::Name(ExprName { id, .. }) = value.as_ref() {
if id.as_str() == "typing" && attr.as_str() == "TYPE_CHECKING" {
return true;
}
}
}
Expr::Name(ExprName { id, .. }) => {
if id == "TYPE_CHECKING" {
/// Checks if we are in a block guarded by `typing.TYPE_CHECKING`.
fn is_guarded_by_type_checking(if_stmt: &StmtIf) -> bool {
match &if_stmt.test.as_ref() {
Expr::Attribute(ExprAttribute { value, attr, .. }) => {
if let Expr::Name(ExprName { id, .. }) = value.as_ref() {
if id.as_str() == "typing" && attr.as_str() == "TYPE_CHECKING" {
return true;
}
}
_ => (),
}
Expr::Name(ExprName { id, .. }) => {
if id == "TYPE_CHECKING" {
return true;
}
}
_ => (),
}

false
Expand Down
2 changes: 0 additions & 2 deletions tests/data/some_imports.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
from __future__ import annotations

import typing
from os import chdir, walk
from pathlib import Path
Expand Down
33 changes: 16 additions & 17 deletions tests/unit/imports/test_extract.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,26 +21,25 @@ def test_import_parser_py() -> None:
some_imports_path = Path("tests/data/some_imports.py")

assert get_imported_modules_from_list_of_files([some_imports_path]) == {
"__future__": [Location(some_imports_path, 1, 1)],
"barfoo": [Location(some_imports_path, 23, 8)],
"baz": [Location(some_imports_path, 19, 5)],
"click": [Location(some_imports_path, 33, 12)],
"foobar": [Location(some_imports_path, 21, 12)],
"httpx": [Location(some_imports_path, 17, 12)],
"module_in_class": [Location(some_imports_path, 44, 16)],
"module_in_func": [Location(some_imports_path, 39, 12)],
"not_click": [Location(some_imports_path, 35, 12)],
"barfoo": [Location(some_imports_path, 21, 8)],
"baz": [Location(some_imports_path, 17, 5)],
"click": [Location(some_imports_path, 31, 12)],
"foobar": [Location(some_imports_path, 19, 12)],
"httpx": [Location(some_imports_path, 15, 12)],
"module_in_class": [Location(some_imports_path, 42, 16)],
"module_in_func": [Location(some_imports_path, 37, 12)],
"not_click": [Location(some_imports_path, 33, 12)],
"numpy": [
Location(some_imports_path, 8, 8),
Location(some_imports_path, 10, 1),
Location(some_imports_path, 6, 8),
Location(some_imports_path, 8, 1),
],
"os": [Location(some_imports_path, 4, 1)],
"pandas": [Location(some_imports_path, 9, 8)],
"pathlib": [Location(some_imports_path, 5, 1)],
"randomizer": [Location(some_imports_path, 24, 1)],
"os": [Location(some_imports_path, 2, 1)],
"pandas": [Location(some_imports_path, 7, 8)],
"pathlib": [Location(some_imports_path, 3, 1)],
"randomizer": [Location(some_imports_path, 22, 1)],
"typing": [
Location(some_imports_path, 3, 8),
Location(some_imports_path, 6, 1),
Location(some_imports_path, 1, 8),
Location(some_imports_path, 4, 1),
],
}

Expand Down

0 comments on commit 1fa8170

Please sign in to comment.