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

[DOC201] Permit explicit None in functions that only return None #13064

Merged
merged 5 commits into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,91 @@ class A(metaclass=abc.abcmeta):
def f(self):
"""Lorem ipsum."""
return True


# OK - implicit None early return
def foo(obj: object) -> None:
"""A very helpful docstring.

Args:
obj (object): An object.
"""
if obj is None:
return
print(obj)


# OK - explicit None early return
def foo(obj: object) -> None:
"""A very helpful docstring.

Args:
obj (object): An object.
"""
if obj is None:
return None
print(obj)


# OK - explicit None early return w/o useful type annotations
def foo(obj):
"""A very helpful docstring.

Args:
obj (object): An object.
"""
if obj is None:
return None
print(obj)


# OK - multiple explicit None early returns
def foo(obj: object) -> None:
"""A very helpful docstring.

Args:
obj (object): An object.
"""
if obj is None:
return None
if obj == "None":
return
if obj == 0:
return None
print(obj)


# DOC201 - non-early return explicit None
def foo(x: int) -> int | None:
"""A very helpful docstring.

Args:
x (int): An interger.
"""
if x < 0:
return None
else:
return x


# DOC201 - non-early return explicit None w/o useful type annotations
def foo(x):
"""A very helpful docstring.

Args:
x (int): An interger.
"""
if x < 0:
return None
else:
return x


# DOC201 - only returns None, but return annotation is not None
def foo(s: str) -> str | None:
"""A very helpful docstring.

Args:
s (str): A string.
"""
return None
102 changes: 102 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pydoclint/DOC201_numpy.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,105 @@ class A(metaclass=abc.abcmeta):
def f(self):
"""Lorem ipsum."""
return True


# OK - implicit None early return
def foo(obj: object) -> None:
"""A very helpful docstring.

Parameters
----------
obj : object
An object.
"""
if obj is None:
return
print(obj)


# OK - explicit None early return
def foo(obj: object) -> None:
"""A very helpful docstring.

Parameters
----------
obj : object
An object.
"""
if obj is None:
return None
print(obj)


# OK - explicit None early return w/o useful type annotations
def foo(obj):
"""A very helpful docstring.

Parameters
----------
obj : object
An object.
"""
if obj is None:
return None
print(obj)


# OK - multiple explicit None early returns
def foo(obj: object) -> None:
"""A very helpful docstring.

Parameters
----------
obj : object
An object.
"""
if obj is None:
return None
if obj == "None":
return
if obj == 0:
return None
print(obj)


# DOC201 - non-early return explicit None
def foo(x: int) -> int | None:
"""A very helpful docstring.

Parameters
----------
x : int
An interger.
"""
if x < 0:
return None
else:
return x


# DOC201 - non-early return explicit None w/o useful type annotations
def foo(x):
"""A very helpful docstring.

Parameters
----------
x : int
An interger.
"""
if x < 0:
return None
else:
return x


# DOC201 - only returns None, but return annotation is not None
def foo(s: str) -> str | None:
"""A very helpful docstring.

Parameters
----------
x : str
A string.
"""
return None
59 changes: 45 additions & 14 deletions crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ use crate::rules::pydocstyle::settings::Convention;
/// Docstrings missing return sections are a sign of incomplete documentation
/// or refactors.
///
/// This rule is not enforced for abstract methods and stubs functions.
/// This rule is not enforced for abstract methods, stubs functions, or
/// functions that only return `None`.
///
/// ## Example
/// ```python
Expand Down Expand Up @@ -494,13 +495,26 @@ fn parse_entries_numpy(content: &str) -> Vec<QualifiedName> {
entries
}

/// An individual documentable statement in a function body.
/// An individual `yield` expression in a function body.
#[derive(Debug)]
struct YieldEntry {
range: TextRange,
}

impl Ranged for YieldEntry {
fn range(&self) -> TextRange {
self.range
}
}

/// An individual `return` statement in a function body.
#[derive(Debug)]
struct Entry {
struct ReturnEntry {
range: TextRange,
is_none_return: bool,
}

impl Ranged for Entry {
impl Ranged for ReturnEntry {
fn range(&self) -> TextRange {
self.range
}
Expand All @@ -522,15 +536,15 @@ impl Ranged for ExceptionEntry<'_> {
/// A summary of documentable statements from the function body
#[derive(Debug)]
struct BodyEntries<'a> {
returns: Vec<Entry>,
yields: Vec<Entry>,
returns: Vec<ReturnEntry>,
yields: Vec<YieldEntry>,
raised_exceptions: Vec<ExceptionEntry<'a>>,
}

/// An AST visitor to extract a summary of documentable statements from a function body.
struct BodyVisitor<'a> {
returns: Vec<Entry>,
yields: Vec<Entry>,
returns: Vec<ReturnEntry>,
yields: Vec<YieldEntry>,
currently_suspended_exceptions: Option<&'a ast::Expr>,
raised_exceptions: Vec<ExceptionEntry<'a>>,
semantic: &'a SemanticModel<'a>,
Expand Down Expand Up @@ -623,9 +637,12 @@ impl<'a> Visitor<'a> for BodyVisitor<'a> {
}
Stmt::Return(ast::StmtReturn {
range,
value: Some(_),
value: Some(value),
}) => {
self.returns.push(Entry { range: *range });
self.returns.push(ReturnEntry {
range: *range,
is_none_return: value.is_none_literal_expr(),
});
}
Stmt::FunctionDef(_) | Stmt::ClassDef(_) => return,
_ => {}
Expand All @@ -640,10 +657,10 @@ impl<'a> Visitor<'a> for BodyVisitor<'a> {
range,
value: Some(_),
}) => {
self.yields.push(Entry { range: *range });
self.yields.push(YieldEntry { range: *range });
}
Expr::YieldFrom(ast::ExprYieldFrom { range, .. }) => {
self.yields.push(Entry { range: *range });
self.yields.push(YieldEntry { range: *range });
}
Expr::Lambda(_) => return,
_ => {}
Expand Down Expand Up @@ -737,8 +754,22 @@ pub(crate) fn check_docstring(
let extra_property_decorators = checker.settings.pydocstyle.property_decorators();
if !definition.is_property(extra_property_decorators, checker.semantic()) {
if let Some(body_return) = body_entries.returns.first() {
let diagnostic = Diagnostic::new(DocstringMissingReturns, body_return.range());
diagnostics.push(diagnostic);
match function_def.returns.as_deref() {
Some(returns) if !Expr::is_none_literal_expr(returns) => diagnostics.push(
Diagnostic::new(DocstringMissingReturns, body_return.range()),
),
None if body_entries
.returns
.iter()
.any(|entry| !entry.is_none_return) =>
{
diagnostics.push(Diagnostic::new(
DocstringMissingReturns,
body_return.range(),
));
}
_ => {}
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,34 @@ DOC201_google.py:121:9: DOC201 `return` is not documented in docstring
| ^^^^^^^^^^^ DOC201
|
= help: Add a "Returns" section to the docstring

DOC201_google.py:184:9: DOC201 `return` is not documented in docstring
|
182 | """
183 | if x < 0:
184 | return None
| ^^^^^^^^^^^ DOC201
185 | else:
186 | return x
|
= help: Add a "Returns" section to the docstring

DOC201_google.py:197:9: DOC201 `return` is not documented in docstring
|
195 | """
196 | if x < 0:
197 | return None
| ^^^^^^^^^^^ DOC201
198 | else:
199 | return x
|
= help: Add a "Returns" section to the docstring

DOC201_google.py:209:5: DOC201 `return` is not documented in docstring
|
207 | s (str): A string.
208 | """
209 | return None
| ^^^^^^^^^^^ DOC201
|
= help: Add a "Returns" section to the docstring
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,34 @@ DOC201_numpy.py:87:9: DOC201 `return` is not documented in docstring
| ^^^^^^^^^^^ DOC201
|
= help: Add a "Returns" section to the docstring

DOC201_numpy.py:160:9: DOC201 `return` is not documented in docstring
|
158 | """
159 | if x < 0:
160 | return None
| ^^^^^^^^^^^ DOC201
161 | else:
162 | return x
|
= help: Add a "Returns" section to the docstring

DOC201_numpy.py:175:9: DOC201 `return` is not documented in docstring
|
173 | """
174 | if x < 0:
175 | return None
| ^^^^^^^^^^^ DOC201
176 | else:
177 | return x
|
= help: Add a "Returns" section to the docstring

DOC201_numpy.py:189:5: DOC201 `return` is not documented in docstring
|
187 | A string.
188 | """
189 | return None
| ^^^^^^^^^^^ DOC201
|
= help: Add a "Returns" section to the docstring
Loading