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

Add semantic analysis of type aliases and parameters #6109

Merged
merged 19 commits into from
Jul 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
103 changes: 103 additions & 0 deletions crates/ruff/resources/test/fixtures/pyflakes/F821_17.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
"""Test type parameters and aliases"""

# Type parameters in type alias statements

from some_module import Bar

type Foo[T] = T # OK
type Foo[T] = list[T] # OK
type Foo[T: ForwardA] = T # OK
type Foo[*Ts] = Bar[Ts] # OK
type Foo[**P] = Bar[P] # OK
class ForwardA: ...

# Types used in aliased assignment must exist

type Foo = DoesNotExist # F821: Undefined name `DoesNotExist`
type Foo = list[DoesNotExist] # F821: Undefined name `DoesNotExist`
Copy link
Member

Choose a reason for hiding this comment

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

Were you able to test these against the updated version of Pyflakes?

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't! I'll do that and confirm they match. I tested for correctness with Pyright and Python runtime errors.

Copy link
Member Author

Choose a reason for hiding this comment

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

The results are identical except that pyflakes emits two incorrect warnings

10:21: undefined name 'Ts'
11:21: undefined name 'P'

They appear to handle binding of type var tuples and param specs incorrectly.


# Type parameters do not escape alias scopes

type Foo[T] = T
T # F821: Undefined name `T` - not accessible afterward alias scope

# Type parameters in functions

def foo[T](t: T) -> T: return t # OK
async def afoo[T](t: T) -> T: return t # OK
def with_forward_ref[T: ForwardB](t: T) -> T: return t # OK
def can_access_inside[T](t: T) -> T: # OK
print(T) # OK
return t # OK
class ForwardB: ...


# Type parameters do not escape function scopes

from some_library import some_decorator

@some_decorator(T) # F821: Undefined name `T` - not accessible in decorators

def foo[T](t: T) -> None: ...
T # F821: Undefined name `T` - not accessible afterward function scope


# Type parameters in classes

class Foo[T](list[T]): ... # OK
class UsesForward[T: ForwardC](list[T]): ... # OK
class ForwardC: ...
class WithinBody[T](list[T]): # OK
t = T # OK
x: T # OK

def foo(self, x: T) -> T: # OK
return x

def foo(self):
T # OK


# Type parameters do not escape class scopes

from some_library import some_decorator
@some_decorator(T) # F821: Undefined name `T` - not accessible in decorators

class Foo[T](list[T]): ...
T # F821: Undefined name `T` - not accessible after class scope

# Types specified in bounds should exist

type Foo[T: DoesNotExist] = T # F821: Undefined name `DoesNotExist`
def foo[T: DoesNotExist](t: T) -> T: return t # F821: Undefined name `DoesNotExist`
class Foo[T: DoesNotExist](list[T]): ... # F821: Undefined name `DoesNotExist`

type Foo[T: (DoesNotExist1, DoesNotExist2)] = T # F821: Undefined name `DoesNotExist1`, Undefined name `DoesNotExist2`
def foo[T: (DoesNotExist1, DoesNotExist2)](t: T) -> T: return t # F821: Undefined name `DoesNotExist1`, Undefined name `DoesNotExist2`
class Foo[T: (DoesNotExist1, DoesNotExist2)](list[T]): ... # F821: Undefined name `DoesNotExist1`, Undefined name `DoesNotExist2`

# Type parameters in nested classes

class Parent[T]:
t = T # OK

def can_use_class_variable(self, x: t) -> t: # OK
return x

class Child:
def can_access_parent_type_parameter(self, x: T) -> T: # OK
T # OK
return x

def cannot_access_parent_variable(self, x: t) -> t: # F821: Undefined name `T`
t # F821: Undefined name `t`
return x

# Type parameters in nested functions

def can_access_inside_nested[T](t: T) -> T: # OK
def bar(x: T) -> T: # OK
T # OK
return x

bar(t)
6 changes: 3 additions & 3 deletions crates/ruff/src/checkers/ast/deferred.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use ruff_python_ast::Expr;
use ruff_text_size::TextRange;

use ruff_python_ast::{Expr, TypeParam};
use ruff_python_semantic::{ScopeId, Snapshot};
use ruff_text_size::TextRange;

