Skip to content

Comments

[pydocstyle] Handle arguments with the same names as sections (D417)#16011

Merged
ntBre merged 11 commits intomainfrom
brent/docstring-nested-attributes
Feb 11, 2025
Merged

[pydocstyle] Handle arguments with the same names as sections (D417)#16011
ntBre merged 11 commits intomainfrom
brent/docstring-nested-attributes

Conversation

@ntBre
Copy link
Contributor

@ntBre ntBre commented Feb 7, 2025

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.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@MichaReiser MichaReiser added the rule Implementing or modifying a lint rule label Feb 7, 2025
# 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:
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind adding a test where one of the function arguments is named like a section header? I'm curious what happens if that argument is undocumented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this what you had in mind? I added these two cases, both of which cause D417. The first one is right, Args is not documented, but the second is still a false positive that I can keep working on.

# undocumented argument with the same name as a section
def f(payload, Args):
    """
    Send a message.

    Args:
        payload:
            The message payload.
    """


# documented argument with the same name as a section
def f(payload, Args):
    """
    Send a message.

    Args:
        payload:
            The message payload.

        Args:
            The other arguments.
    """

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's what I had in mind

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These cases should work now! I initially deleted the inner conditional here

if previous_section.indent_size < indent_size {
if section_kind.as_str() != verbatim {
return false;
}
}

which worked well for D417 but broke other rules like D214 - overindented-section. So I went for the more involved approach of passing down a Definition and extracting known parameter names from it.

One other thing I considered is just searching for a known parameter at line 514 instead of collecting a HashSet up front.

@AlexWaygood AlexWaygood added the docstring Related to docstring linting or formatting label Feb 7, 2025
@ntBre ntBre merged commit 7b487d8 into main Feb 11, 2025
21 checks passed
@ntBre ntBre deleted the brent/docstring-nested-attributes branch February 11, 2025 17:05
dcreager added a commit that referenced this pull request Feb 11, 2025
* main:
  add diagnostic `Span` (couples `File` and `TextRange`) (#16101)
  Remove `Hash` and `Eq` from `AstNodeRef` for types not implementing `Eq` or `Hash` (#16100)
  Fix release build warning about unused todo type message (#16102)
  [`pydocstyle`] Handle arguments with the same names as sections (`D417`) (#16011)
  [red-knot] Reduce usage of `From<Type>` implementations when working with `Symbol`s (#16076)
  Transition to salsa coarse-grained tracked structs (#15763)
  [`pyupgrade`] Handle micro version numbers correctly (`UP036`) (#16091)
  [red-knot] `T | object == object` (#16088)
  [`ruff`] Skip singleton starred expressions for `incorrectly-parenthesized-tuple-in-subscript` (`RUF031`) (#16083)
  Delete left-over `verbosity.rs (#16081)
  [red-knot] User-level configuration (#16021)
  Add `user_configuration_directory` to `System` (#16020)
@dhruvmanila dhruvmanila added bug Something isn't working and removed rule Implementing or modifying a lint rule labels Feb 24, 2025
dhruvmanila added a commit that referenced this pull request Feb 24, 2025
## Summary

This is mainly on me for not noticing this during the last release but I
noticed in the last changelog that there's only 1 bug fix which didn't
seem correct as I saw multiple of them so I looked at a couple of PRs
that are in "Rule changes" section and the PRs that were marked with the
`bug` label was categorized there because

1. It _also_ had other labels like `rule` and `fixes`
(#16080,
#16110,
#16219, etc.)
2. Some PRs didn't have the `bug` label (but the issue as marked as
`bug`) but _only_ labels like "fixes"
(#16011,
#16132, etc.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working docstring Related to docstring linting or formatting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[D214, D405, D417] False positive with arg keyword attributes

4 participants