From 45fb3732a4378dd3d8120d3b5c13c7694bc02ea2 Mon Sep 17 00:00:00 2001 From: Phong Do <45266517+phongddo@users.noreply.github.com> Date: Mon, 8 Dec 2025 20:00:05 +0100 Subject: [PATCH] [`pydocstyle`] Suppress `D417` for parameters with `Unpack` annotations (#21816) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes https://github.com/astral-sh/ruff/issues/8774 This PR fixes `pydocstyle` incorrectly flagging missing argument for arguments with `Unpack` type annotation by extracting the `kwarg` `D417` suppression logic into a helper function for future rules as needed. ## Problem Statement The below example was incorrectly triggering `D417` error for missing `**kwargs` doc. ```python class User(TypedDict): id: int name: str def do_something(some_arg: str, **kwargs: Unpack[User]): """Some doc Args: some_arg: Some argument """ ``` image `**kwargs: Unpack[User]` indicates the function expects keyword arguments that will be unpacked. Ideally, the individual fields of the User `TypedDict` should be documented, not in the `**kwargs` itself. The `**kwargs` parameter acts as a semantic grouping rather than a parameter requiring documentation. ## Solution As discussed in the linked issue, it makes sense to suppress the `D417` for parameters with `Unpack` annotation. I extract a helper function to solely check `D417` should be suppressed with `**kwarg: Unpack[T]` parameter, this function can also be unit tested independently and reduce complexity of current `missing_args` check function. This also makes it easier to add additional rules in the future. _✏️ Note:_ This is my first PR in this repo, as I've learned a ton from it, please call out anything that could be improved. Thanks for making this excellent tool 👏 ## Test Plan Add 2 test cases in `D417.py` and update snapshots. --------- Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com> --- .../test/fixtures/pydocstyle/D417.py | 23 +++++++++++++++++++ .../src/rules/pydocstyle/rules/sections.rs | 19 ++++++++++++++- ...rules__pydocstyle__tests__d417_google.snap | 10 ++++++++ ...ts__d417_google_ignore_var_parameters.snap | 10 ++++++++ ...__pydocstyle__tests__d417_unspecified.snap | 10 ++++++++ ...417_unspecified_ignore_var_parameters.snap | 10 ++++++++ 6 files changed, 81 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pydocstyle/D417.py b/crates/ruff_linter/resources/test/fixtures/pydocstyle/D417.py index 7c5a1f538c..510bf77bf8 100644 --- a/crates/ruff_linter/resources/test/fixtures/pydocstyle/D417.py +++ b/crates/ruff_linter/resources/test/fixtures/pydocstyle/D417.py @@ -218,3 +218,26 @@ def should_not_fail(payload, Args): Args: The other arguments. """ + + +# Test cases for Unpack[TypedDict] kwargs +from typing import TypedDict +from typing_extensions import Unpack + +class User(TypedDict): + id: int + name: str + +def function_with_unpack_args_should_not_fail(query: str, **kwargs: Unpack[User]): + """Function with Unpack kwargs. + + Args: + query: some arg + """ + +def function_with_unpack_and_missing_arg_doc_should_fail(query: str, **kwargs: Unpack[User]): + """Function with Unpack kwargs but missing query arg documentation. + + Args: + **kwargs: keyword arguments + """ diff --git a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs index 7b9fc80ba8..bf84ea85fc 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs @@ -4,7 +4,9 @@ use rustc_hash::FxHashSet; use std::sync::LazyLock; use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::Parameter; use ruff_python_ast::docstrings::{clean_space, leading_space}; +use ruff_python_ast::helpers::map_subscript; use ruff_python_ast::identifier::Identifier; use ruff_python_semantic::analyze::visibility::is_staticmethod; use ruff_python_trivia::textwrap::dedent; @@ -1184,6 +1186,9 @@ impl AlwaysFixableViolation for MissingSectionNameColon { /// This rule is enabled when using the `google` convention, and disabled when /// using the `pep257` and `numpy` conventions. /// +/// Parameters annotated with `typing.Unpack` are exempt from this rule. +/// This follows the Python typing specification for unpacking keyword arguments. +/// /// ## Example /// ```python /// def calculate_speed(distance: float, time: float) -> float: @@ -1233,6 +1238,7 @@ impl AlwaysFixableViolation for MissingSectionNameColon { /// - [PEP 257 – Docstring Conventions](https://peps.python.org/pep-0257/) /// - [PEP 287 – reStructuredText Docstring Format](https://peps.python.org/pep-0287/) /// - [Google Python Style Guide - Docstrings](https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings) +/// - [Python - Unpack for keyword arguments](https://typing.python.org/en/latest/spec/callables.html#unpack-kwargs) #[derive(ViolationMetadata)] #[violation_metadata(stable_since = "v0.0.73")] pub(crate) struct UndocumentedParam { @@ -1808,7 +1814,9 @@ fn missing_args(checker: &Checker, docstring: &Docstring, docstrings_args: &FxHa missing_arg_names.insert(starred_arg_name); } } - if let Some(arg) = function.parameters.kwarg.as_ref() { + if let Some(arg) = function.parameters.kwarg.as_ref() + && !has_unpack_annotation(checker, arg) + { let arg_name = arg.name.as_str(); let starred_arg_name = format!("**{arg_name}"); if !arg_name.starts_with('_') @@ -1834,6 +1842,15 @@ fn missing_args(checker: &Checker, docstring: &Docstring, docstrings_args: &FxHa } } +/// Returns `true` if the parameter is annotated with `typing.Unpack` +fn has_unpack_annotation(checker: &Checker, parameter: &Parameter) -> bool { + parameter.annotation.as_ref().is_some_and(|annotation| { + checker + .semantic() + .match_typing_expr(map_subscript(annotation), "Unpack") + }) +} + // See: `GOOGLE_ARGS_REGEX` in `pydocstyle/checker.py`. static GOOGLE_ARGS_REGEX: LazyLock = LazyLock::new(|| Regex::new(r"^\s*(\*?\*?\w+)\s*(\(.*?\))?\s*:(\r\n|\n)?\s*.+").unwrap()); diff --git a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_google.snap b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_google.snap index 63b975feb9..44b20a8c6b 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_google.snap +++ b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_google.snap @@ -101,3 +101,13 @@ D417 Missing argument description in the docstring for `should_fail`: `Args` 200 | """ 201 | Send a message. | + +D417 Missing argument description in the docstring for `function_with_unpack_and_missing_arg_doc_should_fail`: `query` + --> D417.py:238:5 + | +236 | """ +237 | +238 | def function_with_unpack_and_missing_arg_doc_should_fail(query: str, **kwargs: Unpack[User]): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +239 | """Function with Unpack kwargs but missing query arg documentation. + | diff --git a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_google_ignore_var_parameters.snap b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_google_ignore_var_parameters.snap index f645ff960e..7ff0f72bf0 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_google_ignore_var_parameters.snap +++ b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_google_ignore_var_parameters.snap @@ -83,3 +83,13 @@ D417 Missing argument description in the docstring for `should_fail`: `Args` 200 | """ 201 | Send a message. | + +D417 Missing argument description in the docstring for `function_with_unpack_and_missing_arg_doc_should_fail`: `query` + --> D417.py:238:5 + | +236 | """ +237 | +238 | def function_with_unpack_and_missing_arg_doc_should_fail(query: str, **kwargs: Unpack[User]): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +239 | """Function with Unpack kwargs but missing query arg documentation. + | diff --git a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_unspecified.snap b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_unspecified.snap index 63b975feb9..44b20a8c6b 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_unspecified.snap +++ b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_unspecified.snap @@ -101,3 +101,13 @@ D417 Missing argument description in the docstring for `should_fail`: `Args` 200 | """ 201 | Send a message. | + +D417 Missing argument description in the docstring for `function_with_unpack_and_missing_arg_doc_should_fail`: `query` + --> D417.py:238:5 + | +236 | """ +237 | +238 | def function_with_unpack_and_missing_arg_doc_should_fail(query: str, **kwargs: Unpack[User]): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +239 | """Function with Unpack kwargs but missing query arg documentation. + | diff --git a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_unspecified_ignore_var_parameters.snap b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_unspecified_ignore_var_parameters.snap index 63b975feb9..44b20a8c6b 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_unspecified_ignore_var_parameters.snap +++ b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_unspecified_ignore_var_parameters.snap @@ -101,3 +101,13 @@ D417 Missing argument description in the docstring for `should_fail`: `Args` 200 | """ 201 | Send a message. | + +D417 Missing argument description in the docstring for `function_with_unpack_and_missing_arg_doc_should_fail`: `query` + --> D417.py:238:5 + | +236 | """ +237 | +238 | def function_with_unpack_and_missing_arg_doc_should_fail(query: str, **kwargs: Unpack[User]): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +239 | """Function with Unpack kwargs but missing query arg documentation. + |