Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[flake8-type-checking] Apply TC008 more eagerly in TYPE_CHECKING blocks and disapply it in stubs #15180

Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
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]

a: TypeAlias = 'int' # TC008
b: TypeAlias = 'Dict' # TC008
c: TypeAlias = 'Foo' # TC008
d: TypeAlias = 'Foo[str]' # TC008
e: TypeAlias = 'Foo.bar' # TC008
f: TypeAlias = 'Foo | None' # TC008
g: TypeAlias = 'OptStr' # TC008
h: TypeAlias = 'Bar' # TC008
i: TypeAlias = Foo['str'] # TC008
j: TypeAlias = 'Baz' # OK (this would be treated as use before define)
k: TypeAlias = 'k | None' # False negative in type checking block
l: TypeAlias = 'int' | None # TC008 (because TC010 is not enabled)
m: TypeAlias = ('int' # TC008
| None)
n: TypeAlias = ('int' # TC008 (fix removes comment currently)
' | None')


class Baz: ...
3 changes: 2 additions & 1 deletion crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2324,7 +2324,8 @@ impl<'a> Checker<'a> {
let parsed_expr = parsed_annotation.expression();
self.visit_expr(parsed_expr);
if self.semantic.in_type_alias_value() {
if self.enabled(Rule::QuotedTypeAlias) {
// stub files are covered by PYI020
if !self.source_type.is_stub() && self.enabled(Rule::QuotedTypeAlias) {
Daverball marked this conversation as resolved.
Show resolved Hide resolved
flake8_type_checking::rules::quoted_type_alias(
self,
parsed_expr,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ mod tests {
// so we want to make sure their fixes are not going around in circles.
#[test_case(Rule::UnquotedTypeAlias, Path::new("TC007.py"))]
#[test_case(Rule::QuotedTypeAlias, Path::new("TC008.py"))]
#[test_case(Rule::QuotedTypeAlias, Path::new("TC008_typing_execution_context.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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix, FixAvailab
use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast as ast;
use ruff_python_ast::{Expr, Stmt};
use ruff_python_semantic::{Binding, SemanticModel};
use ruff_python_semantic::{Binding, SemanticModel, TypingOnlyBindingsStatus};
use ruff_python_stdlib::typing::{is_pep_593_generic_type, is_standard_library_literal};
use ruff_text_size::Ranged;

Expand Down Expand Up @@ -68,14 +68,20 @@ impl Violation for UnquotedTypeAlias {
///
/// ## 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
/// in runtime libraries making use of type hints. They can also have 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.
/// PEP-613 type aliases are only flagged by the rule if Ruff can have high
/// confidence that the quotes are unnecessary. Specifically, any PEP-613
/// type alias where the type expression on the right-hand side contains
/// subscripts or attribute accesses will not be flagged. This is because
/// type aliases can reference types that are, for example, generic in stub
/// files but not at runtime. That can mean that a type checker expects the
/// referenced type to be subscripted with type arguments despite the fact
/// that doing so would fail at runtime if the type alias value was not
/// quoted. Similarly, a type alias might need to reference a module-level
/// attribute that exists in a stub file but not at runtime, meaning that
/// the type alias value would need to be quoted to avoid a runtime error.
///
/// ## Example
/// Given:
Expand All @@ -101,6 +107,15 @@ impl Violation for UnquotedTypeAlias {
/// ## Fix safety
/// This rule's fix is marked as safe, unless the type annotation contains comments.
///
/// ## See also
/// This rule only applies to type aliases in non-stub files. For removing quotes in other
/// contexts or in stub files, see:
///
/// - [`quoted-annotation-in-stub`](quoted-annotation-in-stub.md): A rule that
/// removes all quoted annotations from stub files
/// - [`quoted-annotation`](quoted-annotation.md): A rule that removes unnecessary quotes
/// from *annotations* in runtime files.
///
/// ## References
/// - [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)
Expand Down Expand Up @@ -219,7 +234,11 @@ fn collect_typing_references<'a>(
let Some(binding_id) = checker.semantic().resolve_name(name) else {
return;
};
if checker.semantic().simulate_runtime_load(name).is_some() {
if checker
.semantic()
.simulate_runtime_load(name, TypingOnlyBindingsStatus::Disallowed)
.is_some()
{
return;
}

Expand Down Expand Up @@ -291,11 +310,22 @@ fn quotes_are_unremovable(semantic: &SemanticModel, expr: &Expr) -> bool {
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::Subscript(ast::ExprSubscript { value, slice, .. }) => {
// for subscripts 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.
if !semantic.in_type_checking_block() {
return true;
}
quotes_are_unremovable(semantic, value) || quotes_are_unremovable(semantic, slice)
}
Expr::Attribute(ast::ExprAttribute { value, .. }) => {
// for attributes we also don't know whether it's safe
if !semantic.in_type_checking_block() {
return true;
}
quotes_are_unremovable(semantic, value)
}
Expr::List(ast::ExprList { elts, .. }) | Expr::Tuple(ast::ExprTuple { elts, .. }) => {
for elt in elts {
if quotes_are_unremovable(semantic, elt) {
Expand All @@ -305,7 +335,10 @@ fn quotes_are_unremovable(semantic: &SemanticModel, expr: &Expr) -> bool {
false
}
Expr::Name(name) => {
semantic.resolve_name(name).is_some() && semantic.simulate_runtime_load(name).is_none()
semantic.resolve_name(name).is_some()
&& semantic
.simulate_runtime_load(name, semantic.in_type_checking_block().into())
.is_none()
}
_ => false,
}
Expand Down
Loading
Loading