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.

Sign up for GitHub

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
merged 10 commits into from
Jan 8, 2025
+387 −20
Merged

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

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a775f51
[`flake8-type-checking`] Apply `TC008` more eagerly in typing context
Daverball Dec 29, 2024
348fd86
Execution context doesn't work, since it's already set to typing inside
Daverball Dec 29, 2024
f119f26
Merge branch 'main' into feat/tc008-typing-execution-context
Daverball Dec 29, 2024
a9f2d80
Avoids false positives for true forward references
Daverball Dec 29, 2024
39f4e10
Adds some use-before-define test cases
Daverball Dec 29, 2024
44dccdc
Disable `TC008` in stubs. Clean up stuff that's no longer needed.
Daverball Jan 3, 2025
5947f64
Update crates/ruff_linter/src/checkers/ast/mod.rs
Daverball Jan 3, 2025
5db1af5
Applies suggested change
Daverball Jan 3, 2025
ea541a6
Adds note and link to `PYI020`
Daverball Jan 6, 2025
9e8642f
more docs
AlexWaygood Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions ...uff_linter/resources/test/fixtures/flake8_type_checking/TC008_typing_execution_context.py
View file
Open in desktop
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: ...
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2324,7 +2324,7 @@ 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) {
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
View file
Open in desktop
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
32 changes: 25 additions & 7 deletions crates/ruff_linter/src/rules/flake8_type_checking/rules/type_alias_quotes.rs
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we also add a note to the docs for this rule pointing users to PYI020 for stub files? (And we could similarly add a note to the PYI020 docs pointing users to this rule for runtime files)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexWaygood Do you happen to know how to cross reference rules from another rule? I couldn't find any examples. Does linking to the corresponding enum variant work?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm... no, I'm not sure what the best solution is there. @MichaReiser, do you have any guidance/insight?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chatted to @MichaReiser this morning and we concluded that neither of us knows of a great way to do this :-) hardcoding a link to https://docs.astral.sh/ruff/rules/quoted-annotation-in-stub/ might be best for now

Copy link
Member

@MichaReiser MichaReiser Jan 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just double checked and yes, there's no custom handling in

ruff/crates/ruff_dev/src/generate_docs.rs

Lines 118 to 187 in bb12fe9

fn process_documentation(documentation: &str, out: &mut String, rule_name: &str) {
let mut in_options = false;
let mut after = String::new();
let mut referenced_options = HashSet::new();
// HACK: This is an ugly regex hack that's necessary because mkdocs uses
// a non-CommonMark-compliant Markdown parser, which doesn't support code
// tags in link definitions
// (see https://github.com/Python-Markdown/markdown/issues/280).
let documentation = Regex::new(r"\[`([^`]*?)`]($|[^\[(])").unwrap().replace_all(
documentation,
|caps: &Captures| {
format!(
"[`{option}`][{option}]{sep}",
option = &caps[1],
sep = &caps[2]
)
},
);
for line in documentation.split_inclusive('\n') {
if line.starts_with("## ") {
in_options = line == "## Options\n";
} else if in_options {
if let Some(rest) = line.strip_prefix("- `") {
let option = rest.trim_end().trim_end_matches('`');
match Options::metadata().find(option) {
Some(OptionEntry::Field(field)) => {
if field.deprecated.is_some() {
eprintln!("Rule {rule_name} references deprecated option {option}.");
}
}
Some(_) => {}
None => {
panic!("Unknown option {option} referenced by rule {rule_name}");
}
}
let anchor = option.replace('.', "_");
out.push_str(&format!("- [`{option}`][{option}]\n"));
after.push_str(&format!("[{option}]: ../settings.md#{anchor}\n"));
referenced_options.insert(option);
continue;
}
}
out.push_str(line);
}
let re = Regex::new(r"\[`([^`]*?)`]\[(.*?)]").unwrap();
for (_, [option, _]) in re.captures_iter(&documentation).map(|c| c.extract()) {
if let Some(OptionEntry::Field(field)) = Options::metadata().find(option) {
if referenced_options.insert(option) {
let anchor = option.replace('.', "_");
after.push_str(&format!("[{option}]: ../settings.md#{anchor}\n"));
}
if field.deprecated.is_some() {
eprintln!("Rule {rule_name} references deprecated option {option}.");
}
}
}
if !after.is_empty() {
out.push('\n');
out.push('\n');
out.push_str(&after);
}
}

