Implement `property-decorators` configuration option for pydocstyle (#3311)

This commit is contained in:
Yaroslav Chvanov 2023-03-02 21:59:56 +00:00 committed by GitHub
parent ffdf6e35e6
commit 508bc605a5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 90 additions and 20 deletions

View File

@ -2,6 +2,8 @@
from functools import cached_property from functools import cached_property
from gi.repository import GObject
# Bad examples # Bad examples
def bad_liouiwnlkjl(): def bad_liouiwnlkjl():
@ -76,6 +78,11 @@ class Thingy:
"""This property method docstring does not need to be written in imperative mood.""" """This property method docstring does not need to be written in imperative mood."""
return self._beep return self._beep
@GObject.Property
def good_custom_property(self):
"""This property method docstring does not need to be written in imperative mood."""
return self._beep
@cached_property @cached_property
def good_cached_property(self): def good_cached_property(self):
"""This property method docstring does not need to be written in imperative mood.""" """This property method docstring does not need to be written in imperative mood."""

View File

@ -5593,7 +5593,11 @@ impl<'a> Checker<'a> {
pydocstyle::rules::ends_with_period(self, &docstring); pydocstyle::rules::ends_with_period(self, &docstring);
} }
if self.settings.rules.enabled(&Rule::NonImperativeMood) { if self.settings.rules.enabled(&Rule::NonImperativeMood) {
pydocstyle::rules::non_imperative_mood(self, &docstring); pydocstyle::rules::non_imperative_mood(
self,
&docstring,
&self.settings.pydocstyle.property_decorators,
);
} }
if self.settings.rules.enabled(&Rule::NoSignature) { if self.settings.rules.enabled(&Rule::NoSignature) {
pydocstyle::rules::no_signature(self, &docstring); pydocstyle::rules::no_signature(self, &docstring);

View File

@ -577,6 +577,7 @@ mod tests {
pydocstyle: Some(pydocstyle::settings::Options { pydocstyle: Some(pydocstyle::settings::Options {
convention: Some(Convention::Numpy), convention: Some(Convention::Numpy),
ignore_decorators: None, ignore_decorators: None,
property_decorators: None,
}), }),
..default_options([Linter::Pydocstyle.into()]) ..default_options([Linter::Pydocstyle.into()])
}); });

View File

@ -77,6 +77,9 @@ mod tests {
pydocstyle: Settings { pydocstyle: Settings {
convention: None, convention: None,
ignore_decorators: BTreeSet::from_iter(["functools.wraps".to_string()]), ignore_decorators: BTreeSet::from_iter(["functools.wraps".to_string()]),
property_decorators: BTreeSet::from_iter([
"gi.repository.GObject.Property".to_string()
]),
}, },
..settings::Settings::for_rule(rule_code) ..settings::Settings::for_rule(rule_code)
}, },
@ -95,6 +98,7 @@ mod tests {
pydocstyle: Settings { pydocstyle: Settings {
convention: None, convention: None,
ignore_decorators: BTreeSet::new(), ignore_decorators: BTreeSet::new(),
property_decorators: BTreeSet::new(),
}, },
..settings::Settings::for_rule(Rule::UndocumentedParam) ..settings::Settings::for_rule(Rule::UndocumentedParam)
}, },
@ -112,6 +116,7 @@ mod tests {
pydocstyle: Settings { pydocstyle: Settings {
convention: Some(Convention::Google), convention: Some(Convention::Google),
ignore_decorators: BTreeSet::new(), ignore_decorators: BTreeSet::new(),
property_decorators: BTreeSet::new(),
}, },
..settings::Settings::for_rule(Rule::UndocumentedParam) ..settings::Settings::for_rule(Rule::UndocumentedParam)
}, },
@ -129,6 +134,7 @@ mod tests {
pydocstyle: Settings { pydocstyle: Settings {
convention: Some(Convention::Numpy), convention: Some(Convention::Numpy),
ignore_decorators: BTreeSet::new(), ignore_decorators: BTreeSet::new(),
property_decorators: BTreeSet::new(),
}, },
..settings::Settings::for_rule(Rule::UndocumentedParam) ..settings::Settings::for_rule(Rule::UndocumentedParam)
}, },

View File

@ -1,9 +1,12 @@
use std::collections::BTreeSet;
use imperative::Mood; use imperative::Mood;
use once_cell::sync::Lazy; use once_cell::sync::Lazy;
use ruff_macros::{define_violation, derive_message_formats}; use ruff_macros::{define_violation, derive_message_formats};
use crate::ast::cast; use crate::ast::cast;
use crate::ast::types::Range; use crate::ast::helpers::to_call_path;
use crate::ast::types::{CallPath, Range};
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::docstrings::definition::{DefinitionKind, Docstring}; use crate::docstrings::definition::{DefinitionKind, Docstring};
use crate::registry::Diagnostic; use crate::registry::Diagnostic;
@ -14,7 +17,11 @@ use crate::visibility::{is_property, is_test};
static MOOD: Lazy<Mood> = Lazy::new(Mood::new); static MOOD: Lazy<Mood> = Lazy::new(Mood::new);
/// D401 /// D401
pub fn non_imperative_mood(checker: &mut Checker, docstring: &Docstring) { pub fn non_imperative_mood(
checker: &mut Checker,
docstring: &Docstring,
property_decorators: &BTreeSet<String>,
) {
let ( let (
DefinitionKind::Function(parent) DefinitionKind::Function(parent)
| DefinitionKind::NestedFunction(parent) | DefinitionKind::NestedFunction(parent)
@ -22,7 +29,15 @@ pub fn non_imperative_mood(checker: &mut Checker, docstring: &Docstring) {
) = &docstring.kind else { ) = &docstring.kind else {
return; return;
}; };
if is_test(cast::name(parent)) || is_property(checker, cast::decorator_list(parent)) {
let property_decorators = property_decorators
.iter()
.map(|decorator| to_call_path(decorator))
.collect::<Vec<CallPath>>();
if is_test(cast::name(parent))
|| is_property(checker, cast::decorator_list(parent), &property_decorators)
{
return; return;
} }

View File

@ -97,12 +97,25 @@ pub struct Options {
/// specified decorators. Unlike the `pydocstyle`, Ruff accepts an array /// specified decorators. Unlike the `pydocstyle`, Ruff accepts an array
/// of fully-qualified module identifiers, instead of a regular expression. /// of fully-qualified module identifiers, instead of a regular expression.
pub ignore_decorators: Option<Vec<String>>, pub ignore_decorators: Option<Vec<String>>,
#[option(
default = r#"[]"#,
value_type = "list[str]",
example = r#"
property-decorators = ["gi.repository.GObject.Property"]
"#
)]
/// Consider any method decorated with one of these decorators as a property,
/// and consequently allow a docstring which is not in imperative mood.
/// Unlike pydocstyle, supplying this option doesn't disable standard
/// property decorators - `@property` and `@cached_property`.
pub property_decorators: Option<Vec<String>>,
} }
#[derive(Debug, Default, Hash)] #[derive(Debug, Default, Hash)]
pub struct Settings { pub struct Settings {
pub convention: Option<Convention>, pub convention: Option<Convention>,
pub ignore_decorators: BTreeSet<String>, pub ignore_decorators: BTreeSet<String>,
pub property_decorators: BTreeSet<String>,
} }
impl From<Options> for Settings { impl From<Options> for Settings {
@ -110,6 +123,9 @@ impl From<Options> for Settings {
Self { Self {
convention: options.convention, convention: options.convention,
ignore_decorators: BTreeSet::from_iter(options.ignore_decorators.unwrap_or_default()), ignore_decorators: BTreeSet::from_iter(options.ignore_decorators.unwrap_or_default()),
property_decorators: BTreeSet::from_iter(
options.property_decorators.unwrap_or_default(),
),
} }
} }
} }
@ -119,6 +135,7 @@ impl From<Settings> for Options {
Self { Self {
convention: settings.convention, convention: settings.convention,
ignore_decorators: Some(settings.ignore_decorators.into_iter().collect()), ignore_decorators: Some(settings.ignore_decorators.into_iter().collect()),
property_decorators: Some(settings.property_decorators.into_iter().collect()),
} }
} }
} }

View File

@ -1,74 +1,74 @@
--- ---
source: src/rules/pydocstyle/mod.rs source: crates/ruff/src/rules/pydocstyle/mod.rs
expression: diagnostics expression: diagnostics
--- ---
- kind: - kind:
NonImperativeMood: Returns foo. NonImperativeMood: Returns foo.
location: location:
row: 8 row: 10
column: 4 column: 4
end_location: end_location:
row: 8 row: 10
column: 22 column: 22
fix: ~ fix: ~
parent: ~ parent: ~
- kind: - kind:
NonImperativeMood: Constructor for a foo. NonImperativeMood: Constructor for a foo.
location: location:
row: 12 row: 14
column: 4 column: 4
end_location: end_location:
row: 12 row: 14
column: 32 column: 32
fix: ~ fix: ~
parent: ~ parent: ~
- kind: - kind:
NonImperativeMood: Constructor for a boa. NonImperativeMood: Constructor for a boa.
location: location:
row: 16 row: 18
column: 4 column: 4
end_location: end_location:
row: 20 row: 22
column: 7 column: 7
fix: ~ fix: ~
parent: ~ parent: ~
- kind: - kind:
NonImperativeMood: Runs something NonImperativeMood: Runs something
location: location:
row: 24 row: 26
column: 4 column: 4
end_location: end_location:
row: 24 row: 26
column: 24 column: 24
fix: ~ fix: ~
parent: ~ parent: ~
- kind: - kind:
NonImperativeMood: "Runs other things, nested" NonImperativeMood: "Runs other things, nested"
location: location:
row: 27 row: 29
column: 8 column: 8
end_location: end_location:
row: 27 row: 29
column: 39 column: 39
fix: ~ fix: ~
parent: ~ parent: ~
- kind: - kind:
NonImperativeMood: Writes a logical line that NonImperativeMood: Writes a logical line that
location: location:
row: 33 row: 35
column: 4 column: 4
end_location: end_location:
row: 35 row: 37
column: 7 column: 7
fix: ~ fix: ~
parent: ~ parent: ~
- kind: - kind:
NonImperativeMood: This method docstring should be written in imperative mood. NonImperativeMood: This method docstring should be written in imperative mood.
location: location:
row: 72 row: 74
column: 8 column: 8
end_location: end_location:
row: 72 row: 74
column: 73 column: 73
fix: ~ fix: ~
parent: ~ parent: ~

View File

@ -6,6 +6,7 @@ use std::path::Path;
use rustpython_parser::ast::{Expr, Stmt, StmtKind}; use rustpython_parser::ast::{Expr, Stmt, StmtKind};
use crate::ast::helpers::{collect_call_path, map_callable}; use crate::ast::helpers::{collect_call_path, map_callable};
use crate::ast::types::CallPath;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::docstrings::definition::Documentable; use crate::docstrings::definition::Documentable;
@ -77,13 +78,22 @@ pub fn is_abstract(checker: &Checker, decorator_list: &[Expr]) -> bool {
} }
/// Returns `true` if a function definition is a `@property`. /// Returns `true` if a function definition is a `@property`.
pub fn is_property(checker: &Checker, decorator_list: &[Expr]) -> bool { /// `extra_properties` can be used to check additional non-standard
/// `@property`-like decorators.
pub fn is_property(
checker: &Checker,
decorator_list: &[Expr],
extra_properties: &[CallPath],
) -> bool {
decorator_list.iter().any(|expr| { decorator_list.iter().any(|expr| {
checker checker
.resolve_call_path(map_callable(expr)) .resolve_call_path(map_callable(expr))
.map_or(false, |call_path| { .map_or(false, |call_path| {
call_path.as_slice() == ["", "property"] call_path.as_slice() == ["", "property"]
|| call_path.as_slice() == ["functools", "cached_property"] || call_path.as_slice() == ["functools", "cached_property"]
|| extra_properties
.iter()
.any(|extra_property| extra_property.as_slice() == call_path.as_slice())
}) })
}) })
} }

10
ruff.schema.json generated
View File

@ -1268,6 +1268,16 @@
"items": { "items": {
"type": "string" "type": "string"
} }
},
"property-decorators": {
"description": "Consider any method decorated with one of these decorators as a property, and consequently allow a docstring which is not in imperative mood. Unlike pydocstyle, supplying this option doesn't disable standard property decorators - `@property` and `@cached_property`.",
"type": [
"array",
"null"
],
"items": {
"type": "string"
}
} }
}, },
"additionalProperties": false "additionalProperties": false