Python/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff synced 2026-01-21 21:40:51 -05:00
Code Issues Packages Projects Releases Wiki Activity

Implement docstring argument tracking for NumPy-style docstrings (#425)

Browse Source
This commit is contained in:
Charlie Marsh
2022-10-14 10:18:07 -04:00
committed by GitHub
parent 6fb82ab763
commit 9bbfd1d3b2
11 changed files with 471 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
src/check_ast.rs
View File

@@ -1657,7 +1657,6 @@ impl<'a> Checker<'a> {
fn handle_node_delete(&mut self, expr: &Expr) {
if let ExprKind::Name { id, .. } = &expr.node {
// Check if we're on a conditional branch.
if operations::on_conditional_branch(&self.parent_stack, &self.parents) {
return;
}
@@ -1996,20 +1995,23 @@ impl<'a> Checker<'a> {
if self.settings.enabled.contains(&CheckCode::D418) {
docstring_checks::if_needed(self, &docstring);
}
if self.settings.enabled.contains(&CheckCode::D407)
|| self.settings.enabled.contains(&CheckCode::D414)
if self.settings.enabled.contains(&CheckCode::D212)
|| self.settings.enabled.contains(&CheckCode::D214)
|| self.settings.enabled.contains(&CheckCode::D215)
|| self.settings.enabled.contains(&CheckCode::D405)
|| self.settings.enabled.contains(&CheckCode::D406)
|| self.settings.enabled.contains(&CheckCode::D407)
|| self.settings.enabled.contains(&CheckCode::D407)
|| self.settings.enabled.contains(&CheckCode::D212)
|| self.settings.enabled.contains(&CheckCode::D408)
|| self.settings.enabled.contains(&CheckCode::D409)
|| self.settings.enabled.contains(&CheckCode::D414)
|| self.settings.enabled.contains(&CheckCode::D412)
|| self.settings.enabled.contains(&CheckCode::D414)
|| self.settings.enabled.contains(&CheckCode::D405)
|| self.settings.enabled.contains(&CheckCode::D413)
|| self.settings.enabled.contains(&CheckCode::D410)
|| self.settings.enabled.contains(&CheckCode::D411)
|| self.settings.enabled.contains(&CheckCode::D406)
|| self.settings.enabled.contains(&CheckCode::D412)
|| self.settings.enabled.contains(&CheckCode::D413)
|| self.settings.enabled.contains(&CheckCode::D414)
|| self.settings.enabled.contains(&CheckCode::D414)
|| self.settings.enabled.contains(&CheckCode::D414)
|| self.settings.enabled.contains(&CheckCode::D417)
{
docstring_checks::check_sections(self, &docstring);
}

29
src/checks.rs
View File

@@ -174,6 +174,8 @@ pub enum CheckCode {
D211,
D212,
D213,
D214,
D215,
D300,
D400,
D402,
@@ -190,6 +192,7 @@ pub enum CheckCode {
D413,
D414,
D415,
D417,
D418,
D419,
// Meta
@@ -298,6 +301,7 @@ pub enum CheckKind {
BlankLineBeforeSection(String),
CapitalizeSectionName(String),
DashedUnderlineAfterSection(String),
DocumentAllArguments(Vec<String>),
EndsInPeriod,
EndsInPunctuation,
FirstLineCapitalized,
@@ -326,8 +330,10 @@ pub enum CheckKind {
PublicModule,
PublicNestedClass,
PublicPackage,
SectionNotOverIndented(String),
SectionUnderlineAfterName(String),
SectionUnderlineMatchesSectionLength(String),
SectionUnderlineNotOverIndented(String),
SkipDocstring,
UsesTripleQuotes,
// Meta
@@ -489,6 +495,11 @@ impl CheckCode {
CheckCode::D415 => CheckKind::EndsInPunctuation,
CheckCode::D418 => CheckKind::SkipDocstring,
CheckCode::D419 => CheckKind::NonEmpty,
CheckCode::D214 => CheckKind::SectionNotOverIndented("Returns".to_string()),
CheckCode::D215 => CheckKind::SectionUnderlineNotOverIndented("Returns".to_string()),
CheckCode::D417 => {
CheckKind::DocumentAllArguments(vec!["x".to_string(), "y".to_string()])
}
// Meta
CheckCode::M001 => CheckKind::UnusedNOQA(None),
}
@@ -586,6 +597,7 @@ impl CheckKind {
CheckKind::BlankLineBeforeSection(_) => &CheckCode::D411,
CheckKind::CapitalizeSectionName(_) => &CheckCode::D405,
CheckKind::DashedUnderlineAfterSection(_) => &CheckCode::D407,
CheckKind::DocumentAllArguments(_) => &CheckCode::D417,
CheckKind::EndsInPeriod => &CheckCode::D400,
CheckKind::EndsInPunctuation => &CheckCode::D415,
CheckKind::FirstLineCapitalized => &CheckCode::D403,
@@ -614,8 +626,10 @@ impl CheckKind {
CheckKind::PublicModule => &CheckCode::D100,
CheckKind::PublicNestedClass => &CheckCode::D106,
CheckKind::PublicPackage => &CheckCode::D104,
CheckKind::SectionNotOverIndented(_) => &CheckCode::D214,
CheckKind::SectionUnderlineAfterName(_) => &CheckCode::D408,
CheckKind::SectionUnderlineMatchesSectionLength(_) => &CheckCode::D409,
CheckKind::SectionUnderlineNotOverIndented(_) => &CheckCode::D215,
CheckKind::SkipDocstring => &CheckCode::D418,
CheckKind::UsesTripleQuotes => &CheckCode::D300,
// Meta
@@ -962,6 +976,21 @@ impl CheckKind {
)
}
CheckKind::NonEmptySection(name) => format!("Section has no content (\"{name}\")"),
CheckKind::SectionNotOverIndented(name) => {
format!("Section is over-indented (\"{name}\")")
}
CheckKind::SectionUnderlineNotOverIndented(name) => {
format!("Section underline is over-indented (\"{name}\")")
}
CheckKind::DocumentAllArguments(names) => {
if names.len() == 1 {
let name = &names[0];
format!("Missing argument description in the docstring: `{name}`")
} else {
let names = names.iter().map(|name| format!("`{name}`")).join(", ");
format!("Missing argument descriptions in the docstring: {names}")
}
}
// Meta
CheckKind::UnusedNOQA(codes) => match codes {
None => "Unused `noqa` directive".to_string(),

162
src/docstrings/sections.rs
View File

@@ -1,12 +1,16 @@
use itertools::Itertools;
use std::collections::BTreeSet;
use once_cell::sync::Lazy;
use rustpython_ast::{Arg, Expr, Location, StmtKind};
use titlecase::titlecase;
use crate::ast::types::Range;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckCode, CheckKind};
use crate::docstrings::docstring_checks::range_for;
use crate::docstrings::types::Definition;
use crate::docstrings::types::{Definition, DefinitionKind};
use crate::visibility::is_static;
static NUMPY_SECTION_NAMES: Lazy<BTreeSet<&'static str>> = Lazy::new(|| {
BTreeSet::from([
@@ -78,7 +82,21 @@ static NUMPY_SECTION_NAMES_LOWERCASE: Lazy<BTreeSet<&'static str>> = Lazy::new(|
// ])
// });
fn get_leading_words(line: &str) -> String {
fn indentation<'a>(checker: &'a mut Checker, docstring: &Expr) -> &'a str {
let range = range_for(docstring);
checker.locator.slice_source_code_range(&Range {
location: Location::new(range.location.row(), 1),
end_location: Location::new(range.location.row(), range.location.column()),
})
}
fn leading_space(line: &str) -> String {
line.chars()
.take_while(|char| char.is_whitespace())
.collect()
}
fn leading_words(line: &str) -> String {
line.trim()
.chars()
.take_while(|char| char.is_alphanumeric() || char.is_whitespace())
@@ -86,7 +104,7 @@ fn get_leading_words(line: &str) -> String {
}
fn suspected_as_section(line: &str) -> bool {
NUMPY_SECTION_NAMES_LOWERCASE.contains(&get_leading_words(line).to_lowercase().as_str())
NUMPY_SECTION_NAMES_LOWERCASE.contains(&leading_words(line).to_lowercase().as_str())
}
#[derive(Debug)]
@@ -143,7 +161,7 @@ pub fn section_contexts<'a>(lines: &'a [&'a str]) -> Vec<SectionContext<'a>> {
let mut contexts = vec![];
for lineno in suspected_section_indices {
let context = SectionContext {
section_name: get_leading_words(lines[lineno]),
section_name: leading_words(lines[lineno]),
previous_line: lines[lineno - 1],
line: lines[lineno],
following_lines: &lines[lineno + 1..],
@@ -196,14 +214,12 @@ fn check_blanks_and_section_underline(
// Nothing but blank lines after the section header.
if blank_lines_after_header == context.following_lines.len() {
// D407
if checker.settings.enabled.contains(&CheckCode::D407) {
checker.add_check(Check::new(
CheckKind::DashedUnderlineAfterSection(context.section_name.to_string()),
range_for(docstring),
));
}
// D414
if checker.settings.enabled.contains(&CheckCode::D414) {
checker.add_check(Check::new(
CheckKind::NonEmptySection(context.section_name.to_string()),
@@ -219,7 +235,6 @@ fn check_blanks_and_section_underline(
.all(|char| char.is_whitespace() || char == '-');
if !dash_line_found {
// D407
if checker.settings.enabled.contains(&CheckCode::D407) {
checker.add_check(Check::new(
CheckKind::DashedUnderlineAfterSection(context.section_name.to_string()),
@@ -227,7 +242,6 @@ fn check_blanks_and_section_underline(
));
}
if blank_lines_after_header > 0 {
// D212
if checker.settings.enabled.contains(&CheckCode::D212) {
checker.add_check(Check::new(
CheckKind::NoBlankLinesBetweenHeaderAndContent(
@@ -239,7 +253,6 @@ fn check_blanks_and_section_underline(
}
} else {
if blank_lines_after_header > 0 {
// D408
if checker.settings.enabled.contains(&CheckCode::D408) {
checker.add_check(Check::new(
CheckKind::SectionUnderlineAfterName(context.section_name.to_string()),
@@ -255,7 +268,6 @@ fn check_blanks_and_section_underline(
.count()
!= context.section_name.len()
{
// D409
if checker.settings.enabled.contains(&CheckCode::D409) {
checker.add_check(Check::new(
CheckKind::SectionUnderlineMatchesSectionLength(
@@ -266,16 +278,22 @@ fn check_blanks_and_section_underline(
}
}
// TODO(charlie): Implement D215, which requires indentation and leading space tracking.
if checker.settings.enabled.contains(&CheckCode::D215) {
if leading_space(non_empty_line).len() > indentation(checker, docstring).len() {
checker.add_check(Check::new(
CheckKind::SectionUnderlineNotOverIndented(context.section_name.to_string()),
range_for(docstring),
));
}
}
let line_after_dashes_index = blank_lines_after_header + 1;
if line_after_dashes_index < context.following_lines.len() {
let line_after_dashes = context.following_lines[line_after_dashes_index];
if line_after_dashes.trim().is_empty() {
let rest_of_lines = &context.following_lines[line_after_dashes_index..];
if rest_of_lines.iter().all(|line| line.trim().is_empty()) {
// D414
if checker.settings.enabled.contains(&CheckCode::D414) {
checker.add_check(Check::new(
CheckKind::NonEmptySection(context.section_name.to_string()),
@@ -283,7 +301,6 @@ fn check_blanks_and_section_underline(
));
}
} else {
// 412
if checker.settings.enabled.contains(&CheckCode::D412) {
checker.add_check(Check::new(
CheckKind::NoBlankLinesBetweenHeaderAndContent(
@@ -295,7 +312,6 @@ fn check_blanks_and_section_underline(
}
}
} else {
// D414
if checker.settings.enabled.contains(&CheckCode::D414) {
checker.add_check(Check::new(
CheckKind::NonEmptySection(context.section_name.to_string()),
@@ -307,7 +323,6 @@ fn check_blanks_and_section_underline(
}
fn check_common_section(checker: &mut Checker, definition: &Definition, context: &SectionContext) {
// TODO(charlie): Implement D214, which requires indentation and leading space tracking.
let docstring = definition
.docstring
.expect("Sections are only available for docstrings.");
@@ -323,6 +338,15 @@ fn check_common_section(checker: &mut Checker, definition: &Definition, context:
}
}
if checker.settings.enabled.contains(&CheckCode::D214) {
if leading_space(context.line).len() > indentation(checker, docstring).len() {
checker.add_check(Check::new(
CheckKind::SectionNotOverIndented(context.section_name.to_string()),
range_for(docstring),
))
}
}
if context
.following_lines
.last()
@@ -356,12 +380,110 @@ fn check_common_section(checker: &mut Checker, definition: &Definition, context:
}
}
fn check_missing_args(
checker: &mut Checker,
definition: &Definition,
docstrings_args: BTreeSet<&str>,
) {
if let DefinitionKind::Function(parent)
| DefinitionKind::NestedFunction(parent)
| DefinitionKind::Method(parent) = definition.kind
{
if let StmtKind::FunctionDef {
args: arguments, ..
}
| StmtKind::AsyncFunctionDef {
args: arguments, ..
} = &parent.node
{
// Collect all the arguments into a single vector.
let mut all_arguments: Vec<&Arg> = arguments
.args
.iter()
.chain(arguments.posonlyargs.iter())
.chain(arguments.kwonlyargs.iter())
.skip(
// If this is a non-static method, skip `cls` or `self`.
if matches!(definition.kind, DefinitionKind::Method(_)) && !is_static(parent) {
1
} else {
0
},
)
.collect();
if let Some(arg) = &arguments.vararg {
all_arguments.push(arg);
}
if let Some(arg) = &arguments.kwarg {
all_arguments.push(arg);
}
// Look for arguments that weren't included in the docstring.
let mut missing_args: BTreeSet<&str> = Default::default();
for arg in all_arguments {
let arg_name = arg.node.arg.as_str();
if arg_name.starts_with('_') {
continue;
}
if docstrings_args.contains(&arg_name) {
continue;
}
missing_args.insert(arg_name);
}
if !missing_args.is_empty() {
let names = missing_args
.into_iter()
.map(String::from)
.sorted()
.collect();
checker.add_check(Check::new(
CheckKind::DocumentAllArguments(names),
Range::from_located(parent),
));
}
}
}
}
fn check_parameters_section(
checker: &mut Checker,
definition: &Definition,
context: &SectionContext,
) {
// Collect the list of arguments documented in the docstring.
let mut docstring_args: BTreeSet<&str> = Default::default();
let section_level_indent = leading_space(context.line);
for i in 1..context.following_lines.len() {
let current_line = context.following_lines[i - 1];
let current_leading_space = leading_space(current_line);
let next_line = context.following_lines[i];
if current_leading_space == section_level_indent
&& (leading_space(next_line).len() > current_leading_space.len())
&& !next_line.trim().is_empty()
{
let parameters = if let Some(semi_index) = current_line.find(':') {
// If the parameter has a type annotation, exclude it.
&current_line[..semi_index]
} else {
// Otherwise, it's just a list of parameters on the current line.
current_line.trim()
};
// Notably, NumPy lets you put multiple parameters of the same type on the same line.
for parameter in parameters.split(',') {
docstring_args.insert(parameter.trim());
}
}
}
// Validate that all arguments were documented.
check_missing_args(checker, definition, docstring_args);
}
pub fn check_numpy_section(
checker: &mut Checker,
definition: &Definition,
context: &SectionContext,
) {
// TODO(charlie): Implement `_check_parameters_section`.
check_common_section(checker, definition, context);
check_blanks_and_section_underline(checker, definition, context);
@@ -381,4 +503,10 @@ pub fn check_numpy_section(
))
}
}
if checker.settings.enabled.contains(&CheckCode::D417) {
if titlecase(&context.section_name) == "Parameters" {
check_parameters_section(checker, definition, context);
}
}
}

4
src/linter.rs
View File

@@ -277,6 +277,8 @@ mod tests {
#[test_case(CheckCode::D211, Path::new("D.py"); "D211")]
#[test_case(CheckCode::D212, Path::new("D.py"); "D212")]
#[test_case(CheckCode::D213, Path::new("D.py"); "D213")]
#[test_case(CheckCode::D214, Path::new("sections.py"); "D214")]
#[test_case(CheckCode::D215, Path::new("sections.py"); "D215")]
#[test_case(CheckCode::D300, Path::new("D.py"); "D300")]
#[test_case(CheckCode::D400, Path::new("D.py"); "D400")]
#[test_case(CheckCode::D402, Path::new("D.py"); "D402")]
@@ -293,6 +295,8 @@ mod tests {
#[test_case(CheckCode::D413, Path::new("sections.py"); "D413")]
#[test_case(CheckCode::D414, Path::new("sections.py"); "D414")]
#[test_case(CheckCode::D415, Path::new("D.py"); "D415")]
#[test_case(CheckCode::D417, Path::new("sections.py"); "D417_0")]
#[test_case(CheckCode::D417, Path::new("canonical_numpy_examples.py"); "D417_1")]
#[test_case(CheckCode::D418, Path::new("D.py"); "D418")]
#[test_case(CheckCode::D419, Path::new("D.py"); "D419")]
#[test_case(CheckCode::E402, Path::new("E402.py"); "E402")]

14
src/snapshots/ruff__linter__tests__D214_sections.py.snap Normal file
View File

@@ -0,0 +1,14 @@
---
source: src/linter.rs
expression: checks
---
- kind:
SectionNotOverIndented: Returns
location:
row: 135
column: 5
end_location:
row: 141
column: 8
fix: ~

23
src/snapshots/ruff__linter__tests__D215_sections.py.snap Normal file
View File

@@ -0,0 +1,23 @@
---
source: src/linter.rs
expression: checks
---
- kind:
SectionUnderlineNotOverIndented: Returns
location:
row: 147
column: 5
end_location:
row: 153
column: 8
fix: ~
- kind:
SectionUnderlineNotOverIndented: Returns
location:
row: 161
column: 5
end_location:
row: 165
column: 8
fix: ~

6
src/snapshots/ruff__linter__tests__D417_canonical_numpy_examples.py.snap Normal file
View File

@@ -0,0 +1,6 @@
---
source: src/linter.rs
expression: checks
---
[]

50
src/snapshots/ruff__linter__tests__D417_sections.py.snap Normal file
View File

@@ -0,0 +1,50 @@
---
source: src/linter.rs
expression: checks
---
- kind:
DocumentAllArguments:
- y
location:
row: 389
column: 1
end_location:
row: 401
column: 1
fix: ~
- kind:
DocumentAllArguments:
- test
- y
- z
location:
row: 425
column: 5
end_location:
row: 436
column: 5
fix: ~
- kind:
DocumentAllArguments:
- test
- y
- z
location:
row: 440
column: 5
end_location:
row: 455
column: 5
fix: ~
- kind:
DocumentAllArguments:
- a
- z
location:
row: 459
column: 5
end_location:
row: 471
column: 5
fix: ~

11
src/visibility.rs
View File

@@ -26,6 +26,17 @@ pub struct VisibleScope {
pub visibility: Visibility,
}
/// Returns `true` if a function is a "static method".
pub fn is_static(stmt: &Stmt) -> bool {
match &stmt.node {
StmtKind::FunctionDef { decorator_list, .. }
| StmtKind::AsyncFunctionDef { decorator_list, .. } => decorator_list
.iter()
.any(|expr| match_name_or_attr(expr, "staticmethod")),
_ => panic!("Found non-FunctionDef in is_overload"),
}
}
/// Returns `true` if a function definition is an `@overload`.
pub fn is_overload(stmt: &Stmt) -> bool {
match &stmt.node {