[pydoclint] Support NumPy-style comma-separated parameters (DOC102)
#20972
Conversation
|
crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Outdated
Show resolved
Hide resolved
ntBre
added
rule
Refactored the calculation of parameter start and end positions in the Numpy docstring parser to use more direct conversions and remove unnecessary error handling. This improves code clarity and safety by relying on guaranteed substring presence.
Resolved merge conflict in DOC102_numpy.py by combining both test cases: - Kept comma-separated parameter test cases from issue astral-sh#20959 - Added DOC102 false positive test case with Warnings section from upstream
…ameter_DOC102_numpy.py.snap
|
Can you add a test case for a type less docstring with comma, e.g.: def add_numbers(a, b):
"""
Adds two numbers and returns the result.
Parameters
----------
a, b
The numbers to add.
Returns
-------
int
The sum of the two numbers.
"""
return a + b |
Added a new test case to DOC102_numpy.py to verify handling of comma-separated parameters without type annotations in numpy-style docstrings.
| content_start + param_end, | ||
| ), | ||
| }); | ||
| let param_line = before_colon.trim_end(); |
There was a problem hiding this comment.
I suspect that the trim_end call here now is unnecessary, given that you also trim each parameter part.
There was a problem hiding this comment.
Removing it causes incorrect column positions in the diagnostic ranges, from my testing.
Even though we trim() each parameter part, the offset calculation happens before trimming, so the trailing whitespace in before_colon affects the positions.
crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Outdated
Show resolved
Hide resolved
- Track offset incrementally instead of using `find()` - Use `TextRange::at()` instead of `TextRange::new()` - Use `','.text_len()` for comma length instead of hardcoded value
| let param_start_in_line = current_offset | ||
| + TextSize::from(u32::from( | ||
| param_part.text_len() - param_part_trimmed.text_len(), | ||
| )); |
There was a problem hiding this comment.
This seems unnecessarily complicated to go from TextSize -> u32 -> TextSize. Can you say more why you added that?
There was a problem hiding this comment.
Oversight on my part - Since both param_part.text_len() and param_part_trimmed.text_len() return TextSize, we can subtract them directly without the conversion.
4 participants
Summary
Fixes #20959. Enhanced the DOC102 rule to recognize and validate multiple parameters listed together in NumPy-style docstrings using comma separation.
Problem Analysis
The existing DOC102 rule failed to detect extraneous parameters when NumPy-style docstrings combined multiple parameters with commas (e.g.,
x1, x2 : object), causing false negatives.Approach
Updated the
parse_parameters_numpyfunction to split comma-separated parameter names and validate each individually, added comprehensive test cases covering the original issue and edge cases, and ensured code compliance with project standards.