Skip to content

Commit

Permalink
[flake8-type-checking] Avoid false positives for | in TC008
Browse files Browse the repository at this point in the history
  • Loading branch information
Daverball committed Jan 8, 2025
1 parent 339167d commit 89d35c2
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from __future__ import annotations

from typing import Annotated, TypeAlias, TYPE_CHECKING

a: TypeAlias = 'int | None' # OK
b: TypeAlias = 'Annotated[int, 1 | 2]' # False negative in runtime context

if TYPE_CHECKING:
c: TypeAlias = 'int | None' # OK
d: TypeAlias = 'Annotated[int, 1 | 2]' # TC008
e: TypeAlias = 'Annotated[int, 1 + 2]' # TC008
18 changes: 18 additions & 0 deletions crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,24 @@ mod tests {
Ok(())
}

#[test_case(Rule::QuotedTypeAlias, Path::new("TC008_union_syntax_pre_py310.py"))]
fn type_alias_rules_pre_py310(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"pre_py310_{}_{}",
rule_code.as_ref(),
path.to_string_lossy()
);
let diagnostics = test_path(
Path::new("flake8_type_checking").join(path).as_path(),
&settings::LinterSettings {
target_version: settings::types::PythonVersion::Py39,
..settings::LinterSettings::for_rule(rule_code)
},
)?;
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"))]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::registry::Rule;
use ast::{ExprContext, Operator};
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, ViolationMetadata};
Expand All @@ -9,7 +8,10 @@ use ruff_python_stdlib::typing::{is_pep_593_generic_type, is_standard_library_li
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::registry::Rule;
use crate::rules::flake8_type_checking::helpers::quote_type_expression;
use crate::settings::types::PythonVersion;
use crate::settings::LinterSettings;

/// ## What it does
/// Checks if [PEP 613] explicit type aliases contain references to
Expand Down Expand Up @@ -279,7 +281,7 @@ pub(crate) fn quoted_type_alias(

// explicit type aliases require some additional checks to avoid false positives
if checker.semantic().in_annotated_type_alias_value()
&& quotes_are_unremovable(checker.semantic(), expr)
&& quotes_are_unremovable(checker.semantic(), expr, checker.settings)
{
return;
}
Expand All @@ -300,35 +302,68 @@ pub(crate) fn quoted_type_alias(

/// Traverses the type expression and checks if the expression can safely
/// be unquoted
fn quotes_are_unremovable(semantic: &SemanticModel, expr: &Expr) -> bool {
fn quotes_are_unremovable(
semantic: &SemanticModel,
expr: &Expr,
settings: &LinterSettings,
) -> bool {
match expr {
Expr::BinOp(ast::ExprBinOp { left, right, .. }) => {
quotes_are_unremovable(semantic, left) || quotes_are_unremovable(semantic, right)
Expr::BinOp(ast::ExprBinOp {
left, right, op, ..
}) => {
match op {
Operator::BitOr => {
if settings.target_version < PythonVersion::Py310 {
return true;
}
quotes_are_unremovable(semantic, left, settings)
|| quotes_are_unremovable(semantic, right, settings)
}
// for now we'll treat uses of other operators as unremovable quotes
// since that would make it an invalid type expression anyways. We skip
// walking the nested non-type expressions from `typing.Annotated`, so
// we don't produce false negatives in this branch.
_ => true,
}
}
Expr::Starred(ast::ExprStarred {
value,
ctx: ExprContext::Load,
..
}) => quotes_are_unremovable(semantic, value),
}) => quotes_are_unremovable(semantic, value, settings),
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)
if quotes_are_unremovable(semantic, value, settings) {
return true;
}
// for `typing.Annotated`, only analyze the first argument, since the rest may
// contain arbitrary expressions.
if let Some(qualified_name) = semantic.resolve_qualified_name(value) {
if semantic.match_typing_qualified_name(&qualified_name, "Annotated") {
if let Expr::Tuple(ast::ExprTuple { elts, .. }) = slice.as_ref() {
return !elts.is_empty()
&& quotes_are_unremovable(semantic, &elts[0], settings);
}
}
return false;
}
quotes_are_unremovable(semantic, slice, settings)
}
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)
quotes_are_unremovable(semantic, value, settings)
}
Expr::List(ast::ExprList { elts, .. }) | Expr::Tuple(ast::ExprTuple { elts, .. }) => {
for elt in elts {
if quotes_are_unremovable(semantic, elt) {
if quotes_are_unremovable(semantic, elt, settings) {
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---
source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
---
TC008_union_syntax_pre_py310.py:10:20: TC008 [*] Remove quotes from type alias
|
8 | if TYPE_CHECKING:
9 | c: TypeAlias = 'int | None' # OK
10 | d: TypeAlias = 'Annotated[int, 1 | 2]' # TC008
| ^^^^^^^^^^^^^^^^^^^^^^^ TC008
11 | e: TypeAlias = 'Annotated[int, 1 + 2]' # TC008
|
= help: Remove quotes

Safe fix
7 7 |
8 8 | if TYPE_CHECKING:
9 9 | c: TypeAlias = 'int | None' # OK
10 |- d: TypeAlias = 'Annotated[int, 1 | 2]' # TC008
10 |+ d: TypeAlias = Annotated[int, 1 | 2] # TC008
11 11 | e: TypeAlias = 'Annotated[int, 1 + 2]' # TC008

TC008_union_syntax_pre_py310.py:11:20: TC008 [*] Remove quotes from type alias
|
9 | c: TypeAlias = 'int | None' # OK
10 | d: TypeAlias = 'Annotated[int, 1 | 2]' # TC008
11 | e: TypeAlias = 'Annotated[int, 1 + 2]' # TC008
| ^^^^^^^^^^^^^^^^^^^^^^^ TC008
|
= help: Remove quotes

Safe fix
8 8 | if TYPE_CHECKING:
9 9 | c: TypeAlias = 'int | None' # OK
10 10 | d: TypeAlias = 'Annotated[int, 1 | 2]' # TC008
11 |- e: TypeAlias = 'Annotated[int, 1 + 2]' # TC008
11 |+ e: TypeAlias = Annotated[int, 1 + 2] # TC008

0 comments on commit 89d35c2

Please sign in to comment.