Skip to content

[pydoclint] Fix false positive when Sphinx directives follow Raises section (DOC502) #20535

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.

Sign up for GitHub

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

Merged
ntBre merged 6 commits into from
Nov 12, 2025
Merged

[pydoclint] Fix false positive when Sphinx directives follow Raises section (DOC502) #20535

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d97f7ea
fix-18959
danparizher Sep 23, 2025
04d5e16
Refactor Google docstring entry parsing logic
danparizher Oct 15, 2025
fd2a602
Fix DOC502 parsing for dedented section lines in Google style
danparizher Oct 15, 2025
82eb6d1
Merge branch 'main' into fix-18959
ntBre Oct 28, 2025
e645e0b
Handle Sphinx directives in numpy Raises section
danparizher Oct 29, 2025
fea391c
only trim indentation before sphinx check
ntBre Nov 12, 2025
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
13 changes: 13 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pydoclint/DOC502_google.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,16 @@ def calculate_speed(distance: float, time: float) -> float:
except TypeError:
print("Not a number? Shame on you!")
raise

# DOC502 regression for Sphinx directive after Raises (issue #18959)
def foo():
"""First line.

Raises:
ValueError:
some text

.. versionadded:: 0.7.0
The ``init_kwargs`` argument.
"""
raise ValueError
44 changes: 38 additions & 6 deletions crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,13 +476,45 @@ fn parse_entries(content: &str, style: Option<SectionStyle>) -> Vec<QualifiedNam
/// ```
fn parse_entries_google(content: &str) -> Vec<QualifiedName<'_>> {
let mut entries: Vec<QualifiedName> = Vec::new();
for potential in content.lines() {
let Some(colon_idx) = potential.find(':') else {
continue;
};
let entry = potential[..colon_idx].trim();
entries.push(QualifiedName::user_defined(entry));

@ntBre ntBre Oct 13, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about something more like this for this function?

fn parse_entries_google(content: &str) -> Vec<QualifiedName<'_>> {
    let mut entries: Vec<QualifiedName> = Vec::new();
    let mut lines = content.lines().peekable();
    let Some(first) = lines.peek() else {
        return entries;
    };
    let indentation = &first[..first.len() - first.trim_start().len()];
    for potential in lines {
        if let Some(entry) = potential.strip_prefix(indentation) {
            if let Some(first_char) = entry.chars().next() {
                if !first_char.is_whitespace() {
                    if let Some(colon_idx) = entry.find(':') {
                        let entry = entry[..colon_idx].trim();
                        if !entry.is_empty() {
                            entries.push(QualifiedName::user_defined(entry));
                        }
                    }
                }
            }
        }
    }
    entries
}

This is closer to the parse_entries_numpy down below and seems to work on the new test case when I tried it locally.

@danparizher danparizher Oct 15, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is significantly better, thank you! We will have to see what the ecosystem results will look like.

// Determine the indentation of the entries from the first non-empty line.
// Google-style entries are indented relative to the "Raises:" header, e.g.:
// " ValueError: explanation".
let lines = content.lines();
let mut expected_indent: Option<&str> = None;

for raw in lines {
let line = raw.trim_end_matches('\r');

// Stop if we encounter an unindented line or a Sphinx directive starting with ".. ".
if !line.trim().is_empty() {
// Compute indentation of current line
let indent_len = line.len() - line.trim_start().len();
let indent = &line[..indent_len];

// If this looks like a Sphinx directive or any unindented content, the section ends
if indent_len == 0 || line.trim_start().starts_with(".. ") {
break;
}

// Establish expected indentation based on the first valid entry line
if expected_indent.is_none() {
expected_indent = Some(indent);
} else if Some(indent) != expected_indent {
// Different indentation likely starts a new sub-block; stop collecting
break;
}

// Parse only lines that contain a colon and where the token before the colon is non-empty
if let Some(colon_idx) = line.find(':') {
let entry = line[..colon_idx].trim();
if !entry.is_empty() {
entries.push(QualifiedName::user_defined(entry));
}
}
}
}

entries
}

Expand Down
Loading