It might be possible to use relative links as we do in the linter documentation

ruff/docs/linter.md

Lines 165 to 167 in 68eb0a2

For example, [`unnecessary-iterable-allocation-for-first-element`](rules/unnecessary-iterable-allocation-for-first-element.md)
(`RUF015`) is a rule which checks for potentially unperformant use of `list(...)[0]`. The fix
replaces this pattern with `next(iter(...))` which can result in a drastic speedup:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could probably also link /rules/quoted-annotation-in-stub.md so it's only semi-hardlinked, I had to do something similar in a setting to link to a rule.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexWaygood I think a reference from TC008 to PYI020 is sufficient. A reference the other way doesn't make that much sense to me, since most PYI rules are so specific to stub files.

Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,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, false)
.is_some()
{
return;
}

Expand Down Expand Up @@ -291,11 +295,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 +320,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())
.is_none()
}
_ => false,
}
Expand Down
254 changes: 254 additions & 0 deletions ...es__flake8_type_checking__tests__quoted-type-alias_TC008_typing_execution_context.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,254 @@
---
source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
---
TC008_typing_execution_context.py:13:20: TC008 [*] Remove quotes from type alias
|
11 | Bar: TypeAlias = Foo[int]
12 |
13 | a: TypeAlias = 'int' # TC008
| ^^^^^ TC008
14 | b: TypeAlias = 'Dict' # TC008
15 | c: TypeAlias = 'Foo' # TC008
|
= help: Remove quotes

ℹ Safe fix
10 10 | OptStr: TypeAlias = str | None
11 11 | Bar: TypeAlias = Foo[int]
12 12 |
13 |- a: TypeAlias = 'int' # TC008
13 |+ a: TypeAlias = int # TC008
14 14 | b: TypeAlias = 'Dict' # TC008
15 15 | c: TypeAlias = 'Foo' # TC008
16 16 | d: TypeAlias = 'Foo[str]' # TC008

TC008_typing_execution_context.py:14:20: TC008 [*] Remove quotes from type alias
|
13 | a: TypeAlias = 'int' # TC008
14 | b: TypeAlias = 'Dict' # TC008
| ^^^^^^ TC008
15 | c: TypeAlias = 'Foo' # TC008
16 | d: TypeAlias = 'Foo[str]' # TC008
|
= help: Remove quotes

ℹ Safe fix
11 11 | Bar: TypeAlias = Foo[int]
12 12 |
13 13 | a: TypeAlias = 'int' # TC008
14 |- b: TypeAlias = 'Dict' # TC008
14 |+ b: TypeAlias = Dict # TC008
15 15 | c: TypeAlias = 'Foo' # TC008
16 16 | d: TypeAlias = 'Foo[str]' # TC008
17 17 | e: TypeAlias = 'Foo.bar' # TC008

TC008_typing_execution_context.py:15:20: TC008 [*] Remove quotes from type alias
|
13 | a: TypeAlias = 'int' # TC008
14 | b: TypeAlias = 'Dict' # TC008
15 | c: TypeAlias = 'Foo' # TC008
| ^^^^^ TC008
16 | d: TypeAlias = 'Foo[str]' # TC008
17 | e: TypeAlias = 'Foo.bar' # TC008
|
= help: Remove quotes

ℹ Safe fix
12 12 |
13 13 | a: TypeAlias = 'int' # TC008
14 14 | b: TypeAlias = 'Dict' # TC008
15 |- c: TypeAlias = 'Foo' # TC008
15 |+ c: TypeAlias = Foo # TC008
16 16 | d: TypeAlias = 'Foo[str]' # TC008
17 17 | e: TypeAlias = 'Foo.bar' # TC008
18 18 | f: TypeAlias = 'Foo | None' # TC008

TC008_typing_execution_context.py:16:20: TC008 [*] Remove quotes from type alias
|
14 | b: TypeAlias = 'Dict' # TC008
15 | c: TypeAlias = 'Foo' # TC008
16 | d: TypeAlias = 'Foo[str]' # TC008
| ^^^^^^^^^^ TC008
17 | e: TypeAlias = 'Foo.bar' # TC008
18 | f: TypeAlias = 'Foo | None' # TC008
|
= help: Remove quotes

