From 7b0fb1a3b4abae2351cc9e562fc24b07a9efc36c Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 5 Jun 2023 22:37:07 -0400 Subject: [PATCH] Respect noqa directives on `ImportFrom` parents for type-checking rules (#4889) --- .../fixtures/flake8_type_checking/TCH002.py | 14 ++++++ crates/ruff/src/checkers/ast/mod.rs | 18 ++------ .../runtime_import_in_type_checking_block.rs | 5 ++- .../rules/typing_only_runtime_import.rs | 43 ++++++++++--------- crates/ruff_python_semantic/src/binding.rs | 14 ++++++ 5 files changed, 59 insertions(+), 35 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/flake8_type_checking/TCH002.py b/crates/ruff/resources/test/fixtures/flake8_type_checking/TCH002.py index 7b082da74e..82d6d2f10b 100644 --- a/crates/ruff/resources/test/fixtures/flake8_type_checking/TCH002.py +++ b/crates/ruff/resources/test/fixtures/flake8_type_checking/TCH002.py @@ -150,3 +150,17 @@ def f(): def f(): import pandas as pd + + +def f(): + from pandas import DataFrame # noqa: TCH002 + + x: DataFrame = 2 + + +def f(): + from pandas import ( # noqa: TCH002 + DataFrame, + ) + + x: DataFrame = 2 diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 08f9508c85..2eda953a01 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -4410,13 +4410,8 @@ impl<'a> Checker<'a> { }, binding.trimmed_range(&self.semantic_model, self.locator), ); - if let Some(parent) = binding.source { - let parent = self.semantic_model.stmts[parent]; - if parent.is_import_from_stmt() - && parent.range().contains_range(binding.range) - { - diagnostic.set_parent(parent.start()); - } + if let Some(range) = binding.parent_range(&self.semantic_model) { + diagnostic.set_parent(range.start()); } self.diagnostics.push(diagnostic); } @@ -5044,13 +5039,8 @@ impl<'a> Checker<'a> { }, binding.trimmed_range(&self.semantic_model, self.locator), ); - if let Some(parent) = binding - .source - .map(|source| &self.semantic_model.stmts[source]) - { - if parent.is_import_from_stmt() { - diagnostic.set_parent(parent.start()); - } + if let Some(range) = binding.parent_range(&self.semantic_model) { + diagnostic.set_parent(range.start()); } diagnostics.push(diagnostic); } diff --git a/crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs b/crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs index a6c0497596..a2da8d1c98 100644 --- a/crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs +++ b/crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs @@ -86,8 +86,11 @@ pub(crate) fn runtime_import_in_type_checking_block( RuntimeImportInTypeCheckingBlock { qualified_name: qualified_name.to_string(), }, - binding.range, + binding.trimmed_range(checker.semantic_model(), checker.locator), ); + if let Some(range) = binding.parent_range(checker.semantic_model()) { + diagnostic.set_parent(range.start()); + } if checker.patch(diagnostic.kind.rule()) { diagnostic.try_set_fix(|| { 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 a9186c91db..085c565271 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 @@ -1,4 +1,4 @@ -use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation}; +use ruff_diagnostics::{AutofixKind, Diagnostic, DiagnosticKind, Fix, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_semantic::binding::Binding; @@ -263,7 +263,7 @@ pub(crate) fn typing_only_runtime_import( .unwrap(); // Categorize the import. - let mut diagnostic = match categorize( + let kind: DiagnosticKind = match categorize( qualified_name, Some(level), &checker.settings.src, @@ -272,32 +272,35 @@ pub(crate) fn typing_only_runtime_import( checker.settings.target_version, ) { ImportSection::Known(ImportType::LocalFolder | ImportType::FirstParty) => { - Diagnostic::new( - TypingOnlyFirstPartyImport { - qualified_name: qualified_name.to_string(), - }, - binding.range, - ) + TypingOnlyFirstPartyImport { + qualified_name: qualified_name.to_string(), + } + .into() } ImportSection::Known(ImportType::ThirdParty) | ImportSection::UserDefined(_) => { - Diagnostic::new( - TypingOnlyThirdPartyImport { - qualified_name: qualified_name.to_string(), - }, - binding.range, - ) - } - ImportSection::Known(ImportType::StandardLibrary) => Diagnostic::new( - TypingOnlyStandardLibraryImport { + TypingOnlyThirdPartyImport { qualified_name: qualified_name.to_string(), - }, - binding.range, - ), + } + .into() + } + ImportSection::Known(ImportType::StandardLibrary) => TypingOnlyStandardLibraryImport { + qualified_name: qualified_name.to_string(), + } + .into(), + ImportSection::Known(ImportType::Future) => { unreachable!("`__future__` imports should be marked as used") } }; + let mut diagnostic = Diagnostic::new( + kind, + binding.trimmed_range(checker.semantic_model(), checker.locator), + ); + if let Some(range) = binding.parent_range(checker.semantic_model()) { + diagnostic.set_parent(range.start()); + } + if checker.patch(diagnostic.kind.rule()) { diagnostic.try_set_fix(|| { // Step 1) Remove the import. diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index dd862e8ce6..6d30793aa4 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -2,6 +2,7 @@ use std::ops::{Deref, DerefMut}; use bitflags::bitflags; use ruff_text_size::TextRange; +use rustpython_parser::ast::Ranged; use ruff_index::{newtype_index, IndexSlice, IndexVec}; use ruff_python_ast::helpers; @@ -143,6 +144,19 @@ impl<'a> Binding<'a> { _ => self.range, } } + + /// Returns the range of the binding's parent. + pub fn parent_range(&self, semantic_model: &SemanticModel) -> Option { + self.source + .map(|node_id| semantic_model.stmts[node_id]) + .and_then(|parent| { + if parent.is_import_from_stmt() { + Some(parent.range()) + } else { + None + } + }) + } } bitflags! {