From 8a98cfc4b89834e73c7ebace93d5404b14f4dc42 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 9 Feb 2023 11:22:15 -0500 Subject: [PATCH] Treat re-exported annotations as used-at-runtime (#2689) --- .../flake8_type_checking/TCH004_11.py | 6 +++ crates/ruff/src/checkers/ast.rs | 50 ++++++++++--------- .../src/rules/flake8_type_checking/mod.rs | 1 + ...t-in-type-checking-block_TCH004_11.py.snap | 16 ++++++ .../rules/pylint/rules/invalid_all_format.rs | 10 ++-- .../rules/pylint/rules/invalid_all_object.rs | 10 ++-- 6 files changed, 58 insertions(+), 35 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/flake8_type_checking/TCH004_11.py create mode 100644 crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__runtime-import-in-type-checking-block_TCH004_11.py.snap diff --git a/crates/ruff/resources/test/fixtures/flake8_type_checking/TCH004_11.py b/crates/ruff/resources/test/fixtures/flake8_type_checking/TCH004_11.py new file mode 100644 index 0000000000..4a64b209d8 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_type_checking/TCH004_11.py @@ -0,0 +1,6 @@ +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import List + +__all__ = ("List",) diff --git a/crates/ruff/src/checkers/ast.rs b/crates/ruff/src/checkers/ast.rs index 00a16df661..662d0c1183 100644 --- a/crates/ruff/src/checkers/ast.rs +++ b/crates/ruff/src/checkers/ast.rs @@ -4312,17 +4312,33 @@ impl<'a> Checker<'a> { _ => false, } { let (all_names, all_names_flags) = extract_all_names(self, parent, current); + let all_bindings: Vec = all_names + .iter() + .filter_map(|name| current.bindings.get(name.as_str())) + .copied() + .collect(); - if self.settings.rules.enabled(&Rule::InvalidAllFormat) - && matches!(all_names_flags, AllNamesFlags::INVALID_FORMAT) - { - pylint::rules::invalid_all_format(self, expr); + if self.settings.rules.enabled(&Rule::InvalidAllFormat) { + if matches!(all_names_flags, AllNamesFlags::INVALID_FORMAT) { + self.diagnostics + .push(pylint::rules::invalid_all_format(expr)); + } } - if self.settings.rules.enabled(&Rule::InvalidAllObject) - && matches!(all_names_flags, AllNamesFlags::INVALID_OBJECT) - { - pylint::rules::invalid_all_object(self, expr); + if self.settings.rules.enabled(&Rule::InvalidAllObject) { + if matches!(all_names_flags, AllNamesFlags::INVALID_OBJECT) { + self.diagnostics + .push(pylint::rules::invalid_all_object(expr)); + } + } + + // Mark all exported names as used-at-runtime. + for index in all_bindings { + self.bindings[index].mark_used( + GLOBAL_SCOPE_INDEX, + Range::from_located(expr), + ExecutionContext::Runtime, + ); } self.add_binding( @@ -4701,13 +4717,7 @@ impl<'a> Checker<'a> { | BindingKind::StarImportation(..) | BindingKind::FutureImportation ) { - // Skip used exports from `__all__` - if binding.used() - || all_names - .as_ref() - .map(|names| names.contains(name)) - .unwrap_or_default() - { + if binding.used() { continue; } @@ -4829,7 +4839,7 @@ impl<'a> Checker<'a> { let mut ignored: FxHashMap> = FxHashMap::default(); - for (name, index) in &scope.bindings { + for index in scope.bindings.values() { let binding = &self.bindings[*index]; let full_name = match &binding.kind { @@ -4839,13 +4849,7 @@ impl<'a> Checker<'a> { _ => continue, }; - // Skip used exports from `__all__` - if binding.used() - || all_names - .as_ref() - .map(|names| names.contains(name)) - .unwrap_or_default() - { + if binding.used() { continue; } diff --git a/crates/ruff/src/rules/flake8_type_checking/mod.rs b/crates/ruff/src/rules/flake8_type_checking/mod.rs index 7d8f0c26d7..4d3fc729f0 100644 --- a/crates/ruff/src/rules/flake8_type_checking/mod.rs +++ b/crates/ruff/src/rules/flake8_type_checking/mod.rs @@ -28,6 +28,7 @@ mod tests { #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_8.py"); "TCH004_8")] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_9.py"); "TCH004_9")] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_10.py"); "TCH004_10")] + #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_11.py"); "TCH004_11")] #[test_case(Rule::EmptyTypeCheckingBlock, Path::new("TCH005.py"); "TCH005")] #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("strict.py"); "strict")] fn rules(rule_code: Rule, path: &Path) -> Result<()> { diff --git a/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__runtime-import-in-type-checking-block_TCH004_11.py.snap b/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__runtime-import-in-type-checking-block_TCH004_11.py.snap new file mode 100644 index 0000000000..e8aa402677 --- /dev/null +++ b/crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__runtime-import-in-type-checking-block_TCH004_11.py.snap @@ -0,0 +1,16 @@ +--- +source: crates/ruff/src/rules/flake8_type_checking/mod.rs +expression: diagnostics +--- +- kind: + RuntimeImportInTypeCheckingBlock: + full_name: typing.List + location: + row: 4 + column: 23 + end_location: + row: 4 + column: 27 + fix: ~ + parent: ~ + diff --git a/crates/ruff/src/rules/pylint/rules/invalid_all_format.rs b/crates/ruff/src/rules/pylint/rules/invalid_all_format.rs index 85e4009bc3..8a728a4cdb 100644 --- a/crates/ruff/src/rules/pylint/rules/invalid_all_format.rs +++ b/crates/ruff/src/rules/pylint/rules/invalid_all_format.rs @@ -1,8 +1,8 @@ -use ruff_macros::{define_violation, derive_message_formats}; use rustpython_parser::ast::Expr; +use ruff_macros::{define_violation, derive_message_formats}; + use crate::ast::types::Range; -use crate::checkers::ast::Checker; use crate::registry::Diagnostic; use crate::violation::Violation; @@ -17,8 +17,6 @@ impl Violation for InvalidAllFormat { } /// PLE0605 -pub fn invalid_all_format(checker: &mut Checker, expr: &Expr) { - checker - .diagnostics - .push(Diagnostic::new(InvalidAllFormat, Range::from_located(expr))); +pub fn invalid_all_format(expr: &Expr) -> Diagnostic { + Diagnostic::new(InvalidAllFormat, Range::from_located(expr)) } diff --git a/crates/ruff/src/rules/pylint/rules/invalid_all_object.rs b/crates/ruff/src/rules/pylint/rules/invalid_all_object.rs index 2307f6ab24..012b616138 100644 --- a/crates/ruff/src/rules/pylint/rules/invalid_all_object.rs +++ b/crates/ruff/src/rules/pylint/rules/invalid_all_object.rs @@ -1,8 +1,8 @@ -use ruff_macros::{define_violation, derive_message_formats}; use rustpython_parser::ast::Expr; +use ruff_macros::{define_violation, derive_message_formats}; + use crate::ast::types::Range; -use crate::checkers::ast::Checker; use crate::registry::Diagnostic; use crate::violation::Violation; @@ -17,8 +17,6 @@ impl Violation for InvalidAllObject { } /// PLE0604 -pub fn invalid_all_object(checker: &mut Checker, expr: &Expr) { - checker - .diagnostics - .push(Diagnostic::new(InvalidAllObject, Range::from_located(expr))); +pub fn invalid_all_object(expr: &Expr) -> Diagnostic { + Diagnostic::new(InvalidAllObject, Range::from_located(expr)) }