ℹ Safe fix
13 13 | a: TypeAlias = 'int' # TC008
14 14 | b: TypeAlias = 'Dict' # TC008
15 15 | c: TypeAlias = 'Foo' # TC008
16 |- d: TypeAlias = 'Foo[str]' # TC008
16 |+ d: TypeAlias = Foo[str] # TC008
17 17 | e: TypeAlias = 'Foo.bar' # TC008
18 18 | f: TypeAlias = 'Foo | None' # TC008
19 19 | g: TypeAlias = 'OptStr' # TC008

TC008_typing_execution_context.py:17:20: TC008 [*] Remove quotes from type alias
|
15 | c: TypeAlias = 'Foo' # TC008
16 | d: TypeAlias = 'Foo[str]' # TC008
17 | e: TypeAlias = 'Foo.bar' # TC008
| ^^^^^^^^^ TC008
18 | f: TypeAlias = 'Foo | None' # TC008
19 | g: TypeAlias = 'OptStr' # TC008
|
= help: Remove quotes

ℹ Safe fix
14 14 | b: TypeAlias = 'Dict' # TC008
15 15 | c: TypeAlias = 'Foo' # TC008
16 16 | d: TypeAlias = 'Foo[str]' # TC008
17 |- e: TypeAlias = 'Foo.bar' # TC008
17 |+ e: TypeAlias = Foo.bar # TC008
18 18 | f: TypeAlias = 'Foo | None' # TC008
19 19 | g: TypeAlias = 'OptStr' # TC008
20 20 | h: TypeAlias = 'Bar' # TC008

TC008_typing_execution_context.py:18:20: TC008 [*] Remove quotes from type alias
|
16 | d: TypeAlias = 'Foo[str]' # TC008
17 | e: TypeAlias = 'Foo.bar' # TC008
18 | f: TypeAlias = 'Foo | None' # TC008
| ^^^^^^^^^^^^ TC008
19 | g: TypeAlias = 'OptStr' # TC008
20 | h: TypeAlias = 'Bar' # TC008
|
= help: Remove quotes

ℹ Safe fix
15 15 | c: TypeAlias = 'Foo' # TC008
16 16 | d: TypeAlias = 'Foo[str]' # TC008
17 17 | e: TypeAlias = 'Foo.bar' # TC008
18 |- f: TypeAlias = 'Foo | None' # TC008
18 |+ f: TypeAlias = Foo | None # TC008
19 19 | g: TypeAlias = 'OptStr' # TC008
20 20 | h: TypeAlias = 'Bar' # TC008
21 21 | i: TypeAlias = Foo['str'] # TC008

TC008_typing_execution_context.py:19:20: TC008 [*] Remove quotes from type alias
|
17 | e: TypeAlias = 'Foo.bar' # TC008
18 | f: TypeAlias = 'Foo | None' # TC008
19 | g: TypeAlias = 'OptStr' # TC008
| ^^^^^^^^ TC008
20 | h: TypeAlias = 'Bar' # TC008
21 | i: TypeAlias = Foo['str'] # TC008
|
= help: Remove quotes

ℹ Safe fix
16 16 | d: TypeAlias = 'Foo[str]' # TC008
17 17 | e: TypeAlias = 'Foo.bar' # TC008
18 18 | f: TypeAlias = 'Foo | None' # TC008
19 |- g: TypeAlias = 'OptStr' # TC008
19 |+ g: TypeAlias = OptStr # TC008
20 20 | h: TypeAlias = 'Bar' # TC008
21 21 | i: TypeAlias = Foo['str'] # TC008
22 22 | j: TypeAlias = 'Baz' # OK (this would be treated as use before define)

TC008_typing_execution_context.py:20:20: TC008 [*] Remove quotes from type alias
|
18 | f: TypeAlias = 'Foo | None' # TC008
19 | g: TypeAlias = 'OptStr' # TC008
20 | h: TypeAlias = 'Bar' # TC008
| ^^^^^ TC008
21 | i: TypeAlias = Foo['str'] # TC008
22 | j: TypeAlias = 'Baz' # OK (this would be treated as use before define)
|
= help: Remove quotes

ℹ Safe fix
17 17 | e: TypeAlias = 'Foo.bar' # TC008
18 18 | f: TypeAlias = 'Foo | None' # TC008
19 19 | g: TypeAlias = 'OptStr' # TC008
20 |- h: TypeAlias = 'Bar' # TC008
20 |+ h: TypeAlias = Bar # TC008
21 21 | i: TypeAlias = Foo['str'] # TC008
22 22 | j: TypeAlias = 'Baz' # OK (this would be treated as use before define)
23 23 | k: TypeAlias = 'k | None' # False negative in type checking block

