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] Handle arguments with the same names as sections (D417) #16011

Merged
merged 11 commits into from
Feb 11, 2025

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.

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
# 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docstring Related to docstring linting or formatting rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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