mirror of
https://github.com/astral-sh/ruff.git
synced 2025-11-20 04:00:09 +00:00
[pydocstyle] Handle arguments with the same names as sections (D417) (#16011)
## Summary Fixes #16007. The logic from the last fix for this (#9427) was sufficient, it just wasn't being applied because `Attributes` sections aren't expected to have nested sections. I just deleted the outer conditional, which should hopefully fix this for all section types. ## Test Plan New regression test, plus the existing D417 tests.
This commit is contained in:
parent
df1d430294
commit
7b487d853a
6 changed files with 150 additions and 77 deletions
|
|
@ -176,4 +176,45 @@ def f(x, *args, **kwargs):
|
||||||
x: the value
|
x: the value
|
||||||
*args: var-arguments
|
*args: var-arguments
|
||||||
"""
|
"""
|
||||||
return x
|
return x
|
||||||
|
|
||||||
|
|
||||||
|
# regression test for https://github.com/astral-sh/ruff/issues/16007.
|
||||||
|
# attributes is a section name without subsections, so it was failing the
|
||||||
|
# previous workaround for Args: args: sections
|
||||||
|
def send(payload: str, attributes: dict[str, Any]) -> None:
|
||||||
|
"""
|
||||||
|
Send a message.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
payload:
|
||||||
|
The message payload.
|
||||||
|
|
||||||
|
attributes:
|
||||||
|
Additional attributes to be sent alongside the message.
|
||||||
|
"""
|
||||||
|
|
||||||
|
|
||||||
|
# undocumented argument with the same name as a section
|
||||||
|
def should_fail(payload, Args):
|
||||||
|
"""
|
||||||
|
Send a message.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
payload:
|
||||||
|
The message payload.
|
||||||
|
"""
|
||||||
|
|
||||||
|
|
||||||
|
# documented argument with the same name as a section
|
||||||
|
def should_not_fail(payload, Args):
|
||||||
|
"""
|
||||||
|
Send a message.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
payload:
|
||||||
|
The message payload.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
The other arguments.
|
||||||
|
"""
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,7 @@ use std::fmt::{Debug, Formatter};
|
||||||
use std::iter::FusedIterator;
|
use std::iter::FusedIterator;
|
||||||
|
|
||||||
use ruff_python_ast::docstrings::{leading_space, leading_words};
|
use ruff_python_ast::docstrings::{leading_space, leading_words};
|
||||||
|
use ruff_python_semantic::Definition;
|
||||||
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
|
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
|
||||||
use strum_macros::EnumIter;
|
use strum_macros::EnumIter;
|
||||||
|
|
||||||
|
|
@ -130,34 +131,6 @@ impl SectionKind {
|
||||||
Self::Yields => "Yields",
|
Self::Yields => "Yields",
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Returns `true` if a section can contain subsections, as in:
|
|
||||||
/// ```python
|
|
||||||
/// Yields
|
|
||||||
/// ------
|
|
||||||
/// int
|
|
||||||
/// Description of the anonymous integer return value.
|
|
||||||
/// ```
|
|
||||||
///
|
|
||||||
/// For NumPy, see: <https://numpydoc.readthedocs.io/en/latest/format.html>
|
|
||||||
///
|
|
||||||
/// For Google, see: <https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings>
|
|
||||||
pub(crate) fn has_subsections(self) -> bool {
|
|
||||||
matches!(
|
|
||||||
self,
|
|
||||||
Self::Args
|
|
||||||
| Self::Arguments
|
|
||||||
| Self::OtherArgs
|
|
||||||
| Self::OtherParameters
|
|
||||||
| Self::OtherParams
|
|
||||||
| Self::Parameters
|
|
||||||
| Self::Raises
|
|
||||||
| Self::Returns
|
|
||||||
| Self::SeeAlso
|
|
||||||
| Self::Warns
|
|
||||||
| Self::Yields
|
|
||||||
)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) struct SectionContexts<'a> {
|
pub(crate) struct SectionContexts<'a> {
|
||||||
|
|
@ -195,6 +168,7 @@ impl<'a> SectionContexts<'a> {
|
||||||
last.as_ref(),
|
last.as_ref(),
|
||||||
previous_line.as_ref(),
|
previous_line.as_ref(),
|
||||||
lines.peek(),
|
lines.peek(),
|
||||||
|
docstring.definition,
|
||||||
) {
|
) {
|
||||||
if let Some(mut last) = last.take() {
|
if let Some(mut last) = last.take() {
|
||||||
last.range = TextRange::new(last.start(), line.start());
|
last.range = TextRange::new(last.start(), line.start());
|
||||||
|
|
@ -444,6 +418,7 @@ fn suspected_as_section(line: &str, style: SectionStyle) -> Option<SectionKind>
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Check if the suspected context is really a section header.
|
/// Check if the suspected context is really a section header.
|
||||||
|
#[allow(clippy::too_many_arguments)]
|
||||||
fn is_docstring_section(
|
fn is_docstring_section(
|
||||||
line: &Line,
|
line: &Line,
|
||||||
indent_size: TextSize,
|
indent_size: TextSize,
|
||||||
|
|
@ -452,7 +427,9 @@ fn is_docstring_section(
|
||||||
previous_section: Option<&SectionContextData>,
|
previous_section: Option<&SectionContextData>,
|
||||||
previous_line: Option<&Line>,
|
previous_line: Option<&Line>,
|
||||||
next_line: Option<&Line>,
|
next_line: Option<&Line>,
|
||||||
|
definition: &Definition<'_>,
|
||||||
) -> bool {
|
) -> bool {
|
||||||
|
// for function definitions, track the known argument names for more accurate section detection.
|
||||||
// Determine whether the current line looks like a section header, e.g., "Args:".
|
// Determine whether the current line looks like a section header, e.g., "Args:".
|
||||||
let section_name_suffix = line[usize::from(indent_size + section_name_size)..].trim();
|
let section_name_suffix = line[usize::from(indent_size + section_name_size)..].trim();
|
||||||
let this_looks_like_a_section_name =
|
let this_looks_like_a_section_name =
|
||||||
|
|
@ -509,60 +486,80 @@ fn is_docstring_section(
|
||||||
// ```
|
// ```
|
||||||
// However, if the header is an _exact_ match (like `Returns:`, as opposed to `returns:`), then
|
// However, if the header is an _exact_ match (like `Returns:`, as opposed to `returns:`), then
|
||||||
// continue to treat it as a section header.
|
// continue to treat it as a section header.
|
||||||
if section_kind.has_subsections() {
|
if let Some(previous_section) = previous_section {
|
||||||
if let Some(previous_section) = previous_section {
|
let verbatim = &line[TextRange::at(indent_size, section_name_size)];
|
||||||
let verbatim = &line[TextRange::at(indent_size, section_name_size)];
|
|
||||||
|
|
||||||
// If the section is more deeply indented, assume it's a subsection, as in:
|
// If the section is more deeply indented, assume it's a subsection, as in:
|
||||||
// ```python
|
// ```python
|
||||||
// def func(args: tuple[int]):
|
// def func(args: tuple[int]):
|
||||||
// """Toggle the gizmo.
|
// """Toggle the gizmo.
|
||||||
//
|
//
|
||||||
// Args:
|
// Args:
|
||||||
// args: The arguments to the function.
|
// args: The arguments to the function.
|
||||||
// """
|
// """
|
||||||
// ```
|
// ```
|
||||||
if previous_section.indent_size < indent_size {
|
// As noted above, an exact match for a section name (like the inner `Args:` below) is
|
||||||
if section_kind.as_str() != verbatim {
|
// treated as a section header, unless the enclosing `Definition` is a function and contains
|
||||||
return false;
|
// a parameter with the same name, as in:
|
||||||
}
|
// ```python
|
||||||
|
// def func(Args: tuple[int]):
|
||||||
|
// """Toggle the gizmo.
|
||||||
|
//
|
||||||
|
// Args:
|
||||||
|
// Args: The arguments to the function.
|
||||||
|
// """
|
||||||
|
// ```
|
||||||
|
if previous_section.indent_size < indent_size {
|
||||||
|
let section_name = section_kind.as_str();
|
||||||
|
if section_name != verbatim || has_parameter(definition, section_name) {
|
||||||
|
return false;
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// If the section has a preceding empty line, assume it's _not_ a subsection, as in:
|
// If the section has a preceding empty line, assume it's _not_ a subsection, as in:
|
||||||
// ```python
|
// ```python
|
||||||
// def func(args: tuple[int]):
|
// def func(args: tuple[int]):
|
||||||
// """Toggle the gizmo.
|
// """Toggle the gizmo.
|
||||||
//
|
//
|
||||||
// Args:
|
// Args:
|
||||||
// args: The arguments to the function.
|
// args: The arguments to the function.
|
||||||
//
|
//
|
||||||
// returns:
|
// returns:
|
||||||
// The return value of the function.
|
// The return value of the function.
|
||||||
// """
|
// """
|
||||||
// ```
|
// ```
|
||||||
if previous_line.is_some_and(|line| line.trim().is_empty()) {
|
if previous_line.is_some_and(|line| line.trim().is_empty()) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
// If the section isn't underlined, and isn't title-cased, assume it's a subsection,
|
// If the section isn't underlined, and isn't title-cased, assume it's a subsection,
|
||||||
// as in:
|
// as in:
|
||||||
// ```python
|
// ```python
|
||||||
// def func(parameters: tuple[int]):
|
// def func(parameters: tuple[int]):
|
||||||
// """Toggle the gizmo.
|
// """Toggle the gizmo.
|
||||||
//
|
//
|
||||||
// Parameters:
|
// Parameters:
|
||||||
// -----
|
// -----
|
||||||
// parameters:
|
// parameters:
|
||||||
// The arguments to the function.
|
// The arguments to the function.
|
||||||
// """
|
// """
|
||||||
// ```
|
// ```
|
||||||
if !next_line_is_underline && verbatim.chars().next().is_some_and(char::is_lowercase) {
|
if !next_line_is_underline && verbatim.chars().next().is_some_and(char::is_lowercase) {
|
||||||
if section_kind.as_str() != verbatim {
|
if section_kind.as_str() != verbatim {
|
||||||
return false;
|
return false;
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
true
|
true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns whether or not `definition` is a function definition and contains a parameter with the
|
||||||
|
/// same name as `section_name`.
|
||||||
|
fn has_parameter(definition: &Definition, section_name: &str) -> bool {
|
||||||
|
definition.as_function_def().is_some_and(|func| {
|
||||||
|
func.parameters
|
||||||
|
.iter()
|
||||||
|
.any(|param| param.name() == section_name)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -80,3 +80,12 @@ D417.py:172:5: D417 Missing argument description in the docstring for `f`: `**kw
|
||||||
| ^ D417
|
| ^ D417
|
||||||
173 | """Do something.
|
173 | """Do something.
|
||||||
|
|
|
|
||||||
|
|
||||||
|
D417.py:199:5: D417 Missing argument description in the docstring for `should_fail`: `Args`
|
||||||
|
|
|
||||||
|
198 | # undocumented argument with the same name as a section
|
||||||
|
199 | def should_fail(payload, Args):
|
||||||
|
| ^^^^^^^^^^^ D417
|
||||||
|
200 | """
|
||||||
|
201 | Send a message.
|
||||||
|
|
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,5 @@
|
||||||
---
|
---
|
||||||
source: crates/ruff_linter/src/rules/pydocstyle/mod.rs
|
source: crates/ruff_linter/src/rules/pydocstyle/mod.rs
|
||||||
snapshot_kind: text
|
|
||||||
---
|
---
|
||||||
D417.py:1:5: D417 Missing argument descriptions in the docstring for `f`: `y`, `z`
|
D417.py:1:5: D417 Missing argument descriptions in the docstring for `f`: `y`, `z`
|
||||||
|
|
|
|
||||||
|
|
@ -65,3 +64,12 @@ D417.py:155:5: D417 Missing argument description in the docstring for `select_da
|
||||||
156 | query: str,
|
156 | query: str,
|
||||||
157 | args: tuple,
|
157 | args: tuple,
|
||||||
|
|
|
|
||||||
|
|
||||||
|
D417.py:199:5: D417 Missing argument description in the docstring for `should_fail`: `Args`
|
||||||
|
|
|
||||||
|
198 | # undocumented argument with the same name as a section
|
||||||
|
199 | def should_fail(payload, Args):
|
||||||
|
| ^^^^^^^^^^^ D417
|
||||||
|
200 | """
|
||||||
|
201 | Send a message.
|
||||||
|
|
|
||||||
|
|
|
||||||
|
|
@ -80,3 +80,12 @@ D417.py:172:5: D417 Missing argument description in the docstring for `f`: `**kw
|
||||||
| ^ D417
|
| ^ D417
|
||||||
173 | """Do something.
|
173 | """Do something.
|
||||||
|
|
|
|
||||||
|
|
||||||
|
D417.py:199:5: D417 Missing argument description in the docstring for `should_fail`: `Args`
|
||||||
|
|
|
||||||
|
198 | # undocumented argument with the same name as a section
|
||||||
|
199 | def should_fail(payload, Args):
|
||||||
|
| ^^^^^^^^^^^ D417
|
||||||
|
200 | """
|
||||||
|
201 | Send a message.
|
||||||
|
|
|
||||||
|
|
|
||||||
|
|
@ -80,3 +80,12 @@ D417.py:172:5: D417 Missing argument description in the docstring for `f`: `**kw
|
||||||
| ^ D417
|
| ^ D417
|
||||||
173 | """Do something.
|
173 | """Do something.
|
||||||
|
|
|
|
||||||
|
|
||||||
|
D417.py:199:5: D417 Missing argument description in the docstring for `should_fail`: `Args`
|
||||||
|
|
|
||||||
|
198 | # undocumented argument with the same name as a section
|
||||||
|
199 | def should_fail(payload, Args):
|
||||||
|
| ^^^^^^^^^^^ D417
|
||||||
|
200 | """
|
||||||
|
201 | Send a message.
|
||||||
|
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue