chore: remove redundant `Expr::Call` checks (#7678)

## Summary

As we bind the `ast::ExprCall` in the big `match expr` in
`expression.rs`
```rust
Expr::Call(
    call @ ast::ExprCall {
     ...
```

There is no need for additional `let/if let` checks on `ExprCall` in
downstream rules. Found a few older rules which still did this while
working on something else. This PR removes the redundant check from
these rules.

## Test Plan

`cargo test`
This commit is contained in:
qdegraaf 2023-09-27 17:36:20 +02:00 committed by GitHub
parent 70ab4b8b59
commit 34480c0e4d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 44 additions and 56 deletions

View File

@ -474,13 +474,13 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
} }
} }
if checker.enabled(Rule::BlockingHttpCallInAsyncFunction) { if checker.enabled(Rule::BlockingHttpCallInAsyncFunction) {
flake8_async::rules::blocking_http_call(checker, expr); flake8_async::rules::blocking_http_call(checker, call);
} }
if checker.enabled(Rule::OpenSleepOrSubprocessInAsyncFunction) { if checker.enabled(Rule::OpenSleepOrSubprocessInAsyncFunction) {
flake8_async::rules::open_sleep_or_subprocess_call(checker, expr); flake8_async::rules::open_sleep_or_subprocess_call(checker, call);
} }
if checker.enabled(Rule::BlockingOsCallInAsyncFunction) { if checker.enabled(Rule::BlockingOsCallInAsyncFunction) {
flake8_async::rules::blocking_os_call(checker, expr); flake8_async::rules::blocking_os_call(checker, call);
} }
if checker.any_enabled(&[Rule::Print, Rule::PPrint]) { if checker.any_enabled(&[Rule::Print, Rule::PPrint]) {
flake8_print::rules::print_call(checker, call); flake8_print::rules::print_call(checker, call);
@ -508,7 +508,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
Rule::SuspiciousTelnetUsage, Rule::SuspiciousTelnetUsage,
Rule::SuspiciousFTPLibUsage, Rule::SuspiciousFTPLibUsage,
]) { ]) {
flake8_bandit::rules::suspicious_function_call(checker, expr); flake8_bandit::rules::suspicious_function_call(checker, call);
} }
if checker.enabled(Rule::ReSubPositionalArgs) { if checker.enabled(Rule::ReSubPositionalArgs) {
flake8_bugbear::rules::re_sub_positional_args(checker, call); flake8_bugbear::rules::re_sub_positional_args(checker, call);

View File

@ -1,5 +1,4 @@
use ruff_python_ast as ast; use ruff_python_ast::ExprCall;
use ruff_python_ast::Expr;
use ruff_diagnostics::{Diagnostic, Violation}; use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
@ -62,20 +61,18 @@ fn is_blocking_http_call(call_path: &CallPath) -> bool {
} }
/// ASYNC100 /// ASYNC100
pub(crate) fn blocking_http_call(checker: &mut Checker, expr: &Expr) { pub(crate) fn blocking_http_call(checker: &mut Checker, call: &ExprCall) {
if checker.semantic().in_async_context() { if checker.semantic().in_async_context() {
if let Expr::Call(ast::ExprCall { func, .. }) = expr { if checker
if checker .semantic()
.semantic() .resolve_call_path(call.func.as_ref())
.resolve_call_path(func) .as_ref()
.as_ref() .is_some_and(is_blocking_http_call)
.is_some_and(is_blocking_http_call) {
{ checker.diagnostics.push(Diagnostic::new(
checker.diagnostics.push(Diagnostic::new( BlockingHttpCallInAsyncFunction,
BlockingHttpCallInAsyncFunction, call.func.range(),
func.range(), ));
));
}
} }
} }
} }

View File

@ -1,5 +1,4 @@
use ruff_python_ast as ast; use ruff_python_ast::ExprCall;
use ruff_python_ast::Expr;
use ruff_diagnostics::{Diagnostic, Violation}; use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
@ -42,19 +41,18 @@ impl Violation for BlockingOsCallInAsyncFunction {
} }
/// ASYNC102 /// ASYNC102
pub(crate) fn blocking_os_call(checker: &mut Checker, expr: &Expr) { pub(crate) fn blocking_os_call(checker: &mut Checker, call: &ExprCall) {
if checker.semantic().in_async_context() { if checker.semantic().in_async_context() {
if let Expr::Call(ast::ExprCall { func, .. }) = expr { if checker
if checker .semantic()
.semantic() .resolve_call_path(call.func.as_ref())
.resolve_call_path(func) .as_ref()
.as_ref() .is_some_and(is_unsafe_os_method)
.is_some_and(is_unsafe_os_method) {
{ checker.diagnostics.push(Diagnostic::new(
checker BlockingOsCallInAsyncFunction,
.diagnostics call.func.range(),
.push(Diagnostic::new(BlockingOsCallInAsyncFunction, func.range())); ));
}
} }
} }
} }

