Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(visitor): skip type checking blocks without future annotations #662

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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