Skip to content

Commit

Permalink
Remove ExprKind::Call from call path collection (#2666)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh authored Feb 8, 2023
1 parent 124461b commit 286d8c1
Show file tree
Hide file tree
Showing 14 changed files with 73 additions and 52 deletions.
9 changes: 1 addition & 8 deletions crates/ruff/src/ast/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,7 @@ fn collect_call_path_inner<'a>(expr: &'a Expr, parts: &mut CallPath<'a>) -> bool
/// Convert an `Expr` to its [`CallPath`] segments (like `["typing", "List"]`).
pub fn collect_call_path(expr: &Expr) -> CallPath {
let mut segments = smallvec![];
collect_call_path_inner(
if let ExprKind::Call { func, .. } = &expr.node {
func
} else {
expr
},
&mut segments,
);
collect_call_path_inner(expr, &mut segments);
segments
}

Expand Down
10 changes: 5 additions & 5 deletions crates/ruff/src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2280,7 +2280,7 @@ where

// pyupgrade
if self.settings.rules.enabled(&Rule::TypeOfPrimitive) {
pyupgrade::rules::type_of_primitive(self, expr, args);
pyupgrade::rules::type_of_primitive(self, expr, func, args);
}
if self.settings.rules.enabled(&Rule::DeprecatedUnittestAlias) {
pyupgrade::rules::deprecated_unittest_alias(self, func);
Expand All @@ -2301,10 +2301,10 @@ where
pyupgrade::rules::open_alias(self, expr, func);
}
if self.settings.rules.enabled(&Rule::ReplaceUniversalNewlines) {
pyupgrade::rules::replace_universal_newlines(self, expr, keywords);
pyupgrade::rules::replace_universal_newlines(self, func, keywords);
}
if self.settings.rules.enabled(&Rule::ReplaceStdoutStderr) {
pyupgrade::rules::replace_stdout_stderr(self, expr, keywords);
pyupgrade::rules::replace_stdout_stderr(self, expr, func, keywords);
}
if self.settings.rules.enabled(&Rule::OSErrorAlias) {
pyupgrade::rules::os_error_alias(self, &expr);
Expand Down Expand Up @@ -2335,7 +2335,7 @@ where
.rules
.enabled(&Rule::UselessContextlibSuppress)
{
flake8_bugbear::rules::useless_contextlib_suppress(self, expr, args);
flake8_bugbear::rules::useless_contextlib_suppress(self, expr, func, args);
}
if self
.settings
Expand Down Expand Up @@ -3242,7 +3242,7 @@ where
args,
keywords,
} => {
let callable = self.resolve_call_path(expr).and_then(|call_path| {
let callable = self.resolve_call_path(func).and_then(|call_path| {
if self.match_typing_call_path(&call_path, "ForwardRef") {
Some(Callable::ForwardRef)
} else if self.match_typing_call_path(&call_path, "cast") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ const IMMUTABLE_FUNCS: &[&[&str]] = &[
&["re", "compile"],
];

fn is_immutable_func(checker: &Checker, expr: &Expr, extend_immutable_calls: &[CallPath]) -> bool {
checker.resolve_call_path(expr).map_or(false, |call_path| {
fn is_immutable_func(checker: &Checker, func: &Expr, extend_immutable_calls: &[CallPath]) -> bool {
checker.resolve_call_path(func).map_or(false, |call_path| {
IMMUTABLE_FUNCS
.iter()
.any(|target| call_path.as_slice() == *target)
Expand Down Expand Up @@ -67,7 +67,7 @@ where
{
self.diagnostics.push((
FunctionCallArgumentDefault {
name: compose_call_path(expr),
name: compose_call_path(func),
}
.into(),
Range::from_located(expr),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ const IMMUTABLE_GENERIC_TYPES: &[&[&str]] = &[
&["typing", "Tuple"],
];

pub fn is_mutable_func(checker: &Checker, expr: &Expr) -> bool {
checker.resolve_call_path(expr).map_or(false, |call_path| {
pub fn is_mutable_func(checker: &Checker, func: &Expr) -> bool {
checker.resolve_call_path(func).map_or(false, |call_path| {
MUTABLE_FUNCS
.iter()
.any(|target| call_path.as_slice() == *target)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ impl Violation for UselessContextlibSuppress {
}

/// B005
pub fn useless_contextlib_suppress(checker: &mut Checker, expr: &Expr, args: &[Expr]) {
pub fn useless_contextlib_suppress(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) {
if args.is_empty()
&& checker.resolve_call_path(expr).map_or(false, |call_path| {
&& checker.resolve_call_path(func).map_or(false, |call_path| {
call_path.as_slice() == ["contextlib", "suppress"]
})
{
Expand Down
8 changes: 4 additions & 4 deletions crates/ruff/src/rules/flake8_pytest_style/rules/fail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,22 @@ impl Violation for FailWithoutMessage {
}
}

pub fn fail_call(checker: &mut Checker, call: &Expr, args: &[Expr], keywords: &[Keyword]) {
if is_pytest_fail(call, checker) {
pub fn fail_call(checker: &mut Checker, func: &Expr, args: &[Expr], keywords: &[Keyword]) {
if is_pytest_fail(func, checker) {
let call_args = SimpleCallArgs::new(args, keywords);
let msg = call_args.get_argument("msg", Some(0));

if let Some(msg) = msg {
if is_empty_or_null_string(msg) {
checker.diagnostics.push(Diagnostic::new(
FailWithoutMessage,
Range::from_located(call),
Range::from_located(func),
));
}
} else {
checker.diagnostics.push(Diagnostic::new(
FailWithoutMessage,
Range::from_located(call),
Range::from_located(func),
));
}
}
Expand Down
29 changes: 22 additions & 7 deletions crates/ruff/src/rules/flake8_pytest_style/rules/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,26 @@ use crate::checkers::ast::Checker;

const ITERABLE_INITIALIZERS: &[&str] = &["dict", "frozenset", "list", "tuple", "set"];

pub fn get_mark_decorators(decorators: &[Expr]) -> Vec<&Expr> {
/// Given a decorators that can be used with or without explicit call syntax, return
/// the underlying callable.
fn callable_decorator(decorator: &Expr) -> &Expr {
if let ExprKind::Call { func, .. } = &decorator.node {
func
} else {
decorator
}
}

pub fn get_mark_decorators(decorators: &[Expr]) -> impl Iterator<Item = &Expr> {
decorators
.iter()
.filter(|decorator| is_pytest_mark(decorator))
.collect()
}

pub fn get_mark_name(decorator: &Expr) -> &str {
collect_call_path(decorator).last().unwrap()
collect_call_path(callable_decorator(decorator))
.last()
.unwrap()
}

pub fn is_pytest_fail(call: &Expr, checker: &Checker) -> bool {
Expand All @@ -25,14 +36,18 @@ pub fn is_pytest_fail(call: &Expr, checker: &Checker) -> bool {

pub fn is_pytest_fixture(decorator: &Expr, checker: &Checker) -> bool {
checker
.resolve_call_path(decorator)
.resolve_call_path(if let ExprKind::Call { func, .. } = &decorator.node {
func
} else {
decorator
})
.map_or(false, |call_path| {
call_path.as_slice() == ["pytest", "fixture"]
})
}

pub fn is_pytest_mark(decorator: &Expr) -> bool {
let segments = collect_call_path(decorator);
let segments = collect_call_path(callable_decorator(decorator));
if segments.len() > 2 {
segments[0] == "pytest" && segments[1] == "mark"
} else {
Expand All @@ -42,7 +57,7 @@ pub fn is_pytest_mark(decorator: &Expr) -> bool {

pub fn is_pytest_yield_fixture(decorator: &Expr, checker: &Checker) -> bool {
checker
.resolve_call_path(decorator)
.resolve_call_path(callable_decorator(decorator))
.map_or(false, |call_path| {
call_path.as_slice() == ["pytest", "yield_fixture"]
})
Expand Down Expand Up @@ -100,7 +115,7 @@ pub fn is_falsy_constant(expr: &Expr) -> bool {

pub fn is_pytest_parametrize(decorator: &Expr, checker: &Checker) -> bool {
checker
.resolve_call_path(decorator)
.resolve_call_path(callable_decorator(decorator))
.map_or(false, |call_path| {
call_path.as_slice() == ["pytest", "mark", "parametrize"]
})
Expand Down
16 changes: 8 additions & 8 deletions crates/ruff/src/rules/flake8_simplify/rules/ast_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,8 @@ pub fn use_capital_environment_variables(checker: &mut Checker, expr: &Expr) {
return;
}

// check `os.environ.get('foo')` and `os.getenv('foo')``
if !checker.resolve_call_path(expr).map_or(false, |call_path| {
call_path.as_slice() == ["os", "environ", "get"] || call_path.as_slice() == ["os", "getenv"]
}) {
return;
}

let ExprKind::Call { args, .. } = &expr.node else {
// check `os.environ.get('foo')` and `os.getenv('foo')`.
let ExprKind::Call { func, args, .. } = &expr.node else {
return;
};
let Some(arg) = args.get(0) else {
Expand All @@ -51,6 +45,12 @@ pub fn use_capital_environment_variables(checker: &mut Checker, expr: &Expr) {
let ExprKind::Constant { value: Constant::Str(env_var), kind } = &arg.node else {
return;
};
if !checker.resolve_call_path(func).map_or(false, |call_path| {
call_path.as_slice() == ["os", "environ", "get"] || call_path.as_slice() == ["os", "getenv"]
}) {
return;
}

let capital_env_var = env_var.to_ascii_uppercase();
if &capital_env_var == env_var {
return;
Expand Down
17 changes: 13 additions & 4 deletions crates/ruff/src/rules/pep8_naming/helpers.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use itertools::Itertools;
use ruff_python::string::{is_lower, is_upper};
use rustpython_parser::ast::{Stmt, StmtKind};
use rustpython_parser::ast::{ExprKind, Stmt, StmtKind};

use crate::checkers::ast::Checker;

Expand All @@ -26,7 +26,10 @@ pub fn is_namedtuple_assignment(checker: &Checker, stmt: &Stmt) -> bool {
let StmtKind::Assign { value, .. } = &stmt.node else {
return false;
};
checker.resolve_call_path(value).map_or(false, |call_path| {
let ExprKind::Call {func, ..} = &value.node else {
return false;
};
checker.resolve_call_path(func).map_or(false, |call_path| {
call_path.as_slice() == ["collections", "namedtuple"]
|| call_path.as_slice() == ["typing", "NamedTuple"]
})
Expand All @@ -36,7 +39,10 @@ pub fn is_typeddict_assignment(checker: &Checker, stmt: &Stmt) -> bool {
let StmtKind::Assign { value, .. } = &stmt.node else {
return false;
};
checker.resolve_call_path(value).map_or(false, |call_path| {
let ExprKind::Call {func, ..} = &value.node else {
return false;
};
checker.resolve_call_path(func).map_or(false, |call_path| {
call_path.as_slice() == ["typing", "TypedDict"]
})
}
Expand All @@ -45,7 +51,10 @@ pub fn is_type_var_assignment(checker: &Checker, stmt: &Stmt) -> bool {
let StmtKind::Assign { value, .. } = &stmt.node else {
return false;
};
checker.resolve_call_path(value).map_or(false, |call_path| {
let ExprKind::Call {func, ..} = &value.node else {
return false;
};
checker.resolve_call_path(func).map_or(false, |call_path| {
call_path.as_slice() == ["typing", "TypeVar"]
|| call_path.as_slice() == ["typing", "NewType"]
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ fn generate_fix(
}

/// UP022
pub fn replace_stdout_stderr(checker: &mut Checker, expr: &Expr, kwargs: &[Keyword]) {
if checker.resolve_call_path(expr).map_or(false, |call_path| {
pub fn replace_stdout_stderr(checker: &mut Checker, expr: &Expr, func: &Expr, kwargs: &[Keyword]) {
if checker.resolve_call_path(func).map_or(false, |call_path| {
call_path.as_slice() == ["subprocess", "run"]
}) {
// Find `stdout` and `stderr` kwargs.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ impl AlwaysAutofixableViolation for ReplaceUniversalNewlines {
}

/// UP021
pub fn replace_universal_newlines(checker: &mut Checker, expr: &Expr, kwargs: &[Keyword]) {
if checker.resolve_call_path(expr).map_or(false, |call_path| {
pub fn replace_universal_newlines(checker: &mut Checker, func: &Expr, kwargs: &[Keyword]) {
if checker.resolve_call_path(func).map_or(false, |call_path| {
call_path.as_slice() == ["subprocess", "run"]
}) {
let Some(kwarg) = find_keyword(kwargs, "universal_newlines") else { return; };
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/rules/pyupgrade/rules/type_of_primitive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ impl AlwaysAutofixableViolation for TypeOfPrimitive {
}

/// UP003
pub fn type_of_primitive(checker: &mut Checker, expr: &Expr, args: &[Expr]) {
pub fn type_of_primitive(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) {
if args.len() != 1 {
return;
}
if !checker
.resolve_call_path(expr)
.resolve_call_path(func)
.map_or(false, |call_path| call_path.as_slice() == ["", "type"])
{
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ fn check_type_check_test(checker: &mut Checker, test: &Expr) -> bool {
.iter()
.all(|expr| check_type_check_test(checker, expr)),
ExprKind::UnaryOp { operand, .. } => check_type_check_test(checker, operand),
ExprKind::Call { .. } => check_type_check_call(checker, test),
ExprKind::Call { func, .. } => check_type_check_call(checker, func),
_ => false,
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use ruff_macros::{define_violation, derive_message_formats};
use rustpython_parser::ast::Expr;
use rustpython_parser::ast::{Expr, ExprKind};

use crate::ast::types::Range;
use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -59,7 +59,11 @@ impl Violation for RaiseVanillaClass {
/// TRY002
pub fn raise_vanilla_class(checker: &mut Checker, expr: &Expr) {
if checker
.resolve_call_path(expr)
.resolve_call_path(if let ExprKind::Call { func, .. } = &expr.node {
func
} else {
expr
})
.map_or(false, |call_path| call_path.as_slice() == ["", "Exception"])
{
checker.diagnostics.push(Diagnostic::new(
Expand Down

0 comments on commit 286d8c1

Please sign in to comment.