diff --git a/src/check_ast.rs b/src/check_ast.rs index ce13bfe83b..4503e86fa3 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -21,7 +21,7 @@ use crate::ast::visitor::{walk_excepthandler, Visitor}; use crate::ast::{checkers, helpers, operations, visitor}; use crate::autofix::{fixer, fixes}; use crate::checks::{Check, CheckCode, CheckKind}; -use crate::docstrings::{Docstring, DocstringKind}; +use crate::docstrings::{Definition, DefinitionKind, Documentable}; use crate::python::builtins::{BUILTINS, MAGIC_GLOBALS}; use crate::python::future::ALL_FEATURE_NAMES; use crate::settings::{PythonVersion, Settings}; @@ -31,7 +31,7 @@ pub const GLOBAL_SCOPE_INDEX: usize = 0; pub struct Checker<'a> { // Input data. - path: &'a Path, + pub(crate) path: &'a Path, // TODO(charlie): Separate immutable from mutable state. (None of these should ever change.) pub(crate) locator: SourceCodeLocator<'a>, pub(crate) settings: &'a Settings, @@ -39,7 +39,7 @@ pub struct Checker<'a> { // Computed checks. checks: Vec, // Docstring tracking. - docstrings: Vec>, + docstrings: Vec>, // Edit tracking. // TODO(charlie): Instead of exposing deletions, wrap in a public API. pub(crate) deletions: BTreeSet, @@ -125,36 +125,8 @@ where StmtKind::Import { .. } => { self.futures_allowed = false; } - StmtKind::Expr { value } => { - // Track all docstrings: module-, class-, and function-level. - let mut is_module_docstring = false; - if matches!( - &value.node, - ExprKind::Constant { - value: Constant::Str(_), - .. - } - ) { - if let Some(docstring) = docstrings::extract(self, stmt, value) { - if matches!(&docstring.kind, DocstringKind::Module) { - is_module_docstring = true; - } - self.docstrings.push(docstring); - } - } - - if !is_module_docstring { - if !self.seen_import_boundary - && !operations::in_nested_block(&self.parent_stack, &self.parents) - { - self.seen_import_boundary = true; - } - self.futures_allowed = false; - } - } node => { self.futures_allowed = false; - if !self.seen_import_boundary && !helpers::is_assignment_to_a_dunder(node) && !operations::in_nested_block(&self.parent_stack, &self.parents) @@ -594,7 +566,23 @@ where // Recurse. match &stmt.node { - StmtKind::FunctionDef { .. } | StmtKind::AsyncFunctionDef { .. } => { + StmtKind::FunctionDef { body, .. } | StmtKind::AsyncFunctionDef { body, .. } => { + // TODO(charlie): Track public / private. + // Grab all parents, to enable nested definition tracking. (Ignore the most recent + // parent which, confusingly, is `stmt`.) + let parents = self + .parent_stack + .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(( stmt, self.scope_stack.clone(), @@ -602,6 +590,22 @@ where )); } StmtKind::ClassDef { body, .. } => { + // TODO(charlie): Track public / priva + // Grab all parents, to enable nested definition tracking. (Ignore the most recent + // parent which, confusingly, is `stmt`.) + let parents = self + .parent_stack + .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 { self.visit_stmt(stmt); } @@ -1643,6 +1647,22 @@ impl<'a> Checker<'a> { } } + fn visit_docstring<'b>(&mut self, python_ast: &'b Suite) -> bool + where + 'b: 'a, + { + let docstring = docstrings::docstring_from(python_ast); + self.docstrings.push(Definition { + kind: if self.path.ends_with("__init__.py") { + DefinitionKind::Package + } else { + DefinitionKind::Module + }, + docstring, + }); + docstring.is_some() + } + fn check_deferred_annotations(&mut self) { while let Some((expr, scopes, parents)) = self.deferred_annotations.pop() { self.parent_stack = parents; @@ -1988,6 +2008,13 @@ pub fn check_ast( checker.push_scope(Scope::new(ScopeKind::Module)); checker.bind_builtins(); + // Check for module docstring. + let python_ast = if checker.visit_docstring(python_ast) { + &python_ast[1..] + } else { + python_ast + }; + // Iterate over the AST. for stmt in python_ast { checker.visit_stmt(stmt); diff --git a/src/docstrings.rs b/src/docstrings.rs index bddce420ec..3a3f39132e 100644 --- a/src/docstrings.rs +++ b/src/docstrings.rs @@ -7,92 +7,117 @@ use crate::check_ast::Checker; use crate::checks::{Check, CheckCode, CheckKind}; #[derive(Debug)] -pub enum DocstringKind<'a> { +pub enum DefinitionKind<'a> { Module, - Function(&'a Stmt), + Package, Class(&'a Stmt), + NestedClass(&'a Stmt), + Function(&'a Stmt), + NestedFunction(&'a Stmt), + Method(&'a Stmt), } #[derive(Debug)] -pub struct Docstring<'a> { - pub kind: DocstringKind<'a>, - pub expr: &'a Expr, +pub struct Definition<'a> { + pub kind: DefinitionKind<'a>, + pub docstring: Option<&'a Expr>, } -/// Extract a `Docstring` from an `Expr`. -pub fn extract<'a, 'b>( - checker: &'a Checker<'b>, - stmt: &'b Stmt, - expr: &'b Expr, -) -> Option> { - let defined_in = checker - .binding_context() - .defined_in - .map(|index| checker.parents[index]); - match defined_in { - None => { - if checker.initial { - return Some(Docstring { - kind: DocstringKind::Module, - expr, - }); - } - } - Some(parent) => { - if let StmtKind::FunctionDef { body, .. } - | StmtKind::AsyncFunctionDef { body, .. } - | StmtKind::ClassDef { body, .. } = &parent.node - { - if body.first().map(|node| node == stmt).unwrap_or_default() { - return Some(Docstring { - kind: if matches!(&parent.node, StmtKind::ClassDef { .. }) { - DocstringKind::Class(parent) - } else { - DocstringKind::Function(parent) - }, - expr, - }); - } +pub enum Documentable { + Class, + Function, +} + +fn nest(parents: &[&Stmt]) -> Option { + 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 the source code range for a `Docstring`. -fn range_for(docstring: &Docstring) -> Range { +/// Extract a docstring from a function or class body. +pub fn docstring_from(suite: &[Stmt]) -> Option<&Expr> { + if let Some(stmt) = suite.first() { + if let StmtKind::Expr { value } = &stmt.node { + if matches!( + &value.node, + ExprKind::Constant { + value: Constant::Str(_), + .. + } + ) { + return Some(value); + } + } + } + None +} + +/// Extract a `Definition` from the AST node defined by a `Stmt`. +pub fn extract<'a>( + parents: Vec<&'a Stmt>, + stmt: &'a Stmt, + body: &'a [Stmt], + kind: Documentable, +) -> Definition<'a> { + let expr = docstring_from(body); + match kind { + Documentable::Function => Definition { + kind: match nest(&parents) { + None => DefinitionKind::Function(stmt), + Some(Documentable::Function) => DefinitionKind::NestedFunction(stmt), + Some(Documentable::Class) => DefinitionKind::Method(stmt), + }, + docstring: expr, + }, + Documentable::Class => Definition { + kind: match nest(&parents) { + None => DefinitionKind::Class(stmt), + Some(_) => DefinitionKind::NestedClass(stmt), + }, + docstring: expr, + }, + } +} + +/// Extract the source code range for a docstring. +fn range_for(docstring: &Expr) -> Range { // RustPython currently omits the first quotation mark in a string, so offset the location. Range { - location: Location::new( - docstring.expr.location.row(), - docstring.expr.location.column() - 1, - ), - end_location: docstring.expr.end_location, + location: Location::new(docstring.location.row(), docstring.location.column() - 1), + end_location: docstring.end_location, } } /// D200 -pub fn one_liner(checker: &mut Checker, docstring: &Docstring) { - if let ExprKind::Constant { - value: Constant::Str(string), - .. - } = &docstring.expr.node - { - let mut line_count = 0; - let mut non_empty_line_count = 0; - for line in string.lines() { - line_count += 1; - if !line.trim().is_empty() { - non_empty_line_count += 1; +pub fn one_liner(checker: &mut Checker, definition: &Definition) { + if let Some(docstring) = &definition.docstring { + if let ExprKind::Constant { + value: Constant::Str(string), + .. + } = &docstring.node + { + let mut line_count = 0; + let mut non_empty_line_count = 0; + for line in string.lines() { + line_count += 1; + if !line.trim().is_empty() { + non_empty_line_count += 1; + } + if non_empty_line_count > 1 { + break; + } } - if non_empty_line_count > 1 { - break; - } - } - if non_empty_line_count == 1 && line_count > 1 { - checker.add_check(Check::new(CheckKind::FitsOnOneLine, range_for(docstring))); + if non_empty_line_count == 1 && line_count > 1 { + checker.add_check(Check::new(CheckKind::FitsOnOneLine, range_for(docstring))); + } } } } @@ -103,53 +128,59 @@ static INNER_FUNCTION_OR_CLASS_REGEX: Lazy = Lazy::new(|| Regex::new(r"^\s+(?:(?:class|def|async def)\s|@)").unwrap()); /// D201, D202 -pub fn blank_before_after_function(checker: &mut Checker, docstring: &Docstring) { - if let DocstringKind::Function(parent) = &docstring.kind { - if let ExprKind::Constant { - value: Constant::Str(_), - .. - } = &docstring.expr.node +pub fn blank_before_after_function(checker: &mut Checker, definition: &Definition) { + if let Some(docstring) = definition.docstring { + if let DefinitionKind::Function(parent) + | DefinitionKind::NestedFunction(parent) + | DefinitionKind::Method(parent) = &definition.kind { - let (before, _, after) = checker - .locator - .partition_source_code_at(&Range::from_located(parent), &range_for(docstring)); + if let ExprKind::Constant { + value: Constant::Str(_), + .. + } = &docstring.node + { + let (before, _, after) = checker + .locator + .partition_source_code_at(&Range::from_located(parent), &range_for(docstring)); - if checker.settings.enabled.contains(&CheckCode::D201) { - let blank_lines_before = before - .lines() - .rev() - .skip(1) - .take_while(|line| line.trim().is_empty()) - .count(); - if blank_lines_before != 0 { - checker.add_check(Check::new( - CheckKind::NoBlankLineBeforeFunction(blank_lines_before), - range_for(docstring), - )); + if checker.settings.enabled.contains(&CheckCode::D201) { + let blank_lines_before = before + .lines() + .rev() + .skip(1) + .take_while(|line| line.trim().is_empty()) + .count(); + if blank_lines_before != 0 { + checker.add_check(Check::new( + CheckKind::NoBlankLineBeforeFunction(blank_lines_before), + range_for(docstring), + )); + } } - } - if checker.settings.enabled.contains(&CheckCode::D202) { - let blank_lines_after = after - .lines() - .skip(1) - .take_while(|line| line.trim().is_empty()) - .count(); - let all_blank_after = after - .lines() - .skip(1) - .all(|line| line.trim().is_empty() || COMMENT_REGEX.is_match(line)); - // Report a D202 violation if the docstring is followed by a blank line - // and the blank line is not itself followed by an inner function or - // class. - if !all_blank_after - && blank_lines_after != 0 - && !(blank_lines_after == 1 && INNER_FUNCTION_OR_CLASS_REGEX.is_match(after)) - { - checker.add_check(Check::new( - CheckKind::NoBlankLineAfterFunction(blank_lines_after), - range_for(docstring), - )); + if checker.settings.enabled.contains(&CheckCode::D202) { + let blank_lines_after = after + .lines() + .skip(1) + .take_while(|line| line.trim().is_empty()) + .count(); + let all_blank_after = after + .lines() + .skip(1) + .all(|line| line.trim().is_empty() || COMMENT_REGEX.is_match(line)); + // Report a D202 violation if the docstring is followed by a blank line + // and the blank line is not itself followed by an inner function or + // class. + if !all_blank_after + && blank_lines_after != 0 + && !(blank_lines_after == 1 + && INNER_FUNCTION_OR_CLASS_REGEX.is_match(after)) + { + checker.add_check(Check::new( + CheckKind::NoBlankLineAfterFunction(blank_lines_after), + range_for(docstring), + )); + } } } } @@ -157,55 +188,63 @@ pub fn blank_before_after_function(checker: &mut Checker, docstring: &Docstring) } /// D203, D204, D211 -pub fn blank_before_after_class(checker: &mut Checker, docstring: &Docstring) { - if let DocstringKind::Class(parent) = &docstring.kind { - if let ExprKind::Constant { - value: Constant::Str(_), - .. - } = &docstring.expr.node +pub fn blank_before_after_class(checker: &mut Checker, definition: &Definition) { + if let Some(docstring) = &definition.docstring { + if let DefinitionKind::Class(parent) | DefinitionKind::NestedClass(parent) = + &definition.kind { - let (before, _, after) = checker - .locator - .partition_source_code_at(&Range::from_located(parent), &range_for(docstring)); - - if checker.settings.enabled.contains(&CheckCode::D203) - || checker.settings.enabled.contains(&CheckCode::D211) + if let ExprKind::Constant { + value: Constant::Str(_), + .. + } = &docstring.node { - let blank_lines_before = before - .lines() - .rev() - .skip(1) - .take_while(|line| line.trim().is_empty()) - .count(); - if blank_lines_before != 0 && checker.settings.enabled.contains(&CheckCode::D211) { - checker.add_check(Check::new( - CheckKind::NoBlankLineBeforeClass(blank_lines_before), - range_for(docstring), - )); - } - if blank_lines_before != 1 && checker.settings.enabled.contains(&CheckCode::D203) { - checker.add_check(Check::new( - CheckKind::OneBlankLineBeforeClass(blank_lines_before), - range_for(docstring), - )); - } - } + let (before, _, after) = checker + .locator + .partition_source_code_at(&Range::from_located(parent), &range_for(docstring)); - if checker.settings.enabled.contains(&CheckCode::D204) { - let blank_lines_after = after - .lines() - .skip(1) - .take_while(|line| line.trim().is_empty()) - .count(); - let all_blank_after = after - .lines() - .skip(1) - .all(|line| line.trim().is_empty() || COMMENT_REGEX.is_match(line)); - if !all_blank_after && blank_lines_after != 1 { - checker.add_check(Check::new( - CheckKind::OneBlankLineAfterClass(blank_lines_after), - range_for(docstring), - )); + if checker.settings.enabled.contains(&CheckCode::D203) + || checker.settings.enabled.contains(&CheckCode::D211) + { + let blank_lines_before = before + .lines() + .rev() + .skip(1) + .take_while(|line| line.trim().is_empty()) + .count(); + if blank_lines_before != 0 + && checker.settings.enabled.contains(&CheckCode::D211) + { + checker.add_check(Check::new( + CheckKind::NoBlankLineBeforeClass(blank_lines_before), + range_for(docstring), + )); + } + if blank_lines_before != 1 + && checker.settings.enabled.contains(&CheckCode::D203) + { + checker.add_check(Check::new( + CheckKind::OneBlankLineBeforeClass(blank_lines_before), + range_for(docstring), + )); + } + } + + if checker.settings.enabled.contains(&CheckCode::D204) { + let blank_lines_after = after + .lines() + .skip(1) + .take_while(|line| line.trim().is_empty()) + .count(); + let all_blank_after = after + .lines() + .skip(1) + .all(|line| line.trim().is_empty() || COMMENT_REGEX.is_match(line)); + if !all_blank_after && blank_lines_after != 1 { + checker.add_check(Check::new( + CheckKind::OneBlankLineAfterClass(blank_lines_after), + range_for(docstring), + )); + } } } } @@ -213,77 +252,26 @@ pub fn blank_before_after_class(checker: &mut Checker, docstring: &Docstring) { } /// D205 -pub fn blank_after_summary(checker: &mut Checker, docstring: &Docstring) { - if let ExprKind::Constant { - value: Constant::Str(string), - .. - } = &docstring.expr.node - { - let mut lines_count = 1; - let mut blanks_count = 0; - for line in string.trim().lines().skip(1) { - lines_count += 1; - if line.trim().is_empty() { - blanks_count += 1; - } else { - break; - } - } - if lines_count > 1 && blanks_count != 1 { - checker.add_check(Check::new( - CheckKind::NoBlankLineAfterSummary, - range_for(docstring), - )); - } - } -} - -/// D209 -pub fn newline_after_last_paragraph(checker: &mut Checker, docstring: &Docstring) { - if let ExprKind::Constant { - value: Constant::Str(string), - .. - } = &docstring.expr.node - { - let mut line_count = 0; - for line in string.lines() { - if !line.trim().is_empty() { - line_count += 1; - } - if line_count > 1 { - let content = checker - .locator - .slice_source_code_range(&range_for(docstring)); - if let Some(line) = content.lines().last() { - let line = line.trim(); - if line != "\"\"\"" && line != "'''" { - checker.add_check(Check::new( - CheckKind::NewLineAfterLastParagraph, - range_for(docstring), - )); - } +pub fn blank_after_summary(checker: &mut Checker, definition: &Definition) { + if let Some(docstring) = definition.docstring { + if let ExprKind::Constant { + value: Constant::Str(string), + .. + } = &docstring.node + { + let mut lines_count = 1; + let mut blanks_count = 0; + for line in string.trim().lines().skip(1) { + lines_count += 1; + if line.trim().is_empty() { + blanks_count += 1; + } else { + break; } - return; } - } - } -} - -/// D210 -pub fn no_surrounding_whitespace(checker: &mut Checker, docstring: &Docstring) { - if let ExprKind::Constant { - value: Constant::Str(string), - .. - } = &docstring.expr.node - { - let mut lines = string.lines(); - if let Some(line) = lines.next() { - if line.trim().is_empty() { - return; - } - if line.starts_with(' ') || (matches!(lines.next(), None) && line.ends_with(' ')) { + if lines_count > 1 && blanks_count != 1 { checker.add_check(Check::new( - CheckKind::NoSurroundingWhitespace, + CheckKind::NoBlankLineAfterSummary, range_for(docstring), )); } @@ -291,29 +279,55 @@ pub fn no_surrounding_whitespace(checker: &mut Checker, docstring: &Docstring) { } } -/// D212, D213 -pub fn multi_line_summary_start(checker: &mut Checker, docstring: &Docstring) { - if let ExprKind::Constant { - value: Constant::Str(string), - .. - } = &docstring.expr.node - { - if string.lines().nth(1).is_some() { - let content = checker - .locator - .slice_source_code_range(&range_for(docstring)); - if let Some(first_line) = content.lines().next() { - let first_line = first_line.trim(); - if first_line == "\"\"\"" || first_line == "'''" { - if checker.settings.enabled.contains(&CheckCode::D212) { - checker.add_check(Check::new( - CheckKind::MultiLineSummaryFirstLine, - range_for(docstring), - )); +/// D209 +pub fn newline_after_last_paragraph(checker: &mut Checker, definition: &Definition) { + if let Some(docstring) = definition.docstring { + if let ExprKind::Constant { + value: Constant::Str(string), + .. + } = &docstring.node + { + let mut line_count = 0; + for line in string.lines() { + if !line.trim().is_empty() { + line_count += 1; + } + if line_count > 1 { + let content = checker + .locator + .slice_source_code_range(&range_for(docstring)); + if let Some(line) = content.lines().last() { + let line = line.trim(); + if line != "\"\"\"" && line != "'''" { + checker.add_check(Check::new( + CheckKind::NewLineAfterLastParagraph, + range_for(docstring), + )); + } } - } else if checker.settings.enabled.contains(&CheckCode::D213) { + return; + } + } + } + } +} + +/// D210 +pub fn no_surrounding_whitespace(checker: &mut Checker, definition: &Definition) { + if let Some(docstring) = definition.docstring { + if let ExprKind::Constant { + value: Constant::Str(string), + .. + } = &docstring.node + { + let mut lines = string.lines(); + if let Some(line) = lines.next() { + if line.trim().is_empty() { + return; + } + if line.starts_with(' ') || (matches!(lines.next(), None) && line.ends_with(' ')) { checker.add_check(Check::new( - CheckKind::MultiLineSummarySecondLine, + CheckKind::NoSurroundingWhitespace, range_for(docstring), )); } @@ -322,59 +336,104 @@ pub fn multi_line_summary_start(checker: &mut Checker, docstring: &Docstring) { } } +/// D212, D213 +pub fn multi_line_summary_start(checker: &mut Checker, definition: &Definition) { + if let Some(docstring) = definition.docstring { + if let ExprKind::Constant { + value: Constant::Str(string), + .. + } = &docstring.node + { + if string.lines().nth(1).is_some() { + let content = checker + .locator + .slice_source_code_range(&range_for(docstring)); + if let Some(first_line) = content.lines().next() { + let first_line = first_line.trim(); + if first_line == "\"\"\"" || first_line == "'''" { + if checker.settings.enabled.contains(&CheckCode::D212) { + checker.add_check(Check::new( + CheckKind::MultiLineSummaryFirstLine, + range_for(docstring), + )); + } + } else if checker.settings.enabled.contains(&CheckCode::D213) { + checker.add_check(Check::new( + CheckKind::MultiLineSummarySecondLine, + range_for(docstring), + )); + } + } + } + } + } +} + /// D300 -pub fn triple_quotes(checker: &mut Checker, docstring: &Docstring) { - if let ExprKind::Constant { - value: Constant::Str(string), - .. - } = &docstring.expr.node - { - let content = checker - .locator - .slice_source_code_range(&range_for(docstring)); - if string.contains("\"\"\"") { - if !content.starts_with("'''") { +pub fn triple_quotes(checker: &mut Checker, definition: &Definition) { + if let Some(docstring) = definition.docstring { + if let ExprKind::Constant { + value: Constant::Str(string), + .. + } = &docstring.node + { + let content = checker + .locator + .slice_source_code_range(&range_for(docstring)); + if string.contains("\"\"\"") { + if !content.starts_with("'''") { + checker.add_check(Check::new( + CheckKind::UsesTripleQuotes, + range_for(docstring), + )); + } + } else if !content.starts_with("\"\"\"") { checker.add_check(Check::new( CheckKind::UsesTripleQuotes, range_for(docstring), )); } - } else if !content.starts_with("\"\"\"") { - checker.add_check(Check::new( - CheckKind::UsesTripleQuotes, - range_for(docstring), - )); } } } /// D400 -pub fn ends_with_period(checker: &mut Checker, docstring: &Docstring) { - if let ExprKind::Constant { - value: Constant::Str(string), - .. - } = &docstring.expr.node - { - if let Some(string) = string.lines().next() { - if !string.ends_with('.') { - checker.add_check(Check::new(CheckKind::EndsInPeriod, range_for(docstring))); +pub fn ends_with_period(checker: &mut Checker, definition: &Definition) { + if let Some(docstring) = definition.docstring { + if let ExprKind::Constant { + value: Constant::Str(string), + .. + } = &docstring.node + { + if let Some(string) = string.lines().next() { + if !string.ends_with('.') { + checker.add_check(Check::new(CheckKind::EndsInPeriod, range_for(docstring))); + } } } } } /// D402 -pub fn no_signature(checker: &mut Checker, docstring: &Docstring) { - if let DocstringKind::Function(parent) = docstring.kind { - if let StmtKind::FunctionDef { name, .. } = &parent.node { - if let ExprKind::Constant { - value: Constant::Str(string), - .. - } = &docstring.expr.node - { - if let Some(first_line) = string.lines().next() { - if first_line.contains(&format!("{name}(")) { - checker.add_check(Check::new(CheckKind::NoSignature, range_for(docstring))); +pub fn no_signature(checker: &mut Checker, definition: &Definition) { + if let Some(docstring) = definition.docstring { + if let DefinitionKind::Function(parent) + | DefinitionKind::NestedFunction(parent) + | DefinitionKind::Method(parent) = definition.kind + { + if let StmtKind::FunctionDef { name, .. } = &parent.node { + if let ExprKind::Constant { + value: Constant::Str(string), + .. + } = &docstring.node + { + if let Some(first_line) = string.lines().next() { + if first_line.contains(&format!("{name}(")) { + checker.add_check(Check::new( + CheckKind::NoSignature, + range_for(docstring), + )); + } } } } @@ -383,29 +442,51 @@ pub fn no_signature(checker: &mut Checker, docstring: &Docstring) { } /// D403 -pub fn capitalized(checker: &mut Checker, docstring: &Docstring) { - if !matches!(docstring.kind, DocstringKind::Function(_)) { +pub fn capitalized(checker: &mut Checker, definition: &Definition) { + if !matches!(definition.kind, DefinitionKind::Function(_)) { return; } - if let ExprKind::Constant { - value: Constant::Str(string), - .. - } = &docstring.expr.node - { - if let Some(first_word) = string.split(' ').next() { - if first_word == first_word.to_uppercase() { - return; - } - for char in first_word.chars() { - if !char.is_ascii_alphabetic() && char != '\'' { + if let Some(docstring) = definition.docstring { + if let ExprKind::Constant { + value: Constant::Str(string), + .. + } = &docstring.node + { + if let Some(first_word) = string.split(' ').next() { + if first_word == first_word.to_uppercase() { return; } + for char in first_word.chars() { + if !char.is_ascii_alphabetic() && char != '\'' { + return; + } + } + if let Some(first_char) = first_word.chars().next() { + if !first_char.is_uppercase() { + checker.add_check(Check::new( + CheckKind::FirstLineCapitalized, + range_for(docstring), + )); + } + } } - if let Some(first_char) = first_word.chars().next() { - if !first_char.is_uppercase() { + } + } +} + +/// D415 +pub fn ends_with_punctuation(checker: &mut Checker, definition: &Definition) { + if let Some(docstring) = definition.docstring { + if let ExprKind::Constant { + value: Constant::Str(string), + .. + } = &docstring.node + { + if let Some(string) = string.lines().next() { + if !(string.ends_with('.') || string.ends_with('!') || string.ends_with('?')) { checker.add_check(Check::new( - CheckKind::FirstLineCapitalized, + CheckKind::EndsInPunctuation, range_for(docstring), )); } @@ -414,36 +495,20 @@ pub fn capitalized(checker: &mut Checker, docstring: &Docstring) { } } -/// D415 -pub fn ends_with_punctuation(checker: &mut Checker, docstring: &Docstring) { - if let ExprKind::Constant { - value: Constant::Str(string), - .. - } = &docstring.expr.node - { - if let Some(string) = string.lines().next() { - if !(string.ends_with('.') || string.ends_with('!') || string.ends_with('?')) { - checker.add_check(Check::new( - CheckKind::EndsInPunctuation, - range_for(docstring), - )); - } - } - } -} - /// D419 -pub fn not_empty(checker: &mut Checker, docstring: &Docstring) -> bool { - if let ExprKind::Constant { - value: Constant::Str(string), - .. - } = &docstring.expr.node - { - if string.trim().is_empty() { - if checker.settings.enabled.contains(&CheckCode::D419) { - checker.add_check(Check::new(CheckKind::NonEmpty, range_for(docstring))); +pub fn not_empty(checker: &mut Checker, definition: &Definition) -> bool { + if let Some(docstring) = definition.docstring { + if let ExprKind::Constant { + value: Constant::Str(string), + .. + } = &docstring.node + { + if string.trim().is_empty() { + if checker.settings.enabled.contains(&CheckCode::D419) { + checker.add_check(Check::new(CheckKind::NonEmpty, range_for(docstring))); + } + return false; } - return false; } } true