/// A collection of AST nodes that are deferred for later analysis.
/// Used to, e.g., store functions, whose bodies shouldn't be analyzed until all
Expand All @@ -11,6 +10,7 @@ pub(crate) struct Deferred<'a> {
pub(crate) scopes: Vec<ScopeId>,
pub(crate) string_type_definitions: Vec<(TextRange, &'a str, Snapshot)>,
pub(crate) future_type_definitions: Vec<(&'a Expr, Snapshot)>,
pub(crate) type_param_definitions: Vec<(&'a TypeParam, Snapshot)>,
pub(crate) functions: Vec<Snapshot>,
pub(crate) lambdas: Vec<(&'a Expr, Snapshot)>,
pub(crate) for_loops: Vec<Snapshot>,
Expand Down
79 changes: 73 additions & 6 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,12 +453,14 @@ where
args,
decorator_list,
returns,
type_params,
..
})
| Stmt::AsyncFunctionDef(ast::StmtAsyncFunctionDef {
body,
args,
decorator_list,
type_params,
returns,
..
}) => {
Expand All @@ -472,6 +474,12 @@ where
// are enabled.
let runtime_annotation = !self.semantic.future_annotations();

self.semantic.push_scope(ScopeKind::Type);

for type_param in type_params {
self.visit_type_param(type_param);
}

for arg_with_default in args
.posonlyargs
.iter()
Expand Down Expand Up @@ -542,18 +550,25 @@ where
bases,
keywords,
decorator_list,
type_params,
..
},
) => {
for decorator in decorator_list {
self.visit_decorator(decorator);
}

self.semantic.push_scope(ScopeKind::Type);

for type_param in type_params {
self.visit_type_param(type_param);
}
for expr in bases {
self.visit_expr(expr);
}
for keyword in keywords {
self.visit_keyword(keyword);
}
for decorator in decorator_list {
self.visit_decorator(decorator);
}

let definition = docstrings::extraction::extract_definition(
ExtractionTarget::Class,
Expand All @@ -562,7 +577,6 @@ where
&self.semantic.definitions,
);
self.semantic.push_definition(definition);

self.semantic.push_scope(ScopeKind::Class(class_def));

// Extract any global bindings from the class body.
Expand All @@ -572,6 +586,20 @@ where

self.visit_body(body);
}
Stmt::TypeAlias(ast::StmtTypeAlias {
range: _range,
name,
type_params,
value,
}) => {
self.semantic.push_scope(ScopeKind::Type);
for type_param in type_params {
self.visit_type_param(type_param);
}
self.visit_expr(value);
self.semantic.pop_scope();
self.visit_expr(name);
}
Stmt::Try(ast::StmtTry {
body,
handlers,
Expand Down Expand Up @@ -715,8 +743,9 @@ where
| Stmt::AsyncFunctionDef(ast::StmtAsyncFunctionDef { name, .. }) => {
let scope_id = self.semantic.scope_id;
self.deferred.scopes.push(scope_id);
self.semantic.pop_scope();
self.semantic.pop_scope(); // Function scope
self.semantic.pop_definition();
self.semantic.pop_scope(); // Type parameter scope
self.add_binding(
name,
stmt.identifier(),
Expand All @@ -727,8 +756,9 @@ where
Stmt::ClassDef(ast::StmtClassDef { name, .. }) => {
let scope_id = self.semantic.scope_id;
self.deferred.scopes.push(scope_id);
self.semantic.pop_scope();
self.semantic.pop_scope(); // Class scope
self.semantic.pop_definition();
self.semantic.pop_scope(); // Type parameter scope
self.add_binding(
name,
stmt.identifier(),
Expand Down Expand Up @@ -1331,6 +1361,26 @@ where
self.visit_stmt(stmt);
}
}

fn visit_type_param(&mut self, type_param: &'b ast::TypeParam) {
// Step 1: Binding
match type_param {
ast::TypeParam::TypeVar(ast::TypeParamTypeVar { name, range, .. })
| ast::TypeParam::TypeVarTuple(ast::TypeParamTypeVarTuple { name, range })
| ast::TypeParam::ParamSpec(ast::TypeParamParamSpec { name, range }) => {
self.add_binding(
name.as_str(),
*range,
BindingKind::TypeParam,
BindingFlags::empty(),
);
}
}
// Step 2: Traversal
self.deferred
.type_param_definitions
.push((type_param, self.semantic.snapshot()));
}
}

impl<'a> Checker<'a> {
Expand Down Expand Up @@ -1693,6 +1743,22 @@ impl<'a> Checker<'a> {
}
}

fn visit_deferred_type_param_definitions(&mut self) {
while !self.deferred.type_param_definitions.is_empty() {
let type_params = std::mem::take(&mut self.deferred.type_param_definitions);
for (type_param, snapshot) in type_params {
self.semantic.restore(snapshot);

if let ast::TypeParam::TypeVar(ast::TypeParamTypeVar {
bound: Some(bound), ..
}) = type_param
{
self.visit_expr(bound);
}
}
}
}

fn visit_deferred_string_type_definitions(&mut self, allocator: &'a typed_arena::Arena<Expr>) {
while !self.deferred.string_type_definitions.is_empty() {
let type_definitions = std::mem::take(&mut self.deferred.string_type_definitions);
Expand Down Expand Up @@ -1882,6 +1948,7 @@ pub(crate) fn check_ast(
checker.visit_deferred_functions();
checker.visit_deferred_lambdas();
checker.visit_deferred_future_type_definitions();
checker.visit_deferred_type_param_definitions();
let allocator = typed_arena::Arena::new();
checker.visit_deferred_string_type_definitions(&allocator);
checker.visit_exports();
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/renamer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ impl Renamer {
// By default, replace the binding's name with the target name.
BindingKind::Annotation
| BindingKind::Argument
| BindingKind::TypeParam
| BindingKind::NamedExprAssignment
| BindingKind::UnpackedAssignment
| BindingKind::Assignment
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,10 +317,7 @@ pub(crate) fn unused_arguments(
scope: &Scope,
diagnostics: &mut Vec<Diagnostic>,
) {
let Some(parent) = scope
.parent
.map(|scope_id| &checker.semantic().scopes[scope_id])
else {
let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else {
return;
};

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 @@ -128,6 +128,7 @@ mod tests {
#[test_case(Rule::UndefinedName, Path::new("F821_14.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_15.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_16.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_17.py"))]
#[test_case(Rule::UndefinedExport, Path::new("F822_0.py"))]
#[test_case(Rule::UndefinedExport, Path::new("F822_1.py"))]
#[test_case(Rule::UndefinedExport, Path::new("F822_2.py"))]
Expand Down
Loading