TC008_typing_execution_context.py:21:24: TC008 [*] Remove quotes from type alias
|
19 | g: TypeAlias = 'OptStr' # TC008
20 | h: TypeAlias = 'Bar' # TC008
21 | i: TypeAlias = Foo['str'] # TC008
| ^^^^^ TC008
22 | j: TypeAlias = 'Baz' # OK (this would be treated as use before define)
23 | k: TypeAlias = 'k | None' # False negative in type checking block
|
= help: Remove quotes

ℹ Safe fix
18 18 | f: TypeAlias = 'Foo | None' # TC008
19 19 | g: TypeAlias = 'OptStr' # TC008
20 20 | h: TypeAlias = 'Bar' # TC008
21 |- i: TypeAlias = Foo['str'] # TC008
21 |+ i: TypeAlias = Foo[str] # TC008
22 22 | j: TypeAlias = 'Baz' # OK (this would be treated as use before define)
23 23 | k: TypeAlias = 'k | None' # False negative in type checking block
24 24 | l: TypeAlias = 'int' | None # TC008 (because TC010 is not enabled)

TC008_typing_execution_context.py:24:20: TC008 [*] Remove quotes from type alias
|
22 | j: TypeAlias = 'Baz' # OK (this would be treated as use before define)
23 | k: TypeAlias = 'k | None' # False negative in type checking block
24 | l: TypeAlias = 'int' | None # TC008 (because TC010 is not enabled)
| ^^^^^ TC008
25 | m: TypeAlias = ('int' # TC008
26 | | None)
|
= help: Remove quotes

ℹ Safe fix
21 21 | i: TypeAlias = Foo['str'] # TC008
22 22 | j: TypeAlias = 'Baz' # OK (this would be treated as use before define)
23 23 | k: TypeAlias = 'k | None' # False negative in type checking block
24 |- l: TypeAlias = 'int' | None # TC008 (because TC010 is not enabled)
24 |+ l: TypeAlias = int | None # TC008 (because TC010 is not enabled)
25 25 | m: TypeAlias = ('int' # TC008
26 26 | | None)
27 27 | n: TypeAlias = ('int' # TC008 (fix removes comment currently)

TC008_typing_execution_context.py:25:21: TC008 [*] Remove quotes from type alias
|
23 | k: TypeAlias = 'k | None' # False negative in type checking block
24 | l: TypeAlias = 'int' | None # TC008 (because TC010 is not enabled)
25 | m: TypeAlias = ('int' # TC008
| ^^^^^ TC008
26 | | None)
27 | n: TypeAlias = ('int' # TC008 (fix removes comment currently)
|
= help: Remove quotes

ℹ Safe fix
22 22 | j: TypeAlias = 'Baz' # OK (this would be treated as use before define)
23 23 | k: TypeAlias = 'k | None' # False negative in type checking block
24 24 | l: TypeAlias = 'int' | None # TC008 (because TC010 is not enabled)
25 |- m: TypeAlias = ('int' # TC008
25 |+ m: TypeAlias = (int # TC008
26 26 | | None)
27 27 | n: TypeAlias = ('int' # TC008 (fix removes comment currently)
28 28 | ' | None')

TC008_typing_execution_context.py:27:21: TC008 [*] Remove quotes from type alias
|
25 | m: TypeAlias = ('int' # TC008
26 | | None)
27 | n: TypeAlias = ('int' # TC008 (fix removes comment currently)
| _____________________^
28 | | ' | None')
| |_________________^ TC008
|
= help: Remove quotes

ℹ Unsafe fix
24 24 | l: TypeAlias = 'int' | None # TC008 (because TC010 is not enabled)
25 25 | m: TypeAlias = ('int' # TC008
26 26 | | None)
27 |- n: TypeAlias = ('int' # TC008 (fix removes comment currently)
28 |- ' | None')
27 |+ n: TypeAlias = (int | None)
29 28 |
30 29 |
31 30 | class Baz: ...
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ fn should_be_fstring(
id,
literal.range(),
semantic.scope_id,
false,
Daverball marked this conversation as resolved.
Show resolved Hide resolved
)
.map_or(true, |id| semantic.binding(id).kind.is_builtin())
{
Expand Down
Loading
Loading