Skip to content

require Return section only if return is not None nor commentary #25008

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

Merged
merged 45 commits into from
Mar 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
575abc4
Return section only required if at least one return is not None nor c…
Jan 29, 2019
4b6406f
fixed PEP8 issues
Jan 29, 2019
b204f11
added another "return" on test
Jan 29, 2019
0a76a3a
now respects summary in a single line
Jan 29, 2019
d35ec84
only look for return commands after docstring
Jan 29, 2019
e7d9bed
Added comments to regex that finds non-bare return commands
Jan 30, 2019
2cdd367
added space around bitwise or operator
Jan 30, 2019
0e25d93
Replaced [ \t\f\v] for \s on regex.
Jan 31, 2019
d5d9800
Added test with valued return after bare returns
Feb 1, 2019
073a6b6
method_source now returns source without docstring
Feb 2, 2019
0143b7c
Updated to search string without whitespaces.
Feb 2, 2019
76188dd
Simplified regex.
Feb 2, 2019
2051ea6
Created clean_method_source property without comments or docstring.
Feb 2, 2019
ff17a79
Updated clean_method_source to remove comments.
Feb 2, 2019
bafc25d
Updated clean_method_source, remove duplicate and trailing whitespaces.
Feb 2, 2019
172115a
Included test coverage for variable with "return" on the name.
Feb 2, 2019
d5e0de8
Updated regex with word bondaries.
Feb 2, 2019
0e61d2c
Shortened line.
Feb 2, 2019
4c89beb
Merged return_not_documented tests into simplified version.
Feb 3, 2019
b04110a
Removed random elements from tests.
dluiscosta Feb 5, 2019
4b01092
Switched to AST approach.
dluiscosta Feb 5, 2019
0928ba6
Removed clean_method_source property.
dluiscosta Feb 5, 2019
57fdb1e
Updated does_nothing test not to leave variable unused.
dluiscosta Feb 5, 2019
00d5fdb
Reestructured and minimized tests.
dluiscosta Feb 7, 2019
0a0c05e
Encapsulated search for returns on method_returns property.
dluiscosta Feb 7, 2019
6ab2534
Updated method_returns property to ignore nested functions.
dluiscosta Feb 7, 2019
504ea10
Updated ast.parse call.
dluiscosta Feb 7, 2019
17737d1
Fixed linting errors.
dluiscosta Feb 7, 2019
fe23351
Updated method_source to remove common indentation from lines.
dluiscosta Feb 7, 2019
783eb39
Corrected method_source.
dluiscosta Feb 7, 2019
2c41bd7
Reverted ast.parse call.
dluiscosta Feb 7, 2019
6e53215
Updated method_returns to respect empty method_sources.
dluiscosta Feb 8, 2019
2c60e4d
Fixed linting error.
dluiscosta Feb 8, 2019
11b827b
Merge branch 'master' into check-return-necessity
dluiscosta Feb 9, 2019
3e93cbe
Updated method_source property, removing from try block what can't fail.
dluiscosta Feb 13, 2019
1660060
Refactored method_returns to method_returns_something.
dluiscosta Feb 13, 2019
be6c906
Added docstring to method_returns_something.
dluiscosta Feb 13, 2019
0b361de
Improved method_returns_something readability.
dluiscosta Feb 13, 2019
2ebfa33
Corrected method_returns_something property docstring.
dluiscosta Feb 13, 2019
65b0f56
Removed unecessary print from test no_returns.
datapythonista Feb 13, 2019
bbf39d0
Simplified return_not_documented test.
dluiscosta Feb 13, 2019
2c66c89
Removed mistakenly added whitespaces.
dluiscosta Feb 13, 2019
d99edb3
Merge branch 'master' into check-return-necessity
dluiscosta Feb 13, 2019
07a4b01
Added dedent.
dluiscosta Feb 14, 2019
4d6ec7e
Merge branch 'master' into check-return-necessity
dluiscosta Feb 19, 2019
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
23 changes: 22 additions & 1 deletion scripts/tests/test_validate_docstrings.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,27 @@ def good_imports(self):
"""
pass

def no_returns(self):
"""
Say hello and have no returns.
"""
pass

def empty_returns(self):
"""
Say hello and always return None.

Since this function never returns a value, this
docstring doesn't need a return section.
"""
def say_hello():
return "Hello World!"
say_hello()
if True:
return
else:
return None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a test for a property that returns something (without the Returns section, which is the correct).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

such a test was already present before this PR:

    def return_not_documented(self):
        """
        Lacks section for Returns
        """
        return "Hello world!"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is negative test, not like the one being commented here, so it's inside the BadReturns class



class BadGenericDocStrings(object):
"""Everything here has a bad docstring
Expand Down Expand Up @@ -783,7 +804,7 @@ def test_good_class(self, capsys):

@pytest.mark.parametrize("func", [
'plot', 'sample', 'random_letters', 'sample_values', 'head', 'head1',
'contains', 'mode', 'good_imports'])
'contains', 'mode', 'good_imports', 'no_returns', 'empty_returns'])
def test_good_functions(self, capsys, func):
errors = validate_one(self._import_path(
klass='GoodDocStrings', func=func))['errors']
Expand Down
42 changes: 40 additions & 2 deletions scripts/validate_docstrings.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import importlib
import doctest
import tempfile
import ast
import textwrap

import flake8.main.application

Expand Down Expand Up @@ -490,9 +492,45 @@ def yields(self):
@property
def method_source(self):
try:
return inspect.getsource(self.obj)
source = inspect.getsource(self.obj)
except TypeError:
return ''
return textwrap.dedent(source)

@property
def method_returns_something(self):
'''
Check if the docstrings method can return something.

Bare returns, returns valued None and returns from nested functions are
disconsidered.

Returns
-------
bool
Whether the docstrings method can return something.
'''

def get_returns_not_on_nested_functions(node):
returns = [node] if isinstance(node, ast.Return) else []
for child in ast.iter_child_nodes(node):
# Ignore nested functions and its subtrees.
if not isinstance(child, ast.FunctionDef):
child_returns = get_returns_not_on_nested_functions(child)
returns.extend(child_returns)
return returns

tree = ast.parse(self.method_source).body
if tree:
returns = get_returns_not_on_nested_functions(tree[0])
return_values = [r.value for r in returns]
# Replace NameConstant nodes valued None for None.
for i, v in enumerate(return_values):
if isinstance(v, ast.NameConstant) and v.value is None:
return_values[i] = None
return any(return_values)
else:
return False

@property
def first_line_ends_in_dot(self):
Expand Down Expand Up @@ -691,7 +729,7 @@ def get_validation_data(doc):

if doc.is_function_or_method:
if not doc.returns:
if 'return' in doc.method_source:
if doc.method_returns_something:
errs.append(error('RT01'))
else:
if len(doc.returns) == 1 and doc.returns[0][1]:
Expand Down