mirror of https://github.com/astral-sh/ruff
Implement docstring visibility checks (#408)
This commit is contained in:
parent
f30e5e45ab
commit
688fc0cd02
16
README.md
16
README.md
|
|
@ -217,7 +217,7 @@ ruff also implements some of the most popular Flake8 plugins natively, including
|
||||||
- [`flake8-print`](https://pypi.org/project/flake8-print/)
|
- [`flake8-print`](https://pypi.org/project/flake8-print/)
|
||||||
- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) (11/16)
|
- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) (11/16)
|
||||||
- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (3/32)
|
- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (3/32)
|
||||||
- [`flake8-docstrings`](https://pypi.org/project/flake8-docstrings/) (17/48)
|
- [`flake8-docstrings`](https://pypi.org/project/flake8-docstrings/) (25/48)
|
||||||
- [`pyupgrade`](https://pypi.org/project/pyupgrade/) (8/34)
|
- [`pyupgrade`](https://pypi.org/project/pyupgrade/) (8/34)
|
||||||
|
|
||||||
Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis Flake8:
|
Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis Flake8:
|
||||||
|
|
@ -304,6 +304,14 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com
|
||||||
| U006 | UsePEP585Annotation | Use `list` instead of `List` for type annotations | | 🛠 |
|
| U006 | UsePEP585Annotation | Use `list` instead of `List` for type annotations | | 🛠 |
|
||||||
| U007 | UsePEP604Annotation | Use `X \| Y` for type annotations | | 🛠 |
|
| U007 | UsePEP604Annotation | Use `X \| Y` for type annotations | | 🛠 |
|
||||||
| U008 | SuperCallWithParameters | Use `super()` instead of `super(__class__, self)` | | 🛠 |
|
| U008 | SuperCallWithParameters | Use `super()` instead of `super(__class__, self)` | | 🛠 |
|
||||||
|
| D100 | PublicModule | Missing docstring in public module | | |
|
||||||
|
| D101 | PublicClass | Missing docstring in public class | | |
|
||||||
|
| D102 | PublicMethod | Missing docstring in public method | | |
|
||||||
|
| D103 | PublicFunction | Missing docstring in public function | | |
|
||||||
|
| D104 | PublicPackage | Missing docstring in public package | | |
|
||||||
|
| D105 | MagicMethod | Missing docstring in magic method | | |
|
||||||
|
| D106 | PublicNestedClass | Missing docstring in public nested class | | |
|
||||||
|
| D107 | PublicInit | Missing docstring in __init__ | | |
|
||||||
| D200 | FitsOnOneLine | One-line docstring should fit on one line | | |
|
| D200 | FitsOnOneLine | One-line docstring should fit on one line | | |
|
||||||
| D205 | NoBlankLineAfterSummary | 1 blank line required between summary line and description | | |
|
| D205 | NoBlankLineAfterSummary | 1 blank line required between summary line and description | | |
|
||||||
| D209 | NewLineAfterLastParagraph | Multi-line docstring closing quotes should be on a separate line | | |
|
| D209 | NewLineAfterLastParagraph | Multi-line docstring closing quotes should be on a separate line | | |
|
||||||
|
|
@ -318,9 +326,9 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com
|
||||||
| D419 | NonEmpty | Docstring is empty | | |
|
| D419 | NonEmpty | Docstring is empty | | |
|
||||||
| D201 | NoBlankLineBeforeFunction | No blank lines allowed before function docstring (found 1) | | |
|
| D201 | NoBlankLineBeforeFunction | No blank lines allowed before function docstring (found 1) | | |
|
||||||
| D202 | NoBlankLineAfterFunction | No blank lines allowed after function docstring (found 1) | | |
|
| D202 | NoBlankLineAfterFunction | No blank lines allowed after function docstring (found 1) | | |
|
||||||
| D211 | NoBlankLineBeforeClass | NoBlankLineBeforeClass | | |
|
| D211 | NoBlankLineBeforeClass | No blank lines allowed before class docstring | | |
|
||||||
| D203 | OneBlankLineBeforeClass | OneBlankLineBeforeClass | | |
|
| D203 | OneBlankLineBeforeClass | 1 blank line required before class docstring | | |
|
||||||
| D204 | OneBlankLineAfterClass | OneBlankLineAfterClass | | |
|
| D204 | OneBlankLineAfterClass | 1 blank line required after class docstring | | |
|
||||||
| M001 | UnusedNOQA | Unused `noqa` directive | | 🛠 |
|
| M001 | UnusedNOQA | Unused `noqa` directive | | 🛠 |
|
||||||
|
|
||||||
## Integrations
|
## Integrations
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,4 @@
|
||||||
r# No docstring, so we can test D100
|
# No docstring, so we can test D100
|
||||||
from functools import wraps
|
from functools import wraps
|
||||||
import os
|
import os
|
||||||
from .expected import Expectation
|
from .expected import Expectation
|
||||||
|
|
|
||||||
|
|
@ -25,6 +25,7 @@ use crate::docstrings::{Definition, DefinitionKind, Documentable};
|
||||||
use crate::python::builtins::{BUILTINS, MAGIC_GLOBALS};
|
use crate::python::builtins::{BUILTINS, MAGIC_GLOBALS};
|
||||||
use crate::python::future::ALL_FEATURE_NAMES;
|
use crate::python::future::ALL_FEATURE_NAMES;
|
||||||
use crate::settings::{PythonVersion, Settings};
|
use crate::settings::{PythonVersion, Settings};
|
||||||
|
use crate::visibility::{module_visibility, transition_scope, Modifier, VisibleScope};
|
||||||
use crate::{docstrings, plugins};
|
use crate::{docstrings, plugins};
|
||||||
|
|
||||||
pub const GLOBAL_SCOPE_INDEX: usize = 0;
|
pub const GLOBAL_SCOPE_INDEX: usize = 0;
|
||||||
|
|
@ -39,7 +40,7 @@ pub struct Checker<'a> {
|
||||||
// Computed checks.
|
// Computed checks.
|
||||||
checks: Vec<Check>,
|
checks: Vec<Check>,
|
||||||
// Docstring tracking.
|
// Docstring tracking.
|
||||||
docstrings: Vec<Definition<'a>>,
|
docstrings: Vec<(Definition<'a>, VisibleScope)>,
|
||||||
// Edit tracking.
|
// Edit tracking.
|
||||||
// TODO(charlie): Instead of exposing deletions, wrap in a public API.
|
// TODO(charlie): Instead of exposing deletions, wrap in a public API.
|
||||||
pub(crate) deletions: BTreeSet<usize>,
|
pub(crate) deletions: BTreeSet<usize>,
|
||||||
|
|
@ -52,10 +53,11 @@ pub struct Checker<'a> {
|
||||||
dead_scopes: Vec<usize>,
|
dead_scopes: Vec<usize>,
|
||||||
deferred_string_annotations: Vec<(Range, &'a str)>,
|
deferred_string_annotations: Vec<(Range, &'a str)>,
|
||||||
deferred_annotations: Vec<(&'a Expr, Vec<usize>, Vec<usize>)>,
|
deferred_annotations: Vec<(&'a Expr, Vec<usize>, Vec<usize>)>,
|
||||||
deferred_functions: Vec<(&'a Stmt, Vec<usize>, Vec<usize>)>,
|
deferred_functions: Vec<(&'a Stmt, Vec<usize>, Vec<usize>, VisibleScope)>,
|
||||||
deferred_lambdas: Vec<(&'a Expr, Vec<usize>, Vec<usize>)>,
|
deferred_lambdas: Vec<(&'a Expr, Vec<usize>, Vec<usize>)>,
|
||||||
deferred_assignments: Vec<usize>,
|
deferred_assignments: Vec<usize>,
|
||||||
// Internal, derivative state.
|
// Internal, derivative state.
|
||||||
|
visibility: VisibleScope,
|
||||||
in_f_string: Option<Range>,
|
in_f_string: Option<Range>,
|
||||||
in_annotation: bool,
|
in_annotation: bool,
|
||||||
in_literal: bool,
|
in_literal: bool,
|
||||||
|
|
@ -90,6 +92,10 @@ impl<'a> Checker<'a> {
|
||||||
deferred_functions: Default::default(),
|
deferred_functions: Default::default(),
|
||||||
deferred_lambdas: Default::default(),
|
deferred_lambdas: Default::default(),
|
||||||
deferred_assignments: Default::default(),
|
deferred_assignments: Default::default(),
|
||||||
|
visibility: VisibleScope {
|
||||||
|
modifier: Modifier::Module,
|
||||||
|
visibility: module_visibility(path),
|
||||||
|
},
|
||||||
in_f_string: None,
|
in_f_string: None,
|
||||||
in_annotation: Default::default(),
|
in_annotation: Default::default(),
|
||||||
in_literal: Default::default(),
|
in_literal: Default::default(),
|
||||||
|
|
@ -562,46 +568,28 @@ where
|
||||||
}
|
}
|
||||||
|
|
||||||
// Recurse.
|
// Recurse.
|
||||||
|
let prev_visibility = self.visibility.clone();
|
||||||
match &stmt.node {
|
match &stmt.node {
|
||||||
StmtKind::FunctionDef { body, .. } | StmtKind::AsyncFunctionDef { body, .. } => {
|
StmtKind::FunctionDef { body, .. } | StmtKind::AsyncFunctionDef { body, .. } => {
|
||||||
// TODO(charlie): Track public / private.
|
let visibility = transition_scope(&self.visibility, stmt, &Documentable::Function);
|
||||||
// Grab all parents, to enable nested definition tracking. (Ignore the most recent
|
let definition =
|
||||||
// parent which, confusingly, is `stmt`.)
|
docstrings::extract(&self.visibility, stmt, body, &Documentable::Function);
|
||||||
let parents = self
|
self.visibility = visibility.clone();
|
||||||
.parent_stack
|
self.docstrings.push((definition, visibility));
|
||||||
.iter()
|
|
||||||
.take(self.parents.len() - 1)
|
|
||||||
.map(|index| self.parents[*index])
|
|
||||||
.collect();
|
|
||||||
self.docstrings.push(docstrings::extract(
|
|
||||||
parents,
|
|
||||||
stmt,
|
|
||||||
body,
|
|
||||||
Documentable::Function,
|
|
||||||
));
|
|
||||||
|
|
||||||
self.deferred_functions.push((
|
self.deferred_functions.push((
|
||||||
stmt,
|
stmt,
|
||||||
self.scope_stack.clone(),
|
self.scope_stack.clone(),
|
||||||
self.parent_stack.clone(),
|
self.parent_stack.clone(),
|
||||||
|
self.visibility.clone(),
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
StmtKind::ClassDef { body, .. } => {
|
StmtKind::ClassDef { body, .. } => {
|
||||||
// TODO(charlie): Track public / priva
|
let visibility = transition_scope(&self.visibility, stmt, &Documentable::Class);
|
||||||
// Grab all parents, to enable nested definition tracking. (Ignore the most recent
|
let definition =
|
||||||
// parent which, confusingly, is `stmt`.)
|
docstrings::extract(&self.visibility, stmt, body, &Documentable::Class);
|
||||||
let parents = self
|
self.visibility = visibility.clone();
|
||||||
.parent_stack
|
self.docstrings.push((definition, visibility));
|
||||||
.iter()
|
|
||||||
.take(self.parents.len() - 1)
|
|
||||||
.map(|index| self.parents[*index])
|
|
||||||
.collect();
|
|
||||||
self.docstrings.push(docstrings::extract(
|
|
||||||
parents,
|
|
||||||
stmt,
|
|
||||||
body,
|
|
||||||
Documentable::Class,
|
|
||||||
));
|
|
||||||
|
|
||||||
for stmt in body {
|
for stmt in body {
|
||||||
self.visit_stmt(stmt);
|
self.visit_stmt(stmt);
|
||||||
|
|
@ -630,6 +618,7 @@ where
|
||||||
}
|
}
|
||||||
_ => visitor::walk_stmt(self, stmt),
|
_ => visitor::walk_stmt(self, stmt),
|
||||||
};
|
};
|
||||||
|
self.visibility = prev_visibility;
|
||||||
|
|
||||||
// Post-visit.
|
// Post-visit.
|
||||||
if let StmtKind::ClassDef { name, .. } = &stmt.node {
|
if let StmtKind::ClassDef { name, .. } = &stmt.node {
|
||||||
|
|
@ -1649,14 +1638,20 @@ impl<'a> Checker<'a> {
|
||||||
'b: 'a,
|
'b: 'a,
|
||||||
{
|
{
|
||||||
let docstring = docstrings::docstring_from(python_ast);
|
let docstring = docstrings::docstring_from(python_ast);
|
||||||
self.docstrings.push(Definition {
|
self.docstrings.push((
|
||||||
|
Definition {
|
||||||
kind: if self.path.ends_with("__init__.py") {
|
kind: if self.path.ends_with("__init__.py") {
|
||||||
DefinitionKind::Package
|
DefinitionKind::Package
|
||||||
} else {
|
} else {
|
||||||
DefinitionKind::Module
|
DefinitionKind::Module
|
||||||
},
|
},
|
||||||
docstring,
|
docstring,
|
||||||
});
|
},
|
||||||
|
VisibleScope {
|
||||||
|
modifier: Modifier::Module,
|
||||||
|
visibility: module_visibility(self.path),
|
||||||
|
},
|
||||||
|
));
|
||||||
docstring.is_some()
|
docstring.is_some()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -1712,9 +1707,10 @@ impl<'a> Checker<'a> {
|
||||||
}
|
}
|
||||||
|
|
||||||
fn check_deferred_functions(&mut self) {
|
fn check_deferred_functions(&mut self) {
|
||||||
while let Some((stmt, scopes, parents)) = self.deferred_functions.pop() {
|
while let Some((stmt, scopes, parents, visibility)) = self.deferred_functions.pop() {
|
||||||
self.parent_stack = parents;
|
self.parent_stack = parents;
|
||||||
self.scope_stack = scopes;
|
self.scope_stack = scopes;
|
||||||
|
self.visibility = visibility;
|
||||||
self.push_scope(Scope::new(ScopeKind::Function(Default::default())));
|
self.push_scope(Scope::new(ScopeKind::Function(Default::default())));
|
||||||
|
|
||||||
match &stmt.node {
|
match &stmt.node {
|
||||||
|
|
@ -1906,10 +1902,13 @@ impl<'a> Checker<'a> {
|
||||||
}
|
}
|
||||||
|
|
||||||
fn check_docstrings(&mut self) {
|
fn check_docstrings(&mut self) {
|
||||||
while let Some(docstring) = self.docstrings.pop() {
|
while let Some((docstring, scope)) = self.docstrings.pop() {
|
||||||
if !docstrings::not_empty(self, &docstring) {
|
if !docstrings::not_empty(self, &docstring) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
if !docstrings::not_missing(self, &docstring, &scope) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
if self.settings.enabled.contains(&CheckCode::D200) {
|
if self.settings.enabled.contains(&CheckCode::D200) {
|
||||||
docstrings::one_liner(self, &docstring);
|
docstrings::one_liner(self, &docstring);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -151,6 +151,14 @@ pub enum CheckCode {
|
||||||
U007,
|
U007,
|
||||||
U008,
|
U008,
|
||||||
// pydocstyle
|
// pydocstyle
|
||||||
|
D100,
|
||||||
|
D101,
|
||||||
|
D102,
|
||||||
|
D103,
|
||||||
|
D104,
|
||||||
|
D105,
|
||||||
|
D106,
|
||||||
|
D107,
|
||||||
D200,
|
D200,
|
||||||
D205,
|
D205,
|
||||||
D209,
|
D209,
|
||||||
|
|
@ -282,6 +290,14 @@ pub enum CheckKind {
|
||||||
NoBlankLineBeforeClass(usize),
|
NoBlankLineBeforeClass(usize),
|
||||||
OneBlankLineBeforeClass(usize),
|
OneBlankLineBeforeClass(usize),
|
||||||
OneBlankLineAfterClass(usize),
|
OneBlankLineAfterClass(usize),
|
||||||
|
PublicModule,
|
||||||
|
PublicClass,
|
||||||
|
PublicMethod,
|
||||||
|
PublicFunction,
|
||||||
|
PublicPackage,
|
||||||
|
MagicMethod,
|
||||||
|
PublicNestedClass,
|
||||||
|
PublicInit,
|
||||||
// Meta
|
// Meta
|
||||||
UnusedNOQA(Option<Vec<String>>),
|
UnusedNOQA(Option<Vec<String>>),
|
||||||
}
|
}
|
||||||
|
|
@ -391,6 +407,14 @@ impl CheckCode {
|
||||||
CheckCode::U007 => CheckKind::UsePEP604Annotation,
|
CheckCode::U007 => CheckKind::UsePEP604Annotation,
|
||||||
CheckCode::U008 => CheckKind::SuperCallWithParameters,
|
CheckCode::U008 => CheckKind::SuperCallWithParameters,
|
||||||
// pydocstyle
|
// pydocstyle
|
||||||
|
CheckCode::D100 => CheckKind::PublicModule,
|
||||||
|
CheckCode::D101 => CheckKind::PublicClass,
|
||||||
|
CheckCode::D102 => CheckKind::PublicMethod,
|
||||||
|
CheckCode::D103 => CheckKind::PublicFunction,
|
||||||
|
CheckCode::D104 => CheckKind::PublicPackage,
|
||||||
|
CheckCode::D105 => CheckKind::MagicMethod,
|
||||||
|
CheckCode::D106 => CheckKind::PublicNestedClass,
|
||||||
|
CheckCode::D107 => CheckKind::PublicInit,
|
||||||
CheckCode::D200 => CheckKind::FitsOnOneLine,
|
CheckCode::D200 => CheckKind::FitsOnOneLine,
|
||||||
CheckCode::D205 => CheckKind::NoBlankLineAfterSummary,
|
CheckCode::D205 => CheckKind::NoBlankLineAfterSummary,
|
||||||
CheckCode::D209 => CheckKind::NewLineAfterLastParagraph,
|
CheckCode::D209 => CheckKind::NewLineAfterLastParagraph,
|
||||||
|
|
@ -496,6 +520,14 @@ impl CheckKind {
|
||||||
CheckKind::UselessObjectInheritance(_) => &CheckCode::U004,
|
CheckKind::UselessObjectInheritance(_) => &CheckCode::U004,
|
||||||
CheckKind::SuperCallWithParameters => &CheckCode::U008,
|
CheckKind::SuperCallWithParameters => &CheckCode::U008,
|
||||||
// pydocstyle
|
// pydocstyle
|
||||||
|
CheckKind::PublicModule => &CheckCode::D100,
|
||||||
|
CheckKind::PublicClass => &CheckCode::D101,
|
||||||
|
CheckKind::PublicMethod => &CheckCode::D102,
|
||||||
|
CheckKind::PublicFunction => &CheckCode::D103,
|
||||||
|
CheckKind::PublicPackage => &CheckCode::D104,
|
||||||
|
CheckKind::MagicMethod => &CheckCode::D105,
|
||||||
|
CheckKind::PublicNestedClass => &CheckCode::D106,
|
||||||
|
CheckKind::PublicInit => &CheckCode::D107,
|
||||||
CheckKind::FitsOnOneLine => &CheckCode::D200,
|
CheckKind::FitsOnOneLine => &CheckCode::D200,
|
||||||
CheckKind::NoBlankLineAfterSummary => &CheckCode::D205,
|
CheckKind::NoBlankLineAfterSummary => &CheckCode::D205,
|
||||||
CheckKind::NewLineAfterLastParagraph => &CheckCode::D209,
|
CheckKind::NewLineAfterLastParagraph => &CheckCode::D209,
|
||||||
|
|
@ -792,9 +824,23 @@ impl CheckKind {
|
||||||
CheckKind::NoBlankLineAfterFunction(num_lines) => {
|
CheckKind::NoBlankLineAfterFunction(num_lines) => {
|
||||||
format!("No blank lines allowed after function docstring (found {num_lines})")
|
format!("No blank lines allowed after function docstring (found {num_lines})")
|
||||||
}
|
}
|
||||||
CheckKind::NoBlankLineBeforeClass(_) => "NoBlankLineBeforeClass".to_string(),
|
CheckKind::NoBlankLineBeforeClass(_) => {
|
||||||
CheckKind::OneBlankLineBeforeClass(_) => "OneBlankLineBeforeClass".to_string(),
|
"No blank lines allowed before class docstring".to_string()
|
||||||
CheckKind::OneBlankLineAfterClass(_) => "OneBlankLineAfterClass".to_string(),
|
}
|
||||||
|
CheckKind::OneBlankLineBeforeClass(_) => {
|
||||||
|
"1 blank line required before class docstring".to_string()
|
||||||
|
}
|
||||||
|
CheckKind::OneBlankLineAfterClass(_) => {
|
||||||
|
"1 blank line required after class docstring".to_string()
|
||||||
|
}
|
||||||
|
CheckKind::PublicModule => "Missing docstring in public module".to_string(),
|
||||||
|
CheckKind::PublicClass => "Missing docstring in public class".to_string(),
|
||||||
|
CheckKind::PublicMethod => "Missing docstring in public method".to_string(),
|
||||||
|
CheckKind::PublicFunction => "Missing docstring in public function".to_string(),
|
||||||
|
CheckKind::PublicPackage => "Missing docstring in public package".to_string(),
|
||||||
|
CheckKind::MagicMethod => "Missing docstring in magic method".to_string(),
|
||||||
|
CheckKind::PublicNestedClass => "Missing docstring in public nested class".to_string(),
|
||||||
|
CheckKind::PublicInit => "Missing docstring in __init__".to_string(),
|
||||||
// Meta
|
// Meta
|
||||||
CheckKind::UnusedNOQA(codes) => match codes {
|
CheckKind::UnusedNOQA(codes) => match codes {
|
||||||
None => "Unused `noqa` directive".to_string(),
|
None => "Unused `noqa` directive".to_string(),
|
||||||
|
|
|
||||||
|
|
@ -5,6 +5,7 @@ use rustpython_ast::{Constant, Expr, ExprKind, Location, Stmt, StmtKind};
|
||||||
use crate::ast::types::Range;
|
use crate::ast::types::Range;
|
||||||
use crate::check_ast::Checker;
|
use crate::check_ast::Checker;
|
||||||
use crate::checks::{Check, CheckCode, CheckKind};
|
use crate::checks::{Check, CheckCode, CheckKind};
|
||||||
|
use crate::visibility::{is_init, is_magic, is_overload, Modifier, Visibility, VisibleScope};
|
||||||
|
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
pub enum DefinitionKind<'a> {
|
pub enum DefinitionKind<'a> {
|
||||||
|
|
@ -28,19 +29,6 @@ pub enum Documentable {
|
||||||
Function,
|
Function,
|
||||||
}
|
}
|
||||||
|
|
||||||
fn nest(parents: &[&Stmt]) -> Option<Documentable> {
|
|
||||||
for parent in parents.iter().rev() {
|
|
||||||
match &parent.node {
|
|
||||||
StmtKind::FunctionDef { .. } | StmtKind::AsyncFunctionDef { .. } => {
|
|
||||||
return Some(Documentable::Function)
|
|
||||||
}
|
|
||||||
StmtKind::ClassDef { .. } => return Some(Documentable::Class),
|
|
||||||
_ => {}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
None
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Extract a docstring from a function or class body.
|
/// Extract a docstring from a function or class body.
|
||||||
pub fn docstring_from(suite: &[Stmt]) -> Option<&Expr> {
|
pub fn docstring_from(suite: &[Stmt]) -> Option<&Expr> {
|
||||||
if let Some(stmt) = suite.first() {
|
if let Some(stmt) = suite.first() {
|
||||||
|
|
@ -61,28 +49,59 @@ pub fn docstring_from(suite: &[Stmt]) -> Option<&Expr> {
|
||||||
|
|
||||||
/// Extract a `Definition` from the AST node defined by a `Stmt`.
|
/// Extract a `Definition` from the AST node defined by a `Stmt`.
|
||||||
pub fn extract<'a>(
|
pub fn extract<'a>(
|
||||||
parents: Vec<&'a Stmt>,
|
scope: &VisibleScope,
|
||||||
stmt: &'a Stmt,
|
stmt: &'a Stmt,
|
||||||
body: &'a [Stmt],
|
body: &'a [Stmt],
|
||||||
kind: Documentable,
|
kind: &Documentable,
|
||||||
) -> Definition<'a> {
|
) -> Definition<'a> {
|
||||||
let expr = docstring_from(body);
|
let expr = docstring_from(body);
|
||||||
match kind {
|
match kind {
|
||||||
Documentable::Function => Definition {
|
Documentable::Function => match scope {
|
||||||
kind: match nest(&parents) {
|
VisibleScope {
|
||||||
None => DefinitionKind::Function(stmt),
|
modifier: Modifier::Module,
|
||||||
Some(Documentable::Function) => DefinitionKind::NestedFunction(stmt),
|
..
|
||||||
Some(Documentable::Class) => DefinitionKind::Method(stmt),
|
} => Definition {
|
||||||
},
|
kind: DefinitionKind::Function(stmt),
|
||||||
docstring: expr,
|
docstring: expr,
|
||||||
},
|
},
|
||||||
Documentable::Class => Definition {
|
VisibleScope {
|
||||||
kind: match nest(&parents) {
|
modifier: Modifier::Class,
|
||||||
None => DefinitionKind::Class(stmt),
|
..
|
||||||
Some(_) => DefinitionKind::NestedClass(stmt),
|
} => Definition {
|
||||||
},
|
kind: DefinitionKind::Method(stmt),
|
||||||
docstring: expr,
|
docstring: expr,
|
||||||
},
|
},
|
||||||
|
VisibleScope {
|
||||||
|
modifier: Modifier::Function,
|
||||||
|
..
|
||||||
|
} => Definition {
|
||||||
|
kind: DefinitionKind::NestedFunction(stmt),
|
||||||
|
docstring: expr,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
Documentable::Class => match scope {
|
||||||
|
VisibleScope {
|
||||||
|
modifier: Modifier::Module,
|
||||||
|
..
|
||||||
|
} => Definition {
|
||||||
|
kind: DefinitionKind::Class(stmt),
|
||||||
|
docstring: expr,
|
||||||
|
},
|
||||||
|
VisibleScope {
|
||||||
|
modifier: Modifier::Class,
|
||||||
|
..
|
||||||
|
} => Definition {
|
||||||
|
kind: DefinitionKind::NestedClass(stmt),
|
||||||
|
docstring: expr,
|
||||||
|
},
|
||||||
|
VisibleScope {
|
||||||
|
modifier: Modifier::Function,
|
||||||
|
..
|
||||||
|
} => Definition {
|
||||||
|
kind: DefinitionKind::NestedClass(stmt),
|
||||||
|
docstring: expr,
|
||||||
|
},
|
||||||
|
},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -95,6 +114,101 @@ fn range_for(docstring: &Expr) -> Range {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// D100, D101, D102, D103, D104, D105, D106, D107
|
||||||
|
pub fn not_missing(checker: &mut Checker, definition: &Definition, scope: &VisibleScope) -> bool {
|
||||||
|
if definition.docstring.is_some() {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
if matches!(scope.visibility, Visibility::Private) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
match definition.kind {
|
||||||
|
DefinitionKind::Module => {
|
||||||
|
if checker.settings.enabled.contains(&CheckCode::D100) {
|
||||||
|
checker.add_check(Check::new(
|
||||||
|
CheckKind::PublicModule,
|
||||||
|
Range {
|
||||||
|
location: Location::new(1, 1),
|
||||||
|
end_location: Location::new(1, 1),
|
||||||
|
},
|
||||||
|
));
|
||||||
|
}
|
||||||
|
false
|
||||||
|
}
|
||||||
|
DefinitionKind::Package => {
|
||||||
|
if checker.settings.enabled.contains(&CheckCode::D104) {
|
||||||
|
checker.add_check(Check::new(
|
||||||
|
CheckKind::PublicPackage,
|
||||||
|
Range {
|
||||||
|
location: Location::new(1, 1),
|
||||||
|
end_location: Location::new(1, 1),
|
||||||
|
},
|
||||||
|
));
|
||||||
|
}
|
||||||
|
false
|
||||||
|
}
|
||||||
|
DefinitionKind::Class(stmt) => {
|
||||||
|
if checker.settings.enabled.contains(&CheckCode::D101) {
|
||||||
|
checker.add_check(Check::new(
|
||||||
|
CheckKind::PublicClass,
|
||||||
|
Range::from_located(stmt),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
false
|
||||||
|
}
|
||||||
|
DefinitionKind::NestedClass(stmt) => {
|
||||||
|
if checker.settings.enabled.contains(&CheckCode::D106) {
|
||||||
|
checker.add_check(Check::new(
|
||||||
|
CheckKind::PublicNestedClass,
|
||||||
|
Range::from_located(stmt),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
false
|
||||||
|
}
|
||||||
|
DefinitionKind::Function(stmt) | DefinitionKind::NestedFunction(stmt) => {
|
||||||
|
if is_overload(stmt) {
|
||||||
|
true
|
||||||
|
} else {
|
||||||
|
if checker.settings.enabled.contains(&CheckCode::D103) {
|
||||||
|
checker.add_check(Check::new(
|
||||||
|
CheckKind::PublicFunction,
|
||||||
|
Range::from_located(stmt),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
DefinitionKind::Method(stmt) => {
|
||||||
|
if is_overload(stmt) {
|
||||||
|
true
|
||||||
|
} else if is_magic(stmt) {
|
||||||
|
if checker.settings.enabled.contains(&CheckCode::D105) {
|
||||||
|
checker.add_check(Check::new(
|
||||||
|
CheckKind::MagicMethod,
|
||||||
|
Range::from_located(stmt),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
true
|
||||||
|
} else if is_init(stmt) {
|
||||||
|
if checker.settings.enabled.contains(&CheckCode::D107) {
|
||||||
|
checker.add_check(Check::new(CheckKind::PublicInit, Range::from_located(stmt)));
|
||||||
|
}
|
||||||
|
true
|
||||||
|
} else {
|
||||||
|
if checker.settings.enabled.contains(&CheckCode::D102) {
|
||||||
|
checker.add_check(Check::new(
|
||||||
|
CheckKind::PublicMethod,
|
||||||
|
Range::from_located(stmt),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// D200
|
/// D200
|
||||||
pub fn one_liner(checker: &mut Checker, definition: &Definition) {
|
pub fn one_liner(checker: &mut Checker, definition: &Definition) {
|
||||||
if let Some(docstring) = &definition.docstring {
|
if let Some(docstring) = &definition.docstring {
|
||||||
|
|
|
||||||
|
|
@ -28,6 +28,7 @@ pub mod printer;
|
||||||
pub mod pyproject;
|
pub mod pyproject;
|
||||||
mod python;
|
mod python;
|
||||||
pub mod settings;
|
pub mod settings;
|
||||||
|
pub mod visibility;
|
||||||
|
|
||||||
/// Run ruff over Python source code directly.
|
/// Run ruff over Python source code directly.
|
||||||
pub fn check(path: &Path, contents: &str) -> Result<Vec<Message>> {
|
pub fn check(path: &Path, contents: &str) -> Result<Vec<Message>> {
|
||||||
|
|
|
||||||
|
|
@ -1009,10 +1009,94 @@ mod tests {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn d200() -> Result<()> {
|
fn d100() -> Result<()> {
|
||||||
let mut checks = check_path(
|
let mut checks = check_path(
|
||||||
Path::new("./resources/test/fixtures/D.py"),
|
Path::new("./resources/test/fixtures/D.py"),
|
||||||
&settings::Settings::for_rule(CheckCode::D200),
|
&settings::Settings::for_rule(CheckCode::D100),
|
||||||
|
&fixer::Mode::Generate,
|
||||||
|
)?;
|
||||||
|
checks.sort_by_key(|check| check.location);
|
||||||
|
insta::assert_yaml_snapshot!(checks);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn d101() -> Result<()> {
|
||||||
|
let mut checks = check_path(
|
||||||
|
Path::new("./resources/test/fixtures/D.py"),
|
||||||
|
&settings::Settings::for_rule(CheckCode::D101),
|
||||||
|
&fixer::Mode::Generate,
|
||||||
|
)?;
|
||||||
|
checks.sort_by_key(|check| check.location);
|
||||||
|
insta::assert_yaml_snapshot!(checks);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn d102() -> Result<()> {
|
||||||
|
let mut checks = check_path(
|
||||||
|
Path::new("./resources/test/fixtures/D.py"),
|
||||||
|
&settings::Settings::for_rule(CheckCode::D102),
|
||||||
|
&fixer::Mode::Generate,
|
||||||
|
)?;
|
||||||
|
checks.sort_by_key(|check| check.location);
|
||||||
|
insta::assert_yaml_snapshot!(checks);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn d103() -> Result<()> {
|
||||||
|
let mut checks = check_path(
|
||||||
|
Path::new("./resources/test/fixtures/D.py"),
|
||||||
|
&settings::Settings::for_rule(CheckCode::D103),
|
||||||
|
&fixer::Mode::Generate,
|
||||||
|
)?;
|
||||||
|
checks.sort_by_key(|check| check.location);
|
||||||
|
insta::assert_yaml_snapshot!(checks);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn d104() -> Result<()> {
|
||||||
|
let mut checks = check_path(
|
||||||
|
Path::new("./resources/test/fixtures/D.py"),
|
||||||
|
&settings::Settings::for_rule(CheckCode::D104),
|
||||||
|
&fixer::Mode::Generate,
|
||||||
|
)?;
|
||||||
|
checks.sort_by_key(|check| check.location);
|
||||||
|
insta::assert_yaml_snapshot!(checks);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn d105() -> Result<()> {
|
||||||
|
let mut checks = check_path(
|
||||||
|
Path::new("./resources/test/fixtures/D.py"),
|
||||||
|
&settings::Settings::for_rule(CheckCode::D105),
|
||||||
|
&fixer::Mode::Generate,
|
||||||
|
)?;
|
||||||
|
checks.sort_by_key(|check| check.location);
|
||||||
|
insta::assert_yaml_snapshot!(checks);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn d106() -> Result<()> {
|
||||||
|
let mut checks = check_path(
|
||||||
|
Path::new("./resources/test/fixtures/D.py"),
|
||||||
|
&settings::Settings::for_rule(CheckCode::D106),
|
||||||
|
&fixer::Mode::Generate,
|
||||||
|
)?;
|
||||||
|
checks.sort_by_key(|check| check.location);
|
||||||
|
insta::assert_yaml_snapshot!(checks);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn d107() -> Result<()> {
|
||||||
|
let mut checks = check_path(
|
||||||
|
Path::new("./resources/test/fixtures/D.py"),
|
||||||
|
&settings::Settings::for_rule(CheckCode::D107),
|
||||||
&fixer::Mode::Generate,
|
&fixer::Mode::Generate,
|
||||||
)?;
|
)?;
|
||||||
checks.sort_by_key(|check| check.location);
|
checks.sort_by_key(|check| check.location);
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,13 @@
|
||||||
|
---
|
||||||
|
source: src/linter.rs
|
||||||
|
expression: checks
|
||||||
|
---
|
||||||
|
- kind: PublicModule
|
||||||
|
location:
|
||||||
|
row: 1
|
||||||
|
column: 1
|
||||||
|
end_location:
|
||||||
|
row: 1
|
||||||
|
column: 1
|
||||||
|
fix: ~
|
||||||
|
|
||||||
|
|
@ -0,0 +1,13 @@
|
||||||
|
---
|
||||||
|
source: src/linter.rs
|
||||||
|
expression: checks
|
||||||
|
---
|
||||||
|
- kind: PublicClass
|
||||||
|
location:
|
||||||
|
row: 14
|
||||||
|
column: 1
|
||||||
|
end_location:
|
||||||
|
row: 67
|
||||||
|
column: 1
|
||||||
|
fix: ~
|
||||||
|
|
||||||
|
|
@ -0,0 +1,29 @@
|
||||||
|
---
|
||||||
|
source: src/linter.rs
|
||||||
|
expression: checks
|
||||||
|
---
|
||||||
|
- kind: PublicMethod
|
||||||
|
location:
|
||||||
|
row: 22
|
||||||
|
column: 5
|
||||||
|
end_location:
|
||||||
|
row: 25
|
||||||
|
column: 5
|
||||||
|
fix: ~
|
||||||
|
- kind: PublicMethod
|
||||||
|
location:
|
||||||
|
row: 51
|
||||||
|
column: 5
|
||||||
|
end_location:
|
||||||
|
row: 54
|
||||||
|
column: 5
|
||||||
|
fix: ~
|
||||||
|
- kind: PublicMethod
|
||||||
|
location:
|
||||||
|
row: 63
|
||||||
|
column: 5
|
||||||
|
end_location:
|
||||||
|
row: 67
|
||||||
|
column: 1
|
||||||
|
fix: ~
|
||||||
|
|
||||||
|
|
@ -0,0 +1,13 @@
|
||||||
|
---
|
||||||
|
source: src/linter.rs
|
||||||
|
expression: checks
|
||||||
|
---
|
||||||
|
- kind: PublicFunction
|
||||||
|
location:
|
||||||
|
row: 395
|
||||||
|
column: 1
|
||||||
|
end_location:
|
||||||
|
row: 396
|
||||||
|
column: 1
|
||||||
|
fix: ~
|
||||||
|
|
||||||
|
|
@ -0,0 +1,6 @@
|
||||||
|
---
|
||||||
|
source: src/linter.rs
|
||||||
|
expression: checks
|
||||||
|
---
|
||||||
|
[]
|
||||||
|
|
||||||
|
|
@ -0,0 +1,13 @@
|
||||||
|
---
|
||||||
|
source: src/linter.rs
|
||||||
|
expression: checks
|
||||||
|
---
|
||||||
|
- kind: MagicMethod
|
||||||
|
location:
|
||||||
|
row: 59
|
||||||
|
column: 5
|
||||||
|
end_location:
|
||||||
|
row: 62
|
||||||
|
column: 5
|
||||||
|
fix: ~
|
||||||
|
|
||||||
|
|
@ -0,0 +1,6 @@
|
||||||
|
---
|
||||||
|
source: src/linter.rs
|
||||||
|
expression: checks
|
||||||
|
---
|
||||||
|
[]
|
||||||
|
|
||||||
|
|
@ -0,0 +1,21 @@
|
||||||
|
---
|
||||||
|
source: src/linter.rs
|
||||||
|
expression: checks
|
||||||
|
---
|
||||||
|
- kind: PublicInit
|
||||||
|
location:
|
||||||
|
row: 55
|
||||||
|
column: 5
|
||||||
|
end_location:
|
||||||
|
row: 58
|
||||||
|
column: 5
|
||||||
|
fix: ~
|
||||||
|
- kind: PublicInit
|
||||||
|
location:
|
||||||
|
row: 529
|
||||||
|
column: 5
|
||||||
|
end_location:
|
||||||
|
row: 533
|
||||||
|
column: 1
|
||||||
|
fix: ~
|
||||||
|
|
||||||
|
|
@ -0,0 +1,149 @@
|
||||||
|
use std::path::Path;
|
||||||
|
|
||||||
|
use crate::ast::helpers::match_name_or_attr;
|
||||||
|
use rustpython_ast::{Stmt, StmtKind};
|
||||||
|
|
||||||
|
use crate::docstrings::Documentable;
|
||||||
|
|
||||||
|
#[derive(Debug, Clone)]
|
||||||
|
pub enum Modifier {
|
||||||
|
Module,
|
||||||
|
Class,
|
||||||
|
Function,
|
||||||
|
}
|
||||||
|
|
||||||
|
#[derive(Debug, Clone)]
|
||||||
|
pub enum Visibility {
|
||||||
|
Public,
|
||||||
|
Private,
|
||||||
|
}
|
||||||
|
|
||||||
|
#[derive(Debug, Clone)]
|
||||||
|
pub struct VisibleScope {
|
||||||
|
pub modifier: Modifier,
|
||||||
|
pub visibility: Visibility,
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn is_overload(stmt: &Stmt) -> bool {
|
||||||
|
match &stmt.node {
|
||||||
|
StmtKind::FunctionDef { decorator_list, .. }
|
||||||
|
| StmtKind::AsyncFunctionDef { decorator_list, .. } => decorator_list
|
||||||
|
.iter()
|
||||||
|
.any(|expr| match_name_or_attr(expr, "overload")),
|
||||||
|
_ => panic!("Found non-FunctionDef in is_overload"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn is_magic(stmt: &Stmt) -> bool {
|
||||||
|
match &stmt.node {
|
||||||
|
StmtKind::FunctionDef { name, .. } | StmtKind::AsyncFunctionDef { name, .. } => {
|
||||||
|
name.starts_with("__")
|
||||||
|
&& name.ends_with("__")
|
||||||
|
&& name != "__init__"
|
||||||
|
&& name != "__call__"
|
||||||
|
&& name != "__new__"
|
||||||
|
}
|
||||||
|
_ => panic!("Found non-FunctionDef in is_magic"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn is_init(stmt: &Stmt) -> bool {
|
||||||
|
match &stmt.node {
|
||||||
|
StmtKind::FunctionDef { name, .. } | StmtKind::AsyncFunctionDef { name, .. } => {
|
||||||
|
name == "__init__"
|
||||||
|
}
|
||||||
|
_ => panic!("Found non-FunctionDef in is_init"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn is_private_name(module_name: &str) -> bool {
|
||||||
|
module_name.starts_with('_') || (module_name.starts_with("__") && module_name.ends_with("__"))
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn module_visibility(path: &Path) -> Visibility {
|
||||||
|
for component in path.iter().rev() {
|
||||||
|
if is_private_name(&component.to_string_lossy()) {
|
||||||
|
return Visibility::Private;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Visibility::Public
|
||||||
|
}
|
||||||
|
|
||||||
|
fn function_visibility(stmt: &Stmt) -> Visibility {
|
||||||
|
match &stmt.node {
|
||||||
|
StmtKind::FunctionDef { name, .. } | StmtKind::AsyncFunctionDef { name, .. } => {
|
||||||
|
if name.starts_with('_') {
|
||||||
|
Visibility::Private
|
||||||
|
} else {
|
||||||
|
Visibility::Public
|
||||||
|
}
|
||||||
|
}
|
||||||
|
_ => panic!("Found non-FunctionDef in function_visibility"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn method_visibility(stmt: &Stmt) -> Visibility {
|
||||||
|
match &stmt.node {
|
||||||
|
StmtKind::FunctionDef { name, .. } | StmtKind::AsyncFunctionDef { name, .. } => {
|
||||||
|
// Is the method non-private?
|
||||||
|
if !name.starts_with('_') {
|
||||||
|
return Visibility::Public;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Is this a magic method?
|
||||||
|
if name.starts_with("__") && name.ends_with("__") {
|
||||||
|
return Visibility::Public;
|
||||||
|
}
|
||||||
|
|
||||||
|
Visibility::Private
|
||||||
|
}
|
||||||
|
_ => panic!("Found non-FunctionDef in method_visibility"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn class_visibility(stmt: &Stmt) -> Visibility {
|
||||||
|
match &stmt.node {
|
||||||
|
StmtKind::ClassDef { name, .. } => {
|
||||||
|
if name.starts_with('_') {
|
||||||
|
Visibility::Private
|
||||||
|
} else {
|
||||||
|
Visibility::Public
|
||||||
|
}
|
||||||
|
}
|
||||||
|
_ => panic!("Found non-ClassDef in function_visibility"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Transition a `VisibleScope` based on a new `Documentable` definition.
|
||||||
|
pub fn transition_scope(scope: &VisibleScope, stmt: &Stmt, kind: &Documentable) -> VisibleScope {
|
||||||
|
match kind {
|
||||||
|
Documentable::Function => VisibleScope {
|
||||||
|
modifier: Modifier::Function,
|
||||||
|
visibility: match scope {
|
||||||
|
VisibleScope {
|
||||||
|
modifier: Modifier::Module,
|
||||||
|
visibility: Visibility::Public,
|
||||||
|
} => function_visibility(stmt),
|
||||||
|
VisibleScope {
|
||||||
|
modifier: Modifier::Class,
|
||||||
|
visibility: Visibility::Public,
|
||||||
|
} => method_visibility(stmt),
|
||||||
|
_ => Visibility::Private,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
Documentable::Class => VisibleScope {
|
||||||
|
modifier: Modifier::Class,
|
||||||
|
visibility: match scope {
|
||||||
|
VisibleScope {
|
||||||
|
modifier: Modifier::Module,
|
||||||
|
visibility: Visibility::Public,
|
||||||
|
} => class_visibility(stmt),
|
||||||
|
VisibleScope {
|
||||||
|
modifier: Modifier::Class,
|
||||||
|
visibility: Visibility::Public,
|
||||||
|
} => class_visibility(stmt),
|
||||||
|
_ => Visibility::Private,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue