diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 814f2d9f38fab..5c470531f7034 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -405,6 +405,22 @@ impl<'a> Checker<'a> { .map(|node_id| IsolationLevel::Group(node_id.into())) .unwrap_or_default() } + + /// A thin wrapper over [`ruff_python_parser::typing::parse_type_annotation`] + /// that returns `Some(annotation)` only if the annotation is valid Python syntax. + /// + /// In the future this function could possibly add caching to improve performance. + pub(crate) fn parse_string_annotation( + &self, + annotation: &ast::ExprStringLiteral, + ) -> Option<(Parsed, AnnotationKind)> { + let (parsed_annotation, kind) = + parse_type_annotation(annotation, self.locator.contents()).ok()?; + if !parsed_annotation.errors().is_empty() { + return None; + } + Some((parsed_annotation, kind)) + } } impl<'a> Visitor<'a> for Checker<'a> { @@ -2176,9 +2192,7 @@ impl<'a> Checker<'a> { while !self.visit.string_type_definitions.is_empty() { let type_definitions = std::mem::take(&mut self.visit.string_type_definitions); for (string_expr, snapshot) in type_definitions { - if let Ok((parsed_annotation, kind)) = - parse_type_annotation(string_expr, self.locator.contents()) - { + if let Some((parsed_annotation, kind)) = self.parse_string_annotation(string_expr) { let parsed_annotation = allocator.alloc(parsed_annotation); self.parsed_type_annotation = Some(parsed_annotation); diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs index f193646c8724a..80ccb97fd9f2e 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs @@ -79,7 +79,7 @@ pub(crate) fn any_eq_ne_annotation(checker: &mut Checker, name: &str, parameters return; } - if !match_maybe_stringized_annotation(annotation, checker.locator(), |expr| { + if !match_maybe_stringized_annotation(annotation, checker, |expr| { semantic.match_typing_expr(expr, "Any") }) { return; diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/helpers.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/helpers.rs index 28ee42a4083cb..d21531733e2d8 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/helpers.rs @@ -1,6 +1,6 @@ use ruff_python_ast as ast; -use ruff_python_parser::typing::parse_type_annotation; -use ruff_source_file::Locator; + +use crate::checkers::ast::Checker; /// Apply a test to an annotation expression, /// abstracting over the fact that the annotation expression might be "stringized". @@ -9,18 +9,14 @@ use ruff_source_file::Locator; /// `foo: "typing.Any"` means the same thing to a type checker as `foo: typing.Any`. pub(super) fn match_maybe_stringized_annotation( expr: &ast::Expr, - locator: &Locator, + checker: &Checker, match_fn: impl FnOnce(&ast::Expr) -> bool, ) -> bool { if let ast::Expr::StringLiteral(string_annotation) = expr { - let Ok((parsed_annotation, _)) = - parse_type_annotation(string_annotation, locator.contents()) + let Some((parsed_annotation, _)) = checker.parse_string_annotation(string_annotation) else { return false; }; - if !parsed_annotation.errors().is_empty() { - return false; - } let ast::ModExpression { body: annotation, .. } = parsed_annotation.into_syntax(); diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs index 944a73efa0d03..d23c2dbecab2a 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs @@ -3,8 +3,6 @@ use std::fmt; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast as ast; -use ruff_python_semantic::SemanticModel; -use ruff_source_file::Locator; use ruff_text_size::Ranged; use super::helpers::match_maybe_stringized_annotation; @@ -67,7 +65,7 @@ pub(crate) fn no_return_argument_annotation(checker: &mut Checker, parameters: & .iter() .filter_map(ast::AnyParameterRef::annotation) { - if is_no_return(annotation, checker.semantic(), checker.locator()) { + if is_no_return(annotation, checker) { checker.diagnostics.push(Diagnostic::new( NoReturnArgumentAnnotationInStub { module: if checker.settings.target_version >= Py311 { @@ -82,9 +80,9 @@ pub(crate) fn no_return_argument_annotation(checker: &mut Checker, parameters: & } } -fn is_no_return(expr: &ast::Expr, semantic: &SemanticModel, locator: &Locator) -> bool { - match_maybe_stringized_annotation(expr, locator, |expr| { - semantic.match_typing_expr(expr, "NoReturn") +fn is_no_return(expr: &ast::Expr, checker: &Checker) -> bool { + match_maybe_stringized_annotation(expr, checker, |expr| { + checker.semantic().match_typing_expr(expr, "NoReturn") }) } diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/non_self_return_type.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/non_self_return_type.rs index b9c5e4d767e04..8209a1ebb0c20 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/non_self_return_type.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/non_self_return_type.rs @@ -7,7 +7,6 @@ use ruff_python_ast::identifier::Identifier; use ruff_python_semantic::analyze; use ruff_python_semantic::analyze::visibility::{is_abstract, is_final, is_overload}; use ruff_python_semantic::{ScopeKind, SemanticModel}; -use ruff_source_file::Locator; use super::helpers::match_maybe_stringized_annotation; use crate::checkers::ast::Checker; @@ -158,7 +157,7 @@ pub(crate) fn non_self_return_type( // In-place methods that are expected to return `Self`. if is_inplace_bin_op(name) { - if !is_self(returns, semantic, checker.locator()) { + if !is_self(returns, checker) { checker.diagnostics.push(Diagnostic::new( NonSelfReturnType { class_name: class_def.name.to_string(), @@ -243,9 +242,10 @@ fn is_name(expr: &Expr, name: &str) -> bool { id.as_str() == name } -fn is_self(expr: &Expr, semantic: &SemanticModel, locator: &Locator) -> bool { - match_maybe_stringized_annotation(expr, locator, |expr| { - semantic.match_typing_expr(expr, "Self") +/// Return `true` if the given expression resolves to `typing.Self`. +fn is_self(expr: &Expr, checker: &Checker) -> bool { + match_maybe_stringized_annotation(expr, checker, |expr| { + checker.semantic().match_typing_expr(expr, "Self") }) }