Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pydocstyle] Improve heuristics for detecting Google-style docstrings #13142

Merged
merged 1 commit into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading