Wemake-python-styleguide: Forbid useless `return` and `return None` at the end of the function

[sobolevn]

Rule request

Thesis

Some functions return values: return value, some have only early returns if ...: return
Some return None as a value, some just can have it as the last statement.

This is wrong:

def test():
     print('test')
     return

def test():
     print('test')
     return None

def test(some):
     if some:
          return
     print('test')
     return

These are correct:

def test():
     print('test')

def test(some):
     if some:
          return
     print('test')

def test(some):
     if some:
          return some
     print('test')
     return None

The rule is related to #378
The rule is:

1. Check if return is the last statement
2. Check if using pure return, then raise a violation
3. If using return None check if that is the only return in the function, if so raise a violation

Reasoning

This is done for consistency. It should also improve the readability and typing.

[NanduTej]

Hey @sobolevn! Can I pick this up?

[sobolevn]

Sure, feel free to ask me any questions. Thanks for your help!

[NanduTej]

Hey @sobolevn! Can you guide me where I could start for this?

[sobolevn]

@NanduTej sure!

There are several things to start with:

1. Have a look at https://github.com/PyCQA/pylint/blob/5a86fddcb815ebd08fbc3ffcbaf7b11c2ba2e107/pylint/checkers/refactoring.py#L1039
2. Understand the principle: we first visit AnyFunction nodes (which are AsyncFunctionDef + FunctionDef)
   2.1 Then we look at the last node in functions body (we can iterate for all nodes in node.body and get the last one by lineno)
   2.2. We make a decision as it is written in the original task

Please, ask any questions you have or send Work-in-Progress Pull Requests so I can guide you.

[sobolevn]

I would say that ConsistentReturningVariableVisitor is the best class to use

[NanduTej]

Should this function be added in visitors/ast/exceptions.py?

[sobolevn]

visitor/ast/keywords.py::ConsistentReturningVariableVisitor

[NanduTej]

Earlier, tests for these cases seem to have been written here. https://github.com/wemake-services/wemake-python-styleguide/blob/49e232d785a12259fcf8ed0e6b69fc625a166b23/tests/test_visitors/test_ast/test_keywords/test_consistency_returning/test_consistency_return.py#L14. Should these tests be removed from there and added to test_consistent_returning_variable.py? I am not yet done with my work. I just understood the flow but found these while looking around.

[sobolevn]

Yes, they should be changed. I wrote them because it was fine before this rule.

[NanduTej]

Hey! I was trying to reproduce some of the tests code in my terminal. I could not figure out where we are getting default_options https://github.com/wemake-services/wemake-python-styleguide/blob/49e232d785a12259fcf8ed0e6b69fc625a166b23/tests/test_visitors/test_ast/test_keywords/test_consistency_returning/test_consistency_return_variables.py#L140 from. Can you help me out?

[sobolevn]

We are using pytest's fixtures to do that.
Docs: https://docs.pytest.org/en/latest/fixture.html

Code: https://github.com/wemake-services/wemake-python-styleguide/blob/master/tests/conftest.py#L77-L98

[sobolevn]

Hey, @NanduTej, thanks for your help! Check out the final solution.