From 517499cea4d2a2152e12fe55ea64729202de08d9 Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Tue, 29 Aug 2023 20:25:03 +0200 Subject: [PATCH] Move `refurb/helpers` utils to `ruff_python_semantic` for broader use --- crates/ruff/src/rules/refurb/helpers.rs | 193 ----------------- crates/ruff/src/rules/refurb/mod.rs | 1 - .../refurb/rules/check_and_remove_from_set.rs | 2 +- .../rules/refurb/rules/delete_full_slice.rs | 2 +- .../src/rules/refurb/rules/repeated_append.rs | 2 +- .../src/analyze/typing.rs | 196 +++++++++++++++++- 6 files changed, 197 insertions(+), 199 deletions(-) delete mode 100644 crates/ruff/src/rules/refurb/helpers.rs diff --git a/crates/ruff/src/rules/refurb/helpers.rs b/crates/ruff/src/rules/refurb/helpers.rs deleted file mode 100644 index c6523858f46a1..0000000000000 --- a/crates/ruff/src/rules/refurb/helpers.rs +++ /dev/null @@ -1,193 +0,0 @@ -use ast::{ParameterWithDefault, Parameters}; -use ruff_python_ast::helpers::map_subscript; -use ruff_python_ast::{self as ast, Expr, Stmt}; -use ruff_python_semantic::analyze::type_inference::{PythonType, ResolvedPythonType}; -use ruff_python_semantic::{Binding, BindingKind, SemanticModel}; -use ruff_text_size::Ranged; - -/// Abstraction for a type checker, conservatively checks for the intended type(s). -trait TypeChecker { - /// Check annotation expression to match the intended type(s). - fn match_annotation(annotation: &Expr, semantic: &SemanticModel) -> bool; - /// Check initializer expression to match the intended type(s). - fn match_initializer(initializer: &Expr, semantic: &SemanticModel) -> bool; -} - -/// Check if the type checker accepts the given binding with the given name. -/// -/// NOTE: this function doesn't perform more serious type inference, so it won't be able -/// to understand if the value gets initialized from a call to a function always returning -/// lists. This also implies no interfile analysis. -fn check_type(binding: &Binding, semantic: &SemanticModel) -> bool { - match binding.kind { - BindingKind::Assignment => match binding.statement(semantic) { - // ```python - // x = init_expr - // ``` - // - // The type checker might know how to infer the type based on `init_expr`. - Some(Stmt::Assign(ast::StmtAssign { value, .. })) => { - T::match_initializer(value.as_ref(), semantic) - } - - // ```python - // x: annotation = some_expr - // ``` - // - // In this situation, we check only the annotation. - Some(Stmt::AnnAssign(ast::StmtAnnAssign { annotation, .. })) => { - T::match_annotation(annotation.as_ref(), semantic) - } - _ => false, - }, - - BindingKind::Argument => match binding.statement(semantic) { - // ```python - // def foo(x: annotation): - // ... - // ``` - // - // We trust the annotation and see if the type checker matches the annotation. - Some(Stmt::FunctionDef(ast::StmtFunctionDef { parameters, .. })) => { - let Some(parameter) = find_parameter(parameters.as_ref(), binding) else { - return false; - }; - let Some(ref annotation) = parameter.parameter.annotation else { - return false; - }; - T::match_annotation(annotation.as_ref(), semantic) - } - _ => false, - }, - - BindingKind::Annotation => match binding.statement(semantic) { - // ```python - // x: annotation - // ``` - // - // It's a typed declaration, type annotation is the only source of information. - Some(Stmt::AnnAssign(ast::StmtAnnAssign { annotation, .. })) => { - T::match_annotation(annotation.as_ref(), semantic) - } - _ => false, - }, - - _ => false, - } -} - -/// Type checker for builtin types. -trait BuiltinTypeChecker { - /// Builtin type name. - const BUILTIN_TYPE_NAME: &'static str; - /// Type name as found in the `Typing` module. - const TYPING_NAME: &'static str; - /// [`PythonType`] associated with the intended type. - const EXPR_TYPE: PythonType; - - /// Check annotation expression to match the intended type. - fn match_annotation(annotation: &Expr, semantic: &SemanticModel) -> bool { - let value = map_subscript(annotation); - Self::match_builtin_type(value, semantic) - || semantic.match_typing_expr(value, Self::TYPING_NAME) - } - - /// Check initializer expression to match the intended type. - fn match_initializer(initializer: &Expr, semantic: &SemanticModel) -> bool { - Self::match_expr_type(initializer) || Self::match_builtin_constructor(initializer, semantic) - } - - /// Check if the type can be inferred from the given expression. - fn match_expr_type(initializer: &Expr) -> bool { - let init_type: ResolvedPythonType = initializer.into(); - match init_type { - ResolvedPythonType::Atom(atom) => atom == Self::EXPR_TYPE, - _ => false, - } - } - - /// Check if the given expression corresponds to a constructor call of the builtin type. - fn match_builtin_constructor(initializer: &Expr, semantic: &SemanticModel) -> bool { - let Expr::Call(ast::ExprCall { func, .. }) = initializer else { - return false; - }; - Self::match_builtin_type(func.as_ref(), semantic) - } - - /// Check if the given expression names the builtin type. - fn match_builtin_type(type_expr: &Expr, semantic: &SemanticModel) -> bool { - let Expr::Name(ast::ExprName { id, .. }) = type_expr else { - return false; - }; - id == Self::BUILTIN_TYPE_NAME && semantic.is_builtin(Self::BUILTIN_TYPE_NAME) - } -} - -impl TypeChecker for T { - fn match_annotation(annotation: &Expr, semantic: &SemanticModel) -> bool { - ::match_annotation(annotation, semantic) - } - - fn match_initializer(initializer: &Expr, semantic: &SemanticModel) -> bool { - ::match_initializer(initializer, semantic) - } -} - -struct ListChecker; - -impl BuiltinTypeChecker for ListChecker { - const BUILTIN_TYPE_NAME: &'static str = "list"; - const TYPING_NAME: &'static str = "List"; - const EXPR_TYPE: PythonType = PythonType::List; -} - -struct DictChecker; - -impl BuiltinTypeChecker for DictChecker { - const BUILTIN_TYPE_NAME: &'static str = "dict"; - const TYPING_NAME: &'static str = "Dict"; - const EXPR_TYPE: PythonType = PythonType::Dict; -} - -struct SetChecker; - -impl BuiltinTypeChecker for SetChecker { - const BUILTIN_TYPE_NAME: &'static str = "set"; - const TYPING_NAME: &'static str = "Set"; - const EXPR_TYPE: PythonType = PythonType::Set; -} - -/// Test whether the given binding (and the given name) can be considered a list. -/// For this, we check what value might be associated with it through it's initialization and -/// what annotation it has (we consider `list` and `typing.List`). -pub(super) fn is_list(binding: &Binding, semantic: &SemanticModel) -> bool { - check_type::(binding, semantic) -} - -/// Test whether the given binding (and the given name) can be considered a dictionary. -/// For this, we check what value might be associated with it through it's initialization and -/// what annotation it has (we consider `dict` and `typing.Dict`). -pub(super) fn is_dict(binding: &Binding, semantic: &SemanticModel) -> bool { - check_type::(binding, semantic) -} - -/// Test whether the given binding (and the given name) can be considered a set. -/// For this, we check what value might be associated with it through it's initialization and -/// what annotation it has (we consider `set` and `typing.Set`). -pub(super) fn is_set(binding: &Binding, semantic: &SemanticModel) -> bool { - check_type::(binding, semantic) -} - -/// Find the [`ParameterWithDefault`] corresponding to the given [`Binding`]. -#[inline] -fn find_parameter<'a>( - parameters: &'a Parameters, - binding: &Binding, -) -> Option<&'a ParameterWithDefault> { - parameters - .args - .iter() - .chain(parameters.posonlyargs.iter()) - .chain(parameters.kwonlyargs.iter()) - .find(|arg| arg.parameter.name.range() == binding.range()) -} diff --git a/crates/ruff/src/rules/refurb/mod.rs b/crates/ruff/src/rules/refurb/mod.rs index 1dddb615fbf2a..2e5e5a59d9f18 100644 --- a/crates/ruff/src/rules/refurb/mod.rs +++ b/crates/ruff/src/rules/refurb/mod.rs @@ -1,6 +1,5 @@ //! Rules from [refurb](https://pypi.org/project/refurb/)/ -mod helpers; pub(crate) mod rules; #[cfg(test)] diff --git a/crates/ruff/src/rules/refurb/rules/check_and_remove_from_set.rs b/crates/ruff/src/rules/refurb/rules/check_and_remove_from_set.rs index e943d7c895e27..80685be3f3189 100644 --- a/crates/ruff/src/rules/refurb/rules/check_and_remove_from_set.rs +++ b/crates/ruff/src/rules/refurb/rules/check_and_remove_from_set.rs @@ -4,12 +4,12 @@ use ruff_python_ast::comparable::ComparableExpr; use ruff_python_ast::helpers::contains_effect; use ruff_python_ast::{self as ast, CmpOp, Expr, Stmt}; use ruff_python_codegen::Generator; +use ruff_python_semantic::analyze::typing::is_set; use ruff_text_size::{Ranged, TextRange}; use crate::autofix::snippet::SourceCodeSnippet; use crate::checkers::ast::Checker; use crate::registry::AsRule; -use crate::rules::refurb::helpers::is_set; /// ## What it does /// Checks for uses of `set#remove` that can be replaced with `set#discard`. diff --git a/crates/ruff/src/rules/refurb/rules/delete_full_slice.rs b/crates/ruff/src/rules/refurb/rules/delete_full_slice.rs index 7b2c1e7ec730a..b284a744ae571 100644 --- a/crates/ruff/src/rules/refurb/rules/delete_full_slice.rs +++ b/crates/ruff/src/rules/refurb/rules/delete_full_slice.rs @@ -2,12 +2,12 @@ use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{self as ast, Expr}; use ruff_python_codegen::Generator; +use ruff_python_semantic::analyze::typing::{is_dict, is_list}; use ruff_python_semantic::{Binding, SemanticModel}; use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; use crate::registry::AsRule; -use crate::rules::refurb::helpers::{is_dict, is_list}; /// ## What it does /// Checks for `del` statements that delete the entire slice of a list or diff --git a/crates/ruff/src/rules/refurb/rules/repeated_append.rs b/crates/ruff/src/rules/refurb/rules/repeated_append.rs index 0deb6e2ed9b01..8557138f04259 100644 --- a/crates/ruff/src/rules/refurb/rules/repeated_append.rs +++ b/crates/ruff/src/rules/refurb/rules/repeated_append.rs @@ -5,13 +5,13 @@ use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{self as ast, Expr, Stmt}; use ruff_python_codegen::Generator; +use ruff_python_semantic::analyze::typing::is_list; use ruff_python_semantic::{Binding, BindingId, DefinitionId, SemanticModel}; use ruff_text_size::{Ranged, TextRange}; use crate::autofix::snippet::SourceCodeSnippet; use crate::checkers::ast::Checker; use crate::registry::AsRule; -use crate::rules::refurb::helpers::is_list; /// ## What it does /// Checks for consecutive calls to `append`. diff --git a/crates/ruff_python_semantic/src/analyze/typing.rs b/crates/ruff_python_semantic/src/analyze/typing.rs index b2c4b52f3523a..7c75dbb1813bc 100644 --- a/crates/ruff_python_semantic/src/analyze/typing.rs +++ b/crates/ruff_python_semantic/src/analyze/typing.rs @@ -1,16 +1,21 @@ //! Analysis rules for the `typing` module. use num_traits::identities::Zero; -use ruff_python_ast::{self as ast, Constant, Expr, Operator}; +use ruff_python_ast::{ + self as ast, Constant, Expr, Operator, ParameterWithDefault, Parameters, Stmt, +}; +use crate::analyze::type_inference::{PythonType, ResolvedPythonType}; +use crate::{Binding, BindingKind}; use ruff_python_ast::call_path::{from_qualified_name, from_unqualified_name, CallPath}; -use ruff_python_ast::helpers::is_const_false; +use ruff_python_ast::helpers::{is_const_false, map_subscript}; use ruff_python_stdlib::typing::{ as_pep_585_generic, has_pep_585_generic, is_immutable_generic_type, is_immutable_non_generic_type, is_immutable_return_type, is_literal_member, is_mutable_return_type, is_pep_593_generic_member, is_pep_593_generic_type, is_standard_library_generic, is_standard_library_generic_member, is_standard_library_literal, }; +use ruff_text_size::Ranged; use crate::model::SemanticModel; @@ -312,3 +317,190 @@ pub fn is_type_checking_block(stmt: &ast::StmtIf, semantic: &SemanticModel) -> b false } + +/// Abstraction for a type checker, conservatively checks for the intended type(s). +trait TypeChecker { + /// Check annotation expression to match the intended type(s). + fn match_annotation(annotation: &Expr, semantic: &SemanticModel) -> bool; + /// Check initializer expression to match the intended type(s). + fn match_initializer(initializer: &Expr, semantic: &SemanticModel) -> bool; +} + +/// Check if the type checker accepts the given binding with the given name. +/// +/// NOTE: this function doesn't perform more serious type inference, so it won't be able +/// to understand if the value gets initialized from a call to a function always returning +/// lists. This also implies no interfile analysis. +fn check_type(binding: &Binding, semantic: &SemanticModel) -> bool { + match binding.kind { + BindingKind::Assignment => match binding.statement(semantic) { + // ```python + // x = init_expr + // ``` + // + // The type checker might know how to infer the type based on `init_expr`. + Some(Stmt::Assign(ast::StmtAssign { value, .. })) => { + T::match_initializer(value.as_ref(), semantic) + } + + // ```python + // x: annotation = some_expr + // ``` + // + // In this situation, we check only the annotation. + Some(Stmt::AnnAssign(ast::StmtAnnAssign { annotation, .. })) => { + T::match_annotation(annotation.as_ref(), semantic) + } + _ => false, + }, + + BindingKind::Argument => match binding.statement(semantic) { + // ```python + // def foo(x: annotation): + // ... + // ``` + // + // We trust the annotation and see if the type checker matches the annotation. + Some(Stmt::FunctionDef(ast::StmtFunctionDef { parameters, .. })) => { + let Some(parameter) = find_parameter(parameters.as_ref(), binding) else { + return false; + }; + let Some(ref annotation) = parameter.parameter.annotation else { + return false; + }; + T::match_annotation(annotation.as_ref(), semantic) + } + _ => false, + }, + + BindingKind::Annotation => match binding.statement(semantic) { + // ```python + // x: annotation + // ``` + // + // It's a typed declaration, type annotation is the only source of information. + Some(Stmt::AnnAssign(ast::StmtAnnAssign { annotation, .. })) => { + T::match_annotation(annotation.as_ref(), semantic) + } + _ => false, + }, + + _ => false, + } +} + +/// Type checker for builtin types. +trait BuiltinTypeChecker { + /// Builtin type name. + const BUILTIN_TYPE_NAME: &'static str; + /// Type name as found in the `Typing` module. + const TYPING_NAME: &'static str; + /// [`PythonType`] associated with the intended type. + const EXPR_TYPE: PythonType; + + /// Check annotation expression to match the intended type. + fn match_annotation(annotation: &Expr, semantic: &SemanticModel) -> bool { + let value = map_subscript(annotation); + Self::match_builtin_type(value, semantic) + || semantic.match_typing_expr(value, Self::TYPING_NAME) + } + + /// Check initializer expression to match the intended type. + fn match_initializer(initializer: &Expr, semantic: &SemanticModel) -> bool { + Self::match_expr_type(initializer) || Self::match_builtin_constructor(initializer, semantic) + } + + /// Check if the type can be inferred from the given expression. + fn match_expr_type(initializer: &Expr) -> bool { + let init_type: ResolvedPythonType = initializer.into(); + match init_type { + ResolvedPythonType::Atom(atom) => atom == Self::EXPR_TYPE, + _ => false, + } + } + + /// Check if the given expression corresponds to a constructor call of the builtin type. + fn match_builtin_constructor(initializer: &Expr, semantic: &SemanticModel) -> bool { + let Expr::Call(ast::ExprCall { func, .. }) = initializer else { + return false; + }; + Self::match_builtin_type(func.as_ref(), semantic) + } + + /// Check if the given expression names the builtin type. + fn match_builtin_type(type_expr: &Expr, semantic: &SemanticModel) -> bool { + let Expr::Name(ast::ExprName { id, .. }) = type_expr else { + return false; + }; + id == Self::BUILTIN_TYPE_NAME && semantic.is_builtin(Self::BUILTIN_TYPE_NAME) + } +} + +impl TypeChecker for T { + fn match_annotation(annotation: &Expr, semantic: &SemanticModel) -> bool { + ::match_annotation(annotation, semantic) + } + + fn match_initializer(initializer: &Expr, semantic: &SemanticModel) -> bool { + ::match_initializer(initializer, semantic) + } +} + +struct ListChecker; + +impl BuiltinTypeChecker for ListChecker { + const BUILTIN_TYPE_NAME: &'static str = "list"; + const TYPING_NAME: &'static str = "List"; + const EXPR_TYPE: PythonType = PythonType::List; +} + +struct DictChecker; + +impl BuiltinTypeChecker for DictChecker { + const BUILTIN_TYPE_NAME: &'static str = "dict"; + const TYPING_NAME: &'static str = "Dict"; + const EXPR_TYPE: PythonType = PythonType::Dict; +} + +struct SetChecker; + +impl BuiltinTypeChecker for SetChecker { + const BUILTIN_TYPE_NAME: &'static str = "set"; + const TYPING_NAME: &'static str = "Set"; + const EXPR_TYPE: PythonType = PythonType::Set; +} + +/// Test whether the given binding (and the given name) can be considered a list. +/// For this, we check what value might be associated with it through it's initialization and +/// what annotation it has (we consider `list` and `typing.List`). +pub fn is_list(binding: &Binding, semantic: &SemanticModel) -> bool { + check_type::(binding, semantic) +} + +/// Test whether the given binding (and the given name) can be considered a dictionary. +/// For this, we check what value might be associated with it through it's initialization and +/// what annotation it has (we consider `dict` and `typing.Dict`). +pub fn is_dict(binding: &Binding, semantic: &SemanticModel) -> bool { + check_type::(binding, semantic) +} + +/// Test whether the given binding (and the given name) can be considered a set. +/// For this, we check what value might be associated with it through it's initialization and +/// what annotation it has (we consider `set` and `typing.Set`). +pub fn is_set(binding: &Binding, semantic: &SemanticModel) -> bool { + check_type::(binding, semantic) +} + +/// Find the [`ParameterWithDefault`] corresponding to the given [`Binding`]. +#[inline] +fn find_parameter<'a>( + parameters: &'a Parameters, + binding: &Binding, +) -> Option<&'a ParameterWithDefault> { + parameters + .args + .iter() + .chain(parameters.posonlyargs.iter()) + .chain(parameters.kwonlyargs.iter()) + .find(|arg| arg.parameter.name.range() == binding.range()) +}