diff --git a/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs b/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs index 89bffb0d128b4..9f2995de35486 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs @@ -174,7 +174,7 @@ pub(crate) fn definitions(checker: &mut Checker) { if enforce_docstrings || enforce_pydoclint { if pydocstyle::helpers::should_ignore_definition( definition, - &checker.settings.pydocstyle.ignore_decorators, + &checker.settings.pydocstyle, &checker.semantic, ) { continue; @@ -271,7 +271,7 @@ pub(crate) fn definitions(checker: &mut Checker) { pydocstyle::rules::non_imperative_mood( checker, &docstring, - &checker.settings.pydocstyle.property_decorators, + &checker.settings.pydocstyle, ); } if checker.enabled(Rule::NoSignature) { @@ -310,7 +310,7 @@ pub(crate) fn definitions(checker: &mut Checker) { if enforce_sections || enforce_pydoclint { let section_contexts = pydocstyle::helpers::get_section_contexts( &docstring, - checker.settings.pydocstyle.convention.as_ref(), + checker.settings.pydocstyle.convention(), ); if enforce_sections { @@ -318,7 +318,7 @@ pub(crate) fn definitions(checker: &mut Checker) { checker, &docstring, §ion_contexts, - checker.settings.pydocstyle.convention.as_ref(), + checker.settings.pydocstyle.convention(), ); } @@ -327,7 +327,7 @@ pub(crate) fn definitions(checker: &mut Checker) { checker, definition, §ion_contexts, - checker.settings.pydocstyle.convention.as_ref(), + checker.settings.pydocstyle.convention(), ); } } diff --git a/crates/ruff_linter/src/rules/flake8_return/rules/function.rs b/crates/ruff_linter/src/rules/flake8_return/rules/function.rs index 96780d9cedf0a..17e041bed5c8e 100644 --- a/crates/ruff_linter/src/rules/flake8_return/rules/function.rs +++ b/crates/ruff_linter/src/rules/flake8_return/rules/function.rs @@ -6,7 +6,6 @@ use ruff_diagnostics::{AlwaysFixableViolation, FixAvailability, Violation}; use ruff_diagnostics::{Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::helpers::{is_const_false, is_const_true}; -use ruff_python_ast::name::QualifiedName; use ruff_python_ast::stmt_if::elif_else_range; use ruff_python_ast::visitor::Visitor; use ruff_python_ast::whitespace::indentation; @@ -375,18 +374,10 @@ fn unnecessary_return_none(checker: &mut Checker, decorator_list: &[Decorator], continue; } - let extra_property_decorators = checker - .settings - .pydocstyle - .property_decorators - .iter() - .map(|decorator| QualifiedName::from_dotted_name(decorator)) - .collect::>(); - // Skip property functions if is_property( decorator_list, - &extra_property_decorators, + checker.settings.pydocstyle.property_decorators(), checker.semantic(), ) { return; diff --git a/crates/ruff_linter/src/rules/pydoclint/mod.rs b/crates/ruff_linter/src/rules/pydoclint/mod.rs index 77399017677e9..6ef019ce5e99f 100644 --- a/crates/ruff_linter/src/rules/pydoclint/mod.rs +++ b/crates/ruff_linter/src/rules/pydoclint/mod.rs @@ -3,7 +3,6 @@ pub(crate) mod rules; #[cfg(test)] mod tests { - use std::collections::BTreeSet; use std::convert::AsRef; use std::path::Path; @@ -11,7 +10,8 @@ mod tests { use test_case::test_case; use crate::registry::Rule; - use crate::rules::pydocstyle::settings::{Convention, Settings}; + use crate::rules::pydocstyle; + use crate::rules::pydocstyle::settings::Convention; use crate::test::test_path; use crate::{assert_messages, settings}; @@ -35,11 +35,7 @@ mod tests { let diagnostics = test_path( Path::new("pydoclint").join(path).as_path(), &settings::LinterSettings { - pydocstyle: Settings { - convention: Some(Convention::Google), - ignore_decorators: BTreeSet::new(), - property_decorators: BTreeSet::new(), - }, + pydocstyle: pydocstyle::settings::Settings::new(Some(Convention::Google), [], []), ..settings::LinterSettings::for_rule(rule_code) }, )?; @@ -56,11 +52,7 @@ mod tests { let diagnostics = test_path( Path::new("pydoclint").join(path).as_path(), &settings::LinterSettings { - pydocstyle: Settings { - convention: Some(Convention::Numpy), - ignore_decorators: BTreeSet::new(), - property_decorators: BTreeSet::new(), - }, + pydocstyle: pydocstyle::settings::Settings::new(Some(Convention::Numpy), [], []), ..settings::LinterSettings::for_rule(rule_code) }, )?; diff --git a/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs b/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs index 0fdb0d09d9f78..fbe27104382b5 100644 --- a/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs +++ b/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs @@ -444,7 +444,7 @@ pub(crate) fn check_docstring( checker: &mut Checker, definition: &Definition, section_contexts: &SectionContexts, - convention: Option<&Convention>, + convention: Option, ) { let mut diagnostics = Vec::new(); let Definition::Member(member) = definition else { @@ -478,14 +478,8 @@ pub(crate) fn check_docstring( // DOC201 if checker.enabled(Rule::DocstringMissingReturns) && docstring_sections.returns.is_none() { - let extra_property_decorators = checker - .settings - .pydocstyle - .property_decorators - .iter() - .map(|decorator| QualifiedName::from_dotted_name(decorator)) - .collect::>(); - if !definition.is_property(&extra_property_decorators, checker.semantic()) { + let extra_property_decorators = checker.settings.pydocstyle.property_decorators(); + if !definition.is_property(extra_property_decorators, checker.semantic()) { if let Some(body_return) = body_entries.returns.first() { let diagnostic = Diagnostic::new(DocstringMissingReturns, body_return.range()); diagnostics.push(diagnostic); diff --git a/crates/ruff_linter/src/rules/pydocstyle/helpers.rs b/crates/ruff_linter/src/rules/pydocstyle/helpers.rs index 3b78c003c9b8d..e943ac915ce8a 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/helpers.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/helpers.rs @@ -1,14 +1,11 @@ -use std::collections::BTreeSet; - use ruff_python_ast::helpers::map_callable; -use ruff_python_ast::name::QualifiedName; use ruff_python_semantic::{Definition, SemanticModel}; use ruff_source_file::UniversalNewlines; use crate::docstrings::sections::{SectionContexts, SectionKind}; use crate::docstrings::styles::SectionStyle; use crate::docstrings::Docstring; -use crate::rules::pydocstyle::settings::Convention; +use crate::rules::pydocstyle::settings::{Convention, Settings}; /// Return the index of the first logical line in a string. pub(super) fn logical_line(content: &str) -> Option { @@ -45,10 +42,12 @@ pub(super) fn ends_with_backslash(line: &str) -> bool { /// Check decorator list to see if function should be ignored. pub(crate) fn should_ignore_definition( definition: &Definition, - ignore_decorators: &BTreeSet, + settings: &Settings, semantic: &SemanticModel, ) -> bool { - if ignore_decorators.is_empty() { + let ignore_decorators = settings.ignore_decorators(); + + if ignore_decorators.len() == 0 { return false; } @@ -61,15 +60,15 @@ pub(crate) fn should_ignore_definition( .resolve_qualified_name(map_callable(&decorator.expression)) .is_some_and(|qualified_name| { ignore_decorators - .iter() - .any(|decorator| QualifiedName::from_dotted_name(decorator) == qualified_name) + .clone() + .any(|decorator| decorator == qualified_name) }) }) } pub(crate) fn get_section_contexts<'a>( docstring: &'a Docstring<'a>, - convention: Option<&'a Convention>, + convention: Option, ) -> SectionContexts<'a> { match convention { Some(Convention::Google) => { diff --git a/crates/ruff_linter/src/rules/pydocstyle/mod.rs b/crates/ruff_linter/src/rules/pydocstyle/mod.rs index 5b2d237766c46..1ea3ff9ffd3ac 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/mod.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/mod.rs @@ -5,7 +5,6 @@ pub mod settings; #[cfg(test)] mod tests { - use std::collections::BTreeSet; use std::path::Path; use anyhow::Result; @@ -98,13 +97,11 @@ mod tests { let diagnostics = test_path( Path::new("pydocstyle").join(path).as_path(), &settings::LinterSettings { - pydocstyle: Settings { - convention: None, - ignore_decorators: BTreeSet::from_iter(["functools.wraps".to_string()]), - property_decorators: BTreeSet::from_iter([ - "gi.repository.GObject.Property".to_string() - ]), - }, + pydocstyle: Settings::new( + None, + ["functools.wraps".to_string()], + ["gi.repository.GObject.Property".to_string()], + ), ..settings::LinterSettings::for_rule(rule_code) }, )?; @@ -129,11 +126,7 @@ mod tests { &settings::LinterSettings { // When inferring the convention, we'll see a few false negatives. // See: https://github.com/PyCQA/pydocstyle/issues/459. - pydocstyle: Settings { - convention: None, - ignore_decorators: BTreeSet::new(), - property_decorators: BTreeSet::new(), - }, + pydocstyle: Settings::default(), ..settings::LinterSettings::for_rule(Rule::UndocumentedParam) }, )?; @@ -147,11 +140,7 @@ mod tests { Path::new("pydocstyle/D417.py"), &settings::LinterSettings { // With explicit Google convention, we should flag every function. - pydocstyle: Settings { - convention: Some(Convention::Google), - ignore_decorators: BTreeSet::new(), - property_decorators: BTreeSet::new(), - }, + pydocstyle: Settings::new(Some(Convention::Google), [], []), ..settings::LinterSettings::for_rule(Rule::UndocumentedParam) }, )?; @@ -164,12 +153,8 @@ mod tests { let diagnostics = test_path( Path::new("pydocstyle/D417.py"), &settings::LinterSettings { - // With explicit Google convention, we shouldn't flag anything. - pydocstyle: Settings { - convention: Some(Convention::Numpy), - ignore_decorators: BTreeSet::new(), - property_decorators: BTreeSet::new(), - }, + // With explicit numpy convention, we shouldn't flag anything. + pydocstyle: Settings::new(Some(Convention::Numpy), [], []), ..settings::LinterSettings::for_rule(Rule::UndocumentedParam) }, )?; diff --git a/crates/ruff_linter/src/rules/pydocstyle/rules/non_imperative_mood.rs b/crates/ruff_linter/src/rules/pydocstyle/rules/non_imperative_mood.rs index 3a2576775b386..2391aeb11a131 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/rules/non_imperative_mood.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/rules/non_imperative_mood.rs @@ -1,11 +1,8 @@ -use std::collections::BTreeSet; - use imperative::Mood; use once_cell::sync::Lazy; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::name::QualifiedName; use ruff_python_semantic::analyze::visibility::{is_property, is_test}; use ruff_source_file::UniversalNewlines; use ruff_text_size::Ranged; @@ -13,6 +10,7 @@ use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::docstrings::Docstring; use crate::rules::pydocstyle::helpers::normalize_word; +use crate::rules::pydocstyle::settings::Settings; static MOOD: Lazy = Lazy::new(Mood::new); @@ -66,24 +64,21 @@ impl Violation for NonImperativeMood { pub(crate) fn non_imperative_mood( checker: &mut Checker, docstring: &Docstring, - property_decorators: &BTreeSet, + settings: &Settings, ) { let Some(function) = docstring.definition.as_function_def() else { return; }; - let property_decorators = property_decorators - .iter() - .map(|decorator| QualifiedName::from_dotted_name(decorator)) - .collect::>(); + if is_test(&function.name) { + return; + } - if is_test(&function.name) - || is_property( - &function.decorator_list, - &property_decorators, - checker.semantic(), - ) - { + if is_property( + &function.decorator_list, + settings.property_decorators(), + checker.semantic(), + ) { return; } diff --git a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs index 7385226e0904b..95e0c46af6163 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs @@ -1325,7 +1325,7 @@ pub(crate) fn sections( checker: &mut Checker, docstring: &Docstring, section_contexts: &SectionContexts, - convention: Option<&Convention>, + convention: Option, ) { match convention { Some(Convention::Google) => parse_google_sections(checker, docstring, section_contexts), diff --git a/crates/ruff_linter/src/rules/pydocstyle/settings.rs b/crates/ruff_linter/src/rules/pydocstyle/settings.rs index 974c8742f9ec0..c8b05ba3c5012 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/settings.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/settings.rs @@ -2,12 +2,14 @@ use std::collections::BTreeSet; use std::fmt; +use std::iter::FusedIterator; use serde::{Deserialize, Serialize}; -use crate::display_settings; use ruff_macros::CacheKey; +use ruff_python_ast::name::QualifiedName; +use crate::display_settings; use crate::registry::Rule; #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, CacheKey)] @@ -85,9 +87,36 @@ impl fmt::Display for Convention { #[derive(Debug, Clone, Default, CacheKey)] pub struct Settings { - pub convention: Option, - pub ignore_decorators: BTreeSet, - pub property_decorators: BTreeSet, + convention: Option, + ignore_decorators: BTreeSet, + property_decorators: BTreeSet, +} + +impl Settings { + #[must_use] + pub fn new( + convention: Option, + ignore_decorators: impl IntoIterator, + property_decorators: impl IntoIterator, + ) -> Self { + Self { + convention, + ignore_decorators: ignore_decorators.into_iter().collect(), + property_decorators: property_decorators.into_iter().collect(), + } + } + + pub fn convention(&self) -> Option { + self.convention + } + + pub fn ignore_decorators(&self) -> DecoratorIterator { + DecoratorIterator::new(&self.ignore_decorators) + } + + pub fn property_decorators(&self) -> DecoratorIterator { + DecoratorIterator::new(&self.property_decorators) + } } impl fmt::Display for Settings { @@ -104,3 +133,34 @@ impl fmt::Display for Settings { Ok(()) } } + +#[derive(Debug, Clone)] +pub struct DecoratorIterator<'a> { + decorators: std::collections::btree_set::Iter<'a, String>, +} + +impl<'a> DecoratorIterator<'a> { + fn new(decorators: &'a BTreeSet) -> Self { + Self { + decorators: decorators.iter(), + } + } +} + +impl<'a> Iterator for DecoratorIterator<'a> { + type Item = QualifiedName<'a>; + + fn next(&mut self) -> Option> { + self.decorators + .next() + .map(|deco| QualifiedName::from_dotted_name(deco)) + } +} + +impl FusedIterator for DecoratorIterator<'_> {} + +impl ExactSizeIterator for DecoratorIterator<'_> { + fn len(&self) -> usize { + self.decorators.len() + } +} diff --git a/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs b/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs index 7dfa73cece80f..e2f14d09bd329 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs @@ -2,7 +2,6 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast as ast; use ruff_python_ast::identifier::Identifier; -use ruff_python_ast::name::QualifiedName; use ruff_python_semantic::{ analyze::{function_type, visibility}, Scope, ScopeId, ScopeKind, @@ -49,7 +48,9 @@ pub(crate) fn no_self_use( scope: &Scope, diagnostics: &mut Vec, ) { - let Some(parent) = checker.semantic().first_non_type_parent_scope(scope) else { + let semantic = checker.semantic(); + + let Some(parent) = semantic.first_non_type_parent_scope(scope) else { return; }; @@ -69,7 +70,7 @@ pub(crate) fn no_self_use( name, decorator_list, parent, - checker.semantic(), + semantic, &checker.settings.pep8_naming.classmethod_decorators, &checker.settings.pep8_naming.staticmethod_decorators, ), @@ -78,20 +79,14 @@ pub(crate) fn no_self_use( return; } - let property_decorators = checker - .settings - .pydocstyle - .property_decorators - .iter() - .map(|decorator| QualifiedName::from_dotted_name(decorator)) - .collect::>(); + let extra_property_decorators = checker.settings.pydocstyle.property_decorators(); - if function_type::is_stub(func, checker.semantic()) + if function_type::is_stub(func, semantic) || visibility::is_magic(name) - || visibility::is_abstract(decorator_list, checker.semantic()) - || visibility::is_override(decorator_list, checker.semantic()) - || visibility::is_overload(decorator_list, checker.semantic()) - || visibility::is_property(decorator_list, &property_decorators, checker.semantic()) + || visibility::is_abstract(decorator_list, semantic) + || visibility::is_override(decorator_list, semantic) + || visibility::is_overload(decorator_list, semantic) + || visibility::is_property(decorator_list, extra_property_decorators, semantic) { return; } @@ -113,12 +108,12 @@ pub(crate) fn no_self_use( // If the method contains a `super` reference, then it should be considered to use self // implicitly. - if let Some(binding_id) = checker.semantic().global_scope().get("super") { - let binding = checker.semantic().binding(binding_id); + if let Some(binding_id) = semantic.global_scope().get("super") { + let binding = semantic.binding(binding_id); if binding.kind.is_builtin() { if binding .references() - .any(|id| checker.semantic().reference(id).scope_id() == scope_id) + .any(|id| semantic.reference(id).scope_id() == scope_id) { return; } @@ -127,7 +122,7 @@ pub(crate) fn no_self_use( if scope .get("self") - .map(|binding_id| checker.semantic().binding(binding_id)) + .map(|binding_id| semantic.binding(binding_id)) .is_some_and(|binding| binding.kind.is_argument() && !binding.is_used()) { diagnostics.push(Diagnostic::new( diff --git a/crates/ruff_linter/src/rules/pylint/rules/property_with_parameters.rs b/crates/ruff_linter/src/rules/pylint/rules/property_with_parameters.rs index 32b54c5f3f976..16764ff3f715f 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/property_with_parameters.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/property_with_parameters.rs @@ -1,6 +1,5 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::name::QualifiedName; use ruff_python_ast::{identifier::Identifier, Decorator, Parameters, Stmt}; use ruff_python_semantic::analyze::visibility::is_property; @@ -57,14 +56,8 @@ pub(crate) fn property_with_parameters( return; } let semantic = checker.semantic(); - let extra_property_decorators = checker - .settings - .pydocstyle - .property_decorators - .iter() - .map(|decorator| QualifiedName::from_dotted_name(decorator)) - .collect::>(); - if is_property(decorator_list, &extra_property_decorators, semantic) { + let extra_property_decorators = checker.settings.pydocstyle.property_decorators(); + if is_property(decorator_list, extra_property_decorators, semantic) { checker .diagnostics .push(Diagnostic::new(PropertyWithParameters, stmt.identifier())); diff --git a/crates/ruff_python_semantic/src/analyze/visibility.rs b/crates/ruff_python_semantic/src/analyze/visibility.rs index e3f77fcfc1ff3..e28e13d338f7e 100644 --- a/crates/ruff_python_semantic/src/analyze/visibility.rs +++ b/crates/ruff_python_semantic/src/analyze/visibility.rs @@ -63,11 +63,16 @@ pub fn is_abstract(decorator_list: &[Decorator], semantic: &SemanticModel) -> bo /// Returns `true` if a function definition is a `@property`. /// `extra_properties` can be used to check additional non-standard /// `@property`-like decorators. -pub fn is_property( +pub fn is_property<'a, P, I>( decorator_list: &[Decorator], - extra_properties: &[QualifiedName], + extra_properties: P, semantic: &SemanticModel, -) -> bool { +) -> bool +where + P: IntoIterator, + I: Iterator> + Clone, +{ + let extra_properties = extra_properties.into_iter(); decorator_list.iter().any(|decorator| { semantic .resolve_qualified_name(map_callable(&decorator.expression)) @@ -79,8 +84,8 @@ pub fn is_property( | ["abc", "abstractproperty"] | ["types", "DynamicClassAttribute"] ) || extra_properties - .iter() - .any(|extra_property| extra_property.segments() == qualified_name.segments()) + .clone() + .any(|extra_property| extra_property == qualified_name) }) }) } diff --git a/crates/ruff_python_semantic/src/definition.rs b/crates/ruff_python_semantic/src/definition.rs index 667fc8109c7a4..3a14a0be4aaf2 100644 --- a/crates/ruff_python_semantic/src/definition.rs +++ b/crates/ruff_python_semantic/src/definition.rs @@ -151,11 +151,11 @@ impl<'a> Definition<'a> { ) } - pub fn is_property( - &self, - extra_properties: &[QualifiedName], - semantic: &SemanticModel, - ) -> bool { + pub fn is_property(&self, extra_properties: P, semantic: &SemanticModel) -> bool + where + P: IntoIterator, + I: Iterator> + Clone, + { self.as_function_def() .is_some_and(|StmtFunctionDef { decorator_list, .. }| { is_property(decorator_list, extra_properties, semantic) diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index b54e6275f6ad1..30ba34612b545 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -1,5 +1,3 @@ -use std::collections::BTreeSet; - use regex::Regex; use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; use serde::{Deserialize, Serialize}; @@ -2762,11 +2760,16 @@ pub struct PydocstyleOptions { impl PydocstyleOptions { pub fn into_settings(self) -> pydocstyle::settings::Settings { - pydocstyle::settings::Settings { - convention: self.convention, - ignore_decorators: BTreeSet::from_iter(self.ignore_decorators.unwrap_or_default()), - property_decorators: BTreeSet::from_iter(self.property_decorators.unwrap_or_default()), - } + let PydocstyleOptions { + convention, + ignore_decorators, + property_decorators, + } = self; + pydocstyle::settings::Settings::new( + convention, + ignore_decorators.unwrap_or_default(), + property_decorators.unwrap_or_default(), + ) } }