Skip to content

Commit

Permalink
[pydocstyle] Improve heuristics for detecting Google-style docstrin…
Browse files Browse the repository at this point in the history
…gs (#13142)
  • Loading branch information
AlexWaygood authored Aug 29, 2024
1 parent ee258ca commit 281e6d9
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 357 deletions.
14 changes: 14 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pydocstyle/sections.py
Original file line number Diff line number Diff line change
Expand Up @@ -605,3 +605,17 @@ def test_lowercase_sub_section_header_different_kind(returns: int):
some value
"""


# We used to incorrectly infer this as a numpy-style docstring,
# which caused us to emit D406 and D407 on it;
# see https://github.com/astral-sh/ruff/issues/13139
def another_valid_google_style_docstring(a: str) -> str:
"""Foo bar.
Examples:
Some explanation here.
>>> bla bla bla
"""
return a
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/docstrings/styles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::docstrings::google::GOOGLE_SECTIONS;
use crate::docstrings::numpy::NUMPY_SECTIONS;
use crate::docstrings::sections::SectionKind;

#[derive(Copy, Clone)]
#[derive(Copy, Clone, Debug, is_macro::Is)]
pub(crate) enum SectionStyle {
Numpy,
Google,
Expand Down
72 changes: 66 additions & 6 deletions crates/ruff_linter/src/rules/pydocstyle/helpers.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
use std::cmp::Ordering;

use ruff_python_ast::helpers::map_callable;
use ruff_python_semantic::{Definition, SemanticModel};
use ruff_source_file::UniversalNewlines;
use ruff_python_trivia::Cursor;
use ruff_source_file::{Line, UniversalNewlines};
use ruff_text_size::{TextRange, TextSize};

use crate::docstrings::sections::{SectionContexts, SectionKind};
use crate::docstrings::styles::SectionStyle;
Expand Down Expand Up @@ -112,12 +116,68 @@ pub(crate) fn get_section_contexts<'a>(
return google_sections;
}

// Otherwise, use whichever convention matched more sections.
if google_sections.len() > numpy_sections.len() {
google_sections
} else {
numpy_sections
// Otherwise, If one convention matched more sections, return that...
match google_sections.len().cmp(&numpy_sections.len()) {
Ordering::Greater => return google_sections,
Ordering::Less => return numpy_sections,
Ordering::Equal => {}
};

// 0 sections of either convention? Default to numpy
if google_sections.len() == 0 {
return numpy_sections;
}

for section in &google_sections {
// If any section has something that could be an underline
// on the following line, assume Numpy.
// If it *doesn't* have an underline and it *does* have a colon
// at the end of a section name, assume Google.
if let Some(following_line) = section.following_lines().next() {
if find_underline(&following_line, '-').is_some() {
return numpy_sections;
}
}
if section.summary_after_section_name().starts_with(':') {
return google_sections;
}
}

// If all else fails, default to numpy
numpy_sections
}
}
}

/// Returns the [`TextRange`] of the underline, if a line consists of only dashes.
pub(super) fn find_underline(line: &Line, dash: char) -> Option<TextRange> {
let mut cursor = Cursor::new(line.as_str());

// Eat leading whitespace.
cursor.eat_while(char::is_whitespace);

// Determine the start of the dashes.
let offset = cursor.token_len();

// Consume the dashes.
cursor.start_token();
cursor.eat_while(|c| c == dash);

// Determine the end of the dashes.
let len = cursor.token_len();

// If there are no dashes, return None.
if len == TextSize::new(0) {
return None;
}

// Eat trailing whitespace.
cursor.eat_while(char::is_whitespace);

// If there are any characters after the dashes, return None.
if !cursor.is_eof() {
return None;
}

Some(TextRange::at(offset, len) + line.start())
}
51 changes: 10 additions & 41 deletions crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use itertools::Itertools;
use once_cell::sync::Lazy;
use regex::Regex;
use rustc_hash::FxHashSet;
use std::ops::Add;

use ruff_diagnostics::{AlwaysFixableViolation, Violation};
use ruff_diagnostics::{Diagnostic, Edit, Fix};
Expand All @@ -11,15 +10,16 @@ use ruff_python_ast::docstrings::{clean_space, leading_space};
use ruff_python_ast::identifier::Identifier;
use ruff_python_ast::ParameterWithDefault;
use ruff_python_semantic::analyze::visibility::is_staticmethod;
use ruff_python_trivia::{textwrap::dedent, Cursor};
use ruff_source_file::{Line, NewlineWithTrailingNewline};
use ruff_python_trivia::textwrap::dedent;
use ruff_source_file::NewlineWithTrailingNewline;
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};

use crate::checkers::ast::Checker;
use crate::docstrings::sections::{SectionContext, SectionContexts, SectionKind};
use crate::docstrings::styles::SectionStyle;
use crate::docstrings::Docstring;
use crate::registry::Rule;
use crate::rules::pydocstyle::helpers::find_underline;
use crate::rules::pydocstyle::settings::Convention;

/// ## What it does
Expand Down Expand Up @@ -1341,6 +1341,7 @@ fn blanks_and_section_underline(
checker: &mut Checker,
docstring: &Docstring,
context: &SectionContext,
style: SectionStyle,
) {
let mut num_blank_lines_after_header = 0u32;
let mut blank_lines_end = context.following_range().start();
Expand Down Expand Up @@ -1510,7 +1511,7 @@ fn blanks_and_section_underline(
}
}
} else {
if checker.enabled(Rule::DashedUnderlineAfterSection) {
if style.is_numpy() && checker.enabled(Rule::DashedUnderlineAfterSection) {
if let Some(equal_line) = find_underline(&non_blank_line, '=') {
let mut diagnostic = Diagnostic::new(
DashedUnderlineAfterSection {
Expand Down Expand Up @@ -1608,7 +1609,7 @@ fn blanks_and_section_underline(
}
} else {
// Nothing but blank lines after the section header.
if checker.enabled(Rule::DashedUnderlineAfterSection) {
if style.is_numpy() && checker.enabled(Rule::DashedUnderlineAfterSection) {
let mut diagnostic = Diagnostic::new(
DashedUnderlineAfterSection {
name: context.section_name().to_string(),
Expand Down Expand Up @@ -1646,6 +1647,7 @@ fn common_section(
docstring: &Docstring,
context: &SectionContext,
next: Option<&SectionContext>,
style: SectionStyle,
) {
if checker.enabled(Rule::CapitalizeSectionName) {
let capitalized_section_name = context.kind().as_str();
Expand Down Expand Up @@ -1776,7 +1778,7 @@ fn common_section(
}
}

blanks_and_section_underline(checker, docstring, context);
blanks_and_section_underline(checker, docstring, context, style);
}

fn missing_args(checker: &mut Checker, docstring: &Docstring, docstrings_args: &FxHashSet<String>) {
Expand Down Expand Up @@ -1946,7 +1948,7 @@ fn numpy_section(
context: &SectionContext,
next: Option<&SectionContext>,
) {
common_section(checker, docstring, context, next);
common_section(checker, docstring, context, next, SectionStyle::Numpy);

if checker.enabled(Rule::NewLineAfterSectionName) {
let suffix = context.summary_after_section_name();
Expand Down Expand Up @@ -1981,7 +1983,7 @@ fn google_section(
context: &SectionContext,
next: Option<&SectionContext>,
) {
common_section(checker, docstring, context, next);
common_section(checker, docstring, context, next, SectionStyle::Google);

if checker.enabled(Rule::SectionNameEndsInColon) {
let suffix = context.summary_after_section_name();
Expand Down Expand Up @@ -2049,36 +2051,3 @@ fn parse_google_sections(
}
}
}

/// Returns the [`TextRange`] of the underline, if a line consists of only dashes.
fn find_underline(line: &Line, dash: char) -> Option<TextRange> {
let mut cursor = Cursor::new(line.as_str());

// Eat leading whitespace.
cursor.eat_while(char::is_whitespace);

// Determine the start of the dashes.
let offset = cursor.token_len();

// Consume the dashes.
cursor.start_token();
cursor.eat_while(|c| c == dash);

// Determine the end of the dashes.
let len = cursor.token_len();

// If there are no dashes, return None.
if len == TextSize::new(0) {
return None;
}

// Eat trailing whitespace.
cursor.eat_while(char::is_whitespace);

// If there are any characters after the dashes, return None.
if !cursor.is_eof() {
return None;
}

Some(TextRange::at(offset, len).add(line.start()))
}
Loading

0 comments on commit 281e6d9

Please sign in to comment.