diff --git a/crates/ruff/resources/test/fixtures/flake8_self/SLF001.py b/crates/ruff/resources/test/fixtures/flake8_self/SLF001.py index 05351ca70d..04ed45f7eb 100644 --- a/crates/ruff/resources/test/fixtures/flake8_self/SLF001.py +++ b/crates/ruff/resources/test/fixtures/flake8_self/SLF001.py @@ -35,6 +35,13 @@ class Foo(metaclass=BazMeta): return None if self.bar()._private: # SLF001 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 def public_func(self): @@ -61,3 +68,4 @@ print(foo._private_func()) # SLF001 print(foo.__really_private_func(1)) # SLF001 print(foo.bar._private) # SLF001 print(foo()._private_thing) # SLF001 +print(foo()._private_thing__) # SLF001 diff --git a/crates/ruff/src/rules/flake8_self/rules/private_member_access.rs b/crates/ruff/src/rules/flake8_self/rules/private_member_access.rs index eeb2d458c1..656efe6c11 100644 --- a/crates/ruff/src/rules/flake8_self/rules/private_member_access.rs +++ b/crates/ruff/src/rules/flake8_self/rules/private_member_access.rs @@ -3,7 +3,7 @@ use rustpython_parser::ast::{Expr, ExprKind}; use ruff_macros::{define_violation, derive_message_formats}; 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::registry::Diagnostic; use crate::violation::Violation; @@ -60,13 +60,17 @@ impl Violation for PrivateMemberAccess { /// SLF001 pub fn private_member_access(checker: &mut Checker, expr: &Expr) { 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 { + // Ignore `super()` calls. let call_path = collect_call_path(func); if call_path.as_slice() == ["super"] { return; } } else { + // Ignore `self` and `cls` accesses. let call_path = collect_call_path(value); if call_path.as_slice() == ["self"] || call_path.as_slice() == ["cls"] @@ -74,6 +78,32 @@ pub fn private_member_access(checker: &mut Checker, expr: &Expr) { { 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( diff --git a/crates/ruff/src/rules/flake8_self/snapshots/ruff__rules__flake8_self__tests__private-member-access_SLF001.py.snap b/crates/ruff/src/rules/flake8_self/snapshots/ruff__rules__flake8_self__tests__private-member-access_SLF001.py.snap index c184d69741..4452e905df 100644 --- a/crates/ruff/src/rules/flake8_self/snapshots/ruff__rules__flake8_self__tests__private-member-access_SLF001.py.snap +++ b/crates/ruff/src/rules/flake8_self/snapshots/ruff__rules__flake8_self__tests__private-member-access_SLF001.py.snap @@ -28,10 +28,32 @@ expression: diagnostics PrivateMemberAccess: access: _private_thing 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 end_location: - row: 58 + row: 65 column: 24 fix: ~ parent: ~ @@ -39,10 +61,10 @@ expression: diagnostics PrivateMemberAccess: access: __really_private_thing location: - row: 59 + row: 66 column: 6 end_location: - row: 59 + row: 66 column: 32 fix: ~ parent: ~ @@ -50,10 +72,10 @@ expression: diagnostics PrivateMemberAccess: access: _private_func location: - row: 60 + row: 67 column: 6 end_location: - row: 60 + row: 67 column: 23 fix: ~ parent: ~ @@ -61,10 +83,10 @@ expression: diagnostics PrivateMemberAccess: access: __really_private_func location: - row: 61 + row: 68 column: 6 end_location: - row: 61 + row: 68 column: 31 fix: ~ parent: ~ @@ -72,10 +94,10 @@ expression: diagnostics PrivateMemberAccess: access: _private location: - row: 62 + row: 69 column: 6 end_location: - row: 62 + row: 69 column: 22 fix: ~ parent: ~ @@ -83,11 +105,22 @@ expression: diagnostics PrivateMemberAccess: access: _private_thing location: - row: 63 + row: 70 column: 6 end_location: - row: 63 + row: 70 column: 26 fix: ~ parent: ~ +- kind: + PrivateMemberAccess: + access: _private_thing__ + location: + row: 71 + column: 6 + end_location: + row: 71 + column: 28 + fix: ~ + parent: ~