Allow private accesses on current class (#2929)

This commit is contained in:
Charlie Marsh 2023-02-15 11:52:05 -05:00 committed by GitHub
parent 58269a918a
commit 6b0736cf4b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 85 additions and 14 deletions

View file

@ -35,6 +35,13 @@ class Foo(metaclass=BazMeta):
return None return None
if self.bar()._private: # SLF001 if self.bar()._private: # SLF001
return None return None
if Bar._private_thing: # SLF001
return None
if Foo._private_thing:
return None
Foo = Bar()
if Foo._private_thing: # SLF001
return None
return self.bar return self.bar
def public_func(self): def public_func(self):
@ -61,3 +68,4 @@ print(foo._private_func()) # SLF001
print(foo.__really_private_func(1)) # SLF001 print(foo.__really_private_func(1)) # SLF001
print(foo.bar._private) # SLF001 print(foo.bar._private) # SLF001
print(foo()._private_thing) # SLF001 print(foo()._private_thing) # SLF001
print(foo()._private_thing__) # SLF001

View file

@ -3,7 +3,7 @@ use rustpython_parser::ast::{Expr, ExprKind};
use ruff_macros::{define_violation, derive_message_formats}; use ruff_macros::{define_violation, derive_message_formats};
use crate::ast::helpers::collect_call_path; use crate::ast::helpers::collect_call_path;
use crate::ast::types::Range; use crate::ast::types::{BindingKind, Range, ScopeKind};
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::registry::Diagnostic; use crate::registry::Diagnostic;
use crate::violation::Violation; use crate::violation::Violation;
@ -60,13 +60,17 @@ impl Violation for PrivateMemberAccess {
/// SLF001 /// SLF001
pub fn private_member_access(checker: &mut Checker, expr: &Expr) { pub fn private_member_access(checker: &mut Checker, expr: &Expr) {
if let ExprKind::Attribute { value, attr, .. } = &expr.node { if let ExprKind::Attribute { value, attr, .. } = &expr.node {
if !attr.ends_with("__") && (attr.starts_with('_') || attr.starts_with("__")) { if (attr.starts_with("__") && !attr.ends_with("__"))
|| (attr.starts_with('_') && !attr.starts_with("__"))
{
if let ExprKind::Call { func, .. } = &value.node { if let ExprKind::Call { func, .. } = &value.node {
// Ignore `super()` calls.
let call_path = collect_call_path(func); let call_path = collect_call_path(func);
if call_path.as_slice() == ["super"] { if call_path.as_slice() == ["super"] {
return; return;
} }
} else { } else {
// Ignore `self` and `cls` accesses.
let call_path = collect_call_path(value); let call_path = collect_call_path(value);
if call_path.as_slice() == ["self"] if call_path.as_slice() == ["self"]
|| call_path.as_slice() == ["cls"] || call_path.as_slice() == ["cls"]
@ -74,6 +78,32 @@ pub fn private_member_access(checker: &mut Checker, expr: &Expr) {
{ {
return; return;
} }
// Ignore accesses on class members from _within_ the class.
if checker
.scopes
.iter()
.rev()
.find_map(|scope| match &scope.kind {
ScopeKind::Class(class_def) => Some(class_def),
_ => None,
})
.map_or(false, |class_def| {
if call_path.as_slice() == [class_def.name] {
checker
.find_binding(class_def.name)
.map_or(false, |binding| {
// TODO(charlie): Could the name ever be bound to a _different_
// class here?
matches!(binding.kind, BindingKind::ClassDefinition)
})
} else {
false
}
})
{
return;
}
} }
checker.diagnostics.push(Diagnostic::new( checker.diagnostics.push(Diagnostic::new(

View file

@ -28,10 +28,32 @@ expression: diagnostics
PrivateMemberAccess: PrivateMemberAccess:
access: _private_thing access: _private_thing
location: location:
row: 58 row: 38
column: 11
end_location:
row: 38
column: 29
fix: ~
parent: ~
- kind:
PrivateMemberAccess:
access: _private_thing
location:
row: 43
column: 11
end_location:
row: 43
column: 29
fix: ~
parent: ~
- kind:
PrivateMemberAccess:
access: _private_thing
location:
row: 65
column: 6 column: 6
end_location: end_location:
row: 58 row: 65
column: 24 column: 24
fix: ~ fix: ~
parent: ~ parent: ~
@ -39,10 +61,10 @@ expression: diagnostics
PrivateMemberAccess: PrivateMemberAccess:
access: __really_private_thing access: __really_private_thing
location: location:
row: 59 row: 66
column: 6 column: 6
end_location: end_location:
row: 59 row: 66
column: 32 column: 32
fix: ~ fix: ~
parent: ~ parent: ~
@ -50,10 +72,10 @@ expression: diagnostics
PrivateMemberAccess: PrivateMemberAccess:
access: _private_func access: _private_func
location: location:
row: 60 row: 67
column: 6 column: 6
end_location: end_location:
row: 60 row: 67
column: 23 column: 23
fix: ~ fix: ~
parent: ~ parent: ~
@ -61,10 +83,10 @@ expression: diagnostics
PrivateMemberAccess: PrivateMemberAccess:
access: __really_private_func access: __really_private_func
location: location:
row: 61 row: 68
column: 6 column: 6
end_location: end_location:
row: 61 row: 68
column: 31 column: 31
fix: ~ fix: ~
parent: ~ parent: ~
@ -72,10 +94,10 @@ expression: diagnostics
PrivateMemberAccess: PrivateMemberAccess:
access: _private access: _private
location: location:
row: 62 row: 69
column: 6 column: 6
end_location: end_location:
row: 62 row: 69
column: 22 column: 22
fix: ~ fix: ~
parent: ~ parent: ~
@ -83,11 +105,22 @@ expression: diagnostics
PrivateMemberAccess: PrivateMemberAccess:
access: _private_thing access: _private_thing
location: location:
row: 63 row: 70
column: 6 column: 6
end_location: end_location:
row: 63 row: 70
column: 26 column: 26
fix: ~ fix: ~
parent: ~ parent: ~
- kind:
PrivateMemberAccess:
access: _private_thing__
location:
row: 71
column: 6
end_location:
row: 71
column: 28
fix: ~
parent: ~