Skip to content

Commit

Permalink
Avoid raising SIM911 for attribute calls
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Apr 24, 2024
1 parent 7c8c1c7 commit 404d882
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,5 @@ def foo(d: dict[str, str]) -> None:
...

items = zip(x.keys(), x.values()) # OK

items.bar = zip(x.keys(), x.values()) # OK
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use ast::{ExprAttribute, ExprName, Identifier};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Arguments, Expr, ExprCall};
use ruff_python_ast::{self as ast, Arguments, Expr};
use ruff_text_size::Ranged;

use crate::{checkers::ast::Checker, fix::snippet::SourceCodeSnippet};
Expand Down Expand Up @@ -59,8 +59,8 @@ impl AlwaysFixableViolation for ZipDictKeysAndValues {
}

/// SIM911
pub(crate) fn zip_dict_keys_and_values(checker: &mut Checker, expr: &ExprCall) {
let ExprCall {
pub(crate) fn zip_dict_keys_and_values(checker: &mut Checker, expr: &ast::ExprCall) {
let ast::ExprCall {
func,
arguments: Arguments { args, keywords, .. },
..
Expand All @@ -72,9 +72,6 @@ pub(crate) fn zip_dict_keys_and_values(checker: &mut Checker, expr: &ExprCall) {
}] if name.as_str() == "strict" => {}
_ => return,
};
if matches!(func.as_ref(), Expr::Name(ExprName { id, .. }) if id != "zip") {
return;
}
let [arg1, arg2] = &args[..] else {
return;
};
Expand All @@ -87,6 +84,9 @@ pub(crate) fn zip_dict_keys_and_values(checker: &mut Checker, expr: &ExprCall) {
if var1.id != var2.id || attr1 != "keys" || attr2 != "values" {
return;
}
if !checker.semantic().match_builtin_expr(func, "zip") {
return;
}

let Some(binding) = checker
.semantic()
Expand Down
66 changes: 64 additions & 2 deletions crates/ruff_python_semantic/src/analyze/typing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,45 @@ pub fn to_pep604_operator(
})
}

/// Return `true` if `Expr` represents a reference to a type annotation that resolves to a mutable
/// type.
pub fn is_mutable_annotation(expr: &Expr, semantic: &SemanticModel) -> bool {
match expr {
Expr::Name(_) | Expr::Attribute(_) => semantic
.resolve_qualified_name(expr)
.is_some_and(|qualified_name| is_mutable_return_type(qualified_name.segments())),
Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => semantic
.resolve_qualified_name(value)
.is_some_and(|qualified_name| {
if is_mutable_return_type(qualified_name.segments()) {
true
} else if semantic.match_typing_qualified_name(&qualified_name, "Union") {
if let Expr::Tuple(ast::ExprTuple { elts, .. }) = slice.as_ref() {
elts.iter().all(|elt| is_mutable_annotation(elt, semantic))
} else {
false
}
} else if is_pep_593_generic_type(qualified_name.segments()) {
if let Expr::Tuple(ast::ExprTuple { elts, .. }) = slice.as_ref() {
elts.first()
.is_some_and(|elt| is_mutable_annotation(elt, semantic))
} else {
false
}
} else {
false
}
}),
Expr::BinOp(ast::ExprBinOp {
left,
op: Operator::BitOr,
right,
range: _,
}) => is_mutable_annotation(left, semantic) && is_mutable_annotation(right, semantic),
_ => false,
}
}

/// Return `true` if `Expr` represents a reference to a type annotation that resolves to an
/// immutable type.
pub fn is_immutable_annotation(
Expand All @@ -236,15 +275,15 @@ pub fn is_immutable_annotation(
.is_some_and(|qualified_name| {
if is_immutable_generic_type(qualified_name.segments()) {
true
} else if matches!(qualified_name.segments(), ["typing", "Union"]) {
} else if semantic.match_typing_qualified_name(&qualified_name, "Union") {
if let Expr::Tuple(ast::ExprTuple { elts, .. }) = slice.as_ref() {
elts.iter().all(|elt| {
is_immutable_annotation(elt, semantic, extend_immutable_calls)
})
} else {
false
}
} else if matches!(qualified_name.segments(), ["typing", "Optional"]) {
} else if semantic.match_typing_qualified_name(&qualified_name, "Optional") {
is_immutable_annotation(slice, semantic, extend_immutable_calls)
} else if is_pep_593_generic_type(qualified_name.segments()) {
if let Expr::Tuple(ast::ExprTuple { elts, .. }) = slice.as_ref() {
Expand Down Expand Up @@ -311,6 +350,29 @@ pub fn is_mutable_expr(expr: &Expr, semantic: &SemanticModel) -> bool {
}
}

/// Return `true` if `expr` is an expression that resolves to an immutable value.
pub fn is_immutable_expr(
expr: &Expr,
semantic: &SemanticModel,
extend_immutable_calls: &[QualifiedName],
) -> bool {
match expr {
Expr::NoneLiteral(_) => true,
Expr::BooleanLiteral(_) => true,
Expr::NumberLiteral(_) => true,
Expr::StringLiteral(_) => true,
Expr::BytesLiteral(_) => true,
Expr::EllipsisLiteral(_) => true,
Expr::Tuple(ast::ExprTuple { elts, .. }) => elts
.iter()
.all(|elt| is_immutable_expr(elt, semantic, extend_immutable_calls)),
Expr::Call(ast::ExprCall { func, .. }) => {
is_immutable_func(func, semantic, extend_immutable_calls)
}
_ => false,
}
}

/// Return `true` if [`ast::StmtIf`] is a guard for a type-checking block.
pub fn is_type_checking_block(stmt: &ast::StmtIf, semantic: &SemanticModel) -> bool {
let ast::StmtIf { test, .. } = stmt;
Expand Down

0 comments on commit 404d882

Please sign in to comment.