View File

@ -1,5 +1,4 @@
use ruff_python_ast as ast; use ruff_python_ast::ExprCall;
use ruff_python_ast::Expr;
use ruff_diagnostics::{Diagnostic, Violation}; use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
@ -42,20 +41,18 @@ impl Violation for OpenSleepOrSubprocessInAsyncFunction {
} }
/// ASYNC101 /// ASYNC101
pub(crate) fn open_sleep_or_subprocess_call(checker: &mut Checker, expr: &Expr) { pub(crate) fn open_sleep_or_subprocess_call(checker: &mut Checker, call: &ExprCall) {
if checker.semantic().in_async_context() { if checker.semantic().in_async_context() {
if let Expr::Call(ast::ExprCall { func, .. }) = expr { if checker
if checker .semantic()
.semantic() .resolve_call_path(call.func.as_ref())
.resolve_call_path(func) .as_ref()
.as_ref() .is_some_and(is_open_sleep_or_subprocess_call)
.is_some_and(is_open_sleep_or_subprocess_call) {
{ checker.diagnostics.push(Diagnostic::new(
checker.diagnostics.push(Diagnostic::new( OpenSleepOrSubprocessInAsyncFunction,
OpenSleepOrSubprocessInAsyncFunction, call.func.range(),
func.range(), ));
));
}
} }
} }
} }

View File

@ -1,7 +1,7 @@
//! Check for calls to suspicious functions, or calls into suspicious modules. //! Check for calls to suspicious functions, or calls into suspicious modules.
//! //!
//! See: <https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html> //! See: <https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html>
use ruff_python_ast::{self as ast, Expr}; use ruff_python_ast::ExprCall;
use ruff_diagnostics::{Diagnostic, DiagnosticKind, Violation}; use ruff_diagnostics::{Diagnostic, DiagnosticKind, Violation};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
@ -825,12 +825,8 @@ impl Violation for SuspiciousFTPLibUsage {
} }
/// S301, S302, S303, S304, S305, S306, S307, S308, S310, S311, S312, S313, S314, S315, S316, S317, S318, S319, S320, S321, S323 /// S301, S302, S303, S304, S305, S306, S307, S308, S310, S311, S312, S313, S314, S315, S316, S317, S318, S319, S320, S321, S323
pub(crate) fn suspicious_function_call(checker: &mut Checker, expr: &Expr) { pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) {
let Expr::Call(ast::ExprCall { func, .. }) = expr else { let Some(diagnostic_kind) = checker.semantic().resolve_call_path(call.func.as_ref()).and_then(|call_path| {
return;
};
let Some(diagnostic_kind) = checker.semantic().resolve_call_path(func).and_then(|call_path| {
match call_path.as_slice() { match call_path.as_slice() {
// Pickle // Pickle
["pickle" | "dill", "load" | "loads" | "Unpickler"] | ["pickle" | "dill", "load" | "loads" | "Unpickler"] |
@ -888,7 +884,7 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, expr: &Expr) {
return; return;
}; };
let diagnostic = Diagnostic::new::<DiagnosticKind>(diagnostic_kind, expr.range()); let diagnostic = Diagnostic::new::<DiagnosticKind>(diagnostic_kind, call.range());
if checker.enabled(diagnostic.kind.rule()) { if checker.enabled(diagnostic.kind.rule()) {
checker.diagnostics.push(diagnostic); checker.diagnostics.push(diagnostic);
} }