Skip to content

Commit

Permalink
Handle nested string annotations
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Mar 24, 2023
1 parent c721eed commit 3a67a86
Show file tree
Hide file tree
Showing 8 changed files with 195 additions and 41 deletions.
7 changes: 7 additions & 0 deletions Cargo.lock

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

3 changes: 2 additions & 1 deletion crates/ruff/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ strum_macros = { workspace = true }
textwrap = { workspace = true }
thiserror = { version = "1.0.38" }
toml = { workspace = true }
unicode-width = "0.1.10"
typed-arena = { version = "2.0.2" }
unicode-width = {version ="0.1.10"}

[dev-dependencies]
insta = { workspace = true, features = ["yaml", "redactions"] }
Expand Down
7 changes: 7 additions & 0 deletions crates/ruff/resources/test/fixtures/pyflakes/F401_11.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
"""Test: parsing of nested string annotations."""

from typing import List
from pathlib import Path, PurePath


x: """List['Path']""" = []
12 changes: 12 additions & 0 deletions crates/ruff/resources/test/fixtures/pyupgrade/UP006.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,15 @@ def f(x: """List[str]""") -> None:

def f(x: "Li" "st[str]") -> None:
...


def f(x: "List['List[str]']") -> None:
...


def f(x: "List['Li' 'st[str]']") -> None:
...


def f(x: "Li" "st['List[str]']") -> None:
...
82 changes: 42 additions & 40 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4525,47 +4525,49 @@ impl<'a> Checker<'a> {
}
}

