Use presence of convention-specific sections during docstring inference (#3325)

This commit is contained in:
Charlie Marsh
2023-03-03 17:13:11 -05:00
committed by GitHub
parent eb42ce9319
commit dedf8aa5cc
12 changed files with 349 additions and 265 deletions

18
Cargo.lock generated
View File

@@ -1080,12 +1080,6 @@ version = "1.0.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "fad582f4b9e86b6caa621cabeb0963332d92eea04729ab12892c2533951e6440"
[[package]]
name = "joinery"
version = "2.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "72167d68f5fce3b8655487b8038691a3c9984ee769590f93f2a631f4ad64e4f5"
[[package]]
name = "js-sys"
version = "0.3.61"
@@ -2000,7 +1994,6 @@ dependencies = [
"test-case",
"textwrap",
"thiserror",
"titlecase",
"toml",
"wasm-bindgen",
"wasm-bindgen-test",
@@ -2729,17 +2722,6 @@ version = "0.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20"
[[package]]
name = "titlecase"
version = "2.2.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "38397a8cdb017cfeb48bf6c154d6de975ac69ffeed35980fde199d2ee0842042"
dependencies = [
"joinery",
"lazy_static",
"regex",
]
[[package]]
name = "toml"
version = "0.6.0"

View File

@@ -59,7 +59,6 @@ strum = { workspace = true }
strum_macros = { workspace = true }
textwrap = { version = "0.16.0" }
thiserror = { version = "1.0" }
titlecase = { version = "2.2.1" }
toml = { workspace = true }
# https://docs.rs/getrandom/0.2.7/getrandom/#webassembly-support

View File

@@ -1,70 +1,36 @@
//! Abstractions for Google-style docstrings.
use once_cell::sync::Lazy;
use rustc_hash::FxHashSet;
use crate::docstrings::sections::SectionKind;
pub(crate) static GOOGLE_SECTION_NAMES: Lazy<FxHashSet<&'static str>> = Lazy::new(|| {
FxHashSet::from_iter([
"Args",
"Arguments",
"Attention",
"Attributes",
"Caution",
"Danger",
"Error",
"Example",
"Examples",
"Hint",
"Important",
"Keyword Args",
"Keyword Arguments",
"Methods",
"Note",
"Notes",
"Return",
"Returns",
"Raises",
"References",
"See Also",
"Tip",
"Todo",
"Warning",
"Warnings",
"Warns",
"Yield",
"Yields",
])
});
pub(crate) static LOWERCASE_GOOGLE_SECTION_NAMES: Lazy<FxHashSet<&'static str>> = Lazy::new(|| {
FxHashSet::from_iter([
"args",
"arguments",
"attention",
"attributes",
"caution",
"danger",
"error",
"example",
"examples",
"hint",
"important",
"keyword args",
"keyword arguments",
"methods",
"note",
"notes",
"return",
"returns",
"raises",
"references",
"see also",
"tip",
"todo",
"warning",
"warnings",
"warns",
"yield",
"yields",
])
});
pub(crate) static GOOGLE_SECTIONS: &[SectionKind] = &[
SectionKind::Attributes,
SectionKind::Examples,
SectionKind::Methods,
SectionKind::Notes,
SectionKind::Raises,
SectionKind::References,
SectionKind::Returns,
SectionKind::SeeAlso,
SectionKind::Yields,
// Google-only
SectionKind::Args,
SectionKind::Arguments,
SectionKind::Attention,
SectionKind::Caution,
SectionKind::Danger,
SectionKind::Error,
SectionKind::Example,
SectionKind::Hint,
SectionKind::Important,
SectionKind::KeywordArgs,
SectionKind::KeywordArguments,
SectionKind::Note,
SectionKind::Notes,
SectionKind::Return,
SectionKind::Tip,
SectionKind::Todo,
SectionKind::Warning,
SectionKind::Warnings,
SectionKind::Warns,
SectionKind::Yield,
];

View File

@@ -1,40 +1,20 @@
//! Abstractions for NumPy-style docstrings.
use once_cell::sync::Lazy;
use rustc_hash::FxHashSet;
use crate::docstrings::sections::SectionKind;
pub(crate) static LOWERCASE_NUMPY_SECTION_NAMES: Lazy<FxHashSet<&'static str>> = Lazy::new(|| {
FxHashSet::from_iter([
"short summary",
"extended summary",
"parameters",
"returns",
"yields",
"other parameters",
"raises",
"see also",
"notes",
"references",
"examples",
"attributes",
"methods",
])
});
pub(crate) static NUMPY_SECTION_NAMES: Lazy<FxHashSet<&'static str>> = Lazy::new(|| {
FxHashSet::from_iter([
"Short Summary",
"Extended Summary",
"Parameters",
"Returns",
"Yields",
"Other Parameters",
"Raises",
"See Also",
"Notes",
"References",
"Examples",
"Attributes",
"Methods",
])
});
pub(crate) static NUMPY_SECTIONS: &[SectionKind] = &[
SectionKind::Attributes,
SectionKind::Examples,
SectionKind::Methods,
SectionKind::Notes,
SectionKind::Raises,
SectionKind::References,
SectionKind::Returns,
SectionKind::SeeAlso,
SectionKind::Yields,
// NumPy-only
SectionKind::ExtendedSummary,
SectionKind::OtherParameters,
SectionKind::Parameters,
SectionKind::ShortSummary,
];

View File

@@ -1,8 +1,126 @@
use strum_macros::EnumIter;
use crate::ast::whitespace;
use crate::docstrings::styles::SectionStyle;
#[derive(EnumIter, PartialEq, Eq, Debug, Clone, Copy)]
pub enum SectionKind {
Args,
Arguments,
Attention,
Attributes,
Caution,
Danger,
Error,
Example,
Examples,
ExtendedSummary,
Hint,
Important,
KeywordArgs,
KeywordArguments,
Methods,
Note,
Notes,
OtherParameters,
Parameters,
Raises,
References,
Return,
Returns,
SeeAlso,
ShortSummary,
Tip,
Todo,
Warning,
Warnings,
Warns,
Yield,
Yields,
}
impl SectionKind {
pub fn from_str(s: &str) -> Option<Self> {
match s.to_ascii_lowercase().as_str() {
"args" => Some(Self::Args),
"arguments" => Some(Self::Arguments),
"attention" => Some(Self::Attention),
"attributes" => Some(Self::Attributes),
"caution" => Some(Self::Caution),
"danger" => Some(Self::Danger),
"error" => Some(Self::Error),
"example" => Some(Self::Example),
"examples" => Some(Self::Examples),
"extended summary" => Some(Self::ExtendedSummary),
"hint" => Some(Self::Hint),
"important" => Some(Self::Important),
"keyword args" => Some(Self::KeywordArgs),
"keyword arguments" => Some(Self::KeywordArguments),
"methods" => Some(Self::Methods),
"note" => Some(Self::Note),
"notes" => Some(Self::Notes),
"other parameters" => Some(Self::OtherParameters),
"parameters" => Some(Self::Parameters),
"raises" => Some(Self::Raises),
"references" => Some(Self::References),
"return" => Some(Self::Return),
"returns" => Some(Self::Returns),
"see also" => Some(Self::SeeAlso),
"short summary" => Some(Self::ShortSummary),
"tip" => Some(Self::Tip),
"todo" => Some(Self::Todo),
"warning" => Some(Self::Warning),
"warnings" => Some(Self::Warnings),
"warns" => Some(Self::Warns),
"yield" => Some(Self::Yield),
"yields" => Some(Self::Yields),
_ => None,
}
}
pub fn as_str(self) -> &'static str {
match self {
Self::Args => "Args",
Self::Arguments => "Arguments",
Self::Attention => "Attention",
Self::Attributes => "Attributes",
Self::Caution => "Caution",
Self::Danger => "Danger",
Self::Error => "Error",
Self::Example => "Example",
Self::Examples => "Examples",
Self::ExtendedSummary => "Extended Summary",
Self::Hint => "Hint",
Self::Important => "Important",
Self::KeywordArgs => "Keyword Args",
Self::KeywordArguments => "Keyword Arguments",
Self::Methods => "Methods",
Self::Note => "Note",
Self::Notes => "Notes",
Self::OtherParameters => "Other Parameters",
Self::Parameters => "Parameters",
Self::Raises => "Raises",
Self::References => "References",
Self::Return => "Return",
Self::Returns => "Returns",
Self::SeeAlso => "See Also",
Self::ShortSummary => "Short Summary",
Self::Tip => "Tip",
Self::Todo => "Todo",
Self::Warning => "Warning",
Self::Warnings => "Warnings",
Self::Warns => "Warns",
Self::Yield => "Yield",
Self::Yields => "Yields",
}
}
}
#[derive(Debug)]
pub(crate) struct SectionContext<'a> {
/// The "kind" of the section, e.g. "SectionKind::Args" or "SectionKind::Returns".
pub(crate) kind: SectionKind,
/// The name of the section as it appears in the docstring, e.g. "Args" or "Returns".
pub(crate) section_name: &'a str,
pub(crate) previous_line: &'a str,
pub(crate) line: &'a str,
@@ -11,10 +129,13 @@ pub(crate) struct SectionContext<'a> {
pub(crate) original_index: usize,
}
fn suspected_as_section(line: &str, style: &SectionStyle) -> bool {
style
.lowercase_section_names()
.contains(&whitespace::leading_words(line).to_lowercase().as_str())
fn suspected_as_section(line: &str, style: &SectionStyle) -> Option<SectionKind> {
if let Some(kind) = SectionKind::from_str(whitespace::leading_words(line)) {
if style.sections().contains(&kind) {
return Some(kind);
}
}
None
}
/// Check if the suspected context is really a section header.
@@ -49,21 +170,15 @@ pub(crate) fn section_contexts<'a>(
lines: &'a [&'a str],
style: &SectionStyle,
) -> Vec<SectionContext<'a>> {
let suspected_section_indices: Vec<usize> = lines
let mut contexts = vec![];
for (kind, lineno) in lines
.iter()
.enumerate()
.filter_map(|(lineno, line)| {
if lineno > 0 && suspected_as_section(line, style) {
Some(lineno)
} else {
None
}
})
.collect();
let mut contexts = vec![];
for lineno in suspected_section_indices {
.skip(1)
.filter_map(|(lineno, line)| suspected_as_section(line, style).map(|kind| (kind, lineno)))
{
let context = SectionContext {
kind,
section_name: whitespace::leading_words(lines[lineno]),
previous_line: lines[lineno - 1],
line: lines[lineno],
@@ -76,11 +191,12 @@ pub(crate) fn section_contexts<'a>(
}
}
let mut truncated_contexts = vec![];
let mut truncated_contexts = Vec::with_capacity(contexts.len());
let mut end: Option<usize> = None;
for context in contexts.into_iter().rev() {
let next_end = context.original_index;
truncated_contexts.push(SectionContext {
kind: context.kind,
section_name: context.section_name,
previous_line: context.previous_line,
line: context.line,

View File

@@ -1,8 +1,6 @@
use once_cell::sync::Lazy;
use rustc_hash::FxHashSet;
use crate::docstrings::google::{GOOGLE_SECTION_NAMES, LOWERCASE_GOOGLE_SECTION_NAMES};
use crate::docstrings::numpy::{LOWERCASE_NUMPY_SECTION_NAMES, NUMPY_SECTION_NAMES};
use crate::docstrings::google::GOOGLE_SECTIONS;
use crate::docstrings::numpy::NUMPY_SECTIONS;
use crate::docstrings::sections::SectionKind;
pub(crate) enum SectionStyle {
Numpy,
@@ -10,17 +8,10 @@ pub(crate) enum SectionStyle {
}
impl SectionStyle {
pub(crate) fn section_names(&self) -> &Lazy<FxHashSet<&'static str>> {
pub(crate) fn sections(&self) -> &[SectionKind] {
match self {
SectionStyle::Numpy => &NUMPY_SECTION_NAMES,
SectionStyle::Google => &GOOGLE_SECTION_NAMES,
}
}
pub(crate) fn lowercase_section_names(&self) -> &Lazy<FxHashSet<&'static str>> {
match self {
SectionStyle::Numpy => &LOWERCASE_NUMPY_SECTION_NAMES,
SectionStyle::Google => &LOWERCASE_GOOGLE_SECTION_NAMES,
SectionStyle::Numpy => NUMPY_SECTIONS,
SectionStyle::Google => GOOGLE_SECTIONS,
}
}
}

View File

@@ -1,9 +1,11 @@
use strum::IntoEnumIterator;
use ruff_macros::{define_violation, derive_message_formats};
use crate::ast::types::Range;
use crate::checkers::ast::Checker;
use crate::docstrings::definition::Docstring;
use crate::docstrings::styles::SectionStyle;
use crate::docstrings::sections::SectionKind;
use crate::fix::Fix;
use crate::message::Location;
use crate::registry::Diagnostic;
@@ -40,15 +42,13 @@ pub fn ends_with_period(checker: &mut Checker, docstring: &Docstring) {
}
// Avoid false-positives: `Args:`, etc.
for style in [SectionStyle::Google, SectionStyle::Numpy] {
for section_name in style.section_names().iter() {
if let Some(suffix) = trimmed.strip_suffix(section_name) {
if suffix.is_empty() {
return;
}
if suffix == ":" {
return;
}
for section_kind in SectionKind::iter() {
if let Some(suffix) = trimmed.strip_suffix(section_kind.as_str()) {
if suffix.is_empty() {
return;
}
if suffix == ":" {
return;
}
}
}

View File

@@ -1,9 +1,11 @@
use strum::IntoEnumIterator;
use ruff_macros::{define_violation, derive_message_formats};
use crate::ast::types::Range;
use crate::checkers::ast::Checker;
use crate::docstrings::definition::Docstring;
use crate::docstrings::styles::SectionStyle;
use crate::docstrings::sections::SectionKind;
use crate::fix::Fix;
use crate::message::Location;
use crate::registry::Diagnostic;
@@ -40,15 +42,13 @@ pub fn ends_with_punctuation(checker: &mut Checker, docstring: &Docstring) {
}
// Avoid false-positives: `Args:`, etc.
for style in [SectionStyle::Google, SectionStyle::Numpy] {
for section_name in style.section_names().iter() {
if let Some(suffix) = trimmed.strip_suffix(section_name) {
if suffix.is_empty() {
return;
}
if suffix == ":" {
return;
}
for section_kind in SectionKind::iter() {
if let Some(suffix) = trimmed.strip_suffix(section_kind.as_str()) {
if suffix.is_empty() {
return;
}
if suffix == ":" {
return;
}
}
}

View File

@@ -1,17 +1,18 @@
use itertools::Itertools;
use once_cell::sync::Lazy;
use regex::Regex;
use ruff_macros::{define_violation, derive_message_formats};
use rustc_hash::FxHashSet;
use rustpython_parser::ast::StmtKind;
use ruff_macros::{define_violation, derive_message_formats};
use crate::ast::helpers::identifier_range;
use crate::ast::types::Range;
use crate::ast::whitespace::LinesWithTrailingNewline;
use crate::ast::{cast, whitespace};
use crate::checkers::ast::Checker;
use crate::docstrings::definition::{DefinitionKind, Docstring};
use crate::docstrings::sections::{section_contexts, SectionContext};
use crate::docstrings::sections::{section_contexts, SectionContext, SectionKind};
use crate::docstrings::styles::SectionStyle;
use crate::fix::Fix;
use crate::message::Location;
@@ -289,18 +290,46 @@ pub fn sections(checker: &mut Checker, docstring: &Docstring, convention: Option
}
}
Some(Convention::Pep257) | None => {
// First, interpret as NumPy-style sections.
let mut found_numpy_section = false;
for context in &section_contexts(&lines, &SectionStyle::Numpy) {
found_numpy_section = true;
numpy_section(checker, docstring, context);
// There are some overlapping section names, between the Google and NumPy conventions
// (e.g., "Returns", "Raises"). Break ties by checking for the presence of some of the
// section names that are unique to each convention.
// If the docstring contains `Args:` or `Arguments:`, use the Google convention.
let google_sections = section_contexts(&lines, &SectionStyle::Google);
if google_sections
.iter()
.any(|context| matches!(context.kind, SectionKind::Arguments | SectionKind::Args))
{
for context in &google_sections {
google_section(checker, docstring, context);
}
return;
}
// If no such sections were identified, interpret as Google-style sections.
if !found_numpy_section {
for context in &section_contexts(&lines, &SectionStyle::Google) {
// If the docstring contains `Parameters:` or `Other Parameters:`, use the NumPy
// convention.
let numpy_sections = section_contexts(&lines, &SectionStyle::Numpy);
if numpy_sections.iter().any(|context| {
matches!(
context.kind,
SectionKind::Parameters | SectionKind::OtherParameters
)
}) {
for context in &numpy_sections {
numpy_section(checker, docstring, context);
}
return;
}
// Otherwise, use whichever convention matched more sections.
if google_sections.len() > numpy_sections.len() {
for context in &google_sections {
google_section(checker, docstring, context);
}
} else {
for context in &numpy_sections {
numpy_section(checker, docstring, context);
}
}
}
}
@@ -614,47 +643,37 @@ fn blanks_and_section_underline(
}
}
fn common_section(
checker: &mut Checker,
docstring: &Docstring,
context: &SectionContext,
style: &SectionStyle,
) {
fn common_section(checker: &mut Checker, docstring: &Docstring, context: &SectionContext) {
if checker.settings.rules.enabled(&Rule::CapitalizeSectionName) {
if !style.section_names().contains(&context.section_name) {
let capitalized_section_name = titlecase::titlecase(context.section_name);
if style
.section_names()
.contains(capitalized_section_name.as_str())
{
let mut diagnostic = Diagnostic::new(
CapitalizeSectionName {
name: context.section_name.to_string(),
},
Range::from_located(docstring.expr),
);
if checker.patch(diagnostic.kind.rule()) {
// Replace the section title with the capitalized variant. This requires
// locating the start and end of the section name.
if let Some(index) = context.line.find(context.section_name) {
// Map from bytes to characters.
let section_name_start = &context.line[..index].chars().count();
let section_name_length = &context.section_name.chars().count();
diagnostic.amend(Fix::replacement(
capitalized_section_name,
Location::new(
docstring.expr.location.row() + context.original_index,
*section_name_start,
),
Location::new(
docstring.expr.location.row() + context.original_index,
section_name_start + section_name_length,
),
));
}
let capitalized_section_name = context.kind.as_str();
if context.section_name != capitalized_section_name {
let mut diagnostic = Diagnostic::new(
CapitalizeSectionName {
name: context.section_name.to_string(),
},
Range::from_located(docstring.expr),
);
if checker.patch(diagnostic.kind.rule()) {
// Replace the section title with the capitalized variant. This requires
// locating the start and end of the section name.
if let Some(index) = context.line.find(context.section_name) {
// Map from bytes to characters.
let section_name_start = &context.line[..index].chars().count();
let section_name_length = &context.section_name.chars().count();
diagnostic.amend(Fix::replacement(
capitalized_section_name.to_string(),
Location::new(
docstring.expr.location.row() + context.original_index,
*section_name_start,
),
Location::new(
docstring.expr.location.row() + context.original_index,
section_name_start + section_name_length,
),
));
}
checker.diagnostics.push(diagnostic);
}
checker.diagnostics.push(diagnostic);
}
}
@@ -933,7 +952,7 @@ fn parameters_section(checker: &mut Checker, docstring: &Docstring, context: &Se
}
fn numpy_section(checker: &mut Checker, docstring: &Docstring, context: &SectionContext) {
common_section(checker, docstring, context, &SectionStyle::Numpy);
common_section(checker, docstring, context);
if checker
.settings
@@ -977,15 +996,14 @@ fn numpy_section(checker: &mut Checker, docstring: &Docstring, context: &Section
}
if checker.settings.rules.enabled(&Rule::UndocumentedParam) {
let capitalized_section_name = titlecase::titlecase(context.section_name);
if capitalized_section_name == "Parameters" {
if matches!(context.kind, SectionKind::Parameters) {
parameters_section(checker, docstring, context);
}
}
}
fn google_section(checker: &mut Checker, docstring: &Docstring, context: &SectionContext) {
common_section(checker, docstring, context, &SectionStyle::Google);
common_section(checker, docstring, context);
if checker
.settings
@@ -1030,8 +1048,7 @@ fn google_section(checker: &mut Checker, docstring: &Docstring, context: &Sectio
}
if checker.settings.rules.enabled(&Rule::UndocumentedParam) {
let capitalized_section_name = titlecase::titlecase(context.section_name);
if capitalized_section_name == "Args" || capitalized_section_name == "Arguments" {
if matches!(context.kind, SectionKind::Args | SectionKind::Arguments) {
args_section(checker, docstring, context);
}
}

View File

@@ -38,40 +38,4 @@ expression: diagnostics
row: 218
column: 11
parent: ~
- kind:
NewLineAfterSectionName:
name: Returns
location:
row: 252
column: 4
end_location:
row: 262
column: 7
fix:
content: ""
location:
row: 257
column: 11
end_location:
row: 257
column: 12
parent: ~
- kind:
NewLineAfterSectionName:
name: Raises
location:
row: 252
column: 4
end_location:
row: 262
column: 7
fix:
content: ""
location:
row: 259
column: 10
end_location:
row: 259
column: 11
parent: ~

View File

@@ -56,6 +56,24 @@ expression: diagnostics
row: 219
column: 0
parent: ~
- kind:
DashedUnderlineAfterSection:
name: Args
location:
row: 252
column: 4
end_location:
row: 262
column: 7
fix:
content: " ----\n"
location:
row: 255
column: 0
end_location:
row: 255
column: 0
parent: ~
- kind:
DashedUnderlineAfterSection:
name: Returns

View File

@@ -2,6 +2,32 @@
source: crates/ruff/src/rules/pydocstyle/mod.rs
expression: diagnostics
---
- kind:
UndocumentedParam:
names:
- y
- z
location:
row: 1
column: 4
end_location:
row: 1
column: 5
fix: ~
parent: ~
- kind:
UndocumentedParam:
names:
- y
- z
location:
row: 14
column: 4
end_location:
row: 14
column: 5
fix: ~
parent: ~
- kind:
UndocumentedParam:
names:
@@ -15,6 +41,31 @@ expression: diagnostics
column: 5
fix: ~
parent: ~
- kind:
UndocumentedParam:
names:
- y
- z
location:
row: 39
column: 4
end_location:
row: 39
column: 5
fix: ~
parent: ~
- kind:
UndocumentedParam:
names:
- y
location:
row: 52
column: 4
end_location:
row: 52
column: 5
fix: ~
parent: ~
- kind:
UndocumentedParam:
names: