Run dunder method rule on methods directly (#9815)

This stood out in the flamegraph and I realized it requires us to
traverse over all statements in the class (unnecessarily).
This commit is contained in:
Charlie Marsh 2024-02-04 11:24:57 -08:00 committed by GitHub
parent 5c99967c4d
commit ad0121660e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 52 additions and 34 deletions

View File

@ -1,13 +1,15 @@
use ruff_python_ast::str::raw_contents_range; use ruff_python_ast::str::raw_contents_range;
use ruff_text_size::{Ranged, TextRange}; use ruff_text_size::{Ranged, TextRange};
use ruff_python_semantic::{BindingKind, ContextualizedDefinition, Export}; use ruff_python_semantic::{
BindingKind, ContextualizedDefinition, Definition, Export, Member, MemberKind,
};
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::codes::Rule; use crate::codes::Rule;
use crate::docstrings::Docstring; use crate::docstrings::Docstring;
use crate::fs::relativize_path; use crate::fs::relativize_path;
use crate::rules::{flake8_annotations, flake8_pyi, pydocstyle}; use crate::rules::{flake8_annotations, flake8_pyi, pydocstyle, pylint};
use crate::{docstrings, warn_user}; use crate::{docstrings, warn_user};
/// Run lint rules over all [`Definition`] nodes in the [`SemanticModel`]. /// Run lint rules over all [`Definition`] nodes in the [`SemanticModel`].
@ -31,6 +33,7 @@ pub(crate) fn definitions(checker: &mut Checker) {
]); ]);
let enforce_stubs = checker.source_type.is_stub() && checker.enabled(Rule::DocstringInStub); let enforce_stubs = checker.source_type.is_stub() && checker.enabled(Rule::DocstringInStub);
let enforce_stubs_and_runtime = checker.enabled(Rule::IterMethodReturnIterable); let enforce_stubs_and_runtime = checker.enabled(Rule::IterMethodReturnIterable);
let enforce_dunder_method = checker.enabled(Rule::BadDunderMethodName);
let enforce_docstrings = checker.any_enabled(&[ let enforce_docstrings = checker.any_enabled(&[
Rule::BlankLineAfterLastSection, Rule::BlankLineAfterLastSection,
Rule::BlankLineAfterSummary, Rule::BlankLineAfterSummary,
@ -80,7 +83,12 @@ pub(crate) fn definitions(checker: &mut Checker) {
Rule::UndocumentedPublicPackage, Rule::UndocumentedPublicPackage,
]); ]);
if !enforce_annotations && !enforce_docstrings && !enforce_stubs && !enforce_stubs_and_runtime { if !enforce_annotations
&& !enforce_docstrings
&& !enforce_stubs
&& !enforce_stubs_and_runtime
&& !enforce_dunder_method
{
return; return;
} }
@ -147,6 +155,19 @@ pub(crate) fn definitions(checker: &mut Checker) {
} }
} }
// pylint
if enforce_dunder_method {
if checker.enabled(Rule::BadDunderMethodName) {
if let Definition::Member(Member {
kind: MemberKind::Method(method),
..
}) = definition
{
pylint::rules::bad_dunder_method_name(checker, method);
}
}
}
// pydocstyle // pydocstyle
if enforce_docstrings { if enforce_docstrings {
if pydocstyle::helpers::should_ignore_definition( if pydocstyle::helpers::should_ignore_definition(

View File

@ -513,9 +513,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::SingleStringSlots) { if checker.enabled(Rule::SingleStringSlots) {
pylint::rules::single_string_slots(checker, class_def); pylint::rules::single_string_slots(checker, class_def);
} }
if checker.enabled(Rule::BadDunderMethodName) {
pylint::rules::bad_dunder_method_name(checker, body);
}
if checker.enabled(Rule::MetaClassABCMeta) { if checker.enabled(Rule::MetaClassABCMeta) {
refurb::rules::metaclass_abcmeta(checker, class_def); refurb::rules::metaclass_abcmeta(checker, class_def);
} }

View File

@ -1,7 +1,7 @@
use ruff_diagnostics::{Diagnostic, Violation}; use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast as ast;
use ruff_python_ast::identifier::Identifier; use ruff_python_ast::identifier::Identifier;
use ruff_python_ast::Stmt;
use ruff_python_semantic::analyze::visibility; use ruff_python_semantic::analyze::visibility;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
@ -56,32 +56,32 @@ impl Violation for BadDunderMethodName {
} }
/// PLW3201 /// PLW3201
pub(crate) fn bad_dunder_method_name(checker: &mut Checker, class_body: &[Stmt]) { pub(crate) fn bad_dunder_method_name(checker: &mut Checker, method: &ast::StmtFunctionDef) {
for method in class_body // If the name isn't a dunder, skip it.
.iter() if !method.name.starts_with('_') || !method.name.ends_with('_') {
.filter_map(ruff_python_ast::Stmt::as_function_def_stmt) return;
.filter(|method| {
if is_known_dunder_method(&method.name)
|| checker
.settings
.pylint
.allow_dunder_method_names
.contains(method.name.as_str())
|| matches!(method.name.as_str(), "_")
{
return false;
}
method.name.starts_with('_') && method.name.ends_with('_')
})
{
if visibility::is_override(&method.decorator_list, checker.semantic()) {
continue;
}
checker.diagnostics.push(Diagnostic::new(
BadDunderMethodName {
name: method.name.to_string(),
},
method.identifier(),
));
} }
// If the name is explicitly allowed, skip it.
if is_known_dunder_method(&method.name)
|| checker
.settings
.pylint
.allow_dunder_method_names
.contains(method.name.as_str())
|| matches!(method.name.as_str(), "_")
{
return;
}
if visibility::is_override(&method.decorator_list, checker.semantic()) {
return;
}
checker.diagnostics.push(Diagnostic::new(
BadDunderMethodName {
name: method.name.to_string(),
},
method.identifier(),
));
} }