diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI019_0.py b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI019_0.py index 7a2ad05a7350d..cee43b4b750d4 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI019_0.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI019_0.py @@ -99,7 +99,7 @@ def n(self: S) -> S[int]: ... class UsesFullyQualifiedType: @classmethod - def m[S](cls: builtins.type[S]) -> S: ... # PYI019 + def m[S](cls: builtins.type[S]) -> S: ... # False negative (#15821) def shadowed_type(): diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI019_0.pyi b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI019_0.pyi index ae58a2d147ff1..c99f1d74e42e9 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI019_0.pyi +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI019_0.pyi @@ -99,7 +99,7 @@ import builtins class UsesFullyQualifiedType: @classmethod - def m[S](cls: builtins.type[S]) -> S: ... # PYI019 + def m[S](cls: builtins.type[S]) -> S: ... # False negative (#15821) def shadowed_type(): @@ -116,3 +116,26 @@ class SubscriptReturnType: class PEP695TypeParameterAtTheVeryEndOfTheList: def f[T, S](self: S) -> S: ... + + +class PEP695Again: + def mixing_and_nested[T](self: _S695, a: list[_S695], b: dict[_S695, str | T | set[_S695]]) -> _S695: ... + def also_uses_s695_but_should_not_be_edited(self, v: set[tuple[_S695]]) -> _S695: ... + + @classmethod + def comment_in_fix_range[T, S]( + cls: type[ # Lorem ipsum + S + ], + a: T, + b: tuple[S, T] + ) -> S: ... + + def comment_outside_fix_range[T, S]( + self: S, + a: T, + b: tuple[ + # Lorem ipsum + S, T + ] + ) -> S: ... diff --git a/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs b/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs index 69beabf12c812..64dc06984e776 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs @@ -23,6 +23,7 @@ pub(crate) fn bindings(checker: &mut Checker) { Rule::UsedDummyVariable, Rule::PytestUnittestRaisesAssertion, Rule::ForLoopWrites, + Rule::CustomTypeVarReturnType, ]) { return; } @@ -115,5 +116,12 @@ pub(crate) fn bindings(checker: &mut Checker) { checker.diagnostics.push(diagnostic); } } + if checker.enabled(Rule::CustomTypeVarReturnType) { + if let Some(diagnostic) = + flake8_pyi::rules::custom_type_var_return_type(checker, binding) + { + checker.diagnostics.push(diagnostic); + } + } } } diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 292be0169e6b3..9860cb34690fc 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -159,9 +159,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::GeneratorReturnFromIterMethod) { flake8_pyi::rules::bad_generator_return_type(function_def, checker); } - if checker.enabled(Rule::CustomTypeVarReturnType) { - flake8_pyi::rules::custom_type_var_return_type(checker, function_def); - } if checker.source_type.is_stub() { if checker.enabled(Rule::StrOrReprDefinedInStub) { flake8_pyi::rules::str_or_repr_defined_in_stub(checker, stmt); diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/custom_type_var_return_type.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/custom_type_var_return_type.rs index 82fb98497b444..2deb12b4c6c51 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/custom_type_var_return_type.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/custom_type_var_return_type.rs @@ -1,7 +1,6 @@ -use crate::checkers::ast::Checker; -use crate::importer::{ImportRequest, ResolutionError}; -use crate::settings::types::PythonVersion; use itertools::Itertools; +use std::cmp; + use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::{ @@ -9,9 +8,13 @@ use ruff_python_ast::{ }; use ruff_python_semantic::analyze::function_type::{self, FunctionType}; use ruff_python_semantic::analyze::visibility::{is_abstract, is_overload}; -use ruff_python_semantic::SemanticModel; +use ruff_python_semantic::{Binding, ScopeId, SemanticModel}; use ruff_text_size::{Ranged, TextRange, TextSize}; +use crate::checkers::ast::Checker; +use crate::importer::{ImportRequest, ResolutionError}; +use crate::settings::types::PythonVersion; + /// ## What it does /// Checks for methods that define a custom `TypeVar` for their return type /// annotation instead of using `Self`. @@ -52,8 +55,7 @@ use ruff_text_size::{Ranged, TextRange, TextSize}; /// It will try to remove all usages and declarations of the custom type variable. /// Pre-[PEP-695]-style declarations will not be removed. /// -/// If a variable's annotation is too complex to handle, -/// the fix will be marked as display only. +/// If there are any comments within the fix ranges, it will be marked as unsafe. /// Otherwise, it will be marked as safe. /// /// [PEP 673]: https://peps.python.org/pep-0673/#motivation @@ -79,46 +81,43 @@ impl Violation for CustomTypeVarReturnType { /// PYI019 pub(crate) fn custom_type_var_return_type( - checker: &mut Checker, - function_def: &ast::StmtFunctionDef, -) { + checker: &Checker, + binding: &Binding, +) -> Option { + let semantic = checker.semantic(); + let current_scope = &semantic.scopes[binding.scope]; + let function_def = binding.statement(semantic)?.as_function_def_stmt()?; + // Given, e.g., `def foo(self: _S, arg: bytes) -> _T`, extract `_T`. - let Some(returns) = function_def.returns.as_ref() else { - return; - }; + let returns = function_def.returns.as_ref()?; let parameters = &*function_def.parameters; // Given, e.g., `def foo(self: _S, arg: bytes)`, extract `_S`. - let Some(self_or_cls_annotation) = parameters + let self_or_cls_annotation = parameters .posonlyargs .iter() .chain(¶meters.args) - .next() - .and_then(|parameter_with_default| parameter_with_default.annotation()) - else { - return; - }; + .next()? + .annotation()?; let decorator_list = &*function_def.decorator_list; - let semantic = checker.semantic(); - // Skip any abstract, static, and overloaded methods. if is_abstract(decorator_list, semantic) || is_overload(decorator_list, semantic) { - return; + return None; } let method = match function_type::classify( &function_def.name, decorator_list, - semantic.current_scope(), + current_scope, semantic, &checker.settings.pep8_naming.classmethod_decorators, &checker.settings.pep8_naming.staticmethod_decorators, ) { - FunctionType::Function => return, - FunctionType::StaticMethod => return, + FunctionType::Function => return None, + FunctionType::StaticMethod => return None, FunctionType::ClassMethod => Method::Class(ClassMethod { cls_annotation: self_or_cls_annotation, returns, @@ -131,9 +130,21 @@ pub(crate) fn custom_type_var_return_type( }), }; - if method.uses_custom_var(semantic) { - add_diagnostic(checker, function_def, returns); + if !method.uses_custom_var(semantic, binding.scope) { + return None; } + + let mut diagnostic = Diagnostic::new( + CustomTypeVarReturnType { + method_name: function_def.name.to_string(), + }, + returns.range(), + ); + + diagnostic + .try_set_optional_fix(|| replace_custom_typevar_with_self(checker, function_def, returns)); + + Some(diagnostic) } #[derive(Debug)] @@ -143,9 +154,9 @@ enum Method<'a> { } impl Method<'_> { - fn uses_custom_var(&self, semantic: &SemanticModel) -> bool { + fn uses_custom_var(&self, semantic: &SemanticModel, scope: ScopeId) -> bool { match self { - Self::Class(class_method) => class_method.uses_custom_var(semantic), + Self::Class(class_method) => class_method.uses_custom_var(semantic, scope), Self::Instance(instance_method) => instance_method.uses_custom_var(), } } @@ -161,7 +172,7 @@ struct ClassMethod<'a> { impl ClassMethod<'_> { /// Returns `true` if the class method is annotated with /// a custom `TypeVar` that is likely private. - fn uses_custom_var(&self, semantic: &SemanticModel) -> bool { + fn uses_custom_var(&self, semantic: &SemanticModel, scope: ScopeId) -> bool { let Expr::Subscript(ast::ExprSubscript { value: cls_annotation_value, slice: cls_annotation_typevar, @@ -177,7 +188,15 @@ impl ClassMethod<'_> { let cls_annotation_typevar = &cls_annotation_typevar.id; - if !semantic.match_builtin_expr(cls_annotation_value, "type") { + let Expr::Name(ExprName { id, .. }) = &**cls_annotation_value else { + return false; + }; + + if id != "type" { + return false; + } + + if !semantic.has_builtin_binding_in_scope("type", scope) { return false; } @@ -187,7 +206,10 @@ impl ClassMethod<'_> { let Expr::Name(return_annotation_typevar) = &**slice else { return false; }; - if !semantic.match_builtin_expr(value, "type") { + let Expr::Name(ExprName { id, .. }) = &**value else { + return false; + }; + if id != "type" { return false; } &return_annotation_typevar.id @@ -254,20 +276,6 @@ fn is_likely_private_typevar(type_var_name: &str, type_params: Option<&TypeParam }) } -fn add_diagnostic(checker: &mut Checker, function_def: &ast::StmtFunctionDef, returns: &Expr) { - let mut diagnostic = Diagnostic::new( - CustomTypeVarReturnType { - method_name: function_def.name.to_string(), - }, - returns.range(), - ); - - diagnostic - .try_set_optional_fix(|| replace_custom_typevar_with_self(checker, function_def, returns)); - - checker.diagnostics.push(diagnostic); -} - /// Add a "Replace with `Self`" fix that does the following: /// /// * Import `Self` if necessary @@ -275,10 +283,6 @@ fn add_diagnostic(checker: &mut Checker, function_def: &ast::StmtFunctionDef, re /// * Replace the return annotation with `Self` /// * Replace other uses of the original type variable elsewhere in the signature with `Self` /// * Remove that type variable from the PEP 695 type parameter list -/// -/// The fourth step above has the same problem. -/// This function thus only does replacements for the simplest of cases -/// and will mark the fix as unsafe if an annotation cannot be handled. fn replace_custom_typevar_with_self( checker: &Checker, function_def: &ast::StmtFunctionDef, @@ -296,33 +300,58 @@ fn replace_custom_typevar_with_self( } // Non-`Name` return annotations are not currently autofixed - let Expr::Name(typevar_name) = &returns else { + let Expr::Name(typevar) = &returns else { return Ok(None); }; - let typevar_name = &typevar_name.id; + let mut applicability = Applicability::Safe; + + let typevar_name = &typevar.id; let (import_edit, self_symbol_binding) = import_self(checker, returns.start())?; - let mut all_edits = vec![ - import_edit, - replace_return_annotation_with_self(self_symbol_binding, returns), - remove_first_parameter_annotation(&function_def.parameters), - ]; + let mut other_edits = vec![replace_return_annotation_with_self( + &self_symbol_binding, + returns, + )]; + + let replace_references_range = { + let edit = remove_first_parameter_annotation(&function_def.parameters); + let first_parameter_end = edit.end(); + other_edits.push(edit); + TextRange::new(first_parameter_end, returns.start()) + }; - all_edits.extend(remove_typevar_declaration( + other_edits.extend(remove_typevar_declaration( function_def.type_params.as_deref(), typevar_name, )); - let (edits, fix_applicability) = - replace_typevar_usages_with_self(&function_def.parameters, typevar_name); + if let Some(edits) = replace_typevar_usages_with_self( + typevar, + &self_symbol_binding, + replace_references_range, + checker.semantic(), + ) { + other_edits.extend(edits); + } else { + applicability = Applicability::DisplayOnly; + } - all_edits.extend(edits); + let comment_ranges = checker.comment_ranges(); - let (first, rest) = (all_edits.swap_remove(0), all_edits); + if other_edits + .iter() + .any(|edit| comment_ranges.intersects(edit.range())) + { + applicability = cmp::min(applicability, Applicability::Unsafe); + } - Ok(Some(Fix::applicable_edits(first, rest, fix_applicability))) + Ok(Some(Fix::applicable_edits( + import_edit, + other_edits, + applicability, + ))) } fn import_self(checker: &Checker, position: TextSize) -> Result<(Edit, String), ResolutionError> { @@ -345,39 +374,38 @@ fn remove_first_parameter_annotation(parameters: &Parameters) -> Edit { Edit::deletion(first.name().end(), first.end()) } -fn replace_return_annotation_with_self(self_symbol_binding: String, returns: &Expr) -> Edit { - Edit::range_replacement(self_symbol_binding, returns.range()) +fn replace_return_annotation_with_self(self_symbol_binding: &str, returns: &Expr) -> Edit { + Edit::range_replacement(self_symbol_binding.to_string(), returns.range()) } +/// Returns a series of [`Edit`]s that modify all references to the given `typevar`, +/// or `None` when it is not possible to resolve the binding. +/// +/// Only references within `editable_range` will be modified. +/// This ensures that no edit in this series will overlap with other edits. fn replace_typevar_usages_with_self( - parameters: &Parameters, - typevar_name: &str, -) -> (Vec, Applicability) { + typevar: &ast::ExprName, + self_symbol_binding: &str, + editable_range: TextRange, + semantic: &SemanticModel, +) -> Option> { + let binding = semantic + .only_binding(typevar) + .map(|id| semantic.binding(id))?; + let mut edits = vec![]; - let mut could_not_handle_all_usages = false; - for parameter in parameters.iter().skip(1) { - let Some(annotation) = parameter.annotation() else { - continue; - }; - let Expr::Name(name) = annotation else { - could_not_handle_all_usages = true; - continue; - }; + for reference_id in binding.references() { + let reference = semantic.reference(reference_id); + let range = reference.range(); - if name.id.as_str() == typevar_name { - let edit = Edit::range_replacement("Self".to_string(), annotation.range()); + if editable_range.contains_range(range) { + let edit = Edit::range_replacement(self_symbol_binding.to_string(), range); edits.push(edit); - } else { - could_not_handle_all_usages = true; } } - if could_not_handle_all_usages { - (edits, Applicability::DisplayOnly) - } else { - (edits, Applicability::Safe) - } + Some(edits) } fn remove_typevar_declaration(type_params: Option<&TypeParams>, name: &str) -> Option { diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI019_PYI019_0.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI019_PYI019_0.py.snap index 4d3519c350a57..33491f69592ea 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI019_PYI019_0.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI019_PYI019_0.py.snap @@ -207,15 +207,6 @@ PYI019_0.py:89:75: PYI019 Methods like `mixing_old_and_new_style_type_vars` shou | = help: Replace with `Self` -PYI019_0.py:102:40: PYI019 Methods like `m` should return `Self` instead of a custom `TypeVar` - | -100 | class UsesFullyQualifiedType: -101 | @classmethod -102 | def m[S](cls: builtins.type[S]) -> S: ... # PYI019 - | ^ PYI019 - | - = help: Replace with `Self` - PYI019_0.py:114:31: PYI019 Methods like `m` should return `Self` instead of a custom `TypeVar` | 112 | class SubscriptReturnType: diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI019_PYI019_0.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI019_PYI019_0.pyi.snap index 514c92089d06c..7ec7c02093378 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI019_PYI019_0.pyi.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI019_PYI019_0.pyi.snap @@ -207,15 +207,6 @@ PYI019_0.pyi:89:75: PYI019 Methods like `mixing_old_and_new_style_type_vars` sho | = help: Replace with `Self` -PYI019_0.pyi:102:40: PYI019 Methods like `m` should return `Self` instead of a custom `TypeVar` - | -100 | class UsesFullyQualifiedType: -101 | @classmethod -102 | def m[S](cls: builtins.type[S]) -> S: ... # PYI019 - | ^ PYI019 - | - = help: Replace with `Self` - PYI019_0.pyi:114:31: PYI019 Methods like `m` should return `Self` instead of a custom `TypeVar` | 112 | class SubscriptReturnType: @@ -232,3 +223,32 @@ PYI019_0.pyi:118:29: PYI019 Methods like `f` should return `Self` instead of a c | ^ PYI019 | = help: Replace with `Self` + +PYI019_0.pyi:122:100: PYI019 Methods like `mixing_and_nested` should return `Self` instead of a custom `TypeVar` + | +121 | class PEP695Again: +122 | def mixing_and_nested[T](self: _S695, a: list[_S695], b: dict[_S695, str | T | set[_S695]]) -> _S695: ... + | ^^^^^ PYI019 +123 | def also_uses_s695_but_should_not_be_edited(self, v: set[tuple[_S695]]) -> _S695: ... + | + = help: Replace with `Self` + +PYI019_0.pyi:132:10: PYI019 Methods like `comment_in_fix_range` should return `Self` instead of a custom `TypeVar` + | +130 | a: T, +131 | b: tuple[S, T] +132 | ) -> S: ... + | ^ PYI019 +133 | +134 | def comment_outside_fix_range[T, S]( + | + = help: Replace with `Self` + +PYI019_0.pyi:141:10: PYI019 Methods like `comment_outside_fix_range` should return `Self` instead of a custom `TypeVar` + | +139 | S, T +140 | ] +141 | ) -> S: ... + | ^ PYI019 + | + = help: Replace with `Self` diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__preview_PYI019_PYI019_0.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__preview_PYI019_PYI019_0.pyi.snap index 5ad6e45b69c0b..ed1d12eede0c6 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__preview_PYI019_PYI019_0.pyi.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__preview_PYI019_PYI019_0.pyi.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs --- -PYI019_0.pyi:7:62: PYI019 Methods like `__new__` should return `Self` instead of a custom `TypeVar` +PYI019_0.pyi:7:62: PYI019 [*] Methods like `__new__` should return `Self` instead of a custom `TypeVar` | 6 | class BadClass: 7 | def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: ... # PYI019 @@ -9,7 +9,7 @@ PYI019_0.pyi:7:62: PYI019 Methods like `__new__` should return `Self` instead of | = help: Replace with `Self` -ℹ Display-only fix +ℹ Safe fix 4 4 | _S2 = TypeVar("_S2", BadClass, GoodClass) 5 5 | 6 6 | class BadClass: @@ -19,14 +19,14 @@ PYI019_0.pyi:7:62: PYI019 Methods like `__new__` should return `Self` instead of 9 9 | 10 10 | def bad_instance_method(self: _S, arg: bytes) -> _S: ... # PYI019 -PYI019_0.pyi:10:54: PYI019 Methods like `bad_instance_method` should return `Self` instead of a custom `TypeVar` +PYI019_0.pyi:10:54: PYI019 [*] Methods like `bad_instance_method` should return `Self` instead of a custom `TypeVar` | 10 | def bad_instance_method(self: _S, arg: bytes) -> _S: ... # PYI019 | ^^ PYI019 | = help: Replace with `Self` -ℹ Display-only fix +ℹ Safe fix 7 7 | def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: ... # PYI019 8 8 | 9 9 | @@ -36,7 +36,7 @@ PYI019_0.pyi:10:54: PYI019 Methods like `bad_instance_method` should return `Sel 12 12 | 13 13 | @classmethod -PYI019_0.pyi:14:54: PYI019 Methods like `bad_class_method` should return `Self` instead of a custom `TypeVar` +PYI019_0.pyi:14:54: PYI019 [*] Methods like `bad_class_method` should return `Self` instead of a custom `TypeVar` | 13 | @classmethod 14 | def bad_class_method(cls: type[_S], arg: int) -> _S: ... # PYI019 @@ -44,7 +44,7 @@ PYI019_0.pyi:14:54: PYI019 Methods like `bad_class_method` should return `Self` | = help: Replace with `Self` -ℹ Display-only fix +ℹ Safe fix 11 11 | 12 12 | 13 13 | @classmethod @@ -72,7 +72,7 @@ PYI019_0.pyi:18:55: PYI019 [*] Methods like `bad_posonly_class_method` should re 20 20 | 21 21 | @classmethod -PYI019_0.pyi:39:63: PYI019 Methods like `__new__` should return `Self` instead of a custom `TypeVar` +PYI019_0.pyi:39:63: PYI019 [*] Methods like `__new__` should return `Self` instead of a custom `TypeVar` | 37 | # Python > 3.12 38 | class PEP695BadDunderNew[T]: @@ -81,7 +81,7 @@ PYI019_0.pyi:39:63: PYI019 Methods like `__new__` should return `Self` instead o | = help: Replace with `Self` -ℹ Display-only fix +ℹ Safe fix 36 36 | 37 37 | # Python > 3.12 38 38 | class PEP695BadDunderNew[T]: @@ -377,7 +377,7 @@ PYI019_0.pyi:85:81: PYI019 [*] Methods like `instance_method_unbound_with_anothe 87 87 | def multiple_type_vars[S, *Ts, T](self: S, other: S, /, *args: *Ts, a: T, b: list[T]) -> S: ... 88 88 | -PYI019_0.pyi:87:94: PYI019 Methods like `multiple_type_vars` should return `Self` instead of a custom `TypeVar` +PYI019_0.pyi:87:94: PYI019 [*] Methods like `multiple_type_vars` should return `Self` instead of a custom `TypeVar` | 85 | def instance_method_unbound_with_another_parameter[S](self: S, other: S) -> S: ... 86 | @@ -388,7 +388,7 @@ PYI019_0.pyi:87:94: PYI019 Methods like `multiple_type_vars` should return `Self | = help: Replace with `Self` -ℹ Display-only fix +ℹ Safe fix 84 84 | 85 85 | def instance_method_unbound_with_another_parameter[S](self: S, other: S) -> S: ... 86 86 | @@ -398,7 +398,7 @@ PYI019_0.pyi:87:94: PYI019 Methods like `multiple_type_vars` should return `Self 89 89 | def mixing_old_and_new_style_type_vars[T](self: _S695, a: T, b: T) -> _S695: ... 90 90 | -PYI019_0.pyi:89:75: PYI019 Methods like `mixing_old_and_new_style_type_vars` should return `Self` instead of a custom `TypeVar` +PYI019_0.pyi:89:75: PYI019 [*] Methods like `mixing_old_and_new_style_type_vars` should return `Self` instead of a custom `TypeVar` | 87 | def multiple_type_vars[S, *Ts, T](self: S, other: S, /, *args: *Ts, a: T, b: list[T]) -> S: ... 88 | @@ -407,7 +407,7 @@ PYI019_0.pyi:89:75: PYI019 Methods like `mixing_old_and_new_style_type_vars` sho | = help: Replace with `Self` -ℹ Display-only fix +ℹ Safe fix 86 86 | 87 87 | def multiple_type_vars[S, *Ts, T](self: S, other: S, /, *args: *Ts, a: T, b: list[T]) -> S: ... 88 88 | @@ -417,25 +417,6 @@ PYI019_0.pyi:89:75: PYI019 Methods like `mixing_old_and_new_style_type_vars` sho 91 91 | 92 92 | class InvalidButWeDoNotPanic: -PYI019_0.pyi:102:40: PYI019 [*] Methods like `m` should return `Self` instead of a custom `TypeVar` - | -100 | class UsesFullyQualifiedType: -101 | @classmethod -102 | def m[S](cls: builtins.type[S]) -> S: ... # PYI019 - | ^ PYI019 - | - = help: Replace with `Self` - -ℹ Safe fix -99 99 | -100 100 | class UsesFullyQualifiedType: -101 101 | @classmethod -102 |- def m[S](cls: builtins.type[S]) -> S: ... # PYI019 - 102 |+ def m(cls) -> Self: ... # PYI019 -103 103 | -104 104 | -105 105 | def shadowed_type(): - PYI019_0.pyi:114:31: PYI019 Methods like `m` should return `Self` instead of a custom `TypeVar` | 112 | class SubscriptReturnType: @@ -459,3 +440,81 @@ PYI019_0.pyi:118:29: PYI019 [*] Methods like `f` should return `Self` instead of 117 117 | class PEP695TypeParameterAtTheVeryEndOfTheList: 118 |- def f[T, S](self: S) -> S: ... 118 |+ def f[T](self) -> Self: ... +119 119 | +120 120 | +121 121 | class PEP695Again: + +PYI019_0.pyi:122:100: PYI019 [*] Methods like `mixing_and_nested` should return `Self` instead of a custom `TypeVar` + | +121 | class PEP695Again: +122 | def mixing_and_nested[T](self: _S695, a: list[_S695], b: dict[_S695, str | T | set[_S695]]) -> _S695: ... + | ^^^^^ PYI019 +123 | def also_uses_s695_but_should_not_be_edited(self, v: set[tuple[_S695]]) -> _S695: ... + | + = help: Replace with `Self` + +ℹ Safe fix +119 119 | +120 120 | +121 121 | class PEP695Again: +122 |- def mixing_and_nested[T](self: _S695, a: list[_S695], b: dict[_S695, str | T | set[_S695]]) -> _S695: ... + 122 |+ def mixing_and_nested[T](self, a: list[Self], b: dict[Self, str | T | set[Self]]) -> Self: ... +123 123 | def also_uses_s695_but_should_not_be_edited(self, v: set[tuple[_S695]]) -> _S695: ... +124 124 | +125 125 | @classmethod + +PYI019_0.pyi:132:10: PYI019 [*] Methods like `comment_in_fix_range` should return `Self` instead of a custom `TypeVar` + | +130 | a: T, +131 | b: tuple[S, T] +132 | ) -> S: ... + | ^ PYI019 +133 | +134 | def comment_outside_fix_range[T, S]( + | + = help: Replace with `Self` + +ℹ Unsafe fix +123 123 | def also_uses_s695_but_should_not_be_edited(self, v: set[tuple[_S695]]) -> _S695: ... +124 124 | +125 125 | @classmethod +126 |- def comment_in_fix_range[T, S]( +127 |- cls: type[ # Lorem ipsum +128 |- S +129 |- ], + 126 |+ def comment_in_fix_range[T]( + 127 |+ cls, +130 128 | a: T, +131 |- b: tuple[S, T] +132 |- ) -> S: ... + 129 |+ b: tuple[Self, T] + 130 |+ ) -> Self: ... +133 131 | +134 132 | def comment_outside_fix_range[T, S]( +135 133 | self: S, + +PYI019_0.pyi:141:10: PYI019 [*] Methods like `comment_outside_fix_range` should return `Self` instead of a custom `TypeVar` + | +139 | S, T +140 | ] +141 | ) -> S: ... + | ^ PYI019 + | + = help: Replace with `Self` + +ℹ Safe fix +131 131 | b: tuple[S, T] +132 132 | ) -> S: ... +133 133 | +134 |- def comment_outside_fix_range[T, S]( +135 |- self: S, + 134 |+ def comment_outside_fix_range[T]( + 135 |+ self, +136 136 | a: T, +137 137 | b: tuple[ +138 138 | # Lorem ipsum +139 |- S, T + 139 |+ Self, T +140 140 | ] +141 |- ) -> S: ... + 141 |+ ) -> Self: ... diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 4ef240e238b1b..1803aeff7a8c5 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -265,13 +265,22 @@ impl<'a> SemanticModel<'a> { self.shadowed_bindings.get(&binding_id).copied() } - /// Return `true` if `member` is bound as a builtin. + /// Return `true` if `member` is bound as a builtin *in the scope we are currently visiting*. /// /// Note that a "builtin binding" does *not* include explicit lookups via the `builtins` /// module, e.g. `import builtins; builtins.open`. It *only* includes the bindings /// that are pre-populated in Python's global scope before any imports have taken place. pub fn has_builtin_binding(&self, member: &str) -> bool { - self.lookup_symbol(member) + self.has_builtin_binding_in_scope(member, self.scope_id) + } + + /// Return `true` if `member` is bound as a builtin *in a given scope*. + /// + /// Note that a "builtin binding" does *not* include explicit lookups via the `builtins` + /// module, e.g. `import builtins; builtins.open`. It *only* includes the bindings + /// that are pre-populated in Python's global scope before any imports have taken place. + pub fn has_builtin_binding_in_scope(&self, member: &str, scope: ScopeId) -> bool { + self.lookup_symbol_in_scope(member, scope, false) .map(|binding_id| &self.bindings[binding_id]) .is_some_and(|binding| binding.kind.is_builtin()) }