Move match_builtin_expr calls lower down all the rules

astral-sh · Apr 15, 2024 · 4e3c7c0 · 4e3c7c0
1 parent e7227ff
commit 4e3c7c0
Show file tree

Hide file tree

Showing 15 changed files with 80 additions and 76 deletions.
diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs
@@ -54,9 +54,6 @@ pub(crate) fn getattr_with_constant(
     func: &Expr,
     args: &[Expr],
 ) {
-    if !checker.semantic().match_builtin_expr(func, "getattr") {
-        return;
-    };
     let [obj, arg] = args else {
         return;
     };
@@ -72,6 +69,9 @@ pub(crate) fn getattr_with_constant(
     if is_mangled_private(value.to_str()) {
         return;
     }
+    if !checker.semantic().match_builtin_expr(func, "getattr") {
+        return;
+    };
 
     let mut diagnostic = Diagnostic::new(GetAttrWithConstant, expr.range());
     diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs
@@ -68,9 +68,6 @@ pub(crate) fn setattr_with_constant(
     func: &Expr,
     args: &[Expr],
 ) {
-    if !checker.semantic().match_builtin_expr(func, "setattr") {
-        return;
-    }
     let [obj, name, value] = args else {
         return;
     };
@@ -86,6 +83,9 @@ pub(crate) fn setattr_with_constant(
     if is_mangled_private(name.to_str()) {
         return;
     }
+    if !checker.semantic().match_builtin_expr(func, "setattr") {
+        return;
+    }
 
     // We can only replace a `setattr` call (which is an `Expr`) with an assignment
     // (which is a `Stmt`) if the `Expr` is already being used as a `Stmt`

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs
@@ -57,12 +57,6 @@ pub(crate) fn unreliable_callable_check(
     func: &Expr,
     args: &[Expr],
 ) {
-    let Some(qualified_name) = checker.semantic().resolve_qualified_name(func) else {
-        return;
-    };
-    let ["" | "builtins", function @ ("hasattr" | "getattr")] = qualified_name.segments() else {
-        return;
-    };
     let [obj, attr, ..] = args else {
         return;
     };
@@ -72,6 +66,12 @@ pub(crate) fn unreliable_callable_check(
     if value != "__call__" {
         return;
     }
+    let Some(qualified_name) = checker.semantic().resolve_qualified_name(func) else {
+        return;
+    };
+    let ["" | "builtins", function @ ("hasattr" | "getattr")] = qualified_name.segments() else {
+        return;
+    };
 
     let mut diagnostic = Diagnostic::new(UnreliableCallableCheck, expr.range());
     if *function == "hasattr" {

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs
@@ -52,13 +52,15 @@ impl AlwaysFixableViolation for ZipWithoutExplicitStrict {
 
 /// B905
 pub(crate) fn zip_without_explicit_strict(checker: &mut Checker, call: &ast::ExprCall) {
-    if checker.semantic().match_builtin_expr(&call.func, "zip")
+    let semantic = checker.semantic();
+
+    if semantic.match_builtin_expr(&call.func, "zip")
         && call.arguments.find_keyword("strict").is_none()
         && !call
             .arguments
             .args
             .iter()
-            .any(|arg| is_infinite_iterator(arg, checker.semantic()))
+            .any(|arg| is_infinite_iterator(arg, semantic))
     {
         checker.diagnostics.push(
             Diagnostic::new(ZipWithoutExplicitStrict, call.range()).with_fix(Fix::applicable_edit(

diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_range_start.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_range_start.rs
@@ -43,11 +43,6 @@ impl AlwaysFixableViolation for UnnecessaryRangeStart {
 
 /// PIE808
 pub(crate) fn unnecessary_range_start(checker: &mut Checker, call: &ast::ExprCall) {
-    // Verify that the call is to the `range` builtin.
-    if !checker.semantic().match_builtin_expr(&call.func, "range") {
-        return;
-    };
-
     // `range` doesn't accept keyword arguments.
     if !call.arguments.keywords.is_empty() {
         return;
@@ -70,6 +65,11 @@ pub(crate) fn unnecessary_range_start(checker: &mut Checker, call: &ast::ExprCal
         return;
     };
 
+    // Verify that the call is to the `range` builtin.
+    if !checker.semantic().match_builtin_expr(&call.func, "range") {
+        return;
+    };
+
     let mut diagnostic = Diagnostic::new(UnnecessaryRangeStart, start.range());
     diagnostic.try_set_fix(|| {
         remove_argument(

diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs
@@ -53,39 +53,34 @@ impl Violation for UnnecessaryTypeUnion {
 
 /// PYI055
 pub(crate) fn unnecessary_type_union<'a>(checker: &mut Checker, union: &'a Expr) {
+    let semantic = checker.semantic();
+
     // The `|` operator isn't always safe to allow to runtime-evaluated annotations.
-    if checker.semantic().execution_context().is_runtime() {
+    if semantic.execution_context().is_runtime() {
         return;
     }
 
     // Check if `union` is a PEP604 union (e.g. `float | int`) or a `typing.Union[float, int]`
     let subscript = union.as_subscript_expr();
-    if subscript.is_some_and(|subscript| {
-        !checker
-            .semantic()
-            .match_typing_expr(&subscript.value, "Union")
-    }) {
+    if subscript.is_some_and(|subscript| !semantic.match_typing_expr(&subscript.value, "Union")) {
         return;
     }
 
-    let mut type_exprs = Vec::new();
-    let mut other_exprs = Vec::new();
+    let mut type_exprs: Vec<&Expr> = Vec::new();
+    let mut other_exprs: Vec<&Expr> = Vec::new();
 
     let mut collect_type_exprs = |expr: &'a Expr, _parent: &'a Expr| match expr {
-        Expr::Subscript(subscript) => {
-            if checker
-                .semantic()
-                .match_builtin_expr(&subscript.value, "type")
-            {
-                type_exprs.push(&*subscript.slice);
+        Expr::Subscript(ast::ExprSubscript { slice, value, .. }) => {
+            if semantic.match_builtin_expr(value, "type") {
+                type_exprs.push(slice);
             } else {
                 other_exprs.push(expr);
             }
         }
         _ => other_exprs.push(expr),
     };
 
-    traverse_union(&mut collect_type_exprs, checker.semantic(), union);
+    traverse_union(&mut collect_type_exprs, semantic, union);
 
     if type_exprs.len() > 1 {
         let type_members: Vec<String> = type_exprs
@@ -102,7 +97,7 @@ pub(crate) fn unnecessary_type_union<'a>(checker: &mut Checker, union: &'a Expr)
             union.range(),
         );
 
-        if checker.semantic().is_builtin("type") {
+        if semantic.is_builtin("type") {
             let content = if let Some(subscript) = subscript {
                 let types = &Expr::Subscript(ast::ExprSubscript {
                     value: Box::new(Expr::Name(ast::ExprName {

diff --git a/crates/ruff_linter/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs b/crates/ruff_linter/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs
@@ -126,11 +126,14 @@ fn is_open(semantic: &SemanticModel, func: &Expr) -> bool {
     if attr != "open" {
         return false;
     }
-    let Expr::Call(ast::ExprCall { func, .. }) = &**value else {
+    let Expr::Call(ast::ExprCall {
+        func: value_func, ..
+    }) = &**value
+    else {
         return false;
     };
     semantic
-        .resolve_qualified_name(func)
+        .resolve_qualified_name(value_func)
         .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["pathlib", "Path"]))
 }
 

diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/type_comparison.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/type_comparison.rs
@@ -76,7 +76,9 @@ fn deprecated_type_comparison(checker: &mut Checker, compare: &ast::ExprCompare)
             continue;
         };
 
-        if !checker.semantic().match_builtin_expr(func, "type") {
+        let semantic = checker.semantic();
+
+        if !semantic.match_builtin_expr(func, "type") {
             continue;
         }
 
@@ -86,7 +88,7 @@ fn deprecated_type_comparison(checker: &mut Checker, compare: &ast::ExprCompare)
                 func, arguments, ..
             }) => {
                 // Ex) `type(obj) is type(1)`
-                if checker.semantic().match_builtin_expr(func, "type") {
+                if semantic.match_builtin_expr(func, "type") {
                     // Allow comparison for types which are not obvious.
                     if arguments
                         .args
@@ -104,8 +106,7 @@ fn deprecated_type_comparison(checker: &mut Checker, compare: &ast::ExprCompare)
             }
             Expr::Attribute(ast::ExprAttribute { value, .. }) => {
                 // Ex) `type(obj) is types.NoneType`
-                if checker
-                    .semantic()
+                if semantic
                     .resolve_qualified_name(value.as_ref())
                     .is_some_and(|qualified_name| {
                         matches!(qualified_name.segments(), ["types", ..])
@@ -133,7 +134,7 @@ fn deprecated_type_comparison(checker: &mut Checker, compare: &ast::ExprCompare)
                         | "dict"
                         | "set"
                         | "memoryview"
-                ) && checker.semantic().is_builtin(id)
+                ) && semantic.is_builtin(id)
                 {
                     checker.diagnostics.push(Diagnostic::new(
                         TypeComparison {
@@ -180,16 +181,17 @@ fn is_type(expr: &Expr, semantic: &SemanticModel) -> bool {
         Expr::Call(ast::ExprCall {
             func, arguments, ..
         }) => {
-            // Ex) `type(obj) == type(1)`
-            if !semantic.match_builtin_expr(func, "type") {
-                return false;
-            }
-
             // Allow comparison for types which are not obvious.
-            arguments
+            if !arguments
                 .args
                 .first()
                 .is_some_and(|arg| !arg.is_name_expr() && !arg.is_none_literal_expr())
+            {
+                return false;
+            }
+
+            // Ex) `type(obj) == type(1)`
+            semantic.match_builtin_expr(func, "type")
         }
         Expr::Name(ast::ExprName { id, .. }) => {
             // Ex) `type(obj) == int`

diff --git a/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs b/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs
@@ -95,8 +95,10 @@ fn is_open(func: &Expr, semantic: &SemanticModel) -> Option<Kind> {
     if attr != "open" {
         return None;
     }
-    let ast::ExprCall { func, .. } = value.as_call_expr()?;
-    let qualified_name = semantic.resolve_qualified_name(func)?;
+    let ast::ExprCall {
+        func: value_func, ..
+    } = value.as_call_expr()?;
+    let qualified_name = semantic.resolve_qualified_name(value_func)?;
     match qualified_name.segments() {
         ["pathlib", "Path"] => Some(Kind::Pathlib),
         _ => None,

diff --git a/crates/ruff_linter/src/rules/pylint/rules/nan_comparison.rs b/crates/ruff_linter/src/rules/pylint/rules/nan_comparison.rs
@@ -96,23 +96,20 @@ impl std::fmt::Display for Nan {
 
 /// Returns `true` if the expression is a call to `float("NaN")`.
 fn is_nan_float(expr: &Expr, semantic: &SemanticModel) -> bool {
-    let Expr::Call(call) = expr else {
+    let Expr::Call(ast::ExprCall {
+        func,
+        arguments: ast::Arguments { args, keywords, .. },
+        ..
+    }) = expr
+    else {
         return false;
     };
 
-    if !semantic.match_builtin_expr(&call.func, "float") {
-        return false;
-    };
-
-    if !call.arguments.keywords.is_empty() {
+    if !keywords.is_empty() {
         return false;
     }
 
-    let [arg] = call.arguments.args.as_ref() else {
-        return false;
-    };
-
-    let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = arg else {
+    let [Expr::StringLiteral(ast::ExprStringLiteral { value, .. })] = &**args else {
         return false;
     };
 
@@ -123,5 +120,5 @@ fn is_nan_float(expr: &Expr, semantic: &SemanticModel) -> bool {
         return false;
     }
 
-    true
+    semantic.match_builtin_expr(func, "float")
 }
diff --git a/crates/ruff_linter/src/rules/pylint/rules/repeated_isinstance_calls.rs b/crates/ruff_linter/src/rules/pylint/rules/repeated_isinstance_calls.rs
@@ -92,12 +92,12 @@ pub(crate) fn repeated_isinstance_calls(
         else {
             continue;
         };
-        if !checker.semantic().match_builtin_expr(func, "isinstance") {
-            continue;
-        }
         let [obj, types] = &args[..] else {
             continue;
         };
+        if !checker.semantic().match_builtin_expr(func, "isinstance") {
+            continue;
+        }
         let (num_calls, matches) = obj_to_types
             .entry(obj.into())
             .or_insert_with(|| (0, FxHashSet::default()));

diff --git a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs
@@ -124,11 +124,6 @@ fn enumerate_items<'a>(
         func, arguments, ..
     } = call_expr.as_call_expr()?;
 
-    // Check that the function is the `enumerate` builtin.
-    if !semantic.match_builtin_expr(func, "enumerate") {
-        return None;
-    }
-
     let Expr::Tuple(ast::ExprTuple { elts, .. }) = tuple_expr else {
         return None;
     };
@@ -156,6 +151,11 @@ fn enumerate_items<'a>(
         return None;
     };
 
+    // Check that the function is the `enumerate` builtin.
+    if !semantic.match_builtin_expr(func, "enumerate") {
+        return None;
+    }
+
     Some((sequence, index_name, value_name))
 }
 

diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/os_error_alias.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/os_error_alias.rs
@@ -92,7 +92,8 @@ fn atom_diagnostic(checker: &mut Checker, target: &Expr) {
 /// Create a [`Diagnostic`] for a tuple of expressions.
 fn tuple_diagnostic(checker: &mut Checker, tuple: &ast::ExprTuple, aliases: &[&Expr]) {
     let mut diagnostic = Diagnostic::new(OSErrorAlias { name: None }, tuple.range());
-    if checker.semantic().is_builtin("OSError") {
+    let semantic = checker.semantic();
+    if semantic.is_builtin("OSError") {
         // Filter out any `OSErrors` aliases.
         let mut remaining: Vec<Expr> = tuple
             .elts
@@ -110,7 +111,7 @@ fn tuple_diagnostic(checker: &mut Checker, tuple: &ast::ExprTuple, aliases: &[&E
         if tuple
             .elts
             .iter()
-            .all(|elt| !checker.semantic().match_builtin_expr(elt, "OSError"))
+            .all(|elt| !semantic.match_builtin_expr(elt, "OSError"))
         {
             let node = ast::ExprName {
                 id: "OSError".into(),

diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs
@@ -106,7 +106,8 @@ fn atom_diagnostic(checker: &mut Checker, target: &Expr) {
 /// Create a [`Diagnostic`] for a tuple of expressions.
 fn tuple_diagnostic(checker: &mut Checker, tuple: &ast::ExprTuple, aliases: &[&Expr]) {
     let mut diagnostic = Diagnostic::new(TimeoutErrorAlias { name: None }, tuple.range());
-    if checker.semantic().is_builtin("TimeoutError") {
+    let semantic = checker.semantic();
+    if semantic.is_builtin("TimeoutError") {
         // Filter out any `TimeoutErrors` aliases.
         let mut remaining: Vec<Expr> = tuple
             .elts
@@ -124,7 +125,7 @@ fn tuple_diagnostic(checker: &mut Checker, tuple: &ast::ExprTuple, aliases: &[&E
         if tuple
             .elts
             .iter()
-            .all(|elt| !checker.semantic().match_builtin_expr(elt, "TimeoutError"))
+            .all(|elt| !semantic.match_builtin_expr(elt, "TimeoutError"))
         {
             let node = ast::ExprName {
                 id: "TimeoutError".into(),

diff --git a/crates/ruff_linter/src/rules/ruff/rules/mutable_fromkeys_value.rs b/crates/ruff_linter/src/rules/ruff/rules/mutable_fromkeys_value.rs
@@ -73,15 +73,16 @@ pub(crate) fn mutable_fromkeys_value(checker: &mut Checker, call: &ast::ExprCall
     if attr != "fromkeys" {
         return;
     }
-    if !checker.semantic().match_builtin_expr(value, "dict") {
+    let semantic = checker.semantic();
+    if !semantic.match_builtin_expr(value, "dict") {
         return;
     }
 
     // Check that the value parameter is a mutable object.
     let [keys, value] = &*call.arguments.args else {
         return;
     };
-    if !is_mutable_expr(value, checker.semantic()) {
+    if !is_mutable_expr(value, semantic) {
         return;
     }