Skip to content

Commit

Permalink
Add a rule to detect string members in runtime-evaluated unions
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Dec 15, 2023
1 parent d1a7bc3 commit e777927
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x: "int" | str
x: ("int" | str) | "bool"
8 changes: 6 additions & 2 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ use crate::rules::{
flake8_comprehensions, flake8_datetimez, flake8_debugger, flake8_django,
flake8_future_annotations, flake8_gettext, flake8_implicit_str_concat, flake8_logging,
flake8_logging_format, flake8_pie, flake8_print, flake8_pyi, flake8_pytest_style, flake8_self,
flake8_simplify, flake8_tidy_imports, flake8_trio, flake8_use_pathlib, flynt, numpy,
pandas_vet, pep8_naming, pycodestyle, pyflakes, pygrep_hooks, pylint, pyupgrade, refurb, ruff,
flake8_simplify, flake8_tidy_imports, flake8_trio, flake8_type_checking, flake8_use_pathlib,
flynt, numpy, pandas_vet, pep8_naming, pycodestyle, pyflakes, pygrep_hooks, pylint, pyupgrade,
refurb, ruff,
};
use crate::settings::types::PythonVersion;

Expand Down Expand Up @@ -1170,6 +1171,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::UnnecessaryTypeUnion) {
flake8_pyi::rules::unnecessary_type_union(checker, expr);
}
if checker.enabled(Rule::RuntimeStringUnion) {
flake8_type_checking::rules::runtime_string_union(checker, expr);
}
}
}
Expr::UnaryOp(
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,7 @@ 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, "006") => (RuleGroup::Preview, rules::flake8_type_checking::rules::RuntimeStringUnion),

// tryceratops
(Tryceratops, "002") => (RuleGroup::Stable, rules::tryceratops::rules::RaiseVanillaClass),
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 @@ -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::RuntimeStringUnion, Path::new("TCH006.py"))]
#[test_case(Rule::TypingOnlyFirstPartyImport, Path::new("TCH001.py"))]
#[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("TCH003.py"))]
#[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("snapshot.py"))]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
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 typing_only_runtime_import::*;

mod empty_type_checking_block;
mod runtime_import_in_type_checking_block;
mod runtime_string_union;
mod typing_only_runtime_import;
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast as ast;
use ruff_python_ast::{Expr, Operator};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for the presence of string literals in `X | Y`-style union types.
///
/// ## Why is this bad?
/// [PEP 604] introduced a new syntax for union type annotations based on the
/// `|` operator.
///
/// While Python's type annotations can typically be wrapped in strings to
/// avoid runtime evaluation, the use of a string member within an `X | Y`-style
/// union type will cause a runtime error.
///
/// Instead, remove the quotes, wrap the _entire_ union in quotes, or use
/// `from __future__ import annotations` to disable runtime evaluation of
/// annotations entirely.
///
/// ## Example
/// ```python
/// var: str | "int"
/// ```
///
/// Use instead:
/// ```python
/// var: str | int
/// ```
///
/// ## References
/// - [PEP 535](https://peps.python.org/pep-0563/)
/// - [PEP 604](https://peps.python.org/pep-0604/)
///
/// [PEP 604]: https://peps.python.org/pep-0604/
#[violation]
pub struct RuntimeStringUnion;

impl Violation for RuntimeStringUnion {
#[derive_message_formats]
fn message(&self) -> String {
format!("Invalid string member in `X | Y`-style union type")
}
}

/// TCH006
pub(crate) fn runtime_string_union(checker: &mut Checker, expr: &Expr) {
if !checker.semantic().in_type_definition() {
return;
}

if !checker.semantic().execution_context().is_runtime() {
return;
}

// Search for strings within the binary operator.
let mut strings = Vec::new();
traverse_op(expr, &mut strings);

for string in strings {
checker
.diagnostics
.push(Diagnostic::new(RuntimeStringUnion, string.range()));
}
}

/// Collect all string members in possibly-nested binary `|` expressions.
fn traverse_op<'a>(expr: &'a Expr, strings: &mut Vec<&'a Expr>) {
match expr {
Expr::StringLiteral(_) => {
strings.push(expr);
}
Expr::BytesLiteral(_) => {
strings.push(expr);
}
Expr::BinOp(ast::ExprBinOp {
left,
right,
op: Operator::BitOr,
..
}) => {
traverse_op(left, strings);
traverse_op(right, strings);
}
_ => {}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
---
TCH006.py:1:4: TCH006 Invalid string member in `X | Y`-style union type
|
1 | x: "int" | str
| ^^^^^ TCH006
2 | x: ("int" | str) | "bool"
|

TCH006.py:2:5: TCH006 Invalid string member in `X | Y`-style union type
|
1 | x: "int" | str
2 | x: ("int" | str) | "bool"
| ^^^^^ TCH006
|

TCH006.py:2:20: TCH006 Invalid string member in `X | Y`-style union type
|
1 | x: "int" | str
2 | x: ("int" | str) | "bool"
| ^^^^^^ TCH006
|


1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit e777927

Please sign in to comment.