mirror of https://github.com/astral-sh/ruff
Allow `os._exit` accesses in `SLF001` (#6490)
Closes https://github.com/astral-sh/ruff/issues/6483.
This commit is contained in:
parent
1050c4e104
commit
84ae00c395
|
|
@ -73,3 +73,7 @@ print(foo.__dict__)
|
|||
print(foo.__str__())
|
||||
print(foo().__class__)
|
||||
print(foo._asdict())
|
||||
|
||||
import os
|
||||
|
||||
os._exit()
|
||||
|
|
|
|||
|
|
@ -1,8 +1,7 @@
|
|||
use ruff_python_ast::{self as ast, Expr, Ranged};
|
||||
|
||||
use ruff_diagnostics::{Diagnostic, Violation};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::call_path::collect_call_path;
|
||||
use ruff_python_ast::{self as ast, Expr, Ranged};
|
||||
use ruff_python_semantic::ScopeKind;
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
|
|
@ -62,125 +61,136 @@ impl Violation for PrivateMemberAccess {
|
|||
|
||||
/// SLF001
|
||||
pub(crate) fn private_member_access(checker: &mut Checker, expr: &Expr) {
|
||||
if let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = expr {
|
||||
if (attr.starts_with("__") && !attr.ends_with("__"))
|
||||
|| (attr.starts_with('_') && !attr.starts_with("__"))
|
||||
let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = expr else {
|
||||
return;
|
||||
};
|
||||
|
||||
if (attr.starts_with("__") && !attr.ends_with("__"))
|
||||
|| (attr.starts_with('_') && !attr.starts_with("__"))
|
||||
{
|
||||
if checker
|
||||
.settings
|
||||
.flake8_self
|
||||
.ignore_names
|
||||
.contains(attr.as_ref())
|
||||
{
|
||||
if checker
|
||||
.settings
|
||||
.flake8_self
|
||||
.ignore_names
|
||||
.contains(attr.as_ref())
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
// Ignore accesses on instances within special methods (e.g., `__eq__`).
|
||||
if let ScopeKind::Function(ast::StmtFunctionDef { name, .. }) =
|
||||
checker.semantic().current_scope().kind
|
||||
{
|
||||
if matches!(
|
||||
name.as_str(),
|
||||
"__lt__"
|
||||
| "__le__"
|
||||
| "__eq__"
|
||||
| "__ne__"
|
||||
| "__gt__"
|
||||
| "__ge__"
|
||||
| "__add__"
|
||||
| "__sub__"
|
||||
| "__mul__"
|
||||
| "__matmul__"
|
||||
| "__truediv__"
|
||||
| "__floordiv__"
|
||||
| "__mod__"
|
||||
| "__divmod__"
|
||||
| "__pow__"
|
||||
| "__lshift__"
|
||||
| "__rshift__"
|
||||
| "__and__"
|
||||
| "__xor__"
|
||||
| "__or__"
|
||||
| "__radd__"
|
||||
| "__rsub__"
|
||||
| "__rmul__"
|
||||
| "__rmatmul__"
|
||||
| "__rtruediv__"
|
||||
| "__rfloordiv__"
|
||||
| "__rmod__"
|
||||
| "__rdivmod__"
|
||||
| "__rpow__"
|
||||
| "__rlshift__"
|
||||
| "__rrshift__"
|
||||
| "__rand__"
|
||||
| "__rxor__"
|
||||
| "__ror__"
|
||||
| "__iadd__"
|
||||
| "__isub__"
|
||||
| "__imul__"
|
||||
| "__imatmul__"
|
||||
| "__itruediv__"
|
||||
| "__ifloordiv__"
|
||||
| "__imod__"
|
||||
| "__ipow__"
|
||||
| "__ilshift__"
|
||||
| "__irshift__"
|
||||
| "__iand__"
|
||||
| "__ixor__"
|
||||
| "__ior__"
|
||||
) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Allow some documented private methods, like `os._exit()`.
|
||||
if let Some(call_path) = checker.semantic().resolve_call_path(expr) {
|
||||
if matches!(call_path.as_slice(), ["os", "_exit"]) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if let Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() {
|
||||
// Ignore `super()` calls.
|
||||
if let Some(call_path) = collect_call_path(func) {
|
||||
if matches!(call_path.as_slice(), ["super"]) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(call_path) = collect_call_path(value) {
|
||||
// Ignore `self` and `cls` accesses.
|
||||
if matches!(call_path.as_slice(), ["self" | "cls" | "mcs"]) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Ignore accesses on instances within special methods (e.g., `__eq__`).
|
||||
if let ScopeKind::Function(ast::StmtFunctionDef { name, .. }) =
|
||||
checker.semantic().current_scope().kind
|
||||
{
|
||||
if matches!(
|
||||
name.as_str(),
|
||||
"__lt__"
|
||||
| "__le__"
|
||||
| "__eq__"
|
||||
| "__ne__"
|
||||
| "__gt__"
|
||||
| "__ge__"
|
||||
| "__add__"
|
||||
| "__sub__"
|
||||
| "__mul__"
|
||||
| "__matmul__"
|
||||
| "__truediv__"
|
||||
| "__floordiv__"
|
||||
| "__mod__"
|
||||
| "__divmod__"
|
||||
| "__pow__"
|
||||
| "__lshift__"
|
||||
| "__rshift__"
|
||||
| "__and__"
|
||||
| "__xor__"
|
||||
| "__or__"
|
||||
| "__radd__"
|
||||
| "__rsub__"
|
||||
| "__rmul__"
|
||||
| "__rmatmul__"
|
||||
| "__rtruediv__"
|
||||
| "__rfloordiv__"
|
||||
| "__rmod__"
|
||||
| "__rdivmod__"
|
||||
| "__rpow__"
|
||||
| "__rlshift__"
|
||||
| "__rrshift__"
|
||||
| "__rand__"
|
||||
| "__rxor__"
|
||||
| "__ror__"
|
||||
| "__iadd__"
|
||||
| "__isub__"
|
||||
| "__imul__"
|
||||
| "__imatmul__"
|
||||
| "__itruediv__"
|
||||
| "__ifloordiv__"
|
||||
| "__imod__"
|
||||
| "__ipow__"
|
||||
| "__ilshift__"
|
||||
| "__irshift__"
|
||||
| "__iand__"
|
||||
| "__ixor__"
|
||||
| "__ior__"
|
||||
) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if let Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() {
|
||||
// Ignore `super()` calls.
|
||||
if let Some(call_path) = collect_call_path(func) {
|
||||
if matches!(call_path.as_slice(), ["super"]) {
|
||||
return;
|
||||
// Ignore accesses on class members from _within_ the class.
|
||||
if checker
|
||||
.semantic()
|
||||
.scopes
|
||||
.iter()
|
||||
.rev()
|
||||
.find_map(|scope| match &scope.kind {
|
||||
ScopeKind::Class(ast::StmtClassDef { name, .. }) => Some(name),
|
||||
_ => None,
|
||||
})
|
||||
.is_some_and(|name| {
|
||||
if call_path.as_slice() == [name.as_str()] {
|
||||
checker
|
||||
.semantic()
|
||||
.find_binding(name)
|
||||
.is_some_and(|binding| {
|
||||
// TODO(charlie): Could the name ever be bound to a
|
||||
// _different_ class here?
|
||||
binding.kind.is_class_definition()
|
||||
})
|
||||
} else {
|
||||
false
|
||||
}
|
||||
}
|
||||
} else if let Some(call_path) = collect_call_path(value) {
|
||||
// Ignore `self` and `cls` accesses.
|
||||
if matches!(call_path.as_slice(), ["self" | "cls" | "mcs"]) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Ignore accesses on class members from _within_ the class.
|
||||
if checker
|
||||
.semantic()
|
||||
.scopes
|
||||
.iter()
|
||||
.rev()
|
||||
.find_map(|scope| match &scope.kind {
|
||||
ScopeKind::Class(ast::StmtClassDef { name, .. }) => Some(name),
|
||||
_ => None,
|
||||
})
|
||||
.is_some_and(|name| {
|
||||
if call_path.as_slice() == [name.as_str()] {
|
||||
checker
|
||||
.semantic()
|
||||
.find_binding(name)
|
||||
.is_some_and(|binding| {
|
||||
// TODO(charlie): Could the name ever be bound to a
|
||||
// _different_ class here?
|
||||
binding.kind.is_class_definition()
|
||||
})
|
||||
} else {
|
||||
false
|
||||
}
|
||||
})
|
||||
{
|
||||
return;
|
||||
}
|
||||
})
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
checker.diagnostics.push(Diagnostic::new(
|
||||
PrivateMemberAccess {
|
||||
access: attr.to_string(),
|
||||
},
|
||||
expr.range(),
|
||||
));
|
||||
}
|
||||
|
||||
checker.diagnostics.push(Diagnostic::new(
|
||||
PrivateMemberAccess {
|
||||
access: attr.to_string(),
|
||||
},
|
||||
expr.range(),
|
||||
));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -111,8 +111,8 @@ pub(crate) fn logging_call(checker: &mut Checker, call: &ast::ExprCall) {
|
|||
let Some(Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Str(value),
|
||||
..
|
||||
})) = call.arguments
|
||||
.find_positional( 0) else {
|
||||
})) = call.arguments.find_positional(0)
|
||||
else {
|
||||
return;
|
||||
};
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue