Skip to content

Commit

Permalink
Include function name in undocumented-param message (#5818)
Browse files Browse the repository at this point in the history 
Closes #5814.
  • Loading branch information
@charliermarsh
charliermarsh authored Jul 17, 2023
1 parent 94998ae commit be6c744
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 47 deletions.
26 changes: 17 additions & 9 deletions crates/ruff/src/rules/pydocstyle/rules/sections.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1187,19 +1187,22 @@ impl AlwaysAutofixableViolation for SectionNameEndsInColon {
/// - [Google Python Style Guide - Docstrings](https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings)
#[violation]
pub struct UndocumentedParam {
pub names: Vec<String>,
/// The name of the function being documented.
definition: String,
/// The names of the undocumented parameters.
names: Vec<String>,
}

impl Violation for UndocumentedParam {
#[derive_message_formats]
fn message(&self) -> String {
let UndocumentedParam { names } = self;
let UndocumentedParam { definition, names } = self;
if names.len() == 1 {
let name = &names[0];
format!("Missing argument description in the docstring: `{name}`")
format!("Missing argument description in the docstring for `{definition}`: `{name}`")
} else {
let names = names.iter().map(|name| format!("`{name}`")).join(", ");
format!("Missing argument descriptions in the docstring: {names}")
format!("Missing argument descriptions in the docstring for `{definition}`: {names}")
}
}
}
Expand Down Expand Up @@ -1779,11 +1782,16 @@ fn missing_args(checker: &mut Checker, docstring: &Docstring, docstrings_args: &
}

if !missing_arg_names.is_empty() {
let names = missing_arg_names.into_iter().sorted().collect();
checker.diagnostics.push(Diagnostic::new(
UndocumentedParam { names },
stmt.identifier(),
));
if let Some(definition) = docstring.definition.name() {
let names = missing_arg_names.into_iter().sorted().collect();
checker.diagnostics.push(Diagnostic::new(
UndocumentedParam {
definition: definition.to_string(),
names,
},
stmt.identifier(),
));
}
}
}

Expand Down
22 changes: 11 additions & 11 deletions ...ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D417_sections.py.snap
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
source: crates/ruff/src/rules/pydocstyle/mod.rs
---
sections.py:292:9: D417 Missing argument description in the docstring: `y`
sections.py:292:9: D417 Missing argument description in the docstring for `bar`: `y`
|
290 | x = 1
291 |
Expand All @@ -10,7 +10,7 @@ sections.py:292:9: D417 Missing argument description in the docstring: `y`
293 | """Nested function test for docstrings.
|

sections.py:309:5: D417 Missing argument description in the docstring: `y`
sections.py:309:5: D417 Missing argument description in the docstring for `test_missing_google_args`: `y`
|
307 | "(argument(s) y are missing descriptions in "
308 | "'test_missing_google_args' docstring)")
Expand All @@ -19,7 +19,7 @@ sections.py:309:5: D417 Missing argument description in the docstring: `y`
310 | """Toggle the gizmo.
|

sections.py:333:9: D417 Missing argument descriptions in the docstring: `test`, `y`, `z`
sections.py:333:9: D417 Missing argument descriptions in the docstring for `test_missing_args`: `test`, `y`, `z`
|
331 | "(argument(s) test, y, z are missing descriptions in "
332 | "'test_missing_args' docstring)", arg_count=5)
Expand All @@ -28,7 +28,7 @@ sections.py:333:9: D417 Missing argument descriptions in the docstring: `test`,
334 | """Test a valid args section.
|

sections.py:345:9: D417 Missing argument descriptions in the docstring: `test`, `y`, `z`
sections.py:345:9: D417 Missing argument descriptions in the docstring for `test_missing_args_class_method`: `test`, `y`, `z`
|
343 | "(argument(s) test, y, z are missing descriptions in "
344 | "'test_missing_args_class_method' docstring)", arg_count=5)
Expand All @@ -37,7 +37,7 @@ sections.py:345:9: D417 Missing argument descriptions in the docstring: `test`,
346 | """Test a valid args section.
|

sections.py:358:9: D417 Missing argument descriptions in the docstring: `a`, `y`, `z`
sections.py:358:9: D417 Missing argument descriptions in the docstring for `test_missing_args_static_method`: `a`, `y`, `z`
|
356 | "(argument(s) a, y, z are missing descriptions in "
357 | "'test_missing_args_static_method' docstring)", arg_count=4)
Expand All @@ -46,7 +46,7 @@ sections.py:358:9: D417 Missing argument descriptions in the docstring: `a`, `y`
359 | """Test a valid args section.
|

sections.py:370:9: D417 Missing argument descriptions in the docstring: `a`, `b`
sections.py:370:9: D417 Missing argument descriptions in the docstring for `test_missing_docstring`: `a`, `b`
|
368 | "(argument(s) a, b are missing descriptions in "
369 | "'test_missing_docstring' docstring)", arg_count=2)
Expand All @@ -55,7 +55,7 @@ sections.py:370:9: D417 Missing argument descriptions in the docstring: `a`, `b`
371 | """Test a valid args section.
|

sections.py:398:5: D417 Missing argument description in the docstring: `y`
sections.py:398:5: D417 Missing argument description in the docstring for `test_missing_numpy_args`: `y`
|
396 | "(argument(s) y are missing descriptions in "
397 | "'test_missing_numpy_args' docstring)")
Expand All @@ -64,7 +64,7 @@ sections.py:398:5: D417 Missing argument description in the docstring: `y`
399 | """Toggle the gizmo.
|

sections.py:434:9: D417 Missing argument descriptions in the docstring: `test`, `y`, `z`
sections.py:434:9: D417 Missing argument descriptions in the docstring for `test_missing_args`: `test`, `y`, `z`
|
432 | "(argument(s) test, y, z are missing descriptions in "
433 | "'test_missing_args' docstring)", arg_count=5)
Expand All @@ -73,7 +73,7 @@ sections.py:434:9: D417 Missing argument descriptions in the docstring: `test`,
435 | """Test a valid args section.
|

sections.py:449:9: D417 Missing argument descriptions in the docstring: `test`, `y`, `z`
sections.py:449:9: D417 Missing argument descriptions in the docstring for `test_missing_args_class_method`: `test`, `y`, `z`
|
447 | "(argument(s) test, y, z are missing descriptions in "
448 | "'test_missing_args_class_method' docstring)", arg_count=4)
Expand All @@ -82,7 +82,7 @@ sections.py:449:9: D417 Missing argument descriptions in the docstring: `test`,
450 | """Test a valid args section.
|

sections.py:468:9: D417 Missing argument descriptions in the docstring: `a`, `z`
sections.py:468:9: D417 Missing argument descriptions in the docstring for `test_missing_args_static_method`: `a`, `z`
|
466 | "(argument(s) a, z are missing descriptions in "
467 | "'test_missing_args_static_method' docstring)", arg_count=3)
Expand All @@ -91,7 +91,7 @@ sections.py:468:9: D417 Missing argument descriptions in the docstring: `a`, `z`
469 | """Test a valid args section.
|

sections.py:498:9: D417 Missing argument description in the docstring: `y`
sections.py:498:9: D417 Missing argument description in the docstring for `test_incorrect_indent`: `y`
|
496 | "(argument(s) y are missing descriptions in "
497 | "'test_incorrect_indent' docstring)", arg_count=3)
Expand Down
18 changes: 9 additions & 9 deletions crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__d417_google.snap
View file
Original file line number Diff line number Diff line change
@@ -1,63 +1,63 @@
---
source: crates/ruff/src/rules/pydocstyle/mod.rs
---
D417.py:1:5: D417 Missing argument descriptions in the docstring: `y`, `z`
D417.py:1:5: D417 Missing argument descriptions in the docstring for `f`: `y`, `z`
|
1 | def f(x, y, z):
| ^ D417
2 | """Do something.
|

D417.py:14:5: D417 Missing argument descriptions in the docstring: `y`, `z`
D417.py:14:5: D417 Missing argument descriptions in the docstring for `f`: `y`, `z`
|
14 | def f(x, y, z):
| ^ D417
15 | """Do something.
|

D417.py:27:5: D417 Missing argument descriptions in the docstring: `y`, `z`
D417.py:27:5: D417 Missing argument descriptions in the docstring for `f`: `y`, `z`
|
27 | def f(x, y, z):
| ^ D417
28 | """Do something.
|

D417.py:39:5: D417 Missing argument descriptions in the docstring: `y`, `z`
D417.py:39:5: D417 Missing argument descriptions in the docstring for `f`: `y`, `z`
|
39 | def f(x, y, z):
| ^ D417
40 | """Do something.
|

D417.py:52:5: D417 Missing argument description in the docstring: `y`
D417.py:52:5: D417 Missing argument description in the docstring for `f`: `y`
|
52 | def f(x, y, z):
| ^ D417
53 | """Do something.
|

D417.py:65:5: D417 Missing argument description in the docstring: `y`
D417.py:65:5: D417 Missing argument description in the docstring for `f`: `y`
|
65 | def f(x, y, z):
| ^ D417
66 | """Do something.
|

D417.py:77:5: D417 Missing argument description in the docstring: `y`
D417.py:77:5: D417 Missing argument description in the docstring for `f`: `y`
|
77 | def f(x, y, z):
| ^ D417
78 | """Do something.
|

D417.py:98:5: D417 Missing argument description in the docstring: `x`
D417.py:98:5: D417 Missing argument description in the docstring for `f`: `x`
|
98 | def f(x, *args, **kwargs):
| ^ D417
99 | """Do something.
|

D417.py:108:5: D417 Missing argument description in the docstring: `*args`
D417.py:108:5: D417 Missing argument description in the docstring for `f`: `*args`
|
108 | def f(x, *args, **kwargs):
| ^ D417
Expand Down
18 changes: 9 additions & 9 deletions ...ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__d417_unspecified.snap
View file
Original file line number Diff line number Diff line change
@@ -1,63 +1,63 @@
---
source: crates/ruff/src/rules/pydocstyle/mod.rs
---
D417.py:1:5: D417 Missing argument descriptions in the docstring: `y`, `z`
D417.py:1:5: D417 Missing argument descriptions in the docstring for `f`: `y`, `z`
|
1 | def f(x, y, z):
| ^ D417
2 | """Do something.
|

D417.py:14:5: D417 Missing argument descriptions in the docstring: `y`, `z`
D417.py:14:5: D417 Missing argument descriptions in the docstring for `f`: `y`, `z`
|
14 | def f(x, y, z):
| ^ D417
15 | """Do something.
|

D417.py:27:5: D417 Missing argument descriptions in the docstring: `y`, `z`
D417.py:27:5: D417 Missing argument descriptions in the docstring for `f`: `y`, `z`
|
27 | def f(x, y, z):
| ^ D417
28 | """Do something.
|

D417.py:39:5: D417 Missing argument descriptions in the docstring: `y`, `z`
D417.py:39:5: D417 Missing argument descriptions in the docstring for `f`: `y`, `z`
|
39 | def f(x, y, z):
| ^ D417
40 | """Do something.
|

D417.py:52:5: D417 Missing argument description in the docstring: `y`
D417.py:52:5: D417 Missing argument description in the docstring for `f`: `y`
|
52 | def f(x, y, z):
| ^ D417
53 | """Do something.
|

D417.py:65:5: D417 Missing argument description in the docstring: `y`
D417.py:65:5: D417 Missing argument description in the docstring for `f`: `y`
|
65 | def f(x, y, z):
| ^ D417
66 | """Do something.
|

D417.py:77:5: D417 Missing argument description in the docstring: `y`
D417.py:77:5: D417 Missing argument description in the docstring for `f`: `y`
|
77 | def f(x, y, z):
| ^ D417
78 | """Do something.
|

D417.py:98:5: D417 Missing argument description in the docstring: `x`
D417.py:98:5: D417 Missing argument description in the docstring for `f`: `x`
|
98 | def f(x, *args, **kwargs):
| ^ D417
99 | """Do something.
|

D417.py:108:5: D417 Missing argument description in the docstring: `*args`
D417.py:108:5: D417 Missing argument description in the docstring for `f`: `*args`
|
108 | def f(x, *args, **kwargs):
| ^ D417
Expand Down
36 changes: 27 additions & 9 deletions crates/ruff_python_semantic/src/definition.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ impl<'a> Module<'a> {
None
}
}

/// Return the name of the module.
pub fn name(&self) -> Option<&'a str> {
match self.source {
ModuleSource::Path(path) => path.last().map(Deref::deref),
ModuleSource::File(file) => file.file_stem().and_then(std::ffi::OsStr::to_str),
}
}
}

#[derive(Debug, Copy, Clone)]
Expand All @@ -72,12 +80,13 @@ pub struct Member<'a> {
}

impl<'a> Member<'a> {
fn name(&self) -> &'a str {
/// Return the name of the member.
pub fn name(&self) -> Option<&'a str> {
match &self.stmt {
Stmt::FunctionDef(ast::StmtFunctionDef { name, .. }) => name,
Stmt::AsyncFunctionDef(ast::StmtAsyncFunctionDef { name, .. }) => name,
Stmt::ClassDef(ast::StmtClassDef { name, .. }) => name,
_ => unreachable!("Unexpected member kind: {:?}", self.kind),
Stmt::FunctionDef(ast::StmtFunctionDef { name, .. })
| Stmt::AsyncFunctionDef(ast::StmtAsyncFunctionDef { name, .. })
| Stmt::ClassDef(ast::StmtClassDef { name, .. }) => Some(name),
_ => None,
}
}
}
Expand All @@ -100,6 +109,13 @@ impl Definition<'_> {
})
)
}

pub fn name(&self) -> Option<&str> {
match self {
Definition::Module(module) => module.name(),
Definition::Member(member) => member.name(),
}
}
}

/// The definitions within a Python program indexed by [`DefinitionId`].
Expand Down Expand Up @@ -134,8 +150,9 @@ impl<'a> Definitions<'a> {
MemberKind::Class => {
let parent = &definitions[member.parent];
if parent.visibility.is_private()
|| exports
.map_or(false, |exports| !exports.contains(&member.name()))
|| exports.map_or(false, |exports| {
member.name().map_or(false, |name| !exports.contains(&name))
})
{
Visibility::Private
} else {
Expand Down Expand Up @@ -163,8 +180,9 @@ impl<'a> Definitions<'a> {
MemberKind::Function => {
let parent = &definitions[member.parent];
if parent.visibility.is_private()
|| exports
.map_or(false, |exports| !exports.contains(&member.name()))
|| exports.map_or(false, |exports| {
member.name().map_or(false, |name| !exports.contains(&name))
})
{
Visibility::Private
} else {
Expand Down

0 comments on commit be6c744

Please sign in to comment.