From 3de92a516cf729a4de76ccde247aa2e9719581b5 Mon Sep 17 00:00:00 2001 From: David Salvisberg Date: Fri, 16 Aug 2024 13:31:53 +0200 Subject: [PATCH 01/24] Adds initial implementation for TCH008 The implementation for TCH007 is incomplete and also requires some changes to TCH004 in order to avoid conflicts between TCH004 and TCH007 --- .../fixtures/flake8_type_checking/TCH008.py | 29 +++ .../checkers/ast/analyze/deferred_scopes.rs | 4 + crates/ruff_linter/src/checkers/ast/mod.rs | 41 +++- crates/ruff_linter/src/codes.rs | 2 + .../src/rules/flake8_type_checking/mod.rs | 1 + .../rules/flake8_type_checking/rules/mod.rs | 2 + .../rules/runtime_string_union.rs | 2 +- .../rules/type_alias_quotes.rs | 196 +++++++++++++++++ ...g__tests__quoted-type-alias_TCH008.py.snap | 201 ++++++++++++++++++ crates/ruff_python_semantic/src/model.rs | 27 +++ ruff.schema.json | 2 + 11 files changed, 501 insertions(+), 6 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py create mode 100644 crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs create mode 100644 crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py new file mode 100644 index 0000000000000..77b432b6c406b --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py @@ -0,0 +1,29 @@ +from __future__ import annotations + +from typing import TypeAlias, TYPE_CHECKING + +from foo import Foo + +if TYPE_CHECKING: + from typing import Dict + + OptStr: TypeAlias = str | None + Bar: TypeAlias = Foo[int] +else: + Bar = Foo + +a: TypeAlias = 'int' # TCH008 +b: TypeAlias = 'Dict' # OK +c: TypeAlias = 'Foo' # TCH008 +d: TypeAlias = 'Foo[str]' # OK +e: TypeAlias = 'Foo.bar' # OK +f: TypeAlias = 'Foo | None' # TCH008 +g: TypeAlias = 'OptStr' # OK +h: TypeAlias = 'Bar' # TCH008 +i: TypeAlias = Foo['str'] # TCH008 + +type B = 'Dict' # TCH008 +type D = 'Foo[str]' # TCH008 +type E = 'Foo.bar' # TCH008 +type G = 'OptStr' # TCH008 +type I = Foo['str'] # TCH008 diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs index 003db1d741782..1b1bfcd489aca 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs @@ -52,6 +52,8 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { // Identify any valid runtime imports. If a module is imported at runtime, and // used at runtime, then by default, we avoid flagging any other // imports from that model as typing-only. + // FIXME: This does not seem quite right, if only TC004 is enabled + // then we don't need to collect the runtime imports let enforce_typing_imports = !checker.source_type.is_stub() && checker.any_enabled(&[ Rule::RuntimeImportInTypeCheckingBlock, @@ -375,6 +377,8 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { } if matches!(scope.kind, ScopeKind::Function(_) | ScopeKind::Module) { + // FIXME: This does not seem quite right, if only TC004 is enabled + // then we don't need to collect the runtime imports if enforce_typing_imports { let runtime_imports: Vec<&Binding> = checker .semantic diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 814f2d9f38fab..3e4187ed4e32d 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -831,9 +831,7 @@ impl<'a> Visitor<'a> for Checker<'a> { if let Some(type_params) = type_params { self.visit_type_params(type_params); } - self.visit - .type_param_definitions - .push((value, self.semantic.snapshot())); + self.visit_generic_type_alias(value); self.semantic.pop_scope(); self.visit_expr(name); } @@ -904,7 +902,7 @@ impl<'a> Visitor<'a> for Checker<'a> { if let Some(expr) = value { if self.semantic.match_typing_expr(annotation, "TypeAlias") { - self.visit_type_definition(expr); + self.visit_explicit_type_alias(expr); } else { self.visit_expr(expr); } @@ -1790,6 +1788,28 @@ impl<'a> Checker<'a> { self.semantic.flags = snapshot; } + /// Visit an [`Expr`], and treat it as a [PEP 613] explicit type alias. + /// + /// [PEP 613]: https://peps.python.org/pep-0613/ + fn visit_explicit_type_alias(&mut self, expr: &'a Expr) { + let snapshot = self.semantic.flags; + self.semantic.flags |= SemanticModelFlags::EXPLICIT_TYPE_ALIAS; + self.visit_type_definition(expr); + self.semantic.flags = snapshot; + } + + /// Visit an [`Expr`], and treat it as a [PEP 695] generic type alias. + /// + /// [PEP 695]: https://peps.python.org/pep-0695/#generic-type-alias + fn visit_generic_type_alias(&mut self, expr: &'a Expr) { + let snapshot = self.semantic.flags; + self.semantic.flags |= SemanticModelFlags::GENERIC_TYPE_ALIAS; + self.visit + .type_param_definitions + .push((expr, self.semantic.snapshot())); + self.semantic.flags = snapshot; + } + /// Visit an [`Expr`], and treat it as a type definition. fn visit_type_definition(&mut self, expr: &'a Expr) { let snapshot = self.semantic.flags; @@ -2207,7 +2227,18 @@ impl<'a> Checker<'a> { self.semantic.flags |= SemanticModelFlags::TYPE_DEFINITION | type_definition_flag; - self.visit_expr(parsed_annotation.expr()); + let parsed_expr = parsed_annotation.expr(); + self.visit_expr(parsed_expr); + if self.semantic.in_type_alias() { + if self.enabled(Rule::QuotedTypeAlias) { + flake8_type_checking::rules::quoted_type_alias( + self, + parsed_expr, + annotation, + range, + ); + } + } self.parsed_type_annotation = None; } else { if self.enabled(Rule::ForwardAnnotationSyntaxError) { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 9faf8c8c373b2..4bccb8a206b53 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -845,6 +845,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8TypeChecking, "003") => (RuleGroup::Stable, rules::flake8_type_checking::rules::TypingOnlyStandardLibraryImport), (Flake8TypeChecking, "004") => (RuleGroup::Stable, rules::flake8_type_checking::rules::RuntimeImportInTypeCheckingBlock), (Flake8TypeChecking, "005") => (RuleGroup::Stable, rules::flake8_type_checking::rules::EmptyTypeCheckingBlock), + (Flake8TypeChecking, "007") => (RuleGroup::Preview, rules::flake8_type_checking::rules::UnquotedTypeAlias), + (Flake8TypeChecking, "008") => (RuleGroup::Preview, rules::flake8_type_checking::rules::QuotedTypeAlias), (Flake8TypeChecking, "010") => (RuleGroup::Stable, rules::flake8_type_checking::rules::RuntimeStringUnion), // tryceratops diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs index 30fc9c5aa7d8b..41bded76447f9 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs @@ -35,6 +35,7 @@ mod tests { #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_8.py"))] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_9.py"))] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("quote.py"))] + #[test_case(Rule::QuotedTypeAlias, Path::new("TCH008.py"))] #[test_case(Rule::RuntimeStringUnion, Path::new("TCH010_1.py"))] #[test_case(Rule::RuntimeStringUnion, Path::new("TCH010_2.py"))] #[test_case(Rule::TypingOnlyFirstPartyImport, Path::new("TCH001.py"))] diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/mod.rs index 1f94e927c46c6..ef054ad8a6637 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/mod.rs @@ -1,9 +1,11 @@ pub(crate) use empty_type_checking_block::*; pub(crate) use runtime_import_in_type_checking_block::*; pub(crate) use runtime_string_union::*; +pub(crate) use type_alias_quotes::*; pub(crate) use typing_only_runtime_import::*; mod empty_type_checking_block; mod runtime_import_in_type_checking_block; mod runtime_string_union; +mod type_alias_quotes; mod typing_only_runtime_import; diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_string_union.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_string_union.rs index 7d63517a32ca3..66be8359e3508 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_string_union.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_string_union.rs @@ -51,7 +51,7 @@ impl Violation for RuntimeStringUnion { } } -/// TCH006 +/// TCH010 pub(crate) fn runtime_string_union(checker: &mut Checker, expr: &Expr) { if !checker.semantic().in_type_definition() { return; diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs new file mode 100644 index 0000000000000..42fc1fddca6b7 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs @@ -0,0 +1,196 @@ +use ast::ExprContext; +use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast as ast; +use ruff_python_ast::Expr; +use ruff_python_semantic::SemanticModel; +use ruff_text_size::TextRange; +use std::borrow::Borrow; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks if [PEP 613] explicit type aliases contain references to +/// symbols that are not available at runtime. +/// +/// ## Why is this bad? +/// We will get a `NameError` at runtime. +/// +/// ## Example +/// ```python +/// from typing import TYPE_CHECKING, TypeAlias +/// if TYPE_CHECKING: +/// from foo import Foo +/// OptFoo: TypeAlias = Foo | None +/// ``` +/// +/// Use instead: +/// ```python +/// from typing import TYPE_CHECKING, TypeAlias +/// if TYPE_CHECKING: +/// from foo import Foo +/// OptFoo: TypeAlias = "Foo | None" +/// ``` +/// +/// ## References +/// - [PEP 613](https://peps.python.org/pep-0613/) +/// +/// [PEP 613]: https://peps.python.org/pep-0613/ +#[violation] +pub struct UnquotedTypeAlias; + +impl AlwaysFixableViolation for UnquotedTypeAlias { + #[derive_message_formats] + fn message(&self) -> String { + format!("Add quotes to type alias") + } + + fn fix_title(&self) -> String { + "Add quotes".to_string() + } +} + +/// ## What it does +/// Checks for unnecessary quotes in [PEP 613] explicit type aliases +/// and [PEP 695] type statements. +/// +/// ## Why is this bad? +/// Unnecessary string forward references can lead to additional overhead +/// in runtime libraries making use of type hints, as well as lead to bad +/// interactions with other runtime uses like [PEP 604] type unions. +/// +/// For explicit type aliases the quotes are only considered redundant +/// if the type expression contains no subscripts or attribute accesses +/// this is because of stubs packages. Some types will only be subscriptable +/// at type checking time, similarly there may be some module-level +/// attributes like type aliases that are only available in the stubs. +/// +/// ## Example +/// Given: +/// ```python +/// OptInt: TypeAlias = "int | None" +/// ``` +/// +/// Use instead: +/// ```python +/// OptInt: TypeAlias = int | None +/// ``` +/// +/// Given: +/// ```python +/// type OptInt = "int | None" +/// ``` +/// +/// Use instead: +/// ```python +/// type OptInt = int | None +/// ``` +/// +/// ## References +/// - [PEP 613](https://peps.python.org/pep-0613/) +/// - [PEP 695](https://peps.python.org/pep-0695/#generic-type-alias) +/// - [PEP 604](https://peps.python.org/pep-0604/) +/// +/// [PEP 604]: https://peps.python.org/pep-0604/ +/// [PEP 613]: https://peps.python.org/pep-0613/ +/// [PEP 695]: https://peps.python.org/pep-0695/#generic-type-alias +#[violation] +pub struct QuotedTypeAlias; + +impl AlwaysFixableViolation for QuotedTypeAlias { + #[derive_message_formats] + fn message(&self) -> String { + format!("Remove quotes from type alias") + } + + fn fix_title(&self) -> String { + "Remove quotes".to_string() + } +} + +/// TCH007 +/*pub(crate) fn unquoted_type_alias(checker: &mut Checker, expr: &Expr) { + if !checker.semantic().in_explicit_type_alias() { + return; + } + + if checker.semantic().in_forward_reference() { + return; + } + + // TODO implement this +}*/ + +/// Traverses the type expression and checks if any of the encountered names reference +/// a binding that is defined within a typing-only context. +// TODO: Do we want to remove Attribute and Subscript traversal? We already +// skip expressions that don't contain either. But then we can't reuse +// this function for TCH007. Is it worth having two functions where one +// has fewer branches because we know they won't be there? +fn contains_typing_reference<'a>(semantic: &SemanticModel, expr: &'a Expr) -> bool { + match expr { + Expr::BinOp(ast::ExprBinOp { left, right, .. }) => { + contains_typing_reference(semantic, left) || contains_typing_reference(semantic, right) + } + Expr::Starred(ast::ExprStarred { + value, + ctx: ExprContext::Load, + .. + }) + | Expr::Attribute(ast::ExprAttribute { value, .. }) => { + contains_typing_reference(semantic, value) + } + Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { + if contains_typing_reference(semantic, value) { + return true; + } + if let Expr::Name(ast::ExprName { id, .. }) = value.borrow() { + if id.as_str() != "Literal" { + return contains_typing_reference(semantic, slice); + } + } + false + } + Expr::List(ast::ExprList { elts, .. }) | Expr::Tuple(ast::ExprTuple { elts, .. }) => { + for elt in elts { + if contains_typing_reference(semantic, elt) { + return true; + } + } + false + } + Expr::Name(name) => semantic + .resolve_name(name) + .is_some_and(|binding_id| semantic.binding(binding_id).context.is_typing()), + _ => false, + } +} + +/// TCH008 +pub(crate) fn quoted_type_alias( + checker: &mut Checker, + expr: &Expr, + annotation: &str, + range: TextRange, +) { + // explicit type aliases require some additional checks to avoid false positives + if checker.semantic().in_explicit_type_alias() { + // if the expression contains a subscript or attribute access + if annotation.find(|c: char| c == '[' || c == '.').is_some() { + return; + } + + // if the expression contains references to typing-only bindings + // then the quotes are not redundant + if contains_typing_reference(checker.semantic(), expr) { + return; + } + } + + let mut diagnostic = Diagnostic::new(QuotedTypeAlias, range); + diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + annotation.to_string(), + range, + ))); + checker.diagnostics.push(diagnostic); +} diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap new file mode 100644 index 0000000000000..d55270d702a4f --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap @@ -0,0 +1,201 @@ +--- +source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +--- +TCH008.py:15:16: TCH008 [*] Remove quotes from type alias + | +13 | Bar = Foo +14 | +15 | a: TypeAlias = 'int' # TCH008 + | ^^^^^ TCH008 +16 | b: TypeAlias = 'Dict' # OK +17 | c: TypeAlias = 'Foo' # TCH008 + | + = help: Remove quotes + +ℹ Safe fix +12 12 | else: +13 13 | Bar = Foo +14 14 | +15 |-a: TypeAlias = 'int' # TCH008 + 15 |+a: TypeAlias = int # TCH008 +16 16 | b: TypeAlias = 'Dict' # OK +17 17 | c: TypeAlias = 'Foo' # TCH008 +18 18 | d: TypeAlias = 'Foo[str]' # OK + +TCH008.py:17:16: TCH008 [*] Remove quotes from type alias + | +15 | a: TypeAlias = 'int' # TCH008 +16 | b: TypeAlias = 'Dict' # OK +17 | c: TypeAlias = 'Foo' # TCH008 + | ^^^^^ TCH008 +18 | d: TypeAlias = 'Foo[str]' # OK +19 | e: TypeAlias = 'Foo.bar' # OK + | + = help: Remove quotes + +ℹ Safe fix +14 14 | +15 15 | a: TypeAlias = 'int' # TCH008 +16 16 | b: TypeAlias = 'Dict' # OK +17 |-c: TypeAlias = 'Foo' # TCH008 + 17 |+c: TypeAlias = Foo # TCH008 +18 18 | d: TypeAlias = 'Foo[str]' # OK +19 19 | e: TypeAlias = 'Foo.bar' # OK +20 20 | f: TypeAlias = 'Foo | None' # TCH008 + +TCH008.py:20:16: TCH008 [*] Remove quotes from type alias + | +18 | d: TypeAlias = 'Foo[str]' # OK +19 | e: TypeAlias = 'Foo.bar' # OK +20 | f: TypeAlias = 'Foo | None' # TCH008 + | ^^^^^^^^^^^^ TCH008 +21 | g: TypeAlias = 'OptStr' # OK +22 | h: TypeAlias = 'Bar' # TCH008 + | + = help: Remove quotes + +ℹ Safe fix +17 17 | c: TypeAlias = 'Foo' # TCH008 +18 18 | d: TypeAlias = 'Foo[str]' # OK +19 19 | e: TypeAlias = 'Foo.bar' # OK +20 |-f: TypeAlias = 'Foo | None' # TCH008 + 20 |+f: TypeAlias = Foo | None # TCH008 +21 21 | g: TypeAlias = 'OptStr' # OK +22 22 | h: TypeAlias = 'Bar' # TCH008 +23 23 | i: TypeAlias = Foo['str'] # TCH008 + +TCH008.py:22:16: TCH008 [*] Remove quotes from type alias + | +20 | f: TypeAlias = 'Foo | None' # TCH008 +21 | g: TypeAlias = 'OptStr' # OK +22 | h: TypeAlias = 'Bar' # TCH008 + | ^^^^^ TCH008 +23 | i: TypeAlias = Foo['str'] # TCH008 + | + = help: Remove quotes + +ℹ Safe fix +19 19 | e: TypeAlias = 'Foo.bar' # OK +20 20 | f: TypeAlias = 'Foo | None' # TCH008 +21 21 | g: TypeAlias = 'OptStr' # OK +22 |-h: TypeAlias = 'Bar' # TCH008 + 22 |+h: TypeAlias = Bar # TCH008 +23 23 | i: TypeAlias = Foo['str'] # TCH008 +24 24 | +25 25 | type B = 'Dict' # TCH008 + +TCH008.py:23:20: TCH008 [*] Remove quotes from type alias + | +21 | g: TypeAlias = 'OptStr' # OK +22 | h: TypeAlias = 'Bar' # TCH008 +23 | i: TypeAlias = Foo['str'] # TCH008 + | ^^^^^ TCH008 +24 | +25 | type B = 'Dict' # TCH008 + | + = help: Remove quotes + +ℹ Safe fix +20 20 | f: TypeAlias = 'Foo | None' # TCH008 +21 21 | g: TypeAlias = 'OptStr' # OK +22 22 | h: TypeAlias = 'Bar' # TCH008 +23 |-i: TypeAlias = Foo['str'] # TCH008 + 23 |+i: TypeAlias = Foo[str] # TCH008 +24 24 | +25 25 | type B = 'Dict' # TCH008 +26 26 | type D = 'Foo[str]' # TCH008 + +TCH008.py:25:10: TCH008 [*] Remove quotes from type alias + | +23 | i: TypeAlias = Foo['str'] # TCH008 +24 | +25 | type B = 'Dict' # TCH008 + | ^^^^^^ TCH008 +26 | type D = 'Foo[str]' # TCH008 +27 | type E = 'Foo.bar' # TCH008 + | + = help: Remove quotes + +ℹ Safe fix +22 22 | h: TypeAlias = 'Bar' # TCH008 +23 23 | i: TypeAlias = Foo['str'] # TCH008 +24 24 | +25 |-type B = 'Dict' # TCH008 + 25 |+type B = Dict # TCH008 +26 26 | type D = 'Foo[str]' # TCH008 +27 27 | type E = 'Foo.bar' # TCH008 +28 28 | type G = 'OptStr' # TCH008 + +TCH008.py:26:10: TCH008 [*] Remove quotes from type alias + | +25 | type B = 'Dict' # TCH008 +26 | type D = 'Foo[str]' # TCH008 + | ^^^^^^^^^^ TCH008 +27 | type E = 'Foo.bar' # TCH008 +28 | type G = 'OptStr' # TCH008 + | + = help: Remove quotes + +ℹ Safe fix +23 23 | i: TypeAlias = Foo['str'] # TCH008 +24 24 | +25 25 | type B = 'Dict' # TCH008 +26 |-type D = 'Foo[str]' # TCH008 + 26 |+type D = Foo[str] # TCH008 +27 27 | type E = 'Foo.bar' # TCH008 +28 28 | type G = 'OptStr' # TCH008 +29 29 | type I = Foo['str'] # TCH008 + +TCH008.py:27:10: TCH008 [*] Remove quotes from type alias + | +25 | type B = 'Dict' # TCH008 +26 | type D = 'Foo[str]' # TCH008 +27 | type E = 'Foo.bar' # TCH008 + | ^^^^^^^^^ TCH008 +28 | type G = 'OptStr' # TCH008 +29 | type I = Foo['str'] # TCH008 + | + = help: Remove quotes + +ℹ Safe fix +24 24 | +25 25 | type B = 'Dict' # TCH008 +26 26 | type D = 'Foo[str]' # TCH008 +27 |-type E = 'Foo.bar' # TCH008 + 27 |+type E = Foo.bar # TCH008 +28 28 | type G = 'OptStr' # TCH008 +29 29 | type I = Foo['str'] # TCH008 + +TCH008.py:28:10: TCH008 [*] Remove quotes from type alias + | +26 | type D = 'Foo[str]' # TCH008 +27 | type E = 'Foo.bar' # TCH008 +28 | type G = 'OptStr' # TCH008 + | ^^^^^^^^ TCH008 +29 | type I = Foo['str'] # TCH008 + | + = help: Remove quotes + +ℹ Safe fix +25 25 | type B = 'Dict' # TCH008 +26 26 | type D = 'Foo[str]' # TCH008 +27 27 | type E = 'Foo.bar' # TCH008 +28 |-type G = 'OptStr' # TCH008 + 28 |+type G = OptStr # TCH008 +29 29 | type I = Foo['str'] # TCH008 + +TCH008.py:29:14: TCH008 [*] Remove quotes from type alias + | +27 | type E = 'Foo.bar' # TCH008 +28 | type G = 'OptStr' # TCH008 +29 | type I = Foo['str'] # TCH008 + | ^^^^^ TCH008 + | + = help: Remove quotes + +ℹ Safe fix +26 26 | type D = 'Foo[str]' # TCH008 +27 27 | type E = 'Foo.bar' # TCH008 +28 28 | type G = 'OptStr' # TCH008 +29 |-type I = Foo['str'] # TCH008 + 29 |+type I = Foo[str] # TCH008 diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 7ef179b51a9d9..ab87f4afa78f7 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -1646,6 +1646,23 @@ impl<'a> SemanticModel<'a> { || (self.in_future_type_definition() && self.in_typing_only_annotation()) } + /// Return `true` if the model is in an explicit type alias + pub const fn in_explicit_type_alias(&self) -> bool { + self.flags + .intersects(SemanticModelFlags::EXPLICIT_TYPE_ALIAS) + } + + /// Return `true` if the model is in a generic type alias + pub const fn in_generic_type_alias(&self) -> bool { + self.flags + .intersects(SemanticModelFlags::GENERIC_TYPE_ALIAS) + } + + /// Return `true` if the model is in a type alias + pub const fn in_type_alias(&self) -> bool { + self.in_explicit_type_alias() || self.in_generic_type_alias() + } + /// Return `true` if the model is in an exception handler. pub const fn in_exception_handler(&self) -> bool { self.flags.intersects(SemanticModelFlags::EXCEPTION_HANDLER) @@ -2214,6 +2231,16 @@ bitflags! { /// [PEP 257]: https://peps.python.org/pep-0257/#what-is-a-docstring const ATTRIBUTE_DOCSTRING = 1 << 26; + /// The model is in the value expression of a [PEP 613] explicit type alias. + /// + /// [PEP 613]: https://peps.python.org/pep-0613/ + const EXPLICIT_TYPE_ALIAS = 1 << 27; + + /// The model is in the value expression of a [PEP 695] type statement + /// + /// [PEP 695]: https://peps.python.org/pep-0695/#generic-type-alias + const GENERIC_TYPE_ALIAS = 1 << 28; + /// The context is in any type annotation. const ANNOTATION = Self::TYPING_ONLY_ANNOTATION.bits() | Self::RUNTIME_EVALUATED_ANNOTATION.bits() | Self::RUNTIME_REQUIRED_ANNOTATION.bits(); diff --git a/ruff.schema.json b/ruff.schema.json index 83b27a24f71cf..1d1a10d09c5d1 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3903,6 +3903,8 @@ "TCH003", "TCH004", "TCH005", + "TCH007", + "TCH008", "TCH01", "TCH010", "TD", From e1ca904a878fccafd9012f2936bb75a6963883c8 Mon Sep 17 00:00:00 2001 From: David Salvisberg Date: Fri, 16 Aug 2024 14:19:01 +0200 Subject: [PATCH 02/24] Fixes mkdocs and clippy complaints --- .../src/rules/flake8_type_checking/rules/type_alias_quotes.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs index 42fc1fddca6b7..a9e1d0d535b13 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs @@ -19,6 +19,7 @@ use crate::checkers::ast::Checker; /// ## Example /// ```python /// from typing import TYPE_CHECKING, TypeAlias +/// /// if TYPE_CHECKING: /// from foo import Foo /// OptFoo: TypeAlias = Foo | None @@ -27,6 +28,7 @@ use crate::checkers::ast::Checker; /// Use instead: /// ```python /// from typing import TYPE_CHECKING, TypeAlias +/// /// if TYPE_CHECKING: /// from foo import Foo /// OptFoo: TypeAlias = "Foo | None" @@ -127,7 +129,7 @@ impl AlwaysFixableViolation for QuotedTypeAlias { // skip expressions that don't contain either. But then we can't reuse // this function for TCH007. Is it worth having two functions where one // has fewer branches because we know they won't be there? -fn contains_typing_reference<'a>(semantic: &SemanticModel, expr: &'a Expr) -> bool { +fn contains_typing_reference(semantic: &SemanticModel, expr: &Expr) -> bool { match expr { Expr::BinOp(ast::ExprBinOp { left, right, .. }) => { contains_typing_reference(semantic, left) || contains_typing_reference(semantic, right) From 32015889826e2b063c0066fade890d3f9c1199e8 Mon Sep 17 00:00:00 2001 From: David Salvisberg Date: Fri, 16 Aug 2024 15:19:12 +0200 Subject: [PATCH 03/24] Fixes runtime forward reference binding lookup --- .../fixtures/flake8_type_checking/TCH008.py | 6 + .../rules/type_alias_quotes.rs | 23 +-- ...g__tests__quoted-type-alias_TCH008.py.snap | 167 ++++++++++-------- 3 files changed, 115 insertions(+), 81 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py index 77b432b6c406b..f42aecd78be9a 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py @@ -21,9 +21,15 @@ g: TypeAlias = 'OptStr' # OK h: TypeAlias = 'Bar' # TCH008 i: TypeAlias = Foo['str'] # TCH008 +j: TypeAlias = 'Baz' # OK type B = 'Dict' # TCH008 type D = 'Foo[str]' # TCH008 type E = 'Foo.bar' # TCH008 type G = 'OptStr' # TCH008 type I = Foo['str'] # TCH008 +type J = 'Baz' # TCH008 + + +class Baz: + pass diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs index a9e1d0d535b13..9328c74a100e0 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs @@ -3,7 +3,7 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast as ast; use ruff_python_ast::Expr; -use ruff_python_semantic::SemanticModel; +use ruff_python_semantic::{Binding, SemanticModel}; use ruff_text_size::TextRange; use std::borrow::Borrow; @@ -123,16 +123,15 @@ impl AlwaysFixableViolation for QuotedTypeAlias { // TODO implement this }*/ -/// Traverses the type expression and checks if any of the encountered names reference -/// a binding that is defined within a typing-only context. +/// Traverses the type expression and checks the given predicate for each [`Binding`] // TODO: Do we want to remove Attribute and Subscript traversal? We already // skip expressions that don't contain either. But then we can't reuse // this function for TCH007. Is it worth having two functions where one // has fewer branches because we know they won't be there? -fn contains_typing_reference(semantic: &SemanticModel, expr: &Expr) -> bool { +fn check_bindings(semantic: &SemanticModel, expr: &Expr, pred: &impl Fn(&Binding) -> bool) -> bool { match expr { Expr::BinOp(ast::ExprBinOp { left, right, .. }) => { - contains_typing_reference(semantic, left) || contains_typing_reference(semantic, right) + check_bindings(semantic, left, pred) || check_bindings(semantic, right, pred) } Expr::Starred(ast::ExprStarred { value, @@ -140,22 +139,22 @@ fn contains_typing_reference(semantic: &SemanticModel, expr: &Expr) -> bool { .. }) | Expr::Attribute(ast::ExprAttribute { value, .. }) => { - contains_typing_reference(semantic, value) + check_bindings(semantic, value, pred) } Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { - if contains_typing_reference(semantic, value) { + if check_bindings(semantic, value, pred) { return true; } if let Expr::Name(ast::ExprName { id, .. }) = value.borrow() { if id.as_str() != "Literal" { - return contains_typing_reference(semantic, slice); + return check_bindings(semantic, slice, pred); } } false } Expr::List(ast::ExprList { elts, .. }) | Expr::Tuple(ast::ExprTuple { elts, .. }) => { for elt in elts { - if contains_typing_reference(semantic, elt) { + if check_bindings(semantic, elt, pred) { return true; } } @@ -163,7 +162,7 @@ fn contains_typing_reference(semantic: &SemanticModel, expr: &Expr) -> bool { } Expr::Name(name) => semantic .resolve_name(name) - .is_some_and(|binding_id| semantic.binding(binding_id).context.is_typing()), + .is_some_and(|binding_id| pred(semantic.binding(binding_id))), _ => false, } } @@ -184,7 +183,9 @@ pub(crate) fn quoted_type_alias( // if the expression contains references to typing-only bindings // then the quotes are not redundant - if contains_typing_reference(checker.semantic(), expr) { + if check_bindings(checker.semantic(), expr, &|binding| { + binding.context.is_typing() || binding.range.ordering(range).is_gt() + }) { return; } } diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap index d55270d702a4f..1d1e752face0f 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap @@ -71,6 +71,7 @@ TCH008.py:22:16: TCH008 [*] Remove quotes from type alias 22 | h: TypeAlias = 'Bar' # TCH008 | ^^^^^ TCH008 23 | i: TypeAlias = Foo['str'] # TCH008 +24 | j: TypeAlias = 'Baz' # OK | = help: Remove quotes @@ -81,8 +82,8 @@ TCH008.py:22:16: TCH008 [*] Remove quotes from type alias 22 |-h: TypeAlias = 'Bar' # TCH008 22 |+h: TypeAlias = Bar # TCH008 23 23 | i: TypeAlias = Foo['str'] # TCH008 -24 24 | -25 25 | type B = 'Dict' # TCH008 +24 24 | j: TypeAlias = 'Baz' # OK +25 25 | TCH008.py:23:20: TCH008 [*] Remove quotes from type alias | @@ -90,8 +91,7 @@ TCH008.py:23:20: TCH008 [*] Remove quotes from type alias 22 | h: TypeAlias = 'Bar' # TCH008 23 | i: TypeAlias = Foo['str'] # TCH008 | ^^^^^ TCH008 -24 | -25 | type B = 'Dict' # TCH008 +24 | j: TypeAlias = 'Baz' # OK | = help: Remove quotes @@ -101,101 +101,128 @@ TCH008.py:23:20: TCH008 [*] Remove quotes from type alias 22 22 | h: TypeAlias = 'Bar' # TCH008 23 |-i: TypeAlias = Foo['str'] # TCH008 23 |+i: TypeAlias = Foo[str] # TCH008 -24 24 | -25 25 | type B = 'Dict' # TCH008 -26 26 | type D = 'Foo[str]' # TCH008 +24 24 | j: TypeAlias = 'Baz' # OK +25 25 | +26 26 | type B = 'Dict' # TCH008 -TCH008.py:25:10: TCH008 [*] Remove quotes from type alias +TCH008.py:26:10: TCH008 [*] Remove quotes from type alias | -23 | i: TypeAlias = Foo['str'] # TCH008 -24 | -25 | type B = 'Dict' # TCH008 +24 | j: TypeAlias = 'Baz' # OK +25 | +26 | type B = 'Dict' # TCH008 | ^^^^^^ TCH008 -26 | type D = 'Foo[str]' # TCH008 -27 | type E = 'Foo.bar' # TCH008 +27 | type D = 'Foo[str]' # TCH008 +28 | type E = 'Foo.bar' # TCH008 | = help: Remove quotes ℹ Safe fix -22 22 | h: TypeAlias = 'Bar' # TCH008 23 23 | i: TypeAlias = Foo['str'] # TCH008 -24 24 | -25 |-type B = 'Dict' # TCH008 - 25 |+type B = Dict # TCH008 -26 26 | type D = 'Foo[str]' # TCH008 -27 27 | type E = 'Foo.bar' # TCH008 -28 28 | type G = 'OptStr' # TCH008 +24 24 | j: TypeAlias = 'Baz' # OK +25 25 | +26 |-type B = 'Dict' # TCH008 + 26 |+type B = Dict # TCH008 +27 27 | type D = 'Foo[str]' # TCH008 +28 28 | type E = 'Foo.bar' # TCH008 +29 29 | type G = 'OptStr' # TCH008 -TCH008.py:26:10: TCH008 [*] Remove quotes from type alias +TCH008.py:27:10: TCH008 [*] Remove quotes from type alias | -25 | type B = 'Dict' # TCH008 -26 | type D = 'Foo[str]' # TCH008 +26 | type B = 'Dict' # TCH008 +27 | type D = 'Foo[str]' # TCH008 | ^^^^^^^^^^ TCH008 -27 | type E = 'Foo.bar' # TCH008 -28 | type G = 'OptStr' # TCH008 +28 | type E = 'Foo.bar' # TCH008 +29 | type G = 'OptStr' # TCH008 | = help: Remove quotes ℹ Safe fix -23 23 | i: TypeAlias = Foo['str'] # TCH008 -24 24 | -25 25 | type B = 'Dict' # TCH008 -26 |-type D = 'Foo[str]' # TCH008 - 26 |+type D = Foo[str] # TCH008 -27 27 | type E = 'Foo.bar' # TCH008 -28 28 | type G = 'OptStr' # TCH008 -29 29 | type I = Foo['str'] # TCH008 +24 24 | j: TypeAlias = 'Baz' # OK +25 25 | +26 26 | type B = 'Dict' # TCH008 +27 |-type D = 'Foo[str]' # TCH008 + 27 |+type D = Foo[str] # TCH008 +28 28 | type E = 'Foo.bar' # TCH008 +29 29 | type G = 'OptStr' # TCH008 +30 30 | type I = Foo['str'] # TCH008 -TCH008.py:27:10: TCH008 [*] Remove quotes from type alias +TCH008.py:28:10: TCH008 [*] Remove quotes from type alias | -25 | type B = 'Dict' # TCH008 -26 | type D = 'Foo[str]' # TCH008 -27 | type E = 'Foo.bar' # TCH008 +26 | type B = 'Dict' # TCH008 +27 | type D = 'Foo[str]' # TCH008 +28 | type E = 'Foo.bar' # TCH008 | ^^^^^^^^^ TCH008 -28 | type G = 'OptStr' # TCH008 -29 | type I = Foo['str'] # TCH008 +29 | type G = 'OptStr' # TCH008 +30 | type I = Foo['str'] # TCH008 | = help: Remove quotes ℹ Safe fix -24 24 | -25 25 | type B = 'Dict' # TCH008 -26 26 | type D = 'Foo[str]' # TCH008 -27 |-type E = 'Foo.bar' # TCH008 - 27 |+type E = Foo.bar # TCH008 -28 28 | type G = 'OptStr' # TCH008 -29 29 | type I = Foo['str'] # TCH008 - -TCH008.py:28:10: TCH008 [*] Remove quotes from type alias - | -26 | type D = 'Foo[str]' # TCH008 -27 | type E = 'Foo.bar' # TCH008 -28 | type G = 'OptStr' # TCH008 +25 25 | +26 26 | type B = 'Dict' # TCH008 +27 27 | type D = 'Foo[str]' # TCH008 +28 |-type E = 'Foo.bar' # TCH008 + 28 |+type E = Foo.bar # TCH008 +29 29 | type G = 'OptStr' # TCH008 +30 30 | type I = Foo['str'] # TCH008 +31 31 | type J = 'Baz' # TCH008 + +TCH008.py:29:10: TCH008 [*] Remove quotes from type alias + | +27 | type D = 'Foo[str]' # TCH008 +28 | type E = 'Foo.bar' # TCH008 +29 | type G = 'OptStr' # TCH008 | ^^^^^^^^ TCH008 -29 | type I = Foo['str'] # TCH008 +30 | type I = Foo['str'] # TCH008 +31 | type J = 'Baz' # TCH008 | = help: Remove quotes ℹ Safe fix -25 25 | type B = 'Dict' # TCH008 -26 26 | type D = 'Foo[str]' # TCH008 -27 27 | type E = 'Foo.bar' # TCH008 -28 |-type G = 'OptStr' # TCH008 - 28 |+type G = OptStr # TCH008 -29 29 | type I = Foo['str'] # TCH008 - -TCH008.py:29:14: TCH008 [*] Remove quotes from type alias - | -27 | type E = 'Foo.bar' # TCH008 -28 | type G = 'OptStr' # TCH008 -29 | type I = Foo['str'] # TCH008 +26 26 | type B = 'Dict' # TCH008 +27 27 | type D = 'Foo[str]' # TCH008 +28 28 | type E = 'Foo.bar' # TCH008 +29 |-type G = 'OptStr' # TCH008 + 29 |+type G = OptStr # TCH008 +30 30 | type I = Foo['str'] # TCH008 +31 31 | type J = 'Baz' # TCH008 +32 32 | + +TCH008.py:30:14: TCH008 [*] Remove quotes from type alias + | +28 | type E = 'Foo.bar' # TCH008 +29 | type G = 'OptStr' # TCH008 +30 | type I = Foo['str'] # TCH008 | ^^^^^ TCH008 +31 | type J = 'Baz' # TCH008 + | + = help: Remove quotes + +ℹ Safe fix +27 27 | type D = 'Foo[str]' # TCH008 +28 28 | type E = 'Foo.bar' # TCH008 +29 29 | type G = 'OptStr' # TCH008 +30 |-type I = Foo['str'] # TCH008 + 30 |+type I = Foo[str] # TCH008 +31 31 | type J = 'Baz' # TCH008 +32 32 | +33 33 | + +TCH008.py:31:10: TCH008 [*] Remove quotes from type alias + | +29 | type G = 'OptStr' # TCH008 +30 | type I = Foo['str'] # TCH008 +31 | type J = 'Baz' # TCH008 + | ^^^^^ TCH008 | = help: Remove quotes ℹ Safe fix -26 26 | type D = 'Foo[str]' # TCH008 -27 27 | type E = 'Foo.bar' # TCH008 -28 28 | type G = 'OptStr' # TCH008 -29 |-type I = Foo['str'] # TCH008 - 29 |+type I = Foo[str] # TCH008 +28 28 | type E = 'Foo.bar' # TCH008 +29 29 | type G = 'OptStr' # TCH008 +30 30 | type I = Foo['str'] # TCH008 +31 |-type J = 'Baz' # TCH008 + 31 |+type J = Baz # TCH008 +32 32 | +33 33 | +34 34 | class Baz: From bc4514641f83beaf21095f8a8fedf03f236277b3 Mon Sep 17 00:00:00 2001 From: Daverball Date: Sat, 17 Aug 2024 11:38:30 +0200 Subject: [PATCH 04/24] Simulates proper runtime name lookups for more accurate results --- .../fixtures/flake8_type_checking/TCH008.py | 3 +- crates/ruff_linter/src/checkers/ast/mod.rs | 10 +++ .../rules/type_alias_quotes.rs | 25 +++---- ...g__tests__quoted-type-alias_TCH008.py.snap | 16 ++++ crates/ruff_python_semantic/src/binding.rs | 10 +++ crates/ruff_python_semantic/src/model.rs | 74 +++++++++++++++++++ crates/ruff_python_semantic/src/scope.rs | 5 ++ 7 files changed, 129 insertions(+), 14 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py index f42aecd78be9a..71795d8ffbb9c 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py @@ -32,4 +32,5 @@ class Baz: - pass + a: TypeAlias = 'Baz' # OK + type A = 'Baz' # TCH008 diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 3e4187ed4e32d..152bb866639bf 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -820,6 +820,16 @@ impl<'a> Visitor<'a> for Checker<'a> { BindingKind::ClassDefinition(scope_id), BindingFlags::empty(), ); + // FIXME: We should probably pass in a vector when initializing the scope + self.semantic.scopes[scope_id] + .class_names + .push(name.id.as_str()); + for name in self.semantic.scopes[self.semantic.scope_id] + .class_names + .clone() + { + self.semantic.scopes[scope_id].class_names.push(name); + } } Stmt::TypeAlias(ast::StmtTypeAlias { range: _, diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs index 9328c74a100e0..99f8366221335 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs @@ -3,7 +3,7 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast as ast; use ruff_python_ast::Expr; -use ruff_python_semantic::{Binding, SemanticModel}; +use ruff_python_semantic::SemanticModel; use ruff_text_size::TextRange; use std::borrow::Borrow; @@ -128,10 +128,10 @@ impl AlwaysFixableViolation for QuotedTypeAlias { // skip expressions that don't contain either. But then we can't reuse // this function for TCH007. Is it worth having two functions where one // has fewer branches because we know they won't be there? -fn check_bindings(semantic: &SemanticModel, expr: &Expr, pred: &impl Fn(&Binding) -> bool) -> bool { +fn contains_typing_reference(semantic: &SemanticModel, expr: &Expr) -> bool { match expr { Expr::BinOp(ast::ExprBinOp { left, right, .. }) => { - check_bindings(semantic, left, pred) || check_bindings(semantic, right, pred) + contains_typing_reference(semantic, left) || contains_typing_reference(semantic, right) } Expr::Starred(ast::ExprStarred { value, @@ -139,30 +139,31 @@ fn check_bindings(semantic: &SemanticModel, expr: &Expr, pred: &impl Fn(&Binding .. }) | Expr::Attribute(ast::ExprAttribute { value, .. }) => { - check_bindings(semantic, value, pred) + contains_typing_reference(semantic, value) } Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { - if check_bindings(semantic, value, pred) { + if contains_typing_reference(semantic, value) { return true; } if let Expr::Name(ast::ExprName { id, .. }) = value.borrow() { if id.as_str() != "Literal" { - return check_bindings(semantic, slice, pred); + return contains_typing_reference(semantic, slice); } } false } Expr::List(ast::ExprList { elts, .. }) | Expr::Tuple(ast::ExprTuple { elts, .. }) => { for elt in elts { - if check_bindings(semantic, elt, pred) { + if contains_typing_reference(semantic, elt) { return true; } } false } - Expr::Name(name) => semantic - .resolve_name(name) - .is_some_and(|binding_id| pred(semantic.binding(binding_id))), + Expr::Name(name) => { + semantic.lookup_symbol(name.id.as_str()).is_some() + && semantic.simulate_runtime_load(name).is_none() + } _ => false, } } @@ -183,9 +184,7 @@ pub(crate) fn quoted_type_alias( // if the expression contains references to typing-only bindings // then the quotes are not redundant - if check_bindings(checker.semantic(), expr, &|binding| { - binding.context.is_typing() || binding.range.ordering(range).is_gt() - }) { + if contains_typing_reference(checker.semantic(), expr) { return; } } diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap index 1d1e752face0f..defd17a152ecd 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap @@ -226,3 +226,19 @@ TCH008.py:31:10: TCH008 [*] Remove quotes from type alias 32 32 | 33 33 | 34 34 | class Baz: + +TCH008.py:36:14: TCH008 [*] Remove quotes from type alias + | +34 | class Baz: +35 | a: TypeAlias = 'Baz' # OK +36 | type A = 'Baz' # TCH008 + | ^^^^^ TCH008 + | + = help: Remove quotes + +ℹ Safe fix +33 33 | +34 34 | class Baz: +35 35 | a: TypeAlias = 'Baz' # OK +36 |- type A = 'Baz' # TCH008 + 36 |+ type A = Baz # TCH008 diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index 9b9c74aa81fe2..4165e66d9eb1e 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -249,6 +249,16 @@ impl<'a> Binding<'a> { }) } + /// Returns the statement range for AnnAssign and the Name otherwise. + pub fn defn_range(&self, semantic: &SemanticModel) -> TextRange { + if let Some(parent) = self.statement(semantic) { + if parent.is_ann_assign_stmt() { + return parent.range(); + } + } + self.range + } + pub fn as_any_import(&self) -> Option> { match &self.kind { BindingKind::Import(import) => Some(AnyImport::Import(import)), diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index ab87f4afa78f7..1ab2113472edc 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -663,6 +663,80 @@ impl<'a> SemanticModel<'a> { } } + // FIXME: Shouldn't this happen above where `class_variables_visible` is set? + seen_function |= scope.kind.is_function(); + } + + None + } + + /// Simulates a runtime load of a given [`ast::ExprName`]. + /// + /// This should not be run until after all the bindings have been visited. + pub fn simulate_runtime_load(&self, name: &ast::ExprName) -> Option { + let symbol = name.id.as_str(); + let range = name.range; + let mut seen_function = false; + let mut class_variables_visible = true; + let mut lexicographical_lookup = true; + for (index, scope_id) in self.scopes.ancestor_ids(self.scope_id).enumerate() { + let scope = &self.scopes[scope_id]; + if scope.kind.is_class() { + if seen_function && matches!(symbol, "__class__") { + return None; + } + if !class_variables_visible { + continue; + } + } + + // Technically returning here is not quite correct, it would be more accurate + // to remember the specific binding ids that we need to ignore from this point + // on in the loop, but in most cases we probably want to treat this as unresolved + // since without a forward reference, this would probably refer to the wrong binding + if !seen_function && scope.kind.is_class() && scope.class_names.contains(&symbol) { + return None; + } + + class_variables_visible = scope.kind.is_type() && index == 0; + + // once we hit a type scope we should stop doing lexicographical lookups + if scope.kind.is_type() { + lexicographical_lookup = false; + } + + if let Some(binding_id) = scope.get(symbol) { + let candidate_id = match self.bindings[binding_id].kind { + BindingKind::Annotation => continue, + BindingKind::Deletion | BindingKind::UnboundException(None) => return None, + BindingKind::ConditionalDeletion(binding_id) => binding_id, + BindingKind::UnboundException(Some(binding_id)) => binding_id, + _ => binding_id, + }; + + if self.bindings[candidate_id].context.is_typing() { + continue; + } + + if lexicographical_lookup + && self.bindings[candidate_id] + .defn_range(self) + .ordering(range) + .is_gt() + { + continue; + } + + return Some(candidate_id); + } + + if index == 0 && scope.kind.is_class() { + if matches!(symbol, "__module__" | "__qualname__") { + return None; + } + } + + // FIXME: Shouldn't this happen above where `class_variables_visible` is set? seen_function |= scope.kind.is_function(); } diff --git a/crates/ruff_python_semantic/src/scope.rs b/crates/ruff_python_semantic/src/scope.rs index a7a69fd555390..6eb2686b58101 100644 --- a/crates/ruff_python_semantic/src/scope.rs +++ b/crates/ruff_python_semantic/src/scope.rs @@ -42,6 +42,9 @@ pub struct Scope<'a> { /// Flags for the [`Scope`]. flags: ScopeFlags, + + /// A vector of class names that should not be available in this scope + pub class_names: Vec<&'a str>, } impl<'a> Scope<'a> { @@ -54,6 +57,7 @@ impl<'a> Scope<'a> { shadowed_bindings: FxHashMap::default(), globals_id: None, flags: ScopeFlags::empty(), + class_names: Vec::default(), } } @@ -66,6 +70,7 @@ impl<'a> Scope<'a> { shadowed_bindings: FxHashMap::default(), globals_id: None, flags: ScopeFlags::empty(), + class_names: Vec::default(), } } From 0df3d120a63c97dc908f1fdefef4866ec9562a5b Mon Sep 17 00:00:00 2001 From: Daverball Date: Sat, 17 Aug 2024 13:39:49 +0200 Subject: [PATCH 05/24] Adds missing test case back and fixes range overlap check Also fixes lexicographical lookup not taking shadowed bindings into account. --- .../fixtures/flake8_type_checking/TCH008.py | 2 + ...g__tests__quoted-type-alias_TCH008.py.snap | 216 ++++++++++-------- crates/ruff_python_semantic/src/model.rs | 52 +++-- 3 files changed, 154 insertions(+), 116 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py index 71795d8ffbb9c..917f29540661d 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py @@ -22,6 +22,7 @@ h: TypeAlias = 'Bar' # TCH008 i: TypeAlias = Foo['str'] # TCH008 j: TypeAlias = 'Baz' # OK +k: TypeAlias = 'k | None' # OK type B = 'Dict' # TCH008 type D = 'Foo[str]' # TCH008 @@ -29,6 +30,7 @@ type G = 'OptStr' # TCH008 type I = Foo['str'] # TCH008 type J = 'Baz' # TCH008 +type K = 'K | None' # TCH008 class Baz: diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap index defd17a152ecd..2519c2f573003 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap @@ -83,7 +83,7 @@ TCH008.py:22:16: TCH008 [*] Remove quotes from type alias 22 |+h: TypeAlias = Bar # TCH008 23 23 | i: TypeAlias = Foo['str'] # TCH008 24 24 | j: TypeAlias = 'Baz' # OK -25 25 | +25 25 | k: TypeAlias = 'k | None' # OK TCH008.py:23:20: TCH008 [*] Remove quotes from type alias | @@ -92,6 +92,7 @@ TCH008.py:23:20: TCH008 [*] Remove quotes from type alias 23 | i: TypeAlias = Foo['str'] # TCH008 | ^^^^^ TCH008 24 | j: TypeAlias = 'Baz' # OK +25 | k: TypeAlias = 'k | None' # OK | = help: Remove quotes @@ -102,143 +103,164 @@ TCH008.py:23:20: TCH008 [*] Remove quotes from type alias 23 |-i: TypeAlias = Foo['str'] # TCH008 23 |+i: TypeAlias = Foo[str] # TCH008 24 24 | j: TypeAlias = 'Baz' # OK -25 25 | -26 26 | type B = 'Dict' # TCH008 +25 25 | k: TypeAlias = 'k | None' # OK +26 26 | -TCH008.py:26:10: TCH008 [*] Remove quotes from type alias +TCH008.py:27:10: TCH008 [*] Remove quotes from type alias | -24 | j: TypeAlias = 'Baz' # OK -25 | -26 | type B = 'Dict' # TCH008 +25 | k: TypeAlias = 'k | None' # OK +26 | +27 | type B = 'Dict' # TCH008 | ^^^^^^ TCH008 -27 | type D = 'Foo[str]' # TCH008 -28 | type E = 'Foo.bar' # TCH008 +28 | type D = 'Foo[str]' # TCH008 +29 | type E = 'Foo.bar' # TCH008 | = help: Remove quotes ℹ Safe fix -23 23 | i: TypeAlias = Foo['str'] # TCH008 24 24 | j: TypeAlias = 'Baz' # OK -25 25 | -26 |-type B = 'Dict' # TCH008 - 26 |+type B = Dict # TCH008 -27 27 | type D = 'Foo[str]' # TCH008 -28 28 | type E = 'Foo.bar' # TCH008 -29 29 | type G = 'OptStr' # TCH008 +25 25 | k: TypeAlias = 'k | None' # OK +26 26 | +27 |-type B = 'Dict' # TCH008 + 27 |+type B = Dict # TCH008 +28 28 | type D = 'Foo[str]' # TCH008 +29 29 | type E = 'Foo.bar' # TCH008 +30 30 | type G = 'OptStr' # TCH008 -TCH008.py:27:10: TCH008 [*] Remove quotes from type alias +TCH008.py:28:10: TCH008 [*] Remove quotes from type alias | -26 | type B = 'Dict' # TCH008 -27 | type D = 'Foo[str]' # TCH008 +27 | type B = 'Dict' # TCH008 +28 | type D = 'Foo[str]' # TCH008 | ^^^^^^^^^^ TCH008 -28 | type E = 'Foo.bar' # TCH008 -29 | type G = 'OptStr' # TCH008 +29 | type E = 'Foo.bar' # TCH008 +30 | type G = 'OptStr' # TCH008 | = help: Remove quotes ℹ Safe fix -24 24 | j: TypeAlias = 'Baz' # OK -25 25 | -26 26 | type B = 'Dict' # TCH008 -27 |-type D = 'Foo[str]' # TCH008 - 27 |+type D = Foo[str] # TCH008 -28 28 | type E = 'Foo.bar' # TCH008 -29 29 | type G = 'OptStr' # TCH008 -30 30 | type I = Foo['str'] # TCH008 +25 25 | k: TypeAlias = 'k | None' # OK +26 26 | +27 27 | type B = 'Dict' # TCH008 +28 |-type D = 'Foo[str]' # TCH008 + 28 |+type D = Foo[str] # TCH008 +29 29 | type E = 'Foo.bar' # TCH008 +30 30 | type G = 'OptStr' # TCH008 +31 31 | type I = Foo['str'] # TCH008 -TCH008.py:28:10: TCH008 [*] Remove quotes from type alias +TCH008.py:29:10: TCH008 [*] Remove quotes from type alias | -26 | type B = 'Dict' # TCH008 -27 | type D = 'Foo[str]' # TCH008 -28 | type E = 'Foo.bar' # TCH008 +27 | type B = 'Dict' # TCH008 +28 | type D = 'Foo[str]' # TCH008 +29 | type E = 'Foo.bar' # TCH008 | ^^^^^^^^^ TCH008 -29 | type G = 'OptStr' # TCH008 -30 | type I = Foo['str'] # TCH008 +30 | type G = 'OptStr' # TCH008 +31 | type I = Foo['str'] # TCH008 | = help: Remove quotes ℹ Safe fix -25 25 | -26 26 | type B = 'Dict' # TCH008 -27 27 | type D = 'Foo[str]' # TCH008 -28 |-type E = 'Foo.bar' # TCH008 - 28 |+type E = Foo.bar # TCH008 -29 29 | type G = 'OptStr' # TCH008 -30 30 | type I = Foo['str'] # TCH008 -31 31 | type J = 'Baz' # TCH008 +26 26 | +27 27 | type B = 'Dict' # TCH008 +28 28 | type D = 'Foo[str]' # TCH008 +29 |-type E = 'Foo.bar' # TCH008 + 29 |+type E = Foo.bar # TCH008 +30 30 | type G = 'OptStr' # TCH008 +31 31 | type I = Foo['str'] # TCH008 +32 32 | type J = 'Baz' # TCH008 -TCH008.py:29:10: TCH008 [*] Remove quotes from type alias +TCH008.py:30:10: TCH008 [*] Remove quotes from type alias | -27 | type D = 'Foo[str]' # TCH008 -28 | type E = 'Foo.bar' # TCH008 -29 | type G = 'OptStr' # TCH008 +28 | type D = 'Foo[str]' # TCH008 +29 | type E = 'Foo.bar' # TCH008 +30 | type G = 'OptStr' # TCH008 | ^^^^^^^^ TCH008 -30 | type I = Foo['str'] # TCH008 -31 | type J = 'Baz' # TCH008 +31 | type I = Foo['str'] # TCH008 +32 | type J = 'Baz' # TCH008 | = help: Remove quotes ℹ Safe fix -26 26 | type B = 'Dict' # TCH008 -27 27 | type D = 'Foo[str]' # TCH008 -28 28 | type E = 'Foo.bar' # TCH008 -29 |-type G = 'OptStr' # TCH008 - 29 |+type G = OptStr # TCH008 -30 30 | type I = Foo['str'] # TCH008 -31 31 | type J = 'Baz' # TCH008 -32 32 | - -TCH008.py:30:14: TCH008 [*] Remove quotes from type alias - | -28 | type E = 'Foo.bar' # TCH008 -29 | type G = 'OptStr' # TCH008 -30 | type I = Foo['str'] # TCH008 +27 27 | type B = 'Dict' # TCH008 +28 28 | type D = 'Foo[str]' # TCH008 +29 29 | type E = 'Foo.bar' # TCH008 +30 |-type G = 'OptStr' # TCH008 + 30 |+type G = OptStr # TCH008 +31 31 | type I = Foo['str'] # TCH008 +32 32 | type J = 'Baz' # TCH008 +33 33 | type K = 'K | None' # TCH008 + +TCH008.py:31:14: TCH008 [*] Remove quotes from type alias + | +29 | type E = 'Foo.bar' # TCH008 +30 | type G = 'OptStr' # TCH008 +31 | type I = Foo['str'] # TCH008 | ^^^^^ TCH008 -31 | type J = 'Baz' # TCH008 +32 | type J = 'Baz' # TCH008 +33 | type K = 'K | None' # TCH008 | = help: Remove quotes ℹ Safe fix -27 27 | type D = 'Foo[str]' # TCH008 -28 28 | type E = 'Foo.bar' # TCH008 -29 29 | type G = 'OptStr' # TCH008 -30 |-type I = Foo['str'] # TCH008 - 30 |+type I = Foo[str] # TCH008 -31 31 | type J = 'Baz' # TCH008 -32 32 | -33 33 | - -TCH008.py:31:10: TCH008 [*] Remove quotes from type alias - | -29 | type G = 'OptStr' # TCH008 -30 | type I = Foo['str'] # TCH008 -31 | type J = 'Baz' # TCH008 +28 28 | type D = 'Foo[str]' # TCH008 +29 29 | type E = 'Foo.bar' # TCH008 +30 30 | type G = 'OptStr' # TCH008 +31 |-type I = Foo['str'] # TCH008 + 31 |+type I = Foo[str] # TCH008 +32 32 | type J = 'Baz' # TCH008 +33 33 | type K = 'K | None' # TCH008 +34 34 | + +TCH008.py:32:10: TCH008 [*] Remove quotes from type alias + | +30 | type G = 'OptStr' # TCH008 +31 | type I = Foo['str'] # TCH008 +32 | type J = 'Baz' # TCH008 | ^^^^^ TCH008 +33 | type K = 'K | None' # TCH008 | = help: Remove quotes ℹ Safe fix -28 28 | type E = 'Foo.bar' # TCH008 -29 29 | type G = 'OptStr' # TCH008 -30 30 | type I = Foo['str'] # TCH008 -31 |-type J = 'Baz' # TCH008 - 31 |+type J = Baz # TCH008 -32 32 | -33 33 | -34 34 | class Baz: - -TCH008.py:36:14: TCH008 [*] Remove quotes from type alias - | -34 | class Baz: -35 | a: TypeAlias = 'Baz' # OK -36 | type A = 'Baz' # TCH008 +29 29 | type E = 'Foo.bar' # TCH008 +30 30 | type G = 'OptStr' # TCH008 +31 31 | type I = Foo['str'] # TCH008 +32 |-type J = 'Baz' # TCH008 + 32 |+type J = Baz # TCH008 +33 33 | type K = 'K | None' # TCH008 +34 34 | +35 35 | + +TCH008.py:33:10: TCH008 [*] Remove quotes from type alias + | +31 | type I = Foo['str'] # TCH008 +32 | type J = 'Baz' # TCH008 +33 | type K = 'K | None' # TCH008 + | ^^^^^^^^^^ TCH008 + | + = help: Remove quotes + +ℹ Safe fix +30 30 | type G = 'OptStr' # TCH008 +31 31 | type I = Foo['str'] # TCH008 +32 32 | type J = 'Baz' # TCH008 +33 |-type K = 'K | None' # TCH008 + 33 |+type K = K | None # TCH008 +34 34 | +35 35 | +36 36 | class Baz: + +TCH008.py:38:14: TCH008 [*] Remove quotes from type alias + | +36 | class Baz: +37 | a: TypeAlias = 'Baz' # OK +38 | type A = 'Baz' # TCH008 | ^^^^^ TCH008 | = help: Remove quotes ℹ Safe fix -33 33 | -34 34 | class Baz: -35 35 | a: TypeAlias = 'Baz' # OK -36 |- type A = 'Baz' # TCH008 - 36 |+ type A = Baz # TCH008 +35 35 | +36 36 | class Baz: +37 37 | a: TypeAlias = 'Baz' # OK +38 |- type A = 'Baz' # TCH008 + 38 |+ type A = Baz # TCH008 diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 1ab2113472edc..eb61abb7a6f65 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -706,28 +706,42 @@ impl<'a> SemanticModel<'a> { } if let Some(binding_id) = scope.get(symbol) { - let candidate_id = match self.bindings[binding_id].kind { - BindingKind::Annotation => continue, - BindingKind::Deletion | BindingKind::UnboundException(None) => return None, - BindingKind::ConditionalDeletion(binding_id) => binding_id, - BindingKind::UnboundException(Some(binding_id)) => binding_id, - _ => binding_id, - }; + if lexicographical_lookup { + // we need to look through all the shadowed bindings + // since we may be shadowing a valid runtime binding + // with an invalid one + for shadowed_id in scope.shadowed_bindings(binding_id) { + let binding = &self.bindings[shadowed_id]; + if binding.context.is_typing() { + continue; + } + if let BindingKind::Annotation + | BindingKind::Deletion + | BindingKind::UnboundException(..) + | BindingKind::ConditionalDeletion(..) = binding.kind + { + continue; + } - if self.bindings[candidate_id].context.is_typing() { - continue; - } + if binding.defn_range(self).ordering(range).is_lt() { + return Some(shadowed_id); + } + } + } else { + let candidate_id = match self.bindings[binding_id].kind { + BindingKind::Annotation => continue, + BindingKind::Deletion | BindingKind::UnboundException(None) => return None, + BindingKind::ConditionalDeletion(binding_id) => binding_id, + BindingKind::UnboundException(Some(binding_id)) => binding_id, + _ => binding_id, + }; + + if self.bindings[candidate_id].context.is_typing() { + continue; + } - if lexicographical_lookup - && self.bindings[candidate_id] - .defn_range(self) - .ordering(range) - .is_gt() - { - continue; + return Some(candidate_id); } - - return Some(candidate_id); } if index == 0 && scope.kind.is_class() { From 4cb32e1196852c6fd77d550bf49e47737855be8c Mon Sep 17 00:00:00 2001 From: Daverball Date: Sat, 17 Aug 2024 13:49:09 +0200 Subject: [PATCH 06/24] Fixes docstring --- crates/ruff_python_semantic/src/binding.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index 4165e66d9eb1e..099e2c3369944 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -249,7 +249,8 @@ impl<'a> Binding<'a> { }) } - /// Returns the statement range for AnnAssign and the Name otherwise. + /// Returns the statement range for bindings created by `[ast::StmtAnnAssign]` + /// and `[Binding.range]` otherwise. pub fn defn_range(&self, semantic: &SemanticModel) -> TextRange { if let Some(parent) = self.statement(semantic) { if parent.is_ann_assign_stmt() { From d9cea8fdd64437496319485955616ff3013eeccd Mon Sep 17 00:00:00 2001 From: Daverball Date: Sat, 17 Aug 2024 14:57:58 +0200 Subject: [PATCH 07/24] Refactors class name book keeping code --- crates/ruff_linter/src/checkers/ast/mod.rs | 16 +++++------ crates/ruff_python_semantic/src/model.rs | 2 +- crates/ruff_python_semantic/src/scope.rs | 31 +++++++++++++++++++--- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 152bb866639bf..34e261e73b23d 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -820,16 +820,12 @@ impl<'a> Visitor<'a> for Checker<'a> { BindingKind::ClassDefinition(scope_id), BindingFlags::empty(), ); - // FIXME: We should probably pass in a vector when initializing the scope - self.semantic.scopes[scope_id] - .class_names - .push(name.id.as_str()); - for name in self.semantic.scopes[self.semantic.scope_id] - .class_names - .clone() - { - self.semantic.scopes[scope_id].class_names.push(name); - } + + // Record class names that should be unavailable within this scope + let mut class_names = + self.semantic.scopes[self.semantic.scope_id].copy_class_names(); + class_names.push(name.id.as_str()); + self.semantic.scopes[scope_id].set_class_names(class_names); } Stmt::TypeAlias(ast::StmtTypeAlias { range: _, diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index eb61abb7a6f65..01b5380ad4a88 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -694,7 +694,7 @@ impl<'a> SemanticModel<'a> { // to remember the specific binding ids that we need to ignore from this point // on in the loop, but in most cases we probably want to treat this as unresolved // since without a forward reference, this would probably refer to the wrong binding - if !seen_function && scope.kind.is_class() && scope.class_names.contains(&symbol) { + if !seen_function && scope.kind.is_class() && scope.is_class_name(&symbol) { return None; } diff --git a/crates/ruff_python_semantic/src/scope.rs b/crates/ruff_python_semantic/src/scope.rs index 6eb2686b58101..faee0db06eef1 100644 --- a/crates/ruff_python_semantic/src/scope.rs +++ b/crates/ruff_python_semantic/src/scope.rs @@ -44,7 +44,7 @@ pub struct Scope<'a> { flags: ScopeFlags, /// A vector of class names that should not be available in this scope - pub class_names: Vec<&'a str>, + class_names: Option>, } impl<'a> Scope<'a> { @@ -57,7 +57,7 @@ impl<'a> Scope<'a> { shadowed_bindings: FxHashMap::default(), globals_id: None, flags: ScopeFlags::empty(), - class_names: Vec::default(), + class_names: None, } } @@ -70,7 +70,7 @@ impl<'a> Scope<'a> { shadowed_bindings: FxHashMap::default(), globals_id: None, flags: ScopeFlags::empty(), - class_names: Vec::default(), + class_names: None, } } @@ -160,6 +160,31 @@ impl<'a> Scope<'a> { pub const fn uses_locals(&self) -> bool { self.flags.intersects(ScopeFlags::USES_LOCALS) } + + /// Either returns a copy of the class names contained within this scope + /// or an empty vector. + pub fn copy_class_names(&self) -> Vec<&'a str> { + if let Some(names) = &self.class_names { + names.clone() + } else { + Vec::default() + } + } + + /// Sets the class names for this scope. + pub fn set_class_names(&mut self, class_names: Vec<&'a str>) { + self.class_names = Some(class_names); + } + + /// Returns `true` if this name is one of the class names that should not + /// be accessible within this scope. + pub fn is_class_name(&self, name: &str) -> bool { + if let Some(names) = &self.class_names { + names.contains(&name) + } else { + false + } + } } bitflags! { From 096eb9ddc774adcc840c4a5605a35a9417c69ae0 Mon Sep 17 00:00:00 2001 From: Daverball Date: Sat, 17 Aug 2024 15:03:09 +0200 Subject: [PATCH 08/24] Removes unnecessary borrow --- crates/ruff_python_semantic/src/model.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 01b5380ad4a88..a25ffad042715 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -694,7 +694,7 @@ impl<'a> SemanticModel<'a> { // to remember the specific binding ids that we need to ignore from this point // on in the loop, but in most cases we probably want to treat this as unresolved // since without a forward reference, this would probably refer to the wrong binding - if !seen_function && scope.kind.is_class() && scope.is_class_name(&symbol) { + if !seen_function && scope.kind.is_class() && scope.is_class_name(symbol) { return None; } From e42069744b4e42f188d01213f576f6bc7c7b1099 Mon Sep 17 00:00:00 2001 From: Daverball Date: Sun, 18 Aug 2024 09:26:30 +0200 Subject: [PATCH 09/24] Fixes some logical errors and simplifies class name lookups --- .../fixtures/flake8_type_checking/TCH008.py | 4 +++ crates/ruff_linter/src/checkers/ast/mod.rs | 6 ---- ...g__tests__quoted-type-alias_TCH008.py.snap | 21 +++++++++++++ crates/ruff_python_semantic/src/binding.rs | 11 ++++--- crates/ruff_python_semantic/src/model.rs | 23 +++++++------- crates/ruff_python_semantic/src/scope.rs | 30 ------------------- 6 files changed, 40 insertions(+), 55 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py index 917f29540661d..9e766a3732f0b 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py @@ -36,3 +36,7 @@ class Baz: a: TypeAlias = 'Baz' # OK type A = 'Baz' # TCH008 + + class Nested: + a: TypeAlias = 'Baz' # OK + type A = 'Baz' # TCH008 diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 34e261e73b23d..3e4187ed4e32d 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -820,12 +820,6 @@ impl<'a> Visitor<'a> for Checker<'a> { BindingKind::ClassDefinition(scope_id), BindingFlags::empty(), ); - - // Record class names that should be unavailable within this scope - let mut class_names = - self.semantic.scopes[self.semantic.scope_id].copy_class_names(); - class_names.push(name.id.as_str()); - self.semantic.scopes[scope_id].set_class_names(class_names); } Stmt::TypeAlias(ast::StmtTypeAlias { range: _, diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap index 2519c2f573003..fa69f0c3b2a8e 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap @@ -255,6 +255,8 @@ TCH008.py:38:14: TCH008 [*] Remove quotes from type alias 37 | a: TypeAlias = 'Baz' # OK 38 | type A = 'Baz' # TCH008 | ^^^^^ TCH008 +39 | +40 | class Nested: | = help: Remove quotes @@ -264,3 +266,22 @@ TCH008.py:38:14: TCH008 [*] Remove quotes from type alias 37 37 | a: TypeAlias = 'Baz' # OK 38 |- type A = 'Baz' # TCH008 38 |+ type A = Baz # TCH008 +39 39 | +40 40 | class Nested: +41 41 | a: TypeAlias = 'Baz' # OK + +TCH008.py:42:18: TCH008 [*] Remove quotes from type alias + | +40 | class Nested: +41 | a: TypeAlias = 'Baz' # OK +42 | type A = 'Baz' # TCH008 + | ^^^^^ TCH008 + | + = help: Remove quotes + +ℹ Safe fix +39 39 | +40 40 | class Nested: +41 41 | a: TypeAlias = 'Baz' # OK +42 |- type A = 'Baz' # TCH008 + 42 |+ type A = Baz # TCH008 diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index 099e2c3369944..e65ddfc5b4133 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -250,14 +250,13 @@ impl<'a> Binding<'a> { } /// Returns the statement range for bindings created by `[ast::StmtAnnAssign]` - /// and `[Binding.range]` otherwise. + /// or `[ast::StmtClassDef]` and `[Binding.range]` otherwise. pub fn defn_range(&self, semantic: &SemanticModel) -> TextRange { - if let Some(parent) = self.statement(semantic) { - if parent.is_ann_assign_stmt() { - return parent.range(); - } + match self.statement(semantic) { + Some(Stmt::AnnAssign(stmt)) => stmt.range(), + Some(Stmt::ClassDef(stmt)) => stmt.range(), + _ => self.range, } - self.range } pub fn as_any_import(&self) -> Option> { diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index a25ffad042715..9812b74bfd006 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -681,6 +681,16 @@ impl<'a> SemanticModel<'a> { let mut lexicographical_lookup = true; for (index, scope_id) in self.scopes.ancestor_ids(self.scope_id).enumerate() { let scope = &self.scopes[scope_id]; + + // Only once we leave a function scope and its enclosing type scope should + // we stop doing lexicographical lookups. We could e.g. have nested classes + // where we lookup symbols from the innermost class scope, which can only see + // things from the outer class(es) that have been defined before the inner + // class. + if seen_function && !scope.kind.is_type() { + lexicographical_lookup = false; + } + if scope.kind.is_class() { if seen_function && matches!(symbol, "__class__") { return None; @@ -690,21 +700,8 @@ impl<'a> SemanticModel<'a> { } } - // Technically returning here is not quite correct, it would be more accurate - // to remember the specific binding ids that we need to ignore from this point - // on in the loop, but in most cases we probably want to treat this as unresolved - // since without a forward reference, this would probably refer to the wrong binding - if !seen_function && scope.kind.is_class() && scope.is_class_name(symbol) { - return None; - } - class_variables_visible = scope.kind.is_type() && index == 0; - // once we hit a type scope we should stop doing lexicographical lookups - if scope.kind.is_type() { - lexicographical_lookup = false; - } - if let Some(binding_id) = scope.get(symbol) { if lexicographical_lookup { // we need to look through all the shadowed bindings diff --git a/crates/ruff_python_semantic/src/scope.rs b/crates/ruff_python_semantic/src/scope.rs index faee0db06eef1..a7a69fd555390 100644 --- a/crates/ruff_python_semantic/src/scope.rs +++ b/crates/ruff_python_semantic/src/scope.rs @@ -42,9 +42,6 @@ pub struct Scope<'a> { /// Flags for the [`Scope`]. flags: ScopeFlags, - - /// A vector of class names that should not be available in this scope - class_names: Option>, } impl<'a> Scope<'a> { @@ -57,7 +54,6 @@ impl<'a> Scope<'a> { shadowed_bindings: FxHashMap::default(), globals_id: None, flags: ScopeFlags::empty(), - class_names: None, } } @@ -70,7 +66,6 @@ impl<'a> Scope<'a> { shadowed_bindings: FxHashMap::default(), globals_id: None, flags: ScopeFlags::empty(), - class_names: None, } } @@ -160,31 +155,6 @@ impl<'a> Scope<'a> { pub const fn uses_locals(&self) -> bool { self.flags.intersects(ScopeFlags::USES_LOCALS) } - - /// Either returns a copy of the class names contained within this scope - /// or an empty vector. - pub fn copy_class_names(&self) -> Vec<&'a str> { - if let Some(names) = &self.class_names { - names.clone() - } else { - Vec::default() - } - } - - /// Sets the class names for this scope. - pub fn set_class_names(&mut self, class_names: Vec<&'a str>) { - self.class_names = Some(class_names); - } - - /// Returns `true` if this name is one of the class names that should not - /// be accessible within this scope. - pub fn is_class_name(&self, name: &str) -> bool { - if let Some(names) = &self.class_names { - names.contains(&name) - } else { - false - } - } } bitflags! { From fa9d4df876deb425fb4ef4bd3373debbd5af39b4 Mon Sep 17 00:00:00 2001 From: Daverball Date: Sun, 18 Aug 2024 10:31:46 +0200 Subject: [PATCH 10/24] Guards against TC010/TCH008 overlap --- .../fixtures/flake8_type_checking/TCH008.py | 2 + .../src/rules/flake8_type_checking/mod.rs | 11 + .../rules/type_alias_quotes.rs | 14 +- ...g__tests__quoted-type-alias_TCH008.py.snap | 270 ++++++++++-------- ...__tests__tc010_precedence_over_tch008.snap | 27 ++ 5 files changed, 209 insertions(+), 115 deletions(-) create mode 100644 crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tc010_precedence_over_tch008.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py index 9e766a3732f0b..b95bbc8079849 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py @@ -23,6 +23,7 @@ i: TypeAlias = Foo['str'] # TCH008 j: TypeAlias = 'Baz' # OK k: TypeAlias = 'k | None' # OK +l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) type B = 'Dict' # TCH008 type D = 'Foo[str]' # TCH008 @@ -31,6 +32,7 @@ type I = Foo['str'] # TCH008 type J = 'Baz' # TCH008 type K = 'K | None' # TCH008 +type L = 'int' | None # TCH008 (because TC010 is not enabled) class Baz: diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs index 41bded76447f9..05c6c1a582d25 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs @@ -423,6 +423,17 @@ mod tests { ", "multiple_modules_different_types" )] + #[test_case( + r" + from __future__ import annotations + + from typing import TYPE_CHECKING, TypeAlias + + a: TypeAlias = 'int | None' # TCH008 + b: TypeAlias = 'int' | None # TCH010 + ", + "tc010_precedence_over_tch008" + )] fn contents(contents: &str, snapshot: &str) { let diagnostics = test_snippet( contents, diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs index 99f8366221335..c9c1a853c19ec 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs @@ -1,4 +1,5 @@ -use ast::ExprContext; +use crate::registry::Rule; +use ast::{ExprContext, Operator}; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast as ast; @@ -175,6 +176,17 @@ pub(crate) fn quoted_type_alias( annotation: &str, range: TextRange, ) { + if checker.enabled(Rule::RuntimeStringUnion) { + // this should return a TCH010 error instead + if let Some(Expr::BinOp(ast::ExprBinOp { + op: Operator::BitOr, + .. + })) = checker.semantic().current_expression_parent() + { + return; + } + } + // explicit type aliases require some additional checks to avoid false positives if checker.semantic().in_explicit_type_alias() { // if the expression contains a subscript or attribute access diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap index fa69f0c3b2a8e..2120b2e4e8a52 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap @@ -104,184 +104,226 @@ TCH008.py:23:20: TCH008 [*] Remove quotes from type alias 23 |+i: TypeAlias = Foo[str] # TCH008 24 24 | j: TypeAlias = 'Baz' # OK 25 25 | k: TypeAlias = 'k | None' # OK -26 26 | +26 26 | l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) -TCH008.py:27:10: TCH008 [*] Remove quotes from type alias +TCH008.py:26:16: TCH008 [*] Remove quotes from type alias | +24 | j: TypeAlias = 'Baz' # OK 25 | k: TypeAlias = 'k | None' # OK -26 | -27 | type B = 'Dict' # TCH008 - | ^^^^^^ TCH008 -28 | type D = 'Foo[str]' # TCH008 -29 | type E = 'Foo.bar' # TCH008 +26 | l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) + | ^^^^^ TCH008 +27 | +28 | type B = 'Dict' # TCH008 | = help: Remove quotes ℹ Safe fix +23 23 | i: TypeAlias = Foo['str'] # TCH008 24 24 | j: TypeAlias = 'Baz' # OK 25 25 | k: TypeAlias = 'k | None' # OK -26 26 | -27 |-type B = 'Dict' # TCH008 - 27 |+type B = Dict # TCH008 -28 28 | type D = 'Foo[str]' # TCH008 -29 29 | type E = 'Foo.bar' # TCH008 -30 30 | type G = 'OptStr' # TCH008 +26 |-l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) + 26 |+l: TypeAlias = int | None # TCH008 (because TC010 is not enabled) +27 27 | +28 28 | type B = 'Dict' # TCH008 +29 29 | type D = 'Foo[str]' # TCH008 TCH008.py:28:10: TCH008 [*] Remove quotes from type alias | -27 | type B = 'Dict' # TCH008 -28 | type D = 'Foo[str]' # TCH008 - | ^^^^^^^^^^ TCH008 -29 | type E = 'Foo.bar' # TCH008 -30 | type G = 'OptStr' # TCH008 +26 | l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) +27 | +28 | type B = 'Dict' # TCH008 + | ^^^^^^ TCH008 +29 | type D = 'Foo[str]' # TCH008 +30 | type E = 'Foo.bar' # TCH008 | = help: Remove quotes ℹ Safe fix 25 25 | k: TypeAlias = 'k | None' # OK -26 26 | -27 27 | type B = 'Dict' # TCH008 -28 |-type D = 'Foo[str]' # TCH008 - 28 |+type D = Foo[str] # TCH008 -29 29 | type E = 'Foo.bar' # TCH008 -30 30 | type G = 'OptStr' # TCH008 -31 31 | type I = Foo['str'] # TCH008 +26 26 | l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) +27 27 | +28 |-type B = 'Dict' # TCH008 + 28 |+type B = Dict # TCH008 +29 29 | type D = 'Foo[str]' # TCH008 +30 30 | type E = 'Foo.bar' # TCH008 +31 31 | type G = 'OptStr' # TCH008 TCH008.py:29:10: TCH008 [*] Remove quotes from type alias | -27 | type B = 'Dict' # TCH008 -28 | type D = 'Foo[str]' # TCH008 -29 | type E = 'Foo.bar' # TCH008 - | ^^^^^^^^^ TCH008 -30 | type G = 'OptStr' # TCH008 -31 | type I = Foo['str'] # TCH008 +28 | type B = 'Dict' # TCH008 +29 | type D = 'Foo[str]' # TCH008 + | ^^^^^^^^^^ TCH008 +30 | type E = 'Foo.bar' # TCH008 +31 | type G = 'OptStr' # TCH008 | = help: Remove quotes ℹ Safe fix -26 26 | -27 27 | type B = 'Dict' # TCH008 -28 28 | type D = 'Foo[str]' # TCH008 -29 |-type E = 'Foo.bar' # TCH008 - 29 |+type E = Foo.bar # TCH008 -30 30 | type G = 'OptStr' # TCH008 -31 31 | type I = Foo['str'] # TCH008 -32 32 | type J = 'Baz' # TCH008 +26 26 | l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) +27 27 | +28 28 | type B = 'Dict' # TCH008 +29 |-type D = 'Foo[str]' # TCH008 + 29 |+type D = Foo[str] # TCH008 +30 30 | type E = 'Foo.bar' # TCH008 +31 31 | type G = 'OptStr' # TCH008 +32 32 | type I = Foo['str'] # TCH008 TCH008.py:30:10: TCH008 [*] Remove quotes from type alias | -28 | type D = 'Foo[str]' # TCH008 -29 | type E = 'Foo.bar' # TCH008 -30 | type G = 'OptStr' # TCH008 +28 | type B = 'Dict' # TCH008 +29 | type D = 'Foo[str]' # TCH008 +30 | type E = 'Foo.bar' # TCH008 + | ^^^^^^^^^ TCH008 +31 | type G = 'OptStr' # TCH008 +32 | type I = Foo['str'] # TCH008 + | + = help: Remove quotes + +ℹ Safe fix +27 27 | +28 28 | type B = 'Dict' # TCH008 +29 29 | type D = 'Foo[str]' # TCH008 +30 |-type E = 'Foo.bar' # TCH008 + 30 |+type E = Foo.bar # TCH008 +31 31 | type G = 'OptStr' # TCH008 +32 32 | type I = Foo['str'] # TCH008 +33 33 | type J = 'Baz' # TCH008 + +TCH008.py:31:10: TCH008 [*] Remove quotes from type alias + | +29 | type D = 'Foo[str]' # TCH008 +30 | type E = 'Foo.bar' # TCH008 +31 | type G = 'OptStr' # TCH008 | ^^^^^^^^ TCH008 -31 | type I = Foo['str'] # TCH008 -32 | type J = 'Baz' # TCH008 +32 | type I = Foo['str'] # TCH008 +33 | type J = 'Baz' # TCH008 | = help: Remove quotes ℹ Safe fix -27 27 | type B = 'Dict' # TCH008 -28 28 | type D = 'Foo[str]' # TCH008 -29 29 | type E = 'Foo.bar' # TCH008 -30 |-type G = 'OptStr' # TCH008 - 30 |+type G = OptStr # TCH008 -31 31 | type I = Foo['str'] # TCH008 -32 32 | type J = 'Baz' # TCH008 -33 33 | type K = 'K | None' # TCH008 +28 28 | type B = 'Dict' # TCH008 +29 29 | type D = 'Foo[str]' # TCH008 +30 30 | type E = 'Foo.bar' # TCH008 +31 |-type G = 'OptStr' # TCH008 + 31 |+type G = OptStr # TCH008 +32 32 | type I = Foo['str'] # TCH008 +33 33 | type J = 'Baz' # TCH008 +34 34 | type K = 'K | None' # TCH008 -TCH008.py:31:14: TCH008 [*] Remove quotes from type alias +TCH008.py:32:14: TCH008 [*] Remove quotes from type alias | -29 | type E = 'Foo.bar' # TCH008 -30 | type G = 'OptStr' # TCH008 -31 | type I = Foo['str'] # TCH008 +30 | type E = 'Foo.bar' # TCH008 +31 | type G = 'OptStr' # TCH008 +32 | type I = Foo['str'] # TCH008 | ^^^^^ TCH008 -32 | type J = 'Baz' # TCH008 -33 | type K = 'K | None' # TCH008 +33 | type J = 'Baz' # TCH008 +34 | type K = 'K | None' # TCH008 | = help: Remove quotes ℹ Safe fix -28 28 | type D = 'Foo[str]' # TCH008 -29 29 | type E = 'Foo.bar' # TCH008 -30 30 | type G = 'OptStr' # TCH008 -31 |-type I = Foo['str'] # TCH008 - 31 |+type I = Foo[str] # TCH008 -32 32 | type J = 'Baz' # TCH008 -33 33 | type K = 'K | None' # TCH008 -34 34 | +29 29 | type D = 'Foo[str]' # TCH008 +30 30 | type E = 'Foo.bar' # TCH008 +31 31 | type G = 'OptStr' # TCH008 +32 |-type I = Foo['str'] # TCH008 + 32 |+type I = Foo[str] # TCH008 +33 33 | type J = 'Baz' # TCH008 +34 34 | type K = 'K | None' # TCH008 +35 35 | type L = 'int' | None # TCH008 (because TC010 is not enabled) -TCH008.py:32:10: TCH008 [*] Remove quotes from type alias +TCH008.py:33:10: TCH008 [*] Remove quotes from type alias | -30 | type G = 'OptStr' # TCH008 -31 | type I = Foo['str'] # TCH008 -32 | type J = 'Baz' # TCH008 +31 | type G = 'OptStr' # TCH008 +32 | type I = Foo['str'] # TCH008 +33 | type J = 'Baz' # TCH008 | ^^^^^ TCH008 -33 | type K = 'K | None' # TCH008 +34 | type K = 'K | None' # TCH008 +35 | type L = 'int' | None # TCH008 (because TC010 is not enabled) | = help: Remove quotes ℹ Safe fix -29 29 | type E = 'Foo.bar' # TCH008 -30 30 | type G = 'OptStr' # TCH008 -31 31 | type I = Foo['str'] # TCH008 -32 |-type J = 'Baz' # TCH008 - 32 |+type J = Baz # TCH008 -33 33 | type K = 'K | None' # TCH008 -34 34 | -35 35 | +30 30 | type E = 'Foo.bar' # TCH008 +31 31 | type G = 'OptStr' # TCH008 +32 32 | type I = Foo['str'] # TCH008 +33 |-type J = 'Baz' # TCH008 + 33 |+type J = Baz # TCH008 +34 34 | type K = 'K | None' # TCH008 +35 35 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +36 36 | -TCH008.py:33:10: TCH008 [*] Remove quotes from type alias +TCH008.py:34:10: TCH008 [*] Remove quotes from type alias | -31 | type I = Foo['str'] # TCH008 -32 | type J = 'Baz' # TCH008 -33 | type K = 'K | None' # TCH008 +32 | type I = Foo['str'] # TCH008 +33 | type J = 'Baz' # TCH008 +34 | type K = 'K | None' # TCH008 | ^^^^^^^^^^ TCH008 +35 | type L = 'int' | None # TCH008 (because TC010 is not enabled) + | + = help: Remove quotes + +ℹ Safe fix +31 31 | type G = 'OptStr' # TCH008 +32 32 | type I = Foo['str'] # TCH008 +33 33 | type J = 'Baz' # TCH008 +34 |-type K = 'K | None' # TCH008 + 34 |+type K = K | None # TCH008 +35 35 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +36 36 | +37 37 | + +TCH008.py:35:10: TCH008 [*] Remove quotes from type alias + | +33 | type J = 'Baz' # TCH008 +34 | type K = 'K | None' # TCH008 +35 | type L = 'int' | None # TCH008 (because TC010 is not enabled) + | ^^^^^ TCH008 | = help: Remove quotes ℹ Safe fix -30 30 | type G = 'OptStr' # TCH008 -31 31 | type I = Foo['str'] # TCH008 -32 32 | type J = 'Baz' # TCH008 -33 |-type K = 'K | None' # TCH008 - 33 |+type K = K | None # TCH008 -34 34 | -35 35 | -36 36 | class Baz: +32 32 | type I = Foo['str'] # TCH008 +33 33 | type J = 'Baz' # TCH008 +34 34 | type K = 'K | None' # TCH008 +35 |-type L = 'int' | None # TCH008 (because TC010 is not enabled) + 35 |+type L = int | None # TCH008 (because TC010 is not enabled) +36 36 | +37 37 | +38 38 | class Baz: -TCH008.py:38:14: TCH008 [*] Remove quotes from type alias +TCH008.py:40:14: TCH008 [*] Remove quotes from type alias | -36 | class Baz: -37 | a: TypeAlias = 'Baz' # OK -38 | type A = 'Baz' # TCH008 +38 | class Baz: +39 | a: TypeAlias = 'Baz' # OK +40 | type A = 'Baz' # TCH008 | ^^^^^ TCH008 -39 | -40 | class Nested: +41 | +42 | class Nested: | = help: Remove quotes ℹ Safe fix -35 35 | -36 36 | class Baz: -37 37 | a: TypeAlias = 'Baz' # OK -38 |- type A = 'Baz' # TCH008 - 38 |+ type A = Baz # TCH008 -39 39 | -40 40 | class Nested: -41 41 | a: TypeAlias = 'Baz' # OK +37 37 | +38 38 | class Baz: +39 39 | a: TypeAlias = 'Baz' # OK +40 |- type A = 'Baz' # TCH008 + 40 |+ type A = Baz # TCH008 +41 41 | +42 42 | class Nested: +43 43 | a: TypeAlias = 'Baz' # OK -TCH008.py:42:18: TCH008 [*] Remove quotes from type alias +TCH008.py:44:18: TCH008 [*] Remove quotes from type alias | -40 | class Nested: -41 | a: TypeAlias = 'Baz' # OK -42 | type A = 'Baz' # TCH008 +42 | class Nested: +43 | a: TypeAlias = 'Baz' # OK +44 | type A = 'Baz' # TCH008 | ^^^^^ TCH008 | = help: Remove quotes ℹ Safe fix -39 39 | -40 40 | class Nested: -41 41 | a: TypeAlias = 'Baz' # OK -42 |- type A = 'Baz' # TCH008 - 42 |+ type A = Baz # TCH008 +41 41 | +42 42 | class Nested: +43 43 | a: TypeAlias = 'Baz' # OK +44 |- type A = 'Baz' # TCH008 + 44 |+ type A = Baz # TCH008 diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tc010_precedence_over_tch008.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tc010_precedence_over_tch008.snap new file mode 100644 index 0000000000000..34346c84a31f0 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tc010_precedence_over_tch008.snap @@ -0,0 +1,27 @@ +--- +source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +--- +:6:16: TCH008 [*] Remove quotes from type alias + | +4 | from typing import TYPE_CHECKING, TypeAlias +5 | +6 | a: TypeAlias = 'int | None' # TCH008 + | ^^^^^^^^^^^^ TCH008 +7 | b: TypeAlias = 'int' | None # TCH010 + | + = help: Remove quotes + +ℹ Safe fix +3 3 | +4 4 | from typing import TYPE_CHECKING, TypeAlias +5 5 | +6 |-a: TypeAlias = 'int | None' # TCH008 + 6 |+a: TypeAlias = int | None # TCH008 +7 7 | b: TypeAlias = 'int' | None # TCH010 + +:7:16: TCH010 Invalid string member in `X | Y`-style union type + | +6 | a: TypeAlias = 'int | None' # TCH008 +7 | b: TypeAlias = 'int' | None # TCH010 + | ^^^^^ TCH010 + | From dd78b52368a55459cce42240982ea4576bed0e94 Mon Sep 17 00:00:00 2001 From: David Salvisberg Date: Tue, 20 Aug 2024 15:48:50 +0200 Subject: [PATCH 11/24] Adds implementation and tests for TCH007 --- .../fixtures/flake8_type_checking/TCH007.py | 27 ++++ .../src/checkers/ast/analyze/bindings.rs | 12 +- crates/ruff_linter/src/checkers/ast/mod.rs | 15 ++ .../src/rules/flake8_type_checking/helpers.rs | 13 ++ .../src/rules/flake8_type_checking/mod.rs | 1 + .../rules/type_alias_quotes.rs | 150 +++++++++++++----- ..._tests__unquoted-type-alias_TCH007.py.snap | 147 +++++++++++++++++ crates/ruff_python_semantic/src/binding.rs | 28 ++++ crates/ruff_python_semantic/src/model.rs | 5 +- 9 files changed, 352 insertions(+), 46 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH007.py create mode 100644 crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__unquoted-type-alias_TCH007.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH007.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH007.py new file mode 100644 index 0000000000000..68ef803d7c039 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH007.py @@ -0,0 +1,27 @@ +from __future__ import annotations + +from typing import Dict, TypeAlias, TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Dict + + from foo import Foo + + OptStr: TypeAlias = str | None + Bar: TypeAlias = Foo[int] + +a: TypeAlias = int # OK +b: TypeAlias = Dict # OK +c: TypeAlias = Foo # TCH007 +d: TypeAlias = Foo | None # TCH007 +e: TypeAlias = OptStr # TCH007 +f: TypeAlias = Bar # TCH007 +g: TypeAlias = Foo | Bar # TCH007 x2 +h: TypeAlias = Foo[str] # TCH007 + +type C = Foo # OK +type D = Foo | None # OK +type E = OptStr # OK +type F = Bar # OK +type G = Foo | Bar # OK +type H = Foo[str] # OK diff --git a/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs b/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs index fe6faeafa6244..01d0b0b6f813c 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs @@ -3,7 +3,9 @@ use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::codes::Rule; -use crate::rules::{flake8_import_conventions, flake8_pyi, pyflakes, pylint, ruff}; +use crate::rules::{ + flake8_import_conventions, flake8_pyi, flake8_type_checking, pyflakes, pylint, ruff, +}; /// Run lint rules over the [`Binding`]s. pub(crate) fn bindings(checker: &mut Checker) { @@ -15,6 +17,7 @@ pub(crate) fn bindings(checker: &mut Checker) { Rule::UnconventionalImportAlias, Rule::UnsortedDunderSlots, Rule::UnusedVariable, + Rule::UnquotedTypeAlias, ]) { return; } @@ -72,6 +75,13 @@ pub(crate) fn bindings(checker: &mut Checker) { checker.diagnostics.push(diagnostic); } } + if checker.enabled(Rule::UnquotedTypeAlias) { + if let Some(diagnostics) = + flake8_type_checking::rules::unquoted_type_alias(checker, binding) + { + checker.diagnostics.extend(diagnostics); + } + } if checker.enabled(Rule::UnsortedDunderSlots) { if let Some(diagnostic) = ruff::rules::sort_dunder_slots(checker, binding) { checker.diagnostics.push(diagnostic); diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 3e4187ed4e32d..e12aed4032b27 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -1967,6 +1967,21 @@ impl<'a> Checker<'a> { flags.insert(BindingFlags::UNPACKED_ASSIGNMENT); } + match parent { + Stmt::TypeAlias(_) => flags.insert(BindingFlags::GENERIC_TYPE_ALIAS), + Stmt::AnnAssign(ast::StmtAnnAssign { annotation, .. }) => { + // TODO: It is a bit unfortunate that we do this check twice + // maybe we should change how we visit this statement + // so the semantic flag for the type alias sticks around + // until after we've handled this store, so we can check + // the flag instead of duplicating this check + if self.semantic.match_typing_expr(annotation, "TypeAlias") { + flags.insert(BindingFlags::EXPLICIT_TYPE_ALIAS); + } + } + _ => {} + } + let scope = self.semantic.current_scope(); if scope.kind.is_module() diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs index 09973b7ff3637..9fa389cbc7a2b 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs @@ -266,6 +266,19 @@ pub(crate) fn quote_annotation( } } + quote_type_expression(expr, locator, stylist, generator) +} +/// Wrap a type expression in quotes. +/// +/// This function assumes that the callee already expanded expression components +/// to the minimum acceptable range for quoting, i.e. the parent node may not be +/// a [`Expr::Subscript`], [`Expr::Attribute`], `[Expr::Call]` or `[Expr::BinOp]`. +pub(crate) fn quote_type_expression( + expr: &Expr, + locator: &Locator, + stylist: &Stylist, + generator: Generator, +) -> Result { // If the annotation already contains a quote, avoid attempting to re-quote it. For example: // ```python // from typing import Literal diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs index 05c6c1a582d25..ed9aea0d3fb54 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs @@ -35,6 +35,7 @@ mod tests { #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_8.py"))] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_9.py"))] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("quote.py"))] + #[test_case(Rule::UnquotedTypeAlias, Path::new("TCH007.py"))] #[test_case(Rule::QuotedTypeAlias, Path::new("TCH008.py"))] #[test_case(Rule::RuntimeStringUnion, Path::new("TCH010_1.py"))] #[test_case(Rule::RuntimeStringUnion, Path::new("TCH010_2.py"))] diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs index c9c1a853c19ec..b6efdd59c359b 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs @@ -1,14 +1,15 @@ use crate::registry::Rule; use ast::{ExprContext, Operator}; -use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast as ast; -use ruff_python_ast::Expr; -use ruff_python_semantic::SemanticModel; -use ruff_text_size::TextRange; +use ruff_python_ast::{Expr, Stmt}; +use ruff_python_semantic::{Binding, SemanticModel}; +use ruff_text_size::{Ranged, TextRange}; use std::borrow::Borrow; use crate::checkers::ast::Checker; +use crate::rules::flake8_type_checking::helpers::quote_type_expression; /// ## What it does /// Checks if [PEP 613] explicit type aliases contain references to @@ -42,14 +43,16 @@ use crate::checkers::ast::Checker; #[violation] pub struct UnquotedTypeAlias; -impl AlwaysFixableViolation for UnquotedTypeAlias { +impl Violation for UnquotedTypeAlias { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + #[derive_message_formats] fn message(&self) -> String { format!("Add quotes to type alias") } - fn fix_title(&self) -> String { - "Add quotes".to_string() + fn fix_title(&self) -> Option { + Some("Add quotes".to_string()) } } @@ -112,27 +115,64 @@ impl AlwaysFixableViolation for QuotedTypeAlias { } /// TCH007 -/*pub(crate) fn unquoted_type_alias(checker: &mut Checker, expr: &Expr) { - if !checker.semantic().in_explicit_type_alias() { - return; +pub(crate) fn unquoted_type_alias(checker: &Checker, binding: &Binding) -> Option> { + if binding.context.is_typing() { + return None; } - if checker.semantic().in_forward_reference() { - return; + if !binding.is_explicit_type_alias() { + return None; } - // TODO implement this -}*/ + let Some(Stmt::AnnAssign(ast::StmtAnnAssign { + value: Some(expr), .. + })) = binding.statement(checker.semantic()) + else { + return None; + }; -/// Traverses the type expression and checks the given predicate for each [`Binding`] -// TODO: Do we want to remove Attribute and Subscript traversal? We already -// skip expressions that don't contain either. But then we can't reuse -// this function for TCH007. Is it worth having two functions where one -// has fewer branches because we know they won't be there? -fn contains_typing_reference(semantic: &SemanticModel, expr: &Expr) -> bool { + let mut names = Vec::new(); + collect_typing_references(checker.semantic(), expr, &mut names); + if !names.is_empty() { + // We generate a diagnostic for every name that needs to be quoted + // but we currently emit a single shared fix that quotes the entire + // expression. + // + // Eventually we may try to be more clever and come up with the + // minimal set of subexpressions that need to be quoted. + let parent = expr.range().start(); + let edit = quote_type_expression( + expr, + checker.locator(), + checker.stylist(), + checker.generator(), + ) + .ok(); + let mut diagnostics = Vec::with_capacity(names.len()); + for name in names { + let mut diagnostic = Diagnostic::new(UnquotedTypeAlias, name.range()); + diagnostic.set_parent(parent); + if let Some(edit) = edit.as_ref() { + diagnostic.set_fix(Fix::unsafe_edit(edit.clone())); + } + diagnostics.push(diagnostic); + } + return Some(diagnostics); + } + None +} + +/// Traverses the type expression and collects `[Expr::Name]` nodes that are +/// not available at runtime and thus need to be quoted. +fn collect_typing_references<'a>( + semantic: &SemanticModel, + expr: &'a Expr, + names: &mut Vec<&'a ast::ExprName>, +) { match expr { Expr::BinOp(ast::ExprBinOp { left, right, .. }) => { - contains_typing_reference(semantic, left) || contains_typing_reference(semantic, right) + collect_typing_references(semantic, left, names); + collect_typing_references(semantic, right, names); } Expr::Starred(ast::ExprStarred { value, @@ -140,32 +180,29 @@ fn contains_typing_reference(semantic: &SemanticModel, expr: &Expr) -> bool { .. }) | Expr::Attribute(ast::ExprAttribute { value, .. }) => { - contains_typing_reference(semantic, value) + collect_typing_references(semantic, value, names); } Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { - if contains_typing_reference(semantic, value) { - return true; - } + collect_typing_references(semantic, value, names); if let Expr::Name(ast::ExprName { id, .. }) = value.borrow() { if id.as_str() != "Literal" { - return contains_typing_reference(semantic, slice); + collect_typing_references(semantic, slice, names); } } - false } Expr::List(ast::ExprList { elts, .. }) | Expr::Tuple(ast::ExprTuple { elts, .. }) => { for elt in elts { - if contains_typing_reference(semantic, elt) { - return true; - } + collect_typing_references(semantic, elt, names); } - false } Expr::Name(name) => { - semantic.lookup_symbol(name.id.as_str()).is_some() + if semantic.resolve_name(name).is_some() && semantic.simulate_runtime_load(name).is_none() + { + names.push(name); + } } - _ => false, + _ => {} } } @@ -188,17 +225,10 @@ pub(crate) fn quoted_type_alias( } // explicit type aliases require some additional checks to avoid false positives - if checker.semantic().in_explicit_type_alias() { - // if the expression contains a subscript or attribute access - if annotation.find(|c: char| c == '[' || c == '.').is_some() { - return; - } - - // if the expression contains references to typing-only bindings - // then the quotes are not redundant - if contains_typing_reference(checker.semantic(), expr) { - return; - } + if checker.semantic().in_explicit_type_alias() + && quotes_are_unremovable(checker.semantic(), expr) + { + return; } let mut diagnostic = Diagnostic::new(QuotedTypeAlias, range); @@ -208,3 +238,35 @@ pub(crate) fn quoted_type_alias( ))); checker.diagnostics.push(diagnostic); } + +/// Traverses the type expression and checks if the expression can safely +/// be unquoted +fn quotes_are_unremovable(semantic: &SemanticModel, expr: &Expr) -> bool { + match expr { + Expr::BinOp(ast::ExprBinOp { left, right, .. }) => { + quotes_are_unremovable(semantic, left) || quotes_are_unremovable(semantic, right) + } + Expr::Starred(ast::ExprStarred { + value, + ctx: ExprContext::Load, + .. + }) => quotes_are_unremovable(semantic, value), + // for subscripts and attributes we don't know whether it's safe + // to do at runtime, since the operation may only be available at + // type checking time. E.g. stubs only generics. Or stubs only + // type aliases. + Expr::Subscript(_) | Expr::Attribute(_) => true, + Expr::List(ast::ExprList { elts, .. }) | Expr::Tuple(ast::ExprTuple { elts, .. }) => { + for elt in elts { + if quotes_are_unremovable(semantic, elt) { + return true; + } + } + false + } + Expr::Name(name) => { + semantic.resolve_name(name).is_some() && semantic.simulate_runtime_load(name).is_none() + } + _ => false, + } +} diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__unquoted-type-alias_TCH007.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__unquoted-type-alias_TCH007.py.snap new file mode 100644 index 0000000000000..595233125b22c --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__unquoted-type-alias_TCH007.py.snap @@ -0,0 +1,147 @@ +--- +source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +--- +TCH007.py:15:16: TCH007 [*] Add quotes to type alias + | +13 | a: TypeAlias = int # OK +14 | b: TypeAlias = Dict # OK +15 | c: TypeAlias = Foo # TCH007 + | ^^^ TCH007 +16 | d: TypeAlias = Foo | None # TCH007 +17 | e: TypeAlias = OptStr # TCH007 + | + = help: Add quotes + +ℹ Unsafe fix +12 12 | +13 13 | a: TypeAlias = int # OK +14 14 | b: TypeAlias = Dict # OK +15 |-c: TypeAlias = Foo # TCH007 + 15 |+c: TypeAlias = "Foo" # TCH007 +16 16 | d: TypeAlias = Foo | None # TCH007 +17 17 | e: TypeAlias = OptStr # TCH007 +18 18 | f: TypeAlias = Bar # TCH007 + +TCH007.py:16:16: TCH007 [*] Add quotes to type alias + | +14 | b: TypeAlias = Dict # OK +15 | c: TypeAlias = Foo # TCH007 +16 | d: TypeAlias = Foo | None # TCH007 + | ^^^ TCH007 +17 | e: TypeAlias = OptStr # TCH007 +18 | f: TypeAlias = Bar # TCH007 + | + = help: Add quotes + +ℹ Unsafe fix +13 13 | a: TypeAlias = int # OK +14 14 | b: TypeAlias = Dict # OK +15 15 | c: TypeAlias = Foo # TCH007 +16 |-d: TypeAlias = Foo | None # TCH007 + 16 |+d: TypeAlias = "Foo | None" # TCH007 +17 17 | e: TypeAlias = OptStr # TCH007 +18 18 | f: TypeAlias = Bar # TCH007 +19 19 | g: TypeAlias = Foo | Bar # TCH007 x2 + +TCH007.py:17:16: TCH007 [*] Add quotes to type alias + | +15 | c: TypeAlias = Foo # TCH007 +16 | d: TypeAlias = Foo | None # TCH007 +17 | e: TypeAlias = OptStr # TCH007 + | ^^^^^^ TCH007 +18 | f: TypeAlias = Bar # TCH007 +19 | g: TypeAlias = Foo | Bar # TCH007 x2 + | + = help: Add quotes + +ℹ Unsafe fix +14 14 | b: TypeAlias = Dict # OK +15 15 | c: TypeAlias = Foo # TCH007 +16 16 | d: TypeAlias = Foo | None # TCH007 +17 |-e: TypeAlias = OptStr # TCH007 + 17 |+e: TypeAlias = "OptStr" # TCH007 +18 18 | f: TypeAlias = Bar # TCH007 +19 19 | g: TypeAlias = Foo | Bar # TCH007 x2 +20 20 | h: TypeAlias = Foo[str] # TCH007 + +TCH007.py:18:16: TCH007 [*] Add quotes to type alias + | +16 | d: TypeAlias = Foo | None # TCH007 +17 | e: TypeAlias = OptStr # TCH007 +18 | f: TypeAlias = Bar # TCH007 + | ^^^ TCH007 +19 | g: TypeAlias = Foo | Bar # TCH007 x2 +20 | h: TypeAlias = Foo[str] # TCH007 + | + = help: Add quotes + +ℹ Unsafe fix +15 15 | c: TypeAlias = Foo # TCH007 +16 16 | d: TypeAlias = Foo | None # TCH007 +17 17 | e: TypeAlias = OptStr # TCH007 +18 |-f: TypeAlias = Bar # TCH007 + 18 |+f: TypeAlias = "Bar" # TCH007 +19 19 | g: TypeAlias = Foo | Bar # TCH007 x2 +20 20 | h: TypeAlias = Foo[str] # TCH007 +21 21 | + +TCH007.py:19:16: TCH007 [*] Add quotes to type alias + | +17 | e: TypeAlias = OptStr # TCH007 +18 | f: TypeAlias = Bar # TCH007 +19 | g: TypeAlias = Foo | Bar # TCH007 x2 + | ^^^ TCH007 +20 | h: TypeAlias = Foo[str] # TCH007 + | + = help: Add quotes + +ℹ Unsafe fix +16 16 | d: TypeAlias = Foo | None # TCH007 +17 17 | e: TypeAlias = OptStr # TCH007 +18 18 | f: TypeAlias = Bar # TCH007 +19 |-g: TypeAlias = Foo | Bar # TCH007 x2 + 19 |+g: TypeAlias = "Foo | Bar" # TCH007 x2 +20 20 | h: TypeAlias = Foo[str] # TCH007 +21 21 | +22 22 | type C = Foo # OK + +TCH007.py:19:22: TCH007 [*] Add quotes to type alias + | +17 | e: TypeAlias = OptStr # TCH007 +18 | f: TypeAlias = Bar # TCH007 +19 | g: TypeAlias = Foo | Bar # TCH007 x2 + | ^^^ TCH007 +20 | h: TypeAlias = Foo[str] # TCH007 + | + = help: Add quotes + +ℹ Unsafe fix +16 16 | d: TypeAlias = Foo | None # TCH007 +17 17 | e: TypeAlias = OptStr # TCH007 +18 18 | f: TypeAlias = Bar # TCH007 +19 |-g: TypeAlias = Foo | Bar # TCH007 x2 + 19 |+g: TypeAlias = "Foo | Bar" # TCH007 x2 +20 20 | h: TypeAlias = Foo[str] # TCH007 +21 21 | +22 22 | type C = Foo # OK + +TCH007.py:20:16: TCH007 [*] Add quotes to type alias + | +18 | f: TypeAlias = Bar # TCH007 +19 | g: TypeAlias = Foo | Bar # TCH007 x2 +20 | h: TypeAlias = Foo[str] # TCH007 + | ^^^ TCH007 +21 | +22 | type C = Foo # OK + | + = help: Add quotes + +ℹ Unsafe fix +17 17 | e: TypeAlias = OptStr # TCH007 +18 18 | f: TypeAlias = Bar # TCH007 +19 19 | g: TypeAlias = Foo | Bar # TCH007 x2 +20 |-h: TypeAlias = Foo[str] # TCH007 + 20 |+h: TypeAlias = "Foo[str]" # TCH007 +21 21 | +22 22 | type C = Foo # OK +23 23 | type D = Foo | None # OK diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index e65ddfc5b4133..82c885f502015 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -136,6 +136,21 @@ impl<'a> Binding<'a> { self.flags.contains(BindingFlags::IN_EXCEPT_HANDLER) } + /// Return `true` if this [`Binding`] represents an explicit type alias + pub const fn is_explicit_type_alias(&self) -> bool { + self.flags.intersects(BindingFlags::EXPLICIT_TYPE_ALIAS) + } + + /// Return `true` if this [`Binding`] represents a generic type alias + pub const fn is_generic_type_alias(&self) -> bool { + self.flags.intersects(BindingFlags::GENERIC_TYPE_ALIAS) + } + + /// Return `true` if this [`Binding`] represents a type alias + pub const fn is_type_alias(&self) -> bool { + self.flags.intersects(BindingFlags::TYPE_ALIAS) + } + /// Return `true` if this binding "redefines" the given binding, as per Pyflake's definition of /// redefinition. pub fn redefines(&self, existing: &Binding) -> bool { @@ -377,6 +392,19 @@ bitflags! { /// y = 42 /// ``` const IN_EXCEPT_HANDLER = 1 << 10; + + /// The binding represents a [PEP 613] explicit type alias. + /// + /// [PEP 613]: https://peps.python.org/pep-0613/ + const EXPLICIT_TYPE_ALIAS = 1 << 11; + + /// The binding represents a [PEP 695] type statement + /// + /// [PEP 695]: https://peps.python.org/pep-0695/#generic-type-alias + const GENERIC_TYPE_ALIAS = 1 << 12; + + /// The binding represents any type alias. + const TYPE_ALIAS = Self::EXPLICIT_TYPE_ALIAS.bits() | Self::GENERIC_TYPE_ALIAS.bits(); } } diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 9812b74bfd006..c9c80a194ee55 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -1745,7 +1745,7 @@ impl<'a> SemanticModel<'a> { /// Return `true` if the model is in a type alias pub const fn in_type_alias(&self) -> bool { - self.in_explicit_type_alias() || self.in_generic_type_alias() + self.flags.intersects(SemanticModelFlags::TYPE_ALIAS) } /// Return `true` if the model is in an exception handler. @@ -2342,6 +2342,9 @@ bitflags! { /// The context is in a typing-only context. const TYPING_CONTEXT = Self::TYPE_CHECKING_BLOCK.bits() | Self::TYPING_ONLY_ANNOTATION.bits() | Self::STRING_TYPE_DEFINITION.bits() | Self::TYPE_PARAM_DEFINITION.bits(); + + /// The context is in any type alias. + const TYPE_ALIAS = Self::EXPLICIT_TYPE_ALIAS.bits() | Self::GENERIC_TYPE_ALIAS.bits(); } } From 08b7ddcf96b1b4d366f2e9ad4f0ad41cdc4a2f8e Mon Sep 17 00:00:00 2001 From: David Salvisberg Date: Tue, 20 Aug 2024 17:42:45 +0200 Subject: [PATCH 12/24] Fixes TCH004/TCH007 overlap --- .../fixtures/flake8_type_checking/quote.py | 9 +++++ .../src/rules/flake8_type_checking/mod.rs | 12 ++++++ .../runtime_import_in_type_checking_block.rs | 4 +- .../rules/type_alias_quotes.rs | 40 +++++++++++++------ ...mport-in-type-checking-block_quote.py.snap | 15 +++++++ ...ping-only-third-party-import_quote.py.snap | 4 ++ ...mport-in-type-checking-block_quote.py.snap | 22 ++++++++++ ...__tests__tc010_precedence_over_tch008.snap | 4 +- ..._tests__tch004_precedence_over_tch007.snap | 24 +++++++++++ crates/ruff_python_semantic/src/reference.rs | 6 +++ 10 files changed, 125 insertions(+), 15 deletions(-) create mode 100644 crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tch004_precedence_over_tch007.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote.py index d13e8cd970f8c..537447a015d36 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote.py @@ -97,3 +97,12 @@ def f(): def func(self) -> DataFrame | list[Series]: pass + + +def f(): + from typing import TypeAlias, TYPE_CHECKING + + if TYPE_CHECKING: + from pandas import DataFrame + + x: TypeAlias = DataFrame | None diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs index ed9aea0d3fb54..0ba4583746fe9 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs @@ -429,6 +429,18 @@ mod tests { from __future__ import annotations from typing import TYPE_CHECKING, TypeAlias + if TYPE_CHECKING: + from foo import Foo # TCH004 + + a: TypeAlias = Foo | None # OK + ", + "tch004_precedence_over_tch007" + )] + #[test_case( + r" + from __future__ import annotations + + from typing import TypeAlias a: TypeAlias = 'int | None' # TCH008 b: TypeAlias = 'int' | None # TCH010 diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs index d9d7b45e2e5c6..2881cce44d07b 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs @@ -154,7 +154,9 @@ pub(crate) fn runtime_import_in_type_checking_block( if checker.settings.flake8_type_checking.quote_annotations && binding.references().all(|reference_id| { let reference = checker.semantic().reference(reference_id); - reference.in_typing_context() || reference.in_runtime_evaluated_annotation() + reference.in_typing_context() + || reference.in_runtime_evaluated_annotation() + || reference.in_explicit_type_alias() }) { actions diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs index b6efdd59c359b..9da026a98d3f5 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs @@ -132,7 +132,7 @@ pub(crate) fn unquoted_type_alias(checker: &Checker, binding: &Binding) -> Optio }; let mut names = Vec::new(); - collect_typing_references(checker.semantic(), expr, &mut names); + collect_typing_references(checker, expr, &mut names); if !names.is_empty() { // We generate a diagnostic for every name that needs to be quoted // but we currently emit a single shared fix that quotes the entire @@ -163,16 +163,17 @@ pub(crate) fn unquoted_type_alias(checker: &Checker, binding: &Binding) -> Optio } /// Traverses the type expression and collects `[Expr::Name]` nodes that are -/// not available at runtime and thus need to be quoted. +/// not available at runtime and thus need to be quoted, unless they would +/// become available through `[Rule::RuntimeImportInTypeCheckingBlock]`. fn collect_typing_references<'a>( - semantic: &SemanticModel, + checker: &Checker, expr: &'a Expr, names: &mut Vec<&'a ast::ExprName>, ) { match expr { Expr::BinOp(ast::ExprBinOp { left, right, .. }) => { - collect_typing_references(semantic, left, names); - collect_typing_references(semantic, right, names); + collect_typing_references(checker, left, names); + collect_typing_references(checker, right, names); } Expr::Starred(ast::ExprStarred { value, @@ -180,27 +181,42 @@ fn collect_typing_references<'a>( .. }) | Expr::Attribute(ast::ExprAttribute { value, .. }) => { - collect_typing_references(semantic, value, names); + collect_typing_references(checker, value, names); } Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { - collect_typing_references(semantic, value, names); + collect_typing_references(checker, value, names); if let Expr::Name(ast::ExprName { id, .. }) = value.borrow() { if id.as_str() != "Literal" { - collect_typing_references(semantic, slice, names); + collect_typing_references(checker, slice, names); } } } Expr::List(ast::ExprList { elts, .. }) | Expr::Tuple(ast::ExprTuple { elts, .. }) => { for elt in elts { - collect_typing_references(semantic, elt, names); + collect_typing_references(checker, elt, names); } } Expr::Name(name) => { - if semantic.resolve_name(name).is_some() - && semantic.simulate_runtime_load(name).is_none() + let Some(binding_id) = checker.semantic().resolve_name(name) else { + return; + }; + if checker.semantic().simulate_runtime_load(name).is_some() { + return; + } + + // if TCH004 is enabled we shouldn't emit a TCH007 for a reference to + // a binding that would emit a TCH004, otherwise the fixes will never + // stabilize and keep going in circles + if checker.enabled(Rule::RuntimeImportInTypeCheckingBlock) + && checker + .semantic() + .binding(binding_id) + .references() + .any(|id| checker.semantic().reference(id).in_runtime_context()) { - names.push(name); + return; } + names.push(name); } _ => {} } diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_runtime-import-in-type-checking-block_quote.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_runtime-import-in-type-checking-block_quote.py.snap index ae71c56c8195a..54207373d458a 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_runtime-import-in-type-checking-block_quote.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_runtime-import-in-type-checking-block_quote.py.snap @@ -21,4 +21,19 @@ quote.py:64:28: TCH004 [*] Quote references to `pandas.DataFrame`. Import is in 68 68 | 69 69 | +quote.py:106:28: TCH004 [*] Quote references to `pandas.DataFrame`. Import is in a type-checking block. + | +105 | if TYPE_CHECKING: +106 | from pandas import DataFrame + | ^^^^^^^^^ TCH004 +107 | +108 | x: TypeAlias = DataFrame | None + | + = help: Quote references +ℹ Unsafe fix +105 105 | if TYPE_CHECKING: +106 106 | from pandas import DataFrame +107 107 | +108 |- x: TypeAlias = DataFrame | None + 108 |+ x: TypeAlias = "DataFrame | None" diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_typing-only-third-party-import_quote.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_typing-only-third-party-import_quote.py.snap index d9bb6a9824812..2f4625a60ff4a 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_typing-only-third-party-import_quote.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_typing-only-third-party-import_quote.py.snap @@ -369,6 +369,8 @@ quote.py:96:24: TCH002 [*] Move third-party import `pandas.DataFrame` into a typ 98 |- def func(self) -> DataFrame | list[Series]: 101 |+ def func(self) -> "DataFrame | list[Series]": 99 102 | pass +100 103 | +101 104 | quote.py:96:35: TCH002 [*] Move third-party import `pandas.Series` into a type-checking block | @@ -397,3 +399,5 @@ quote.py:96:35: TCH002 [*] Move third-party import `pandas.Series` into a type-c 98 |- def func(self) -> DataFrame | list[Series]: 101 |+ def func(self) -> "DataFrame | list[Series]": 99 102 | pass +100 103 | +101 104 | diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__runtime-import-in-type-checking-block_quote.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__runtime-import-in-type-checking-block_quote.py.snap index c09777853b97a..3d01e7d47af72 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__runtime-import-in-type-checking-block_quote.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__runtime-import-in-type-checking-block_quote.py.snap @@ -26,4 +26,26 @@ quote.py:64:28: TCH004 [*] Move import `pandas.DataFrame` out of type-checking b 66 67 | def func(value: DataFrame): 67 68 | ... +quote.py:106:28: TCH004 [*] Move import `pandas.DataFrame` out of type-checking block. Import is used for more than type hinting. + | +105 | if TYPE_CHECKING: +106 | from pandas import DataFrame + | ^^^^^^^^^ TCH004 +107 | +108 | x: TypeAlias = DataFrame | None + | + = help: Move out of type-checking block +ℹ Unsafe fix + 1 |+from pandas import DataFrame +1 2 | def f(): +2 3 | from pandas import DataFrame +3 4 | +-------------------------------------------------------------------------------- +103 104 | from typing import TypeAlias, TYPE_CHECKING +104 105 | +105 106 | if TYPE_CHECKING: +106 |- from pandas import DataFrame + 107 |+ pass +107 108 | +108 109 | x: TypeAlias = DataFrame | None diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tc010_precedence_over_tch008.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tc010_precedence_over_tch008.snap index 34346c84a31f0..f99741fd4865d 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tc010_precedence_over_tch008.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tc010_precedence_over_tch008.snap @@ -3,7 +3,7 @@ source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs --- :6:16: TCH008 [*] Remove quotes from type alias | -4 | from typing import TYPE_CHECKING, TypeAlias +4 | from typing import TypeAlias 5 | 6 | a: TypeAlias = 'int | None' # TCH008 | ^^^^^^^^^^^^ TCH008 @@ -13,7 +13,7 @@ source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs ℹ Safe fix 3 3 | -4 4 | from typing import TYPE_CHECKING, TypeAlias +4 4 | from typing import TypeAlias 5 5 | 6 |-a: TypeAlias = 'int | None' # TCH008 6 |+a: TypeAlias = int | None # TCH008 diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tch004_precedence_over_tch007.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tch004_precedence_over_tch007.snap new file mode 100644 index 0000000000000..a6e05ea19728e --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tch004_precedence_over_tch007.snap @@ -0,0 +1,24 @@ +--- +source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +--- +:6:21: TCH004 [*] Move import `foo.Foo` out of type-checking block. Import is used for more than type hinting. + | +4 | from typing import TYPE_CHECKING, TypeAlias +5 | if TYPE_CHECKING: +6 | from foo import Foo # TCH004 + | ^^^ TCH004 +7 | +8 | a: TypeAlias = Foo | None # OK + | + = help: Move out of type-checking block + +ℹ Unsafe fix +2 2 | from __future__ import annotations +3 3 | +4 4 | from typing import TYPE_CHECKING, TypeAlias + 5 |+from foo import Foo +5 6 | if TYPE_CHECKING: +6 |- from foo import Foo # TCH004 + 7 |+ pass # TCH004 +7 8 | +8 9 | a: TypeAlias = Foo | None # OK diff --git a/crates/ruff_python_semantic/src/reference.rs b/crates/ruff_python_semantic/src/reference.rs index 1b79347684bea..91681c08b8211 100644 --- a/crates/ruff_python_semantic/src/reference.rs +++ b/crates/ruff_python_semantic/src/reference.rs @@ -99,6 +99,12 @@ impl ResolvedReference { self.flags .intersects(SemanticModelFlags::DUNDER_ALL_DEFINITION) } + + /// Return `true` if the context is in an explicit type alias. + pub const fn in_explicit_type_alias(&self) -> bool { + self.flags + .intersects(SemanticModelFlags::EXPLICIT_TYPE_ALIAS) + } } impl Ranged for ResolvedReference { From b17c95967abe83cf6c2eb77f643697bf08866c67 Mon Sep 17 00:00:00 2001 From: David Salvisberg Date: Thu, 24 Oct 2024 15:40:00 +0200 Subject: [PATCH 13/24] Clean up unused import that was accidentally left in --- crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs index f005ea1fadcc4..ad982130ae071 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs @@ -1,7 +1,6 @@ use anyhow::Result; use ast::visitor::source_order; use ruff_python_ast::visitor::source_order::SourceOrderVisitor; -use ruff_source_file::Locator; use std::cmp::Reverse; use ruff_diagnostics::Edit; From 643ddb6ea9d4ed04df37d22d804d22691b315d36 Mon Sep 17 00:00:00 2001 From: David Salvisberg Date: Thu, 24 Oct 2024 15:47:45 +0200 Subject: [PATCH 14/24] Simplifies `quote_type_expression` --- .../src/rules/flake8_type_checking/helpers.rs | 9 +++------ .../flake8_type_checking/rules/type_alias_quotes.rs | 6 ++---- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs index ad982130ae071..23fd3d1cbff9c 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs @@ -265,7 +265,7 @@ pub(crate) fn quote_annotation( } } - quote_type_expression(expr, semantic, stylist) + Ok(quote_type_expression(expr, semantic, stylist)) } /// Wrap a type expression in quotes. @@ -277,17 +277,14 @@ pub(crate) fn quote_type_expression( expr: &Expr, semantic: &SemanticModel, stylist: &Stylist, -) -> Result { +) -> Edit { // Quote the entire expression. let quote = stylist.quote(); let mut quote_annotator = QuoteAnnotator::new(semantic, stylist); quote_annotator.visit_expr(expr); let annotation = quote_annotator.into_annotation(); - Ok(Edit::range_replacement( - format!("{quote}{annotation}{quote}"), - expr.range(), - )) + Edit::range_replacement(format!("{quote}{annotation}{quote}"), expr.range()) } /// Filter out any [`Edit`]s that are completely contained by any other [`Edit`]. diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs index beceed7fd8605..9459327b0e905 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs @@ -141,14 +141,12 @@ pub(crate) fn unquoted_type_alias(checker: &Checker, binding: &Binding) -> Optio // Eventually we may try to be more clever and come up with the // minimal set of subexpressions that need to be quoted. let parent = expr.range().start(); - let edit = quote_type_expression(expr, checker.semantic(), checker.stylist()).ok(); + let edit = quote_type_expression(expr, checker.semantic(), checker.stylist()); let mut diagnostics = Vec::with_capacity(names.len()); for name in names { let mut diagnostic = Diagnostic::new(UnquotedTypeAlias, name.range()); diagnostic.set_parent(parent); - if let Some(edit) = edit.as_ref() { - diagnostic.set_fix(Fix::unsafe_edit(edit.clone())); - } + diagnostic.set_fix(Fix::unsafe_edit(edit.clone())); diagnostics.push(diagnostic); } return Some(diagnostics); From 09986c7fe3fe515546940b6d845ec06dac2b0d9c Mon Sep 17 00:00:00 2001 From: David Salvisberg Date: Sat, 9 Nov 2024 14:58:59 +0100 Subject: [PATCH 15/24] Apply suggestions from code review Co-authored-by: Simon Brugman --- .../rules/flake8_type_checking/rules/type_alias_quotes.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs index 9459327b0e905..77bbbd7c4a95e 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs @@ -37,7 +37,7 @@ use crate::rules::flake8_type_checking::helpers::quote_type_expression; /// ``` /// /// ## References -/// - [PEP 613](https://peps.python.org/pep-0613/) +/// - [PEP 613 – Explicit Type Aliases](https://peps.python.org/pep-0613/) /// /// [PEP 613]: https://peps.python.org/pep-0613/ #[violation] @@ -93,9 +93,9 @@ impl Violation for UnquotedTypeAlias { /// ``` /// /// ## References -/// - [PEP 613](https://peps.python.org/pep-0613/) -/// - [PEP 695](https://peps.python.org/pep-0695/#generic-type-alias) -/// - [PEP 604](https://peps.python.org/pep-0604/) +/// - [PEP 613 – Explicit Type Aliases](https://peps.python.org/pep-0613/) +/// - [PEP 695: Generic Type Alias](https://peps.python.org/pep-0695/#generic-type-alias) +/// - [PEP 604 – Allow writing union types as `X | Y`](https://peps.python.org/pep-0604/) /// /// [PEP 604]: https://peps.python.org/pep-0604/ /// [PEP 613]: https://peps.python.org/pep-0613/ From 4044dfadcb17929fc36299e6a5c38b4674381ef5 Mon Sep 17 00:00:00 2001 From: Daverball Date: Sat, 9 Nov 2024 16:03:24 +0100 Subject: [PATCH 16/24] Re-formats code slightly. Marks TCH007 fix as unsafe, since it may remove comments. Adds additional test cases. --- .../fixtures/flake8_type_checking/TCH007.py | 4 + .../fixtures/flake8_type_checking/TCH008.py | 8 + .../rules/type_alias_quotes.rs | 39 +- ...g__tests__quoted-type-alias_TCH008.py.snap | 411 +++++++++++------- ...__tests__tc010_precedence_over_tch008.snap | 2 +- ..._tests__unquoted-type-alias_TCH007.py.snap | 65 ++- 6 files changed, 339 insertions(+), 190 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH007.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH007.py index 68ef803d7c039..82a595b44626a 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH007.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH007.py @@ -18,6 +18,8 @@ f: TypeAlias = Bar # TCH007 g: TypeAlias = Foo | Bar # TCH007 x2 h: TypeAlias = Foo[str] # TCH007 +i: TypeAlias = (Foo | # TCH007 x2 (fix removes comment currently) + Bar) type C = Foo # OK type D = Foo | None # OK @@ -25,3 +27,5 @@ type F = Bar # OK type G = Foo | Bar # OK type H = Foo[str] # OK +type I = (Foo | # OK + Bar) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py index b95bbc8079849..45df895e67ded 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py @@ -24,6 +24,10 @@ j: TypeAlias = 'Baz' # OK k: TypeAlias = 'k | None' # OK l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) +m: TypeAlias = ('int' # TCH008 + | None) +n: TypeAlias = ('int' # TCH008 (fix removes comment currently) + ' | None') type B = 'Dict' # TCH008 type D = 'Foo[str]' # TCH008 @@ -33,6 +37,10 @@ type J = 'Baz' # TCH008 type K = 'K | None' # TCH008 type L = 'int' | None # TCH008 (because TC010 is not enabled) +type M = ('int' # TCH008 + | None) +type N = ('int' # TCH008 (fix removes comment currently) + ' | None') class Baz: diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs index 77bbbd7c4a95e..eb2dcef213c66 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs @@ -133,25 +133,26 @@ pub(crate) fn unquoted_type_alias(checker: &Checker, binding: &Binding) -> Optio let mut names = Vec::new(); collect_typing_references(checker, expr, &mut names); - if !names.is_empty() { - // We generate a diagnostic for every name that needs to be quoted - // but we currently emit a single shared fix that quotes the entire - // expression. - // - // Eventually we may try to be more clever and come up with the - // minimal set of subexpressions that need to be quoted. - let parent = expr.range().start(); - let edit = quote_type_expression(expr, checker.semantic(), checker.stylist()); - let mut diagnostics = Vec::with_capacity(names.len()); - for name in names { - let mut diagnostic = Diagnostic::new(UnquotedTypeAlias, name.range()); - diagnostic.set_parent(parent); - diagnostic.set_fix(Fix::unsafe_edit(edit.clone())); - diagnostics.push(diagnostic); - } - return Some(diagnostics); + if names.is_empty() { + return None; + } + + // We generate a diagnostic for every name that needs to be quoted + // but we currently emit a single shared fix that quotes the entire + // expression. + // + // Eventually we may try to be more clever and come up with the + // minimal set of subexpressions that need to be quoted. + let parent = expr.range().start(); + let edit = quote_type_expression(expr, checker.semantic(), checker.stylist()); + let mut diagnostics = Vec::with_capacity(names.len()); + for name in names { + let mut diagnostic = Diagnostic::new(UnquotedTypeAlias, name.range()); + diagnostic.set_parent(parent); + diagnostic.set_fix(Fix::unsafe_edit(edit.clone())); + diagnostics.push(diagnostic); } - None + return Some(diagnostics); } /// Traverses the type expression and collects `[Expr::Name]` nodes that are @@ -240,7 +241,7 @@ pub(crate) fn quoted_type_alias( } let mut diagnostic = Diagnostic::new(QuotedTypeAlias, range); - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( annotation.to_string(), range, ))); diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap index 2120b2e4e8a52..d368dd45ffdc7 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap @@ -12,7 +12,7 @@ TCH008.py:15:16: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Safe fix +ℹ Unsafe fix 12 12 | else: 13 13 | Bar = Foo 14 14 | @@ -33,7 +33,7 @@ TCH008.py:17:16: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Safe fix +ℹ Unsafe fix 14 14 | 15 15 | a: TypeAlias = 'int' # TCH008 16 16 | b: TypeAlias = 'Dict' # OK @@ -54,7 +54,7 @@ TCH008.py:20:16: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Safe fix +ℹ Unsafe fix 17 17 | c: TypeAlias = 'Foo' # TCH008 18 18 | d: TypeAlias = 'Foo[str]' # OK 19 19 | e: TypeAlias = 'Foo.bar' # OK @@ -75,7 +75,7 @@ TCH008.py:22:16: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Safe fix +ℹ Unsafe fix 19 19 | e: TypeAlias = 'Foo.bar' # OK 20 20 | f: TypeAlias = 'Foo | None' # TCH008 21 21 | g: TypeAlias = 'OptStr' # OK @@ -96,7 +96,7 @@ TCH008.py:23:20: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Safe fix +ℹ Unsafe fix 20 20 | f: TypeAlias = 'Foo | None' # TCH008 21 21 | g: TypeAlias = 'OptStr' # OK 22 22 | h: TypeAlias = 'Bar' # TCH008 @@ -112,218 +112,309 @@ TCH008.py:26:16: TCH008 [*] Remove quotes from type alias 25 | k: TypeAlias = 'k | None' # OK 26 | l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) | ^^^^^ TCH008 -27 | -28 | type B = 'Dict' # TCH008 +27 | m: TypeAlias = ('int' # TCH008 +28 | | None) | = help: Remove quotes -ℹ Safe fix +ℹ Unsafe fix 23 23 | i: TypeAlias = Foo['str'] # TCH008 24 24 | j: TypeAlias = 'Baz' # OK 25 25 | k: TypeAlias = 'k | None' # OK 26 |-l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) 26 |+l: TypeAlias = int | None # TCH008 (because TC010 is not enabled) -27 27 | -28 28 | type B = 'Dict' # TCH008 -29 29 | type D = 'Foo[str]' # TCH008 +27 27 | m: TypeAlias = ('int' # TCH008 +28 28 | | None) +29 29 | n: TypeAlias = ('int' # TCH008 (fix removes comment currently) -TCH008.py:28:10: TCH008 [*] Remove quotes from type alias +TCH008.py:27:17: TCH008 [*] Remove quotes from type alias | +25 | k: TypeAlias = 'k | None' # OK 26 | l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) -27 | -28 | type B = 'Dict' # TCH008 - | ^^^^^^ TCH008 -29 | type D = 'Foo[str]' # TCH008 -30 | type E = 'Foo.bar' # TCH008 +27 | m: TypeAlias = ('int' # TCH008 + | ^^^^^ TCH008 +28 | | None) +29 | n: TypeAlias = ('int' # TCH008 (fix removes comment currently) | = help: Remove quotes -ℹ Safe fix +ℹ Unsafe fix +24 24 | j: TypeAlias = 'Baz' # OK 25 25 | k: TypeAlias = 'k | None' # OK 26 26 | l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) -27 27 | -28 |-type B = 'Dict' # TCH008 - 28 |+type B = Dict # TCH008 -29 29 | type D = 'Foo[str]' # TCH008 -30 30 | type E = 'Foo.bar' # TCH008 -31 31 | type G = 'OptStr' # TCH008 +27 |-m: TypeAlias = ('int' # TCH008 + 27 |+m: TypeAlias = (int # TCH008 +28 28 | | None) +29 29 | n: TypeAlias = ('int' # TCH008 (fix removes comment currently) +30 30 | ' | None') -TCH008.py:29:10: TCH008 [*] Remove quotes from type alias +TCH008.py:29:17: TCH008 [*] Remove quotes from type alias | -28 | type B = 'Dict' # TCH008 -29 | type D = 'Foo[str]' # TCH008 - | ^^^^^^^^^^ TCH008 -30 | type E = 'Foo.bar' # TCH008 -31 | type G = 'OptStr' # TCH008 +27 | m: TypeAlias = ('int' # TCH008 +28 | | None) +29 | n: TypeAlias = ('int' # TCH008 (fix removes comment currently) + | _________________^ +30 | | ' | None') + | |_____________^ TCH008 +31 | +32 | type B = 'Dict' # TCH008 | = help: Remove quotes -ℹ Safe fix +ℹ Unsafe fix 26 26 | l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) -27 27 | -28 28 | type B = 'Dict' # TCH008 -29 |-type D = 'Foo[str]' # TCH008 - 29 |+type D = Foo[str] # TCH008 -30 30 | type E = 'Foo.bar' # TCH008 -31 31 | type G = 'OptStr' # TCH008 -32 32 | type I = Foo['str'] # TCH008 - -TCH008.py:30:10: TCH008 [*] Remove quotes from type alias - | -28 | type B = 'Dict' # TCH008 -29 | type D = 'Foo[str]' # TCH008 -30 | type E = 'Foo.bar' # TCH008 +27 27 | m: TypeAlias = ('int' # TCH008 +28 28 | | None) +29 |-n: TypeAlias = ('int' # TCH008 (fix removes comment currently) +30 |- ' | None') + 29 |+n: TypeAlias = (int | None) +31 30 | +32 31 | type B = 'Dict' # TCH008 +33 32 | type D = 'Foo[str]' # TCH008 + +TCH008.py:32:10: TCH008 [*] Remove quotes from type alias + | +30 | ' | None') +31 | +32 | type B = 'Dict' # TCH008 + | ^^^^^^ TCH008 +33 | type D = 'Foo[str]' # TCH008 +34 | type E = 'Foo.bar' # TCH008 + | + = help: Remove quotes + +ℹ Unsafe fix +29 29 | n: TypeAlias = ('int' # TCH008 (fix removes comment currently) +30 30 | ' | None') +31 31 | +32 |-type B = 'Dict' # TCH008 + 32 |+type B = Dict # TCH008 +33 33 | type D = 'Foo[str]' # TCH008 +34 34 | type E = 'Foo.bar' # TCH008 +35 35 | type G = 'OptStr' # TCH008 + +TCH008.py:33:10: TCH008 [*] Remove quotes from type alias + | +32 | type B = 'Dict' # TCH008 +33 | type D = 'Foo[str]' # TCH008 + | ^^^^^^^^^^ TCH008 +34 | type E = 'Foo.bar' # TCH008 +35 | type G = 'OptStr' # TCH008 + | + = help: Remove quotes + +ℹ Unsafe fix +30 30 | ' | None') +31 31 | +32 32 | type B = 'Dict' # TCH008 +33 |-type D = 'Foo[str]' # TCH008 + 33 |+type D = Foo[str] # TCH008 +34 34 | type E = 'Foo.bar' # TCH008 +35 35 | type G = 'OptStr' # TCH008 +36 36 | type I = Foo['str'] # TCH008 + +TCH008.py:34:10: TCH008 [*] Remove quotes from type alias + | +32 | type B = 'Dict' # TCH008 +33 | type D = 'Foo[str]' # TCH008 +34 | type E = 'Foo.bar' # TCH008 | ^^^^^^^^^ TCH008 -31 | type G = 'OptStr' # TCH008 -32 | type I = Foo['str'] # TCH008 +35 | type G = 'OptStr' # TCH008 +36 | type I = Foo['str'] # TCH008 | = help: Remove quotes -ℹ Safe fix -27 27 | -28 28 | type B = 'Dict' # TCH008 -29 29 | type D = 'Foo[str]' # TCH008 -30 |-type E = 'Foo.bar' # TCH008 - 30 |+type E = Foo.bar # TCH008 -31 31 | type G = 'OptStr' # TCH008 -32 32 | type I = Foo['str'] # TCH008 -33 33 | type J = 'Baz' # TCH008 - -TCH008.py:31:10: TCH008 [*] Remove quotes from type alias - | -29 | type D = 'Foo[str]' # TCH008 -30 | type E = 'Foo.bar' # TCH008 -31 | type G = 'OptStr' # TCH008 +ℹ Unsafe fix +31 31 | +32 32 | type B = 'Dict' # TCH008 +33 33 | type D = 'Foo[str]' # TCH008 +34 |-type E = 'Foo.bar' # TCH008 + 34 |+type E = Foo.bar # TCH008 +35 35 | type G = 'OptStr' # TCH008 +36 36 | type I = Foo['str'] # TCH008 +37 37 | type J = 'Baz' # TCH008 + +TCH008.py:35:10: TCH008 [*] Remove quotes from type alias + | +33 | type D = 'Foo[str]' # TCH008 +34 | type E = 'Foo.bar' # TCH008 +35 | type G = 'OptStr' # TCH008 | ^^^^^^^^ TCH008 -32 | type I = Foo['str'] # TCH008 -33 | type J = 'Baz' # TCH008 +36 | type I = Foo['str'] # TCH008 +37 | type J = 'Baz' # TCH008 | = help: Remove quotes -ℹ Safe fix -28 28 | type B = 'Dict' # TCH008 -29 29 | type D = 'Foo[str]' # TCH008 -30 30 | type E = 'Foo.bar' # TCH008 -31 |-type G = 'OptStr' # TCH008 - 31 |+type G = OptStr # TCH008 -32 32 | type I = Foo['str'] # TCH008 -33 33 | type J = 'Baz' # TCH008 -34 34 | type K = 'K | None' # TCH008 - -TCH008.py:32:14: TCH008 [*] Remove quotes from type alias - | -30 | type E = 'Foo.bar' # TCH008 -31 | type G = 'OptStr' # TCH008 -32 | type I = Foo['str'] # TCH008 +ℹ Unsafe fix +32 32 | type B = 'Dict' # TCH008 +33 33 | type D = 'Foo[str]' # TCH008 +34 34 | type E = 'Foo.bar' # TCH008 +35 |-type G = 'OptStr' # TCH008 + 35 |+type G = OptStr # TCH008 +36 36 | type I = Foo['str'] # TCH008 +37 37 | type J = 'Baz' # TCH008 +38 38 | type K = 'K | None' # TCH008 + +TCH008.py:36:14: TCH008 [*] Remove quotes from type alias + | +34 | type E = 'Foo.bar' # TCH008 +35 | type G = 'OptStr' # TCH008 +36 | type I = Foo['str'] # TCH008 | ^^^^^ TCH008 -33 | type J = 'Baz' # TCH008 -34 | type K = 'K | None' # TCH008 +37 | type J = 'Baz' # TCH008 +38 | type K = 'K | None' # TCH008 | = help: Remove quotes -ℹ Safe fix -29 29 | type D = 'Foo[str]' # TCH008 -30 30 | type E = 'Foo.bar' # TCH008 -31 31 | type G = 'OptStr' # TCH008 -32 |-type I = Foo['str'] # TCH008 - 32 |+type I = Foo[str] # TCH008 -33 33 | type J = 'Baz' # TCH008 -34 34 | type K = 'K | None' # TCH008 -35 35 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +ℹ Unsafe fix +33 33 | type D = 'Foo[str]' # TCH008 +34 34 | type E = 'Foo.bar' # TCH008 +35 35 | type G = 'OptStr' # TCH008 +36 |-type I = Foo['str'] # TCH008 + 36 |+type I = Foo[str] # TCH008 +37 37 | type J = 'Baz' # TCH008 +38 38 | type K = 'K | None' # TCH008 +39 39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) -TCH008.py:33:10: TCH008 [*] Remove quotes from type alias +TCH008.py:37:10: TCH008 [*] Remove quotes from type alias | -31 | type G = 'OptStr' # TCH008 -32 | type I = Foo['str'] # TCH008 -33 | type J = 'Baz' # TCH008 +35 | type G = 'OptStr' # TCH008 +36 | type I = Foo['str'] # TCH008 +37 | type J = 'Baz' # TCH008 | ^^^^^ TCH008 -34 | type K = 'K | None' # TCH008 -35 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +38 | type K = 'K | None' # TCH008 +39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) | = help: Remove quotes -ℹ Safe fix -30 30 | type E = 'Foo.bar' # TCH008 -31 31 | type G = 'OptStr' # TCH008 -32 32 | type I = Foo['str'] # TCH008 -33 |-type J = 'Baz' # TCH008 - 33 |+type J = Baz # TCH008 -34 34 | type K = 'K | None' # TCH008 -35 35 | type L = 'int' | None # TCH008 (because TC010 is not enabled) -36 36 | +ℹ Unsafe fix +34 34 | type E = 'Foo.bar' # TCH008 +35 35 | type G = 'OptStr' # TCH008 +36 36 | type I = Foo['str'] # TCH008 +37 |-type J = 'Baz' # TCH008 + 37 |+type J = Baz # TCH008 +38 38 | type K = 'K | None' # TCH008 +39 39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +40 40 | type M = ('int' # TCH008 -TCH008.py:34:10: TCH008 [*] Remove quotes from type alias +TCH008.py:38:10: TCH008 [*] Remove quotes from type alias | -32 | type I = Foo['str'] # TCH008 -33 | type J = 'Baz' # TCH008 -34 | type K = 'K | None' # TCH008 +36 | type I = Foo['str'] # TCH008 +37 | type J = 'Baz' # TCH008 +38 | type K = 'K | None' # TCH008 | ^^^^^^^^^^ TCH008 -35 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +40 | type M = ('int' # TCH008 | = help: Remove quotes -ℹ Safe fix -31 31 | type G = 'OptStr' # TCH008 -32 32 | type I = Foo['str'] # TCH008 -33 33 | type J = 'Baz' # TCH008 -34 |-type K = 'K | None' # TCH008 - 34 |+type K = K | None # TCH008 -35 35 | type L = 'int' | None # TCH008 (because TC010 is not enabled) -36 36 | -37 37 | +ℹ Unsafe fix +35 35 | type G = 'OptStr' # TCH008 +36 36 | type I = Foo['str'] # TCH008 +37 37 | type J = 'Baz' # TCH008 +38 |-type K = 'K | None' # TCH008 + 38 |+type K = K | None # TCH008 +39 39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +40 40 | type M = ('int' # TCH008 +41 41 | | None) -TCH008.py:35:10: TCH008 [*] Remove quotes from type alias +TCH008.py:39:10: TCH008 [*] Remove quotes from type alias | -33 | type J = 'Baz' # TCH008 -34 | type K = 'K | None' # TCH008 -35 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +37 | type J = 'Baz' # TCH008 +38 | type K = 'K | None' # TCH008 +39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) | ^^^^^ TCH008 +40 | type M = ('int' # TCH008 +41 | | None) | = help: Remove quotes -ℹ Safe fix -32 32 | type I = Foo['str'] # TCH008 -33 33 | type J = 'Baz' # TCH008 -34 34 | type K = 'K | None' # TCH008 -35 |-type L = 'int' | None # TCH008 (because TC010 is not enabled) - 35 |+type L = int | None # TCH008 (because TC010 is not enabled) -36 36 | -37 37 | -38 38 | class Baz: - -TCH008.py:40:14: TCH008 [*] Remove quotes from type alias - | -38 | class Baz: -39 | a: TypeAlias = 'Baz' # OK -40 | type A = 'Baz' # TCH008 +ℹ Unsafe fix +36 36 | type I = Foo['str'] # TCH008 +37 37 | type J = 'Baz' # TCH008 +38 38 | type K = 'K | None' # TCH008 +39 |-type L = 'int' | None # TCH008 (because TC010 is not enabled) + 39 |+type L = int | None # TCH008 (because TC010 is not enabled) +40 40 | type M = ('int' # TCH008 +41 41 | | None) +42 42 | type N = ('int' # TCH008 (fix removes comment currently) + +TCH008.py:40:11: TCH008 [*] Remove quotes from type alias + | +38 | type K = 'K | None' # TCH008 +39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +40 | type M = ('int' # TCH008 + | ^^^^^ TCH008 +41 | | None) +42 | type N = ('int' # TCH008 (fix removes comment currently) + | + = help: Remove quotes + +ℹ Unsafe fix +37 37 | type J = 'Baz' # TCH008 +38 38 | type K = 'K | None' # TCH008 +39 39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +40 |-type M = ('int' # TCH008 + 40 |+type M = (int # TCH008 +41 41 | | None) +42 42 | type N = ('int' # TCH008 (fix removes comment currently) +43 43 | ' | None') + +TCH008.py:42:11: TCH008 [*] Remove quotes from type alias + | +40 | type M = ('int' # TCH008 +41 | | None) +42 | type N = ('int' # TCH008 (fix removes comment currently) + | ___________^ +43 | | ' | None') + | |_____________^ TCH008 + | + = help: Remove quotes + +ℹ Unsafe fix +39 39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +40 40 | type M = ('int' # TCH008 +41 41 | | None) +42 |-type N = ('int' # TCH008 (fix removes comment currently) +43 |- ' | None') + 42 |+type N = (int | None) +44 43 | +45 44 | +46 45 | class Baz: + +TCH008.py:48:14: TCH008 [*] Remove quotes from type alias + | +46 | class Baz: +47 | a: TypeAlias = 'Baz' # OK +48 | type A = 'Baz' # TCH008 | ^^^^^ TCH008 -41 | -42 | class Nested: +49 | +50 | class Nested: | = help: Remove quotes -ℹ Safe fix -37 37 | -38 38 | class Baz: -39 39 | a: TypeAlias = 'Baz' # OK -40 |- type A = 'Baz' # TCH008 - 40 |+ type A = Baz # TCH008 -41 41 | -42 42 | class Nested: -43 43 | a: TypeAlias = 'Baz' # OK - -TCH008.py:44:18: TCH008 [*] Remove quotes from type alias - | -42 | class Nested: -43 | a: TypeAlias = 'Baz' # OK -44 | type A = 'Baz' # TCH008 +ℹ Unsafe fix +45 45 | +46 46 | class Baz: +47 47 | a: TypeAlias = 'Baz' # OK +48 |- type A = 'Baz' # TCH008 + 48 |+ type A = Baz # TCH008 +49 49 | +50 50 | class Nested: +51 51 | a: TypeAlias = 'Baz' # OK + +TCH008.py:52:18: TCH008 [*] Remove quotes from type alias + | +50 | class Nested: +51 | a: TypeAlias = 'Baz' # OK +52 | type A = 'Baz' # TCH008 | ^^^^^ TCH008 | = help: Remove quotes -ℹ Safe fix -41 41 | -42 42 | class Nested: -43 43 | a: TypeAlias = 'Baz' # OK -44 |- type A = 'Baz' # TCH008 - 44 |+ type A = Baz # TCH008 +ℹ Unsafe fix +49 49 | +50 50 | class Nested: +51 51 | a: TypeAlias = 'Baz' # OK +52 |- type A = 'Baz' # TCH008 + 52 |+ type A = Baz # TCH008 diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tc010_precedence_over_tch008.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tc010_precedence_over_tch008.snap index f99741fd4865d..60eec4e8edb75 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tc010_precedence_over_tch008.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tc010_precedence_over_tch008.snap @@ -11,7 +11,7 @@ source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs | = help: Remove quotes -ℹ Safe fix +ℹ Unsafe fix 3 3 | 4 4 | from typing import TypeAlias 5 5 | diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__unquoted-type-alias_TCH007.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__unquoted-type-alias_TCH007.py.snap index 595233125b22c..1f3ce227c8c68 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__unquoted-type-alias_TCH007.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__unquoted-type-alias_TCH007.py.snap @@ -83,7 +83,7 @@ TCH007.py:18:16: TCH007 [*] Add quotes to type alias 18 |+f: TypeAlias = "Bar" # TCH007 19 19 | g: TypeAlias = Foo | Bar # TCH007 x2 20 20 | h: TypeAlias = Foo[str] # TCH007 -21 21 | +21 21 | i: TypeAlias = (Foo | # TCH007 x2 (fix removes comment currently) TCH007.py:19:16: TCH007 [*] Add quotes to type alias | @@ -92,6 +92,7 @@ TCH007.py:19:16: TCH007 [*] Add quotes to type alias 19 | g: TypeAlias = Foo | Bar # TCH007 x2 | ^^^ TCH007 20 | h: TypeAlias = Foo[str] # TCH007 +21 | i: TypeAlias = (Foo | # TCH007 x2 (fix removes comment currently) | = help: Add quotes @@ -102,8 +103,8 @@ TCH007.py:19:16: TCH007 [*] Add quotes to type alias 19 |-g: TypeAlias = Foo | Bar # TCH007 x2 19 |+g: TypeAlias = "Foo | Bar" # TCH007 x2 20 20 | h: TypeAlias = Foo[str] # TCH007 -21 21 | -22 22 | type C = Foo # OK +21 21 | i: TypeAlias = (Foo | # TCH007 x2 (fix removes comment currently) +22 22 | Bar) TCH007.py:19:22: TCH007 [*] Add quotes to type alias | @@ -112,6 +113,7 @@ TCH007.py:19:22: TCH007 [*] Add quotes to type alias 19 | g: TypeAlias = Foo | Bar # TCH007 x2 | ^^^ TCH007 20 | h: TypeAlias = Foo[str] # TCH007 +21 | i: TypeAlias = (Foo | # TCH007 x2 (fix removes comment currently) | = help: Add quotes @@ -122,8 +124,8 @@ TCH007.py:19:22: TCH007 [*] Add quotes to type alias 19 |-g: TypeAlias = Foo | Bar # TCH007 x2 19 |+g: TypeAlias = "Foo | Bar" # TCH007 x2 20 20 | h: TypeAlias = Foo[str] # TCH007 -21 21 | -22 22 | type C = Foo # OK +21 21 | i: TypeAlias = (Foo | # TCH007 x2 (fix removes comment currently) +22 22 | Bar) TCH007.py:20:16: TCH007 [*] Add quotes to type alias | @@ -131,8 +133,8 @@ TCH007.py:20:16: TCH007 [*] Add quotes to type alias 19 | g: TypeAlias = Foo | Bar # TCH007 x2 20 | h: TypeAlias = Foo[str] # TCH007 | ^^^ TCH007 -21 | -22 | type C = Foo # OK +21 | i: TypeAlias = (Foo | # TCH007 x2 (fix removes comment currently) +22 | Bar) | = help: Add quotes @@ -142,6 +144,49 @@ TCH007.py:20:16: TCH007 [*] Add quotes to type alias 19 19 | g: TypeAlias = Foo | Bar # TCH007 x2 20 |-h: TypeAlias = Foo[str] # TCH007 20 |+h: TypeAlias = "Foo[str]" # TCH007 -21 21 | -22 22 | type C = Foo # OK -23 23 | type D = Foo | None # OK +21 21 | i: TypeAlias = (Foo | # TCH007 x2 (fix removes comment currently) +22 22 | Bar) +23 23 | + +TCH007.py:21:17: TCH007 [*] Add quotes to type alias + | +19 | g: TypeAlias = Foo | Bar # TCH007 x2 +20 | h: TypeAlias = Foo[str] # TCH007 +21 | i: TypeAlias = (Foo | # TCH007 x2 (fix removes comment currently) + | ^^^ TCH007 +22 | Bar) + | + = help: Add quotes + +ℹ Unsafe fix +18 18 | f: TypeAlias = Bar # TCH007 +19 19 | g: TypeAlias = Foo | Bar # TCH007 x2 +20 20 | h: TypeAlias = Foo[str] # TCH007 +21 |-i: TypeAlias = (Foo | # TCH007 x2 (fix removes comment currently) +22 |- Bar) + 21 |+i: TypeAlias = ("Foo | Bar") +23 22 | +24 23 | type C = Foo # OK +25 24 | type D = Foo | None # OK + +TCH007.py:22:5: TCH007 [*] Add quotes to type alias + | +20 | h: TypeAlias = Foo[str] # TCH007 +21 | i: TypeAlias = (Foo | # TCH007 x2 (fix removes comment currently) +22 | Bar) + | ^^^ TCH007 +23 | +24 | type C = Foo # OK + | + = help: Add quotes + +ℹ Unsafe fix +18 18 | f: TypeAlias = Bar # TCH007 +19 19 | g: TypeAlias = Foo | Bar # TCH007 x2 +20 20 | h: TypeAlias = Foo[str] # TCH007 +21 |-i: TypeAlias = (Foo | # TCH007 x2 (fix removes comment currently) +22 |- Bar) + 21 |+i: TypeAlias = ("Foo | Bar") +23 22 | +24 23 | type C = Foo # OK +25 24 | type D = Foo | None # OK From 81bb17290d3cdd5d7529bbc1bc0cdadf7bfaa4f7 Mon Sep 17 00:00:00 2001 From: Daverball Date: Sat, 9 Nov 2024 16:40:29 +0100 Subject: [PATCH 17/24] Fixes errors and only marks fix as unsafe when there are comments --- crates/ruff_linter/src/checkers/ast/mod.rs | 3 +- .../rules/type_alias_quotes.rs | 23 +++++++----- ...g__tests__quoted-type-alias_TCH008.py.snap | 36 +++++++++---------- ...__tests__tc010_precedence_over_tch008.snap | 2 +- 4 files changed, 34 insertions(+), 30 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 2291cbf2f94ce..cacffb4a8868b 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -2299,8 +2299,7 @@ impl<'a> Checker<'a> { flake8_type_checking::rules::quoted_type_alias( self, parsed_expr, - annotation, - range, + string_expr, ); } } diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs index eb2dcef213c66..bec1a2ae11c68 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs @@ -5,7 +5,7 @@ use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast as ast; use ruff_python_ast::{Expr, Stmt}; use ruff_python_semantic::{Binding, SemanticModel}; -use ruff_text_size::{Ranged, TextRange}; +use ruff_text_size::Ranged; use std::borrow::Borrow; use crate::checkers::ast::Checker; @@ -48,7 +48,7 @@ impl Violation for UnquotedTypeAlias { #[derive_message_formats] fn message(&self) -> String { - format!("Add quotes to type alias") + "Add quotes to type alias".to_string() } fn fix_title(&self) -> Option { @@ -106,7 +106,7 @@ pub struct QuotedTypeAlias; impl AlwaysFixableViolation for QuotedTypeAlias { #[derive_message_formats] fn message(&self) -> String { - format!("Remove quotes from type alias") + "Remove quotes from type alias".to_string() } fn fix_title(&self) -> String { @@ -219,8 +219,7 @@ fn collect_typing_references<'a>( pub(crate) fn quoted_type_alias( checker: &mut Checker, expr: &Expr, - annotation: &str, - range: TextRange, + annotation_expr: &ast::ExprStringLiteral, ) { if checker.enabled(Rule::RuntimeStringUnion) { // this should return a TCH010 error instead @@ -240,11 +239,17 @@ pub(crate) fn quoted_type_alias( return; } + let range = annotation_expr.range(); let mut diagnostic = Diagnostic::new(QuotedTypeAlias, range); - diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( - annotation.to_string(), - range, - ))); + let edit = Edit::range_replacement(annotation_expr.value.to_string(), range); + if checker + .comment_ranges() + .has_comments(expr, checker.source()) + { + diagnostic.set_fix(Fix::unsafe_edit(edit)); + } else { + diagnostic.set_fix(Fix::safe_edit(edit)); + } checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap index d368dd45ffdc7..87552bed9bba6 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap @@ -12,7 +12,7 @@ TCH008.py:15:16: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 12 12 | else: 13 13 | Bar = Foo 14 14 | @@ -33,7 +33,7 @@ TCH008.py:17:16: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 14 14 | 15 15 | a: TypeAlias = 'int' # TCH008 16 16 | b: TypeAlias = 'Dict' # OK @@ -54,7 +54,7 @@ TCH008.py:20:16: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 17 17 | c: TypeAlias = 'Foo' # TCH008 18 18 | d: TypeAlias = 'Foo[str]' # OK 19 19 | e: TypeAlias = 'Foo.bar' # OK @@ -75,7 +75,7 @@ TCH008.py:22:16: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 19 19 | e: TypeAlias = 'Foo.bar' # OK 20 20 | f: TypeAlias = 'Foo | None' # TCH008 21 21 | g: TypeAlias = 'OptStr' # OK @@ -96,7 +96,7 @@ TCH008.py:23:20: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 20 20 | f: TypeAlias = 'Foo | None' # TCH008 21 21 | g: TypeAlias = 'OptStr' # OK 22 22 | h: TypeAlias = 'Bar' # TCH008 @@ -117,7 +117,7 @@ TCH008.py:26:16: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 23 23 | i: TypeAlias = Foo['str'] # TCH008 24 24 | j: TypeAlias = 'Baz' # OK 25 25 | k: TypeAlias = 'k | None' # OK @@ -138,7 +138,7 @@ TCH008.py:27:17: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 24 24 | j: TypeAlias = 'Baz' # OK 25 25 | k: TypeAlias = 'k | None' # OK 26 26 | l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) @@ -183,7 +183,7 @@ TCH008.py:32:10: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 29 29 | n: TypeAlias = ('int' # TCH008 (fix removes comment currently) 30 30 | ' | None') 31 31 | @@ -203,7 +203,7 @@ TCH008.py:33:10: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 30 30 | ' | None') 31 31 | 32 32 | type B = 'Dict' # TCH008 @@ -224,7 +224,7 @@ TCH008.py:34:10: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 31 31 | 32 32 | type B = 'Dict' # TCH008 33 33 | type D = 'Foo[str]' # TCH008 @@ -245,7 +245,7 @@ TCH008.py:35:10: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 32 32 | type B = 'Dict' # TCH008 33 33 | type D = 'Foo[str]' # TCH008 34 34 | type E = 'Foo.bar' # TCH008 @@ -266,7 +266,7 @@ TCH008.py:36:14: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 33 33 | type D = 'Foo[str]' # TCH008 34 34 | type E = 'Foo.bar' # TCH008 35 35 | type G = 'OptStr' # TCH008 @@ -287,7 +287,7 @@ TCH008.py:37:10: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 34 34 | type E = 'Foo.bar' # TCH008 35 35 | type G = 'OptStr' # TCH008 36 36 | type I = Foo['str'] # TCH008 @@ -308,7 +308,7 @@ TCH008.py:38:10: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 35 35 | type G = 'OptStr' # TCH008 36 36 | type I = Foo['str'] # TCH008 37 37 | type J = 'Baz' # TCH008 @@ -329,7 +329,7 @@ TCH008.py:39:10: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 36 36 | type I = Foo['str'] # TCH008 37 37 | type J = 'Baz' # TCH008 38 38 | type K = 'K | None' # TCH008 @@ -350,7 +350,7 @@ TCH008.py:40:11: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 37 37 | type J = 'Baz' # TCH008 38 38 | type K = 'K | None' # TCH008 39 39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) @@ -393,7 +393,7 @@ TCH008.py:48:14: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 45 45 | 46 46 | class Baz: 47 47 | a: TypeAlias = 'Baz' # OK @@ -412,7 +412,7 @@ TCH008.py:52:18: TCH008 [*] Remove quotes from type alias | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 49 49 | 50 50 | class Nested: 51 51 | a: TypeAlias = 'Baz' # OK diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tc010_precedence_over_tch008.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tc010_precedence_over_tch008.snap index 60eec4e8edb75..f99741fd4865d 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tc010_precedence_over_tch008.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__tc010_precedence_over_tch008.snap @@ -11,7 +11,7 @@ source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs | = help: Remove quotes -ℹ Unsafe fix +ℹ Safe fix 3 3 | 4 4 | from typing import TypeAlias 5 5 | From 2555f8af382daa194affc59857049ad09d0cb22a Mon Sep 17 00:00:00 2001 From: Daverball Date: Sat, 9 Nov 2024 16:45:58 +0100 Subject: [PATCH 18/24] Removes unnecessary `return` statement --- .../src/rules/flake8_type_checking/rules/type_alias_quotes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs index bec1a2ae11c68..5775360d01a6d 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs @@ -152,7 +152,7 @@ pub(crate) fn unquoted_type_alias(checker: &Checker, binding: &Binding) -> Optio diagnostic.set_fix(Fix::unsafe_edit(edit.clone())); diagnostics.push(diagnostic); } - return Some(diagnostics); + Some(diagnostics) } /// Traverses the type expression and collects `[Expr::Name]` nodes that are From 6799411dffcc3de689205f0e18f55e0f435e3380 Mon Sep 17 00:00:00 2001 From: Daverball Date: Thu, 14 Nov 2024 10:22:48 +0100 Subject: [PATCH 19/24] Fixes new semantic flag values --- crates/ruff_python_semantic/src/model.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 7733212a0132b..a806bb060553d 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -2321,12 +2321,12 @@ bitflags! { /// The model is in the value expression of a [PEP 613] explicit type alias. /// /// [PEP 613]: https://peps.python.org/pep-0613/ - const EXPLICIT_TYPE_ALIAS = 1 << 27; + const EXPLICIT_TYPE_ALIAS = 1 << 26; /// The model is in the value expression of a [PEP 695] type statement /// /// [PEP 695]: https://peps.python.org/pep-0695/#generic-type-alias - const GENERIC_TYPE_ALIAS = 1 << 28; + const GENERIC_TYPE_ALIAS = 1 << 27; /// The context is in any type annotation. const ANNOTATION = Self::TYPING_ONLY_ANNOTATION.bits() | Self::RUNTIME_EVALUATED_ANNOTATION.bits() | Self::RUNTIME_REQUIRED_ANNOTATION.bits(); From bfffb7badf4382140c125a6d34ec4764044c4140 Mon Sep 17 00:00:00 2001 From: Daverball Date: Sat, 16 Nov 2024 14:44:21 +0100 Subject: [PATCH 20/24] `quote_type_annotation` is once again no longer guaranteed to work --- .../src/rules/flake8_type_checking/helpers.rs | 9 ++++++--- .../flake8_type_checking/rules/type_alias_quotes.rs | 6 ++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs index 0c75213f009ea..d94403e2d9b0c 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs @@ -269,7 +269,7 @@ pub(crate) fn quote_annotation( } } - Ok(quote_type_expression(expr, semantic, stylist)) + quote_type_expression(expr, semantic, stylist) } /// Wrap a type expression in quotes. @@ -281,14 +281,17 @@ pub(crate) fn quote_type_expression( expr: &Expr, semantic: &SemanticModel, stylist: &Stylist, -) -> Edit { +) -> Result { // Quote the entire expression. let quote = stylist.quote(); let mut quote_annotator = QuoteAnnotator::new(semantic, stylist); quote_annotator.visit_expr(expr); let annotation = quote_annotator.into_annotation()?; - Edit::range_replacement(format!("{quote}{annotation}{quote}"), expr.range()) + Ok(Edit::range_replacement( + format!("{quote}{annotation}{quote}"), + expr.range(), + )) } /// Filter out any [`Edit`]s that are completely contained by any other [`Edit`]. diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs index 5775360d01a6d..cdecf09d75e27 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs @@ -144,12 +144,14 @@ pub(crate) fn unquoted_type_alias(checker: &Checker, binding: &Binding) -> Optio // Eventually we may try to be more clever and come up with the // minimal set of subexpressions that need to be quoted. let parent = expr.range().start(); - let edit = quote_type_expression(expr, checker.semantic(), checker.stylist()); + let edit = quote_type_expression(expr, checker.semantic(), checker.stylist()).ok(); let mut diagnostics = Vec::with_capacity(names.len()); for name in names { let mut diagnostic = Diagnostic::new(UnquotedTypeAlias, name.range()); diagnostic.set_parent(parent); - diagnostic.set_fix(Fix::unsafe_edit(edit.clone())); + if let Some(edit) = edit.as_ref() { + diagnostic.set_fix(Fix::unsafe_edit(edit.clone())); + } diagnostics.push(diagnostic); } Some(diagnostics) From fd051a86893ff13474e654e93114579cd6783c0a Mon Sep 17 00:00:00 2001 From: Daverball Date: Tue, 19 Nov 2024 15:58:45 +0100 Subject: [PATCH 21/24] Addressed some of the feedback from the code review --- crates/ruff_linter/src/checkers/ast/mod.rs | 3 +++ .../src/rules/flake8_type_checking/helpers.rs | 3 +++ .../rules/type_alias_quotes.rs | 22 ++++++++++++++---- crates/ruff_python_semantic/src/binding.rs | 18 +++++---------- crates/ruff_python_semantic/src/model.rs | 23 ++++++++++++++----- 5 files changed, 46 insertions(+), 23 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 615be60f2c664..f3c8a0032d950 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -1855,6 +1855,9 @@ impl<'a> Checker<'a> { /// [PEP 695]: https://peps.python.org/pep-0695/#generic-type-alias fn visit_generic_type_alias(&mut self, expr: &'a Expr) { let snapshot = self.semantic.flags; + // even though we don't visit these nodes immediately we need to + // modify the semantic flags before we push the expression and its + // corresponding semantic snapshot self.semantic.flags |= SemanticModelFlags::GENERIC_TYPE_ALIAS; self.visit .type_param_definitions diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs index d94403e2d9b0c..e0e0aed35b83f 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs @@ -277,6 +277,9 @@ pub(crate) fn quote_annotation( /// This function assumes that the callee already expanded expression components /// to the minimum acceptable range for quoting, i.e. the parent node may not be /// a [`Expr::Subscript`], [`Expr::Attribute`], `[Expr::Call]` or `[Expr::BinOp]`. +/// +/// In most cases you want to call [`quote_annotation`] instead, which provides +/// that guarantee by expanding the expression before calling into this function. pub(crate) fn quote_type_expression( expr: &Expr, semantic: &SemanticModel, diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs index cdecf09d75e27..ce47096f4c810 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs @@ -5,8 +5,8 @@ use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast as ast; use ruff_python_ast::{Expr, Stmt}; use ruff_python_semantic::{Binding, SemanticModel}; +use ruff_python_stdlib::typing::{is_pep_593_generic_type, is_standard_library_literal}; use ruff_text_size::Ranged; -use std::borrow::Borrow; use crate::checkers::ast::Checker; use crate::rules::flake8_type_checking::helpers::quote_type_expression; @@ -16,7 +16,7 @@ use crate::rules::flake8_type_checking::helpers::quote_type_expression; /// symbols that are not available at runtime. /// /// ## Why is this bad? -/// We will get a `NameError` at runtime. +/// Referencing type-checking only symbols results in a `NameError` at runtime. /// /// ## Example /// ```python @@ -180,11 +180,23 @@ fn collect_typing_references<'a>( } Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { collect_typing_references(checker, value, names); - if let Expr::Name(ast::ExprName { id, .. }) = value.borrow() { - if id.as_str() != "Literal" { - collect_typing_references(checker, slice, names); + if let Some(qualified_name) = checker.semantic().resolve_qualified_name(value) { + if is_standard_library_literal(qualified_name.segments()) { + return; + } + if is_pep_593_generic_type(qualified_name.segments()) { + // First argument is a type (including forward references); the + // rest are arbitrary Python objects. + if let Expr::Tuple(ast::ExprTuple { elts, .. }) = slice.as_ref() { + let mut iter = elts.iter(); + if let Some(expr) = iter.next() { + collect_typing_references(checker, expr, names); + } + } + return; } } + collect_typing_references(checker, slice, names); } Expr::List(ast::ExprList { elts, .. }) | Expr::Tuple(ast::ExprTuple { elts, .. }) => { for elt in elts { diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index 371ed1cdaf89c..909ddad0e4722 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -135,12 +135,16 @@ impl<'a> Binding<'a> { self.flags.contains(BindingFlags::IN_EXCEPT_HANDLER) } - /// Return `true` if this [`Binding`] represents an explicit type alias + /// Return `true` if this [`Binding`] represents a [PEP 613] explicit type alias + /// + /// [PEP 613]: https://peps.python.org/pep-0613/ pub const fn is_explicit_type_alias(&self) -> bool { self.flags.intersects(BindingFlags::EXPLICIT_TYPE_ALIAS) } - /// Return `true` if this [`Binding`] represents a generic type alias + /// Return `true` if this [`Binding`] represents a [PEP 695] generic type alias + /// + /// [PEP 695]: https://peps.python.org/pep-0695/#generic-type-alias pub const fn is_generic_type_alias(&self) -> bool { self.flags.intersects(BindingFlags::GENERIC_TYPE_ALIAS) } @@ -263,16 +267,6 @@ impl<'a> Binding<'a> { }) } - /// Returns the statement range for bindings created by `[ast::StmtAnnAssign]` - /// or `[ast::StmtClassDef]` and `[Binding.range]` otherwise. - pub fn defn_range(&self, semantic: &SemanticModel) -> TextRange { - match self.statement(semantic) { - Some(Stmt::AnnAssign(stmt)) => stmt.range(), - Some(Stmt::ClassDef(stmt)) => stmt.range(), - _ => self.range, - } - } - pub fn as_any_import(&self) -> Option> { match &self.kind { BindingKind::Import(import) => Some(AnyImport::Import(import)), diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index a6ff8dc10a563..cdaded3c9120e 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -700,6 +700,7 @@ impl<'a> SemanticModel<'a> { } class_variables_visible = scope.kind.is_type() && index == 0; + seen_function |= scope.kind.is_function(); if let Some(binding_id) = scope.get(symbol) { if lexicographical_lookup { @@ -719,7 +720,16 @@ impl<'a> SemanticModel<'a> { continue; } - if binding.defn_range(self).ordering(range).is_lt() { + // This ensures we perform the correct lexicographical lookup + // since the ranges for these two types of bindings are trimmed + // but the name is not available until the end of the statement + let binding_range = match binding.statement(self) { + Some(Stmt::AnnAssign(stmt)) => stmt.range(), + Some(Stmt::ClassDef(stmt)) => stmt.range(), + _ => binding.range, + }; + + if binding_range.ordering(range).is_lt() { return Some(shadowed_id); } } @@ -745,9 +755,6 @@ impl<'a> SemanticModel<'a> { return None; } } - - // FIXME: Shouldn't this happen above where `class_variables_visible` is set? - seen_function |= scope.kind.is_function(); } None @@ -1745,13 +1752,17 @@ impl<'a> SemanticModel<'a> { || (self.in_future_type_definition() && self.in_typing_only_annotation()) } - /// Return `true` if the model is in an explicit type alias + /// Return `true` if the model is in a [PEP 613] explicit type alias + /// + /// [PEP 613]: https://peps.python.org/pep-0613/ pub const fn in_explicit_type_alias(&self) -> bool { self.flags .intersects(SemanticModelFlags::EXPLICIT_TYPE_ALIAS) } - /// Return `true` if the model is in a generic type alias + /// Return `true` if the model is in a [PEP 695] generic type alias + /// + /// [PEP 695]: https://peps.python.org/pep-0695/#generic-type-alias pub const fn in_generic_type_alias(&self) -> bool { self.flags .intersects(SemanticModelFlags::GENERIC_TYPE_ALIAS) From 0fc0d4b528852925f8604d4b537c573c71cfc3f6 Mon Sep 17 00:00:00 2001 From: Daverball Date: Tue, 19 Nov 2024 16:53:00 +0100 Subject: [PATCH 22/24] Avoids changing the behavior of `TCH004` for now. Ensures there's no circularity issues between `TCH007` and `TCH008` by always running test cases with both rules enabled. --- .../src/rules/flake8_type_checking/mod.rs | 21 ++++++++++++++++-- .../runtime_import_in_type_checking_block.rs | 12 +++++++--- ...mport-in-type-checking-block_quote.py.snap | 22 ++++++++++++------- 3 files changed, 42 insertions(+), 13 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs index 358537789736b..27cb4812e4740 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs @@ -13,6 +13,7 @@ mod tests { use test_case::test_case; use crate::registry::{Linter, Rule}; + use crate::settings::types::PreviewMode; use crate::test::{test_path, test_snippet}; use crate::{assert_messages, settings}; @@ -35,8 +36,6 @@ mod tests { #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_8.py"))] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_9.py"))] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("quote.py"))] - #[test_case(Rule::UnquotedTypeAlias, Path::new("TCH007.py"))] - #[test_case(Rule::QuotedTypeAlias, Path::new("TCH008.py"))] #[test_case(Rule::RuntimeStringUnion, Path::new("TCH010_1.py"))] #[test_case(Rule::RuntimeStringUnion, Path::new("TCH010_2.py"))] #[test_case(Rule::TypingOnlyFirstPartyImport, Path::new("TCH001.py"))] @@ -64,6 +63,24 @@ mod tests { Ok(()) } + #[test_case(Rule::UnquotedTypeAlias, Path::new("TCH007.py"))] + #[test_case(Rule::QuotedTypeAlias, Path::new("TCH008.py"))] + fn type_alias_rules(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy()); + let diagnostics = test_path( + Path::new("flake8_type_checking").join(path).as_path(), + &settings::LinterSettings { + preview: PreviewMode::Enabled, + ..settings::LinterSettings::for_rules(vec![ + Rule::UnquotedTypeAlias, + Rule::QuotedTypeAlias, + ]) + }, + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } + #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("quote.py"))] #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("quote.py"))] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("quote2.py"))] diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs index 142bb9c079685..e18fd368e0207 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs @@ -154,9 +154,15 @@ pub(crate) fn runtime_import_in_type_checking_block( if checker.settings.flake8_type_checking.quote_annotations && binding.references().all(|reference_id| { let reference = checker.semantic().reference(reference_id); - reference.in_typing_context() - || reference.in_runtime_evaluated_annotation() - || reference.in_explicit_type_alias() + reference.in_typing_context() || reference.in_runtime_evaluated_annotation() + // TODO: We should check `reference.in_explicit_type_alias()` + // as well to match the behavior of the flake8 plugin + // although maybe the best way forward is to add an + // additional setting to configure whether quoting + // or moving the import is preferred for type aliases + // since some people will consistently use their + // type aliases at runtimes, while others won't, so + // the best solution is unclear. }) { actions diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_runtime-import-in-type-checking-block_quote.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_runtime-import-in-type-checking-block_quote.py.snap index 11a589b23aeed..7ee0fbdfa2246 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_runtime-import-in-type-checking-block_quote.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_runtime-import-in-type-checking-block_quote.py.snap @@ -1,6 +1,5 @@ --- source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs -snapshot_kind: text --- quote.py:57:28: TCH004 [*] Quote references to `pandas.DataFrame`. Import is in a type-checking block. | @@ -22,7 +21,7 @@ quote.py:57:28: TCH004 [*] Quote references to `pandas.DataFrame`. Import is in 61 61 | 62 62 | -quote.py:110:28: TCH004 [*] Quote references to `pandas.DataFrame`. Import is in a type-checking block. +quote.py:110:28: TCH004 [*] Move import `pandas.DataFrame` out of type-checking block. Import is used for more than type hinting. | 109 | if TYPE_CHECKING: 110 | from pandas import DataFrame @@ -30,11 +29,18 @@ quote.py:110:28: TCH004 [*] Quote references to `pandas.DataFrame`. Import is in 111 | 112 | x: TypeAlias = DataFrame | None | - = help: Quote references + = help: Move out of type-checking block ℹ Unsafe fix -109 109 | if TYPE_CHECKING: -110 110 | from pandas import DataFrame -111 111 | -112 |- x: TypeAlias = DataFrame | None - 112 |+ x: TypeAlias = "DataFrame | None" + 1 |+from pandas import DataFrame +1 2 | def f(): +2 3 | from pandas import DataFrame +3 4 | +-------------------------------------------------------------------------------- +107 108 | from typing import TypeAlias, TYPE_CHECKING +108 109 | +109 110 | if TYPE_CHECKING: +110 |- from pandas import DataFrame + 111 |+ pass +111 112 | +112 113 | x: TypeAlias = DataFrame | None From f1ec697b9c10886b64975b693bb2786e28565d3d Mon Sep 17 00:00:00 2001 From: David Salvisberg Date: Wed, 20 Nov 2024 08:20:05 +0100 Subject: [PATCH 23/24] Apply suggestions from code review Co-authored-by: Carl Meyer --- .../resources/test/fixtures/flake8_type_checking/TCH008.py | 2 +- crates/ruff_python_semantic/src/model.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py index 45df895e67ded..661a41ce34efd 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py @@ -36,7 +36,7 @@ type I = Foo['str'] # TCH008 type J = 'Baz' # TCH008 type K = 'K | None' # TCH008 -type L = 'int' | None # TCH008 (because TC010 is not enabled) +type L = 'int' | None # TCH008 (because TCH010 is not enabled) type M = ('int' # TCH008 | None) type N = ('int' # TCH008 (fix removes comment currently) diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index cdaded3c9120e..67d66dc4dfb6c 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -720,9 +720,9 @@ impl<'a> SemanticModel<'a> { continue; } - // This ensures we perform the correct lexicographical lookup - // since the ranges for these two types of bindings are trimmed - // but the name is not available until the end of the statement + // This ensures we perform the correct source-order lookup, + // since the ranges for these two types of bindings are trimmed to just the target, + // but the name is not available until the end of the entire statement let binding_range = match binding.statement(self) { Some(Stmt::AnnAssign(stmt)) => stmt.range(), Some(Stmt::ClassDef(stmt)) => stmt.range(), From fd8c04508c3aa4c4db0f44de76d6b18b29e3e96e Mon Sep 17 00:00:00 2001 From: Daverball Date: Wed, 20 Nov 2024 08:34:56 +0100 Subject: [PATCH 24/24] Fixes snapshot and applies a couple of other suggestions --- .../fixtures/flake8_type_checking/TCH008.py | 2 +- ...g__tests__quoted-type-alias_TCH008.py.snap | 36 +++++++++---------- crates/ruff_python_semantic/src/model.rs | 17 ++++----- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py index 661a41ce34efd..2225a99dd3f0c 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH008.py @@ -23,7 +23,7 @@ i: TypeAlias = Foo['str'] # TCH008 j: TypeAlias = 'Baz' # OK k: TypeAlias = 'k | None' # OK -l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) +l: TypeAlias = 'int' | None # TCH008 (because TCH010 is not enabled) m: TypeAlias = ('int' # TCH008 | None) n: TypeAlias = ('int' # TCH008 (fix removes comment currently) diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap index 87552bed9bba6..48d068868d875 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quoted-type-alias_TCH008.py.snap @@ -104,13 +104,13 @@ TCH008.py:23:20: TCH008 [*] Remove quotes from type alias 23 |+i: TypeAlias = Foo[str] # TCH008 24 24 | j: TypeAlias = 'Baz' # OK 25 25 | k: TypeAlias = 'k | None' # OK -26 26 | l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) +26 26 | l: TypeAlias = 'int' | None # TCH008 (because TCH010 is not enabled) TCH008.py:26:16: TCH008 [*] Remove quotes from type alias | 24 | j: TypeAlias = 'Baz' # OK 25 | k: TypeAlias = 'k | None' # OK -26 | l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) +26 | l: TypeAlias = 'int' | None # TCH008 (because TCH010 is not enabled) | ^^^^^ TCH008 27 | m: TypeAlias = ('int' # TCH008 28 | | None) @@ -121,8 +121,8 @@ TCH008.py:26:16: TCH008 [*] Remove quotes from type alias 23 23 | i: TypeAlias = Foo['str'] # TCH008 24 24 | j: TypeAlias = 'Baz' # OK 25 25 | k: TypeAlias = 'k | None' # OK -26 |-l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) - 26 |+l: TypeAlias = int | None # TCH008 (because TC010 is not enabled) +26 |-l: TypeAlias = 'int' | None # TCH008 (because TCH010 is not enabled) + 26 |+l: TypeAlias = int | None # TCH008 (because TCH010 is not enabled) 27 27 | m: TypeAlias = ('int' # TCH008 28 28 | | None) 29 29 | n: TypeAlias = ('int' # TCH008 (fix removes comment currently) @@ -130,7 +130,7 @@ TCH008.py:26:16: TCH008 [*] Remove quotes from type alias TCH008.py:27:17: TCH008 [*] Remove quotes from type alias | 25 | k: TypeAlias = 'k | None' # OK -26 | l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) +26 | l: TypeAlias = 'int' | None # TCH008 (because TCH010 is not enabled) 27 | m: TypeAlias = ('int' # TCH008 | ^^^^^ TCH008 28 | | None) @@ -141,7 +141,7 @@ TCH008.py:27:17: TCH008 [*] Remove quotes from type alias ℹ Safe fix 24 24 | j: TypeAlias = 'Baz' # OK 25 25 | k: TypeAlias = 'k | None' # OK -26 26 | l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) +26 26 | l: TypeAlias = 'int' | None # TCH008 (because TCH010 is not enabled) 27 |-m: TypeAlias = ('int' # TCH008 27 |+m: TypeAlias = (int # TCH008 28 28 | | None) @@ -162,7 +162,7 @@ TCH008.py:29:17: TCH008 [*] Remove quotes from type alias = help: Remove quotes ℹ Unsafe fix -26 26 | l: TypeAlias = 'int' | None # TCH008 (because TC010 is not enabled) +26 26 | l: TypeAlias = 'int' | None # TCH008 (because TCH010 is not enabled) 27 27 | m: TypeAlias = ('int' # TCH008 28 28 | | None) 29 |-n: TypeAlias = ('int' # TCH008 (fix removes comment currently) @@ -274,7 +274,7 @@ TCH008.py:36:14: TCH008 [*] Remove quotes from type alias 36 |+type I = Foo[str] # TCH008 37 37 | type J = 'Baz' # TCH008 38 38 | type K = 'K | None' # TCH008 -39 39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +39 39 | type L = 'int' | None # TCH008 (because TCH010 is not enabled) TCH008.py:37:10: TCH008 [*] Remove quotes from type alias | @@ -283,7 +283,7 @@ TCH008.py:37:10: TCH008 [*] Remove quotes from type alias 37 | type J = 'Baz' # TCH008 | ^^^^^ TCH008 38 | type K = 'K | None' # TCH008 -39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +39 | type L = 'int' | None # TCH008 (because TCH010 is not enabled) | = help: Remove quotes @@ -294,7 +294,7 @@ TCH008.py:37:10: TCH008 [*] Remove quotes from type alias 37 |-type J = 'Baz' # TCH008 37 |+type J = Baz # TCH008 38 38 | type K = 'K | None' # TCH008 -39 39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +39 39 | type L = 'int' | None # TCH008 (because TCH010 is not enabled) 40 40 | type M = ('int' # TCH008 TCH008.py:38:10: TCH008 [*] Remove quotes from type alias @@ -303,7 +303,7 @@ TCH008.py:38:10: TCH008 [*] Remove quotes from type alias 37 | type J = 'Baz' # TCH008 38 | type K = 'K | None' # TCH008 | ^^^^^^^^^^ TCH008 -39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +39 | type L = 'int' | None # TCH008 (because TCH010 is not enabled) 40 | type M = ('int' # TCH008 | = help: Remove quotes @@ -314,7 +314,7 @@ TCH008.py:38:10: TCH008 [*] Remove quotes from type alias 37 37 | type J = 'Baz' # TCH008 38 |-type K = 'K | None' # TCH008 38 |+type K = K | None # TCH008 -39 39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +39 39 | type L = 'int' | None # TCH008 (because TCH010 is not enabled) 40 40 | type M = ('int' # TCH008 41 41 | | None) @@ -322,7 +322,7 @@ TCH008.py:39:10: TCH008 [*] Remove quotes from type alias | 37 | type J = 'Baz' # TCH008 38 | type K = 'K | None' # TCH008 -39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +39 | type L = 'int' | None # TCH008 (because TCH010 is not enabled) | ^^^^^ TCH008 40 | type M = ('int' # TCH008 41 | | None) @@ -333,8 +333,8 @@ TCH008.py:39:10: TCH008 [*] Remove quotes from type alias 36 36 | type I = Foo['str'] # TCH008 37 37 | type J = 'Baz' # TCH008 38 38 | type K = 'K | None' # TCH008 -39 |-type L = 'int' | None # TCH008 (because TC010 is not enabled) - 39 |+type L = int | None # TCH008 (because TC010 is not enabled) +39 |-type L = 'int' | None # TCH008 (because TCH010 is not enabled) + 39 |+type L = int | None # TCH008 (because TCH010 is not enabled) 40 40 | type M = ('int' # TCH008 41 41 | | None) 42 42 | type N = ('int' # TCH008 (fix removes comment currently) @@ -342,7 +342,7 @@ TCH008.py:39:10: TCH008 [*] Remove quotes from type alias TCH008.py:40:11: TCH008 [*] Remove quotes from type alias | 38 | type K = 'K | None' # TCH008 -39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +39 | type L = 'int' | None # TCH008 (because TCH010 is not enabled) 40 | type M = ('int' # TCH008 | ^^^^^ TCH008 41 | | None) @@ -353,7 +353,7 @@ TCH008.py:40:11: TCH008 [*] Remove quotes from type alias ℹ Safe fix 37 37 | type J = 'Baz' # TCH008 38 38 | type K = 'K | None' # TCH008 -39 39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +39 39 | type L = 'int' | None # TCH008 (because TCH010 is not enabled) 40 |-type M = ('int' # TCH008 40 |+type M = (int # TCH008 41 41 | | None) @@ -372,7 +372,7 @@ TCH008.py:42:11: TCH008 [*] Remove quotes from type alias = help: Remove quotes ℹ Unsafe fix -39 39 | type L = 'int' | None # TCH008 (because TC010 is not enabled) +39 39 | type L = 'int' | None # TCH008 (because TCH010 is not enabled) 40 40 | type M = ('int' # TCH008 41 41 | | None) 42 |-type N = ('int' # TCH008 (fix removes comment currently) diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 67d66dc4dfb6c..634fbf3523f0a 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -677,17 +677,17 @@ impl<'a> SemanticModel<'a> { let range = name.range; let mut seen_function = false; let mut class_variables_visible = true; - let mut lexicographical_lookup = true; + let mut source_order_sensitive_lookup = true; for (index, scope_id) in self.scopes.ancestor_ids(self.scope_id).enumerate() { let scope = &self.scopes[scope_id]; // Only once we leave a function scope and its enclosing type scope should - // we stop doing lexicographical lookups. We could e.g. have nested classes + // we stop doing source-order lookups. We could e.g. have nested classes // where we lookup symbols from the innermost class scope, which can only see // things from the outer class(es) that have been defined before the inner // class. if seen_function && !scope.kind.is_type() { - lexicographical_lookup = false; + source_order_sensitive_lookup = false; } if scope.kind.is_class() { @@ -703,10 +703,10 @@ impl<'a> SemanticModel<'a> { seen_function |= scope.kind.is_function(); if let Some(binding_id) = scope.get(symbol) { - if lexicographical_lookup { + if source_order_sensitive_lookup { // we need to look through all the shadowed bindings - // since we may be shadowing a valid runtime binding - // with an invalid one + // since we may be shadowing a source-order accurate + // runtime binding with a source-order inaccurate one for shadowed_id in scope.shadowed_bindings(binding_id) { let binding = &self.bindings[shadowed_id]; if binding.context.is_typing() { @@ -721,8 +721,9 @@ impl<'a> SemanticModel<'a> { } // This ensures we perform the correct source-order lookup, - // since the ranges for these two types of bindings are trimmed to just the target, - // but the name is not available until the end of the entire statement + // since the ranges for these two types of bindings are trimmed + // to just the target, but the name is not available until the + // end of the entire statement let binding_range = match binding.statement(self) { Some(Stmt::AnnAssign(stmt)) => stmt.range(), Some(Stmt::ClassDef(stmt)) => stmt.range(),