wemake-python-styleguide: Early return should not take `return None` form when it is not required

Rule request

Thesis

Right now this code:

    def _check_parent(self, node: ast.If) -> None:
        parent = get_parent(node)

        if parent is None:
            return None

        if isinstance(parent, _ELSE_NODES):
            body = parent.body + parent.orelse
        else:
            body = getattr(parent, 'body', [node])

        next_index_in_parent = body.index(node) + 1
        if keywords.next_node_returns_bool(body, next_index_in_parent):
            self.add_violation(SimplifiableReturningIfViolation(node))

passes our style check. Note return None part there. It is the only return. And it means that None part is not required. Moreover, it should not be allowed. Simple early returns must be just return`.

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 21 (20 by maintainers)

Most upvoted comments

We allow:

def some():
   if condition:
       return None
   return other

We don’t allow:

def some():
   if condition:
       return None
   print()

sobolevn on Aug 25, 2021

Are you running pythno3.9?

sobolevn on Aug 31, 2021

Ok, we also need to take a closer look at:

def some_funct():
    if first_cond:
        return None
    if second_cond:
        return None
    print('a')

I would re-formulate the rule as: if all function return nodes are return None -> than we must use plain return.

@DhirajChauhan40 Something like (untested):

is_all_none = (
            issubclass(returning_type, ast.Return) and
            has_values and
            all(
                ret_node.value.value is None
                for ret_node in return_nodes
                if isinstance(ret_node.value, (compat.nodes.Constant, ast.NameConstant))
            )
        )
        if is_all_none:
            self.add_violation(...)

You can put it here: https://github.com/wemake-services/wemake-python-styleguide/blob/d41d4b9d70dcad0ece5e7ed7320560de58999fcf/wemake_python_styleguide/visitors/ast/keywords.py#L123

You would still have to:

Add new violation type in violations/consistency.py
Add tests
Have fun 🙂

sobolevn on Aug 29, 2021