fn check_deferred_string_type_definitions(&mut self, allocator: &'a mut Vec<Expr>) {
let mut stacks = Vec::with_capacity(self.deferred.string_type_definitions.len());
self.deferred.string_type_definitions.reverse();
while let Some((range, value, (in_annotation, in_type_checking_block), deferral)) =
self.deferred.string_type_definitions.pop()
{
if let Ok((expr, kind)) = parse_type_annotation(value, range, self.locator) {
if in_annotation && self.ctx.annotations_future_enabled {
if self.settings.rules.enabled(Rule::QuotedAnnotation) {
pyupgrade::rules::quoted_annotation(self, value, range);
fn check_deferred_string_type_definitions(&mut self, allocator: &'a typed_arena::Arena<Expr>) {
let mut type_definitions = std::mem::take(&mut self.deferred.string_type_definitions);
loop {
for (range, value, (in_annotation, in_type_checking_block), (scopes, parents)) in
type_definitions.into_iter().rev()
{
if let Ok((expr, kind)) = parse_type_annotation(value, range, self.locator) {
if in_annotation && self.ctx.annotations_future_enabled {
if self.settings.rules.enabled(Rule::QuotedAnnotation) {
pyupgrade::rules::quoted_annotation(self, value, range);
}
}

let expr = allocator.alloc(expr);

self.ctx.scope_stack = scopes;
self.ctx.parents = parents;
self.ctx.in_annotation = in_annotation;
self.ctx.in_type_checking_block = in_type_checking_block;
self.ctx.in_type_definition = true;
self.ctx.in_deferred_string_type_definition = Some(kind);
self.visit_expr(expr);
self.ctx.in_deferred_string_type_definition = None;
self.ctx.in_type_definition = false;
} else {
if self
.settings
.rules
.enabled(Rule::ForwardAnnotationSyntaxError)
{
self.diagnostics.push(Diagnostic::new(
pyflakes::rules::ForwardAnnotationSyntaxError {
body: value.to_string(),
},
range,
));
}
}
allocator.push(expr);
stacks.push((kind, (in_annotation, in_type_checking_block), deferral));
} else {
if self
.settings
.rules
.enabled(Rule::ForwardAnnotationSyntaxError)
{
self.diagnostics.push(Diagnostic::new(
pyflakes::rules::ForwardAnnotationSyntaxError {
body: value.to_string(),
},
range,
));
}
}
}
for (expr, (kind, (in_annotation, in_type_checking_block), (scopes, parents))) in
allocator.iter().zip(stacks)
{
self.ctx.scope_stack = scopes;
self.ctx.parents = parents;
self.ctx.in_annotation = in_annotation;
self.ctx.in_type_checking_block = in_type_checking_block;
self.ctx.in_type_definition = true;
self.ctx.in_deferred_string_type_definition = Some(kind);
self.visit_expr(expr);
self.ctx.in_deferred_string_type_definition = None;
self.ctx.in_type_definition = false;
if self.deferred.string_type_definitions.is_empty() {
break;
}
type_definitions = std::mem::take(&mut self.deferred.string_type_definitions);
}
}

Expand Down Expand Up @@ -5394,8 +5396,8 @@ pub fn check_ast(
checker.check_deferred_functions();
checker.check_deferred_lambdas();
checker.check_deferred_type_definitions();
let mut allocator = vec![];
checker.check_deferred_string_type_definitions(&mut allocator);
let allocator = typed_arena::Arena::new();
checker.check_deferred_string_type_definitions(&allocator);
checker.check_deferred_assignments();
checker.check_deferred_for_loops();

Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/pyflakes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ mod tests {
#[test_case(Rule::UnusedImport, Path::new("F401_8.py"); "F401_8")]
#[test_case(Rule::UnusedImport, Path::new("F401_9.py"); "F401_9")]
#[test_case(Rule::UnusedImport, Path::new("F401_10.py"); "F401_10")]
#[test_case(Rule::UnusedImport, Path::new("F401_11.py"); "F401_11")]
#[test_case(Rule::ImportShadowedByLoopVar, Path::new("F402.py"); "F402")]
#[test_case(Rule::UndefinedLocalWithImportStar, Path::new("F403.py"); "F403")]
#[test_case(Rule::LateFutureImport, Path::new("F404.py"); "F404")]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
source: crates/ruff/src/rules/pyflakes/mod.rs
expression: diagnostics
---
- kind:
name: UnusedImport
body: "`pathlib.PurePath` imported but unused"
suggestion: "Remove unused import: `pathlib.PurePath`"
fixable: true
location:
row: 4
column: 26
end_location:
row: 4
column: 34
fix:
content: from pathlib import Path
location:
row: 4
column: 0
end_location:
row: 4
column: 34
parent: ~

Original file line number Diff line number Diff line change
Expand Up @@ -175,4 +175,103 @@ expression: diagnostics
column: 23
fix: ~
parent: ~
- kind:
name: NonPEP585Annotation
body: "Use `list` instead of `List` for type annotations"
suggestion: "Replace `List` with `list`"
fixable: true
location:
row: 49
column: 10
end_location:
row: 49
column: 14
fix:
content: list
location:
row: 49
column: 10
end_location:
row: 49
column: 14
parent: ~
- kind:
name: NonPEP585Annotation
body: "Use `list` instead of `List` for type annotations"
suggestion: "Replace `List` with `list`"
fixable: true
location:
row: 49
column: 16
end_location:
row: 49
column: 20
fix:
content: list
location:
row: 49
column: 16
end_location:
row: 49
column: 20
parent: ~
- kind:
name: NonPEP585Annotation
body: "Use `list` instead of `List` for type annotations"
suggestion: "Replace `List` with `list`"
fixable: true
location:
row: 53
column: 10
end_location:
row: 53
column: 14
fix:
content: list
location:
row: 53
column: 10
end_location:
row: 53
column: 14
parent: ~
- kind:
name: NonPEP585Annotation
body: "Use `list` instead of `List` for type annotations"
suggestion: ~
fixable: false
location:
row: 53
column: 15
end_location:
row: 53
column: 29
fix: ~
parent: ~
- kind:
name: NonPEP585Annotation
body: "Use `list` instead of `List` for type annotations"
suggestion: ~
fixable: false
location:
row: 57
column: 9
end_location:
row: 57
column: 31
fix: ~
parent: ~
- kind:
name: NonPEP585Annotation
body: "Use `list` instead of `List` for type annotations"
suggestion: ~
fixable: false
location:
row: 57
column: 9
end_location:
row: 57
column: 31
fix: ~
parent: ~

0 comments on commit 3a67a86

Please sign in to comment.