Wemake-python-styleguide: WPS324 false-positive with docstring

[nkhitrov]

Bug report

What's wrong

This code produces false positive WPS324 due to docstring line.

from botx import Collector

collector = Collector()


@collector.default(include_in_status=False)
def default_handler() -> None:
    """This is the problem line."""

    return None

How is that should be

Docstring should not cause false positives.

System information

flake8 information

Contents of flake8 --bug-report:

Details

{
"dependencies": [
{
"dependency": "entrypoints",
"version": "0.3"
}
],
"platform": {
"python_implementation": "CPython",
"python_version": "3.8.2",
"system": "Linux"
},
"plugins": [
{
"is_local": false,
"plugin": "flake8-bandit",
"version": "2.1.2"
},
{
"is_local": false,
"plugin": "flake8-broken-line",
"version": "0.2.0"
},
{
"is_local": false,
"plugin": "flake8-bugbear",
"version": "19.8.0"
},
{
"is_local": false,
"plugin": "flake8-comprehensions",
"version": "3.2.2"
},
{
"is_local": false,
"plugin": "flake8-darglint",
"version": "0.4.1"
},
{
"is_local": false,
"plugin": "flake8-debugger",
"version": "3.2.1"
},
{
"is_local": false,
"plugin": "flake8-docstrings",
"version": "1.5.0, pydocstyle: 5.0.2"
},
{
"is_local": false,
"plugin": "flake8-eradicate",
"version": "0.3.0"
},
{
"is_local": false,
"plugin": "flake8-string-format",
"version": "0.2.3"
},
{
"is_local": false,
"plugin": "flake8_commas",
"version": "2.0.0"
},
{
"is_local": false,
"plugin": "flake8_isort",
"version": "2.9.1"
},
{
"is_local": false,
"plugin": "flake8_quotes",
"version": "2.1.2"
},
{
"is_local": false,
"plugin": "mccabe",
"version": "0.6.1"
},
{
"is_local": false,
"plugin": "naming",
"version": "0.9.1"
},
{
"is_local": false,
"plugin": "pycodestyle",
"version": "2.5.0"
},
{
"is_local": false,
"plugin": "pyflakes",
"version": "2.1.1"
},
{
"is_local": false,
"plugin": "rst-docstrings",
"version": "0.0.12"
},
{
"is_local": false,
"plugin": "wemake_python_styleguide",
"version": "0.14.0"
}
],
"version": "3.7.9"
}

pip information

Contents of pip freeze:

Details

astor==0.8.1
attrs==19.3.0
bandit==1.6.2
botx==0.14.0
certifi==2020.4.5.1
chardet==3.0.4
darglint==1.2.3
docutils==0.16
entrypoints==0.3
eradicate==1.0
flake8==3.7.9
flake8-bandit==2.1.2
flake8-broken-line==0.2.0
flake8-bugbear==19.8.0
flake8-commas==2.0.0
flake8-comprehensions==3.2.2
flake8-debugger==3.2.1
flake8-docstrings==1.5.0
flake8-eradicate==0.3.0
flake8-isort==2.9.1
flake8-polyfill==1.0.2
flake8-quotes==2.1.2
flake8-rst-docstrings==0.0.12
flake8-string-format==0.2.3
gitdb==4.0.4
GitPython==3.1.1
h11==0.9.0
h2==3.2.0
hpack==3.0.0
hstspreload==2020.4.24
httpx==0.11.1
hyperframe==5.2.0
idna==2.9
isort==4.3.21
loguru==0.4.1
mccabe==0.6.1
pbr==5.4.5
pep8-naming==0.9.1
pycodestyle==2.5.0
pydantic==1.5.1
pydocstyle==5.0.2
pyflakes==2.1.1
Pygments==2.6.1
PyYAML==5.3.1
restructuredtext-lint==1.3.0
rfc3986==1.4.0
six==1.14.0
smmap==3.0.2
sniffio==1.1.0
snowballstemmer==2.0.0
stevedore==1.32.0
testfixtures==6.14.1
toml==0.10.0
typing-extensions==3.7.4.2
urllib3==1.25.9
wemake-python-styleguide==0.14.0

[sobolevn]

@Slyfoxy thanks a lot for the bug report! Are you willing to fix it?

[nkhitrov]

@sobolevn ok, I'll do it.

[sobolevn]

Awesome! Thanks!

Tests: https://github.com/wemake-services/wemake-python-styleguide/blob/58b96219a39390c35353aa9a29d483d5ed3f1cf7/tests/test_visitors/test_ast/test_keywords/test_consistency_returning/test_consistency_return.py

Logic: https://github.com/wemake-services/wemake-python-styleguide/blob/256de41bebbe53f60cd5ce87cb4031f120eb9615/wemake_python_styleguide/visitors/ast/keywords.py#L93

[nsidnev]

@sobolevn Hi! I'm checking this now with @Slyfoxy and I'm a bit confused. Is it really a bug that an error appears on this code? Shouldn't this code be written like this:

def function():
    """Returns nothing."""

?

Also, there is a test that such code:

def function():
    return

is bad. And this code isn't:

def function():
    return None

Is that right? Shouldn't both of these functions be written using just the docstring?

[sobolevn]

@nsidnev that's a really good question. I think that there are no reason for a function to just return None

Why would someone need this function? (Even in functional programming world)

So, I guess that it might be a feature instead. We can add a test that explicitly covers this case: one function, one docstring, one return (with None and plain one), one violation.

Also, there is a test that such code is bad. And this code isn't. Is that right?

This question is more like "why would anyone want to do this?"
Probably we can make both these cases invalid.

[nsidnev]

Probably we can make both these cases invalid.

Yes, I was thinking about the same solution.