Skip to content

Commit

Permalink
Change to Python 3.11 and lower implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
qdegraaf committed Jul 31, 2023
1 parent e0b15c2 commit bcb0c8d
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 84 deletions.
10 changes: 0 additions & 10 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,3 @@ def good_cls_method_1(cls: type[Self], arg: int) -> Self: ...
def good_cls_method_2(cls, arg1: _S, arg2: _S) -> _S: ...
@staticmethod
def static_method(arg1: _S) -> _S: ...


# Python > 3.12
class PEP695BadDunderNew[T]:
def __new__[S](cls: type[S], *args: Any, ** kwargs: Any) -> S: ... # Ok

def generic_instance_method[S](self: S) -> S: ... # Ok

class PEP695GoodDunderNew[T]:
def __new__(cls, *args: Any, **kwargs: Any) -> Self: ...
10 changes: 0 additions & 10 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,3 @@ class GoodClass:
def good_cls_method_2(cls, arg1: _S, arg2: _S) -> _S: ...
@staticmethod
def static_method(arg1: _S) -> _S: ...


# Python > 3.12
class PEP695BadDunderNew[T]:
def __new__[S](cls: type[S], *args: Any, ** kwargs: Any) -> S: ... # PYI019

def generic_instance_method[S](self: S) -> S: ... # PYI019

class PEP695GoodDunderNew[T]:
def __new__(cls, *args: Any, **kwargs: Any) -> Self: ...
73 changes: 9 additions & 64 deletions crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
use crate::checkers::ast::Checker;
use crate::settings::types::PythonVersion;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast as ast;
use ruff_python_ast::{ArgWithDefault, Arguments, Decorator, Expr, Ranged, Stmt};
use ruff_python_ast::{ArgWithDefault, Arguments, Decorator, Expr, Ranged};
use ruff_python_semantic::analyze::visibility::{
is_abstract, is_classmethod, is_overload, is_staticmethod,
};
use ruff_python_semantic::ScopeKind;

// TODO: Check docs for accuracy
/// ## What it does
/// Checks if certain methods define a custom `TypeVar`s for their return annotation instead of
/// using `typing_extensions.Self`. This check currently applies for instance methods that return
Expand Down Expand Up @@ -102,12 +100,15 @@ pub(crate) fn custom_typevar_return_type(

let is_violation: bool =
if is_classmethod(decorator_list, checker.semantic()) || name == "__new__" {
check_class_method_for_bad_typevars(checker, args, return_annotation)
println!("Class: {}", name.to_string());
check_class_method_for_bad_typevars(args, return_annotation)
} else {
// If not static, or a class method or __new__ we know it is an instance method
check_instance_method_for_bad_typevars(checker, args, return_annotation)
println!("Instance: {}", name.to_string());
check_instance_method_for_bad_typevars(args, return_annotation)
};

println!("{:?}", is_violation);
if is_violation {
checker.diagnostics.push(Diagnostic::new(
CustomTypeVarReturnType {
Expand All @@ -119,7 +120,6 @@ pub(crate) fn custom_typevar_return_type(
}

fn check_class_method_for_bad_typevars(
checker: &Checker,
args: &Arguments,
return_annotation: &Expr,
) -> bool {
Expand All @@ -133,12 +133,6 @@ fn check_class_method_for_bad_typevars(
return false
};

// Don't error if the first argument is annotated with `builtins.type[T]` or `typing.Type[T]`
// These are edge cases, and it's hard to give good error messages for them.
if let Expr::Name(ast::ExprName { id: id_value, .. }) = value.as_ref() {
return id_value == "type";
}

let Expr::Name(ast::ExprName { id: id_slice, .. }) = slice.as_ref() else {
return false
};
Expand All @@ -147,11 +141,10 @@ fn check_class_method_for_bad_typevars(
return false
};

return_type == id_slice && is_likely_private_typevar(checker, args, id_slice)
return_type == id_slice && is_likely_private_typevar(id_slice)
}

fn check_instance_method_for_bad_typevars(
checker: &Checker,
args: &Arguments,
return_annotation: &Expr,
) -> bool {
Expand All @@ -173,60 +166,12 @@ fn check_instance_method_for_bad_typevars(
return false;
}

is_likely_private_typevar(checker, args, first_arg_type)
is_likely_private_typevar(first_arg_type)
}

fn is_likely_private_typevar(checker: &Checker, args: &Arguments, tvar_name: &str) -> bool {
fn is_likely_private_typevar(tvar_name: &str) -> bool {
if tvar_name.starts_with('_') {
return true;
}
if checker.settings.target_version < PythonVersion::Py312 {
return false;
}
for ArgWithDefault { def, .. } in &args.args {
if def.arg.to_string() != tvar_name {
continue;
}

let Some(annotation) = &def.annotation else {
continue
};

let Expr::Name(ast::ExprName{id,..}) = annotation.as_ref() else {
continue
};

// Check if the binding used in an annotation is an assignment to typing.TypeVar
let scope = checker.semantic().scope();
let Some(binding_id) = scope.get(id) else {
continue
};

let binding = checker.semantic().binding(binding_id);
if binding.kind.is_assignment() || binding.kind.is_named_expr_assignment() {
if let Some(parent_id) = binding.source {
let parent = checker.semantic().stmts[parent_id];
if let Stmt::Assign(ast::StmtAssign { value, .. })
| Stmt::AnnAssign(ast::StmtAnnAssign {
value: Some(value), ..
})
| Stmt::AugAssign(ast::StmtAugAssign { value, .. }) = parent
{
let Expr::Call(ast::ExprCall{func, ..}) = value.as_ref() else {
continue
};
let Some(call_path) = checker.semantic().resolve_call_path(func) else {
continue
};
if checker
.semantic()
.match_typing_call_path(&call_path, "TypeVar")
{
return true;
}
}
}
}
}
false
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---
PYI019.pyi:7:62: PYI019 Methods like __new__ should return `typing.Self` instead of a custom TypeVar
|
6 | class BadClass:
7 | def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: ... # PYI019
| ^^ PYI019
8 | def bad_instance_method(self: _S, arg: bytes) -> _S: ... # PYI019
9 | @classmethod
|

PYI019.pyi:8:54: PYI019 Methods like bad_instance_method should return `typing.Self` instead of a custom TypeVar
|
6 | class BadClass:
7 | def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: ... # PYI019
8 | def bad_instance_method(self: _S, arg: bytes) -> _S: ... # PYI019
| ^^ PYI019
9 | @classmethod
10 | def bad_class_method(cls: type[_S], arg: int) -> _S: ... # PYI019
|

PYI019.pyi:10:54: PYI019 Methods like bad_class_method should return `typing.Self` instead of a custom TypeVar
|
8 | def bad_instance_method(self: _S, arg: bytes) -> _S: ... # PYI019
9 | @classmethod
10 | def bad_class_method(cls: type[_S], arg: int) -> _S: ... # PYI019
| ^^ PYI019
11 |
12 | class GoodClass:
|


0 comments on commit bcb0c8d

Please sign in to comment.