Wemake-python-styleguide: Forbid complex default values

[sobolevn]

Rule request

Thesis

Today I saw this code:

    def __init__(self, tree, filename='(none)', builtins=None,
                 withDoctest='PYFLAKES_DOCTEST' in os.environ, tokens=()):

https://github.com/PyCQA/pyflakes/blob/ac0aeba0eddb0b8e2285f5ac568e4621426b3aa7/pyflakes/checker.py#L635

What it does? It uses an expression as a default value. I am strongly against that.
We should forbid anything that is not a ast.Name or ast.Attribute to be used as a default value.

Reasoning

It can be tricky. Nothing stops you from making database calls or http requests in such expressions.
It is also not readable for me. I would prefer to write:

SHOULD_USE_DOCTEST = 'PYFLAKES_DOCTEST' in os.environ

def __init__(self, tree, filename='(none)', builtins=None,
                 withDoctest=SHOULD_USE_DOCTEST, tokens=()):

[kxepal]

At the same time are you fine with pytest.mark.skipIf and pytest.mark.xfail cases? How they are different from this one?

[mikhail-akimov]

I`ll do it!

[mikhail-akimov]

It will be a challenge for you, because everything is new to me.

I'm a little confused. I realized that I need to add a violation, but I’m not sure that I need to add a new visitor (I continue to think about this pattern).

I think the correct base class for this violation is ASTViolation. Am i right? I think i should add the new violation to "best_practises"? But i can not figure out where to add and how.

Could you direct me?

[sobolevn]

@mikhail-akimov yes, you are absolutly correct.

You need to create:

1. a new ast violation in best_practices.py
2. then, create a new method in this class: https://github.com/wemake-services/wemake-python-styleguide/blob/master/wemake_python_styleguide/visitors/ast/functions.py#L98 call it _check_argument_default_values

[mikhail-akimov]

I found out that in order to prohibit the expression in default, it is enough to forbid ast.Compare, but what about ast.Tuple, ast.NameConstant, ast.Tuple? We have to forbid them too?

[mikhail-akimov]

What then will be the correct example?

[sobolevn]

The rule is: we should allow any immutable simple values and variables:

• None, True, False
• 1, 2.0
• 'some string', b'some data string'
• (1, 2)
• variable_name, module.CONTANT

[sobolevn]

Closed via https://github.com/wemake-services/wemake-python-styleguide/pull/486