mirror of https://github.com/astral-sh/ruff
[`pydocstyle`] Suppress `D417` for parameters with `Unpack` annotations (#21816)
<!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## 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 """ ``` <img width="1135" height="276" alt="image" src="https://github.com/user-attachments/assets/42fa4bb9-61a5-4a70-a79c-0c8922a3ee66" /> `**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>
This commit is contained in:
parent
0ab8521171
commit
45fb3732a4
|
|
@ -218,3 +218,26 @@ def should_not_fail(payload, Args):
|
||||||
Args:
|
Args:
|
||||||
The other arguments.
|
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
|
||||||
|
"""
|
||||||
|
|
|
||||||
|
|
@ -4,7 +4,9 @@ use rustc_hash::FxHashSet;
|
||||||
use std::sync::LazyLock;
|
use std::sync::LazyLock;
|
||||||
|
|
||||||
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
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::docstrings::{clean_space, leading_space};
|
||||||
|
use ruff_python_ast::helpers::map_subscript;
|
||||||
use ruff_python_ast::identifier::Identifier;
|
use ruff_python_ast::identifier::Identifier;
|
||||||
use ruff_python_semantic::analyze::visibility::is_staticmethod;
|
use ruff_python_semantic::analyze::visibility::is_staticmethod;
|
||||||
use ruff_python_trivia::textwrap::dedent;
|
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
|
/// This rule is enabled when using the `google` convention, and disabled when
|
||||||
/// using the `pep257` and `numpy` conventions.
|
/// 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
|
/// ## Example
|
||||||
/// ```python
|
/// ```python
|
||||||
/// def calculate_speed(distance: float, time: float) -> float:
|
/// 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 257 – Docstring Conventions](https://peps.python.org/pep-0257/)
|
||||||
/// - [PEP 287 – reStructuredText Docstring Format](https://peps.python.org/pep-0287/)
|
/// - [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)
|
/// - [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)]
|
#[derive(ViolationMetadata)]
|
||||||
#[violation_metadata(stable_since = "v0.0.73")]
|
#[violation_metadata(stable_since = "v0.0.73")]
|
||||||
pub(crate) struct UndocumentedParam {
|
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);
|
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 arg_name = arg.name.as_str();
|
||||||
let starred_arg_name = format!("**{arg_name}");
|
let starred_arg_name = format!("**{arg_name}");
|
||||||
if !arg_name.starts_with('_')
|
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`.
|
// See: `GOOGLE_ARGS_REGEX` in `pydocstyle/checker.py`.
|
||||||
static GOOGLE_ARGS_REGEX: LazyLock<Regex> =
|
static GOOGLE_ARGS_REGEX: LazyLock<Regex> =
|
||||||
LazyLock::new(|| Regex::new(r"^\s*(\*?\*?\w+)\s*(\(.*?\))?\s*:(\r\n|\n)?\s*.+").unwrap());
|
LazyLock::new(|| Regex::new(r"^\s*(\*?\*?\w+)\s*(\(.*?\))?\s*:(\r\n|\n)?\s*.+").unwrap());
|
||||||
|
|
|
||||||
|
|
@ -101,3 +101,13 @@ D417 Missing argument description in the docstring for `should_fail`: `Args`
|
||||||
200 | """
|
200 | """
|
||||||
201 | Send a message.
|
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.
|
||||||
|
|
|
||||||
|
|
|
||||||
|
|
@ -83,3 +83,13 @@ D417 Missing argument description in the docstring for `should_fail`: `Args`
|
||||||
200 | """
|
200 | """
|
||||||
201 | Send a message.
|
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.
|
||||||
|
|
|
||||||
|
|
|
||||||
|
|
@ -101,3 +101,13 @@ D417 Missing argument description in the docstring for `should_fail`: `Args`
|
||||||
200 | """
|
200 | """
|
||||||
201 | Send a message.
|
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.
|
||||||
|
|
|
||||||
|
|
|
||||||
|
|
@ -101,3 +101,13 @@ D417 Missing argument description in the docstring for `should_fail`: `Args`
|
||||||
200 | """
|
200 | """
|
||||||
201 | Send a message.
|
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.
|
||||||
|
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue