From 00fbbe4223d0cbfc3014403ace659f48368e1852 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 3 Jul 2023 12:29:59 -0400 Subject: [PATCH] Remove some additional manual iterator matches (#5482) ## Summary I've done a few of these PRs, I thought I'd caught them all, but missed this pattern. --- crates/ruff/src/checkers/ast/mod.rs | 5 +- .../rules/request_with_no_cert_validation.rs | 30 +++--------- .../rules/nullable_model_string_field.rs | 20 +++----- .../src/analyze/function_type.rs | 35 +++++++------- crates/ruff_python_stdlib/src/identifiers.rs | 8 ++-- crates/ruff_python_stdlib/src/keyword.rs | 46 ++++++++++++++++--- 6 files changed, 75 insertions(+), 69 deletions(-) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 0d8a631558..7abc449cdb 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -644,10 +644,7 @@ where }, ) => { if self.enabled(Rule::DjangoNullableModelStringField) { - self.diagnostics - .extend(flake8_django::rules::nullable_model_string_field( - self, body, - )); + flake8_django::rules::nullable_model_string_field(self, body); } if self.enabled(Rule::DjangoExcludeWithModelForm) { if let Some(diagnostic) = diff --git a/crates/ruff/src/rules/flake8_bandit/rules/request_with_no_cert_validation.rs b/crates/ruff/src/rules/flake8_bandit/rules/request_with_no_cert_validation.rs index b03020e7eb..27c22af441 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/request_with_no_cert_validation.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/request_with_no_cert_validation.rs @@ -21,21 +21,6 @@ impl Violation for RequestWithNoCertValidation { } } -const REQUESTS_HTTP_VERBS: [&str; 7] = ["get", "options", "head", "post", "put", "patch", "delete"]; -const HTTPX_METHODS: [&str; 11] = [ - "get", - "options", - "head", - "post", - "put", - "patch", - "delete", - "request", - "stream", - "Client", - "AsyncClient", -]; - /// S501 pub(crate) fn request_with_no_cert_validation( checker: &mut Checker, @@ -46,16 +31,13 @@ pub(crate) fn request_with_no_cert_validation( if let Some(target) = checker .semantic() .resolve_call_path(func) - .and_then(|call_path| { - if call_path.len() == 2 { - if call_path[0] == "requests" && REQUESTS_HTTP_VERBS.contains(&call_path[1]) { - return Some("requests"); - } - if call_path[0] == "httpx" && HTTPX_METHODS.contains(&call_path[1]) { - return Some("httpx"); - } + .and_then(|call_path| match call_path.as_slice() { + ["requests", "get" | "options" | "head" | "post" | "put" | "patch" | "delete"] => { + Some("requests") } - None + ["httpx", "get" | "options" | "head" | "post" | "put" | "patch" | "delete" | "request" + | "stream" | "Client" | "AsyncClient"] => Some("httpx"), + _ => None, }) { let call_args = SimpleCallArgs::new(args, keywords); diff --git a/crates/ruff/src/rules/flake8_django/rules/nullable_model_string_field.rs b/crates/ruff/src/rules/flake8_django/rules/nullable_model_string_field.rs index 6da8d4ba50..b74bf19a8c 100644 --- a/crates/ruff/src/rules/flake8_django/rules/nullable_model_string_field.rs +++ b/crates/ruff/src/rules/flake8_django/rules/nullable_model_string_field.rs @@ -51,24 +51,14 @@ impl Violation for DjangoNullableModelStringField { } } -const NOT_NULL_TRUE_FIELDS: [&str; 6] = [ - "CharField", - "TextField", - "SlugField", - "EmailField", - "FilePathField", - "URLField", -]; - /// DJ001 -pub(crate) fn nullable_model_string_field(checker: &Checker, body: &[Stmt]) -> Vec { - let mut errors = Vec::new(); +pub(crate) fn nullable_model_string_field(checker: &mut Checker, body: &[Stmt]) { for statement in body.iter() { let Stmt::Assign(ast::StmtAssign { value, .. }) = statement else { continue; }; if let Some(field_name) = is_nullable_field(checker, value) { - errors.push(Diagnostic::new( + checker.diagnostics.push(Diagnostic::new( DjangoNullableModelStringField { field_name: field_name.to_string(), }, @@ -76,7 +66,6 @@ pub(crate) fn nullable_model_string_field(checker: &Checker, body: &[Stmt]) -> V )); } } - errors } fn is_nullable_field<'a>(checker: &'a Checker, value: &'a Expr) -> Option<&'a str> { @@ -88,7 +77,10 @@ fn is_nullable_field<'a>(checker: &'a Checker, value: &'a Expr) -> Option<&'a st return None; }; - if !NOT_NULL_TRUE_FIELDS.contains(&valid_field_name) { + if !matches!( + valid_field_name, + "CharField" | "TextField" | "SlugField" | "EmailField" | "FilePathField" | "URLField" + ) { return None; } diff --git a/crates/ruff_python_semantic/src/analyze/function_type.rs b/crates/ruff_python_semantic/src/analyze/function_type.rs index 63f9b8ce71..95b0f3845e 100644 --- a/crates/ruff_python_semantic/src/analyze/function_type.rs +++ b/crates/ruff_python_semantic/src/analyze/function_type.rs @@ -6,10 +6,7 @@ use ruff_python_ast::helpers::map_callable; use crate::model::SemanticModel; use crate::scope::{Scope, ScopeKind}; -const CLASS_METHODS: [&str; 3] = ["__new__", "__init_subclass__", "__class_getitem__"]; -const METACLASS_BASES: [(&str, &str); 2] = [("", "type"), ("abc", "ABCMeta")]; - -#[derive(Copy, Clone)] +#[derive(Debug, Copy, Clone)] pub enum FunctionType { Function, Method, @@ -44,24 +41,28 @@ pub fn classify( }) }) { FunctionType::StaticMethod - } else if CLASS_METHODS.contains(&name) - // Special-case class method, like `__new__`. + } else if matches!(name, "__new__" | "__init_subclass__" | "__class_getitem__") + // Special-case class method, like `__new__`. || scope.bases.iter().any(|expr| { // The class itself extends a known metaclass, so all methods are class methods. - semantic.resolve_call_path(map_callable(expr)).map_or(false, |call_path| { - METACLASS_BASES - .iter() - .any(|(module, member)| call_path.as_slice() == [*module, *member]) - }) + semantic + .resolve_call_path(map_callable(expr)) + .map_or(false, |call_path| { + matches!(call_path.as_slice(), ["", "type"] | ["abc", "ABCMeta"]) + }) }) || decorator_list.iter().any(|decorator| { // The method is decorated with a class method decorator (like `@classmethod`). - semantic.resolve_call_path(map_callable(&decorator.expression)).map_or(false, |call_path| { - matches!(call_path.as_slice(), ["", "classmethod"] | ["abc", "abstractclassmethod"]) || - classmethod_decorators - .iter() - .any(|decorator| call_path == from_qualified_name(decorator)) - }) + semantic + .resolve_call_path(map_callable(&decorator.expression)) + .map_or(false, |call_path| { + matches!( + call_path.as_slice(), + ["", "classmethod"] | ["abc", "abstractclassmethod"] + ) || classmethod_decorators + .iter() + .any(|decorator| call_path == from_qualified_name(decorator)) + }) }) { FunctionType::ClassMethod diff --git a/crates/ruff_python_stdlib/src/identifiers.rs b/crates/ruff_python_stdlib/src/identifiers.rs index 2dca484a3c..169959bee2 100644 --- a/crates/ruff_python_stdlib/src/identifiers.rs +++ b/crates/ruff_python_stdlib/src/identifiers.rs @@ -1,4 +1,4 @@ -use crate::keyword::KWLIST; +use crate::keyword::is_keyword; /// Returns `true` if a string is a valid Python identifier (e.g., variable /// name). @@ -18,7 +18,7 @@ pub fn is_identifier(name: &str) -> bool { } // Is the identifier a keyword? - if KWLIST.contains(&name) { + if is_keyword(name) { return false; } @@ -52,7 +52,7 @@ pub fn is_module_name(name: &str) -> bool { } // Is the identifier a keyword? - if KWLIST.contains(&name) { + if is_keyword(name) { return false; } @@ -70,7 +70,7 @@ pub fn is_migration_name(name: &str) -> bool { } // Is the identifier a keyword? - if KWLIST.contains(&name) { + if is_keyword(name) { return false; } diff --git a/crates/ruff_python_stdlib/src/keyword.rs b/crates/ruff_python_stdlib/src/keyword.rs index ddaec03c80..7f361c0b69 100644 --- a/crates/ruff_python_stdlib/src/keyword.rs +++ b/crates/ruff_python_stdlib/src/keyword.rs @@ -1,7 +1,41 @@ // See: https://github.com/python/cpython/blob/9d692841691590c25e6cf5b2250a594d3bf54825/Lib/keyword.py#L18 -pub(crate) const KWLIST: [&str; 35] = [ - "False", "None", "True", "and", "as", "assert", "async", "await", "break", "class", "continue", - "def", "del", "elif", "else", "except", "finally", "for", "from", "global", "if", "import", - "in", "is", "lambda", "nonlocal", "not", "or", "pass", "raise", "return", "try", "while", - "with", "yield", -]; +pub(crate) fn is_keyword(name: &str) -> bool { + matches!( + name, + "False" + | "None" + | "True" + | "and" + | "as" + | "assert" + | "async" + | "await" + | "break" + | "class" + | "continue" + | "def" + | "del" + | "elif" + | "else" + | "except" + | "finally" + | "for" + | "from" + | "global" + | "if" + | "import" + | "in" + | "is" + | "lambda" + | "nonlocal" + | "not" + | "or" + | "pass" + | "raise" + | "return" + | "try" + | "while" + | "with" + | "yield", + ) +}