Pylint: Feature Request: Ability to Ignore Unused Argument given a Function Name Expression

[sanscore]

Steps to reproduce

1. Define a Function with an Argument
2. Write a function body that does NOT utilize said Argument.

def test_function(never_used):
    """Test that does not utilize the 'never_used' argument"""
    assert False

Current behavior

Expected "unused-argument" Warning

Pylint, correctly, gives the warning [unused-argument] Unused argument 'never_used' message.

Rename Argument to a "dummy" Variable

With the default configuration, one could simply make use of dummy variables which will not cause a warning:

def test_function(_unused):
    """Test that does not utilize the '_unused' argument"""
    assert False

_Note:_ The argument _never_used still throws a warning because the dummy-variables-rgx option, by default (_+[a-zA-Z0-9]*?$)|dummy, doesn't allow for underscores (_) in the root of name. I'll file a separate issue for this problem.

Redefine "dummy" Variable Regular Expression

One could, also, expand on the default behavior to permit custom "dummy" variables with the dummy-variables-rgx option. For instance, permit the prefix never_.

pylintrc:

[VARIABLES]
dummy-variables-rgx=(_+[a-zA-Z0-9]*?$)|dummy|(never_+[a-zA-Z0-9]*?$)

Expected behavior

All of the behavior described the "Current Behavior" section is correct and works as expected.

However, it would be nice to be able to permit all "unused-arguments" on a function given an expression that matches against the function name.

My current use case is for compatibility with pytest fixtures. Pytest's fixture concept can be described briefly as a two step process.

First, you write a helper function decorated with @pytest.fixture. This function will serve as reusable piece of code that may be consumed by multiple tests. It can have any name that you choose. Fixtures are utilized in the argument list of a test function (or another fixture). Through pytest "magic" (introspection), the fixture function will be executed prior to the test function, and the return value of the fixture is accessible by treating the fixture as any other argument.

Here are two examples, the first, does _not_ provide (return) any useful feedback and, the second, returns an object.

"""conftest.py for a fictional "process" application"""

@pytest.fixture
def setup_process():
    """Fixture that starts 'process'."""
    process_lib.setup()
    process_lib.start()

@pytest.fixture
def process_config():
    """Fixture that retrieves the 'process' config as a python object."""
    return process_lib.get_config()

Second, write the test function that consumes the fixtures.

def test_process_config_exists(setup_process, process_config):
    assert process_config, "Config does not exist."

Now, we get to the problem. When running pylint, it will complain that setup_process is never used. I've already stated two ways that I could fix this problem.

Workaround 1 - _dummy_fixture

I could rename all of my fixtures with the "dummy" underscore prefix.

Workaround 2 - Redefine dummy-variables-rgx

I could redefine the dummy-variables-rgx to:

1. Exhaustively, add all my fixtures, or
2. Add my own prefix (or suffix), as described in the "Current Behavior" section

Counter Argument

Both workaround 1 and 2.2 require either extreme foresight and team cooperation to ensure all fixtures are prefixed (or suffixed) OR large, intrusive refactor of a current codeabse.

Workaround 2.1 is complex and will permit "unused-argument"s where I don't want them.

Feature Request - dummy-arguments-function-rgx

The request is to add an option that will permit unused-arguments based upon the function name; suggested name (open to better ideas) dummy-arguments-function-rgx.

By default, this option will be empty; No function will be permissive to unused-arguments.

Given some value, i.e. test_([a-z_][a-z0-9_]{2,30})$|([a-z_][a-z0-9_]{2,30}$)_test$, these warnings will go away.

pylint --version output

pylint 1.6.4,
$ pylint --version
astroid 1.4.7
Python 2.7.11 (default, Mar 15 2016, 09:47:58)
[GCC 4.2.1 Compatible Apple LLVM 7.0.2 (clang-700.1.81)]

[The-Compiler]

I'd argue that there are better solutions to this from the pytest side :wink:

For simple cases, you could make the fixture an autouse fixture (perhaps with tests grouped in a class).

Alternatively, for tests needing a fixture for its side effects but not using it otherwise, I'd recommend using @pytest.mark.usefixtures instead of requesting them as arguments.

Finally, fixtures can also request other fixtures, and your test_ regex would not catch those (though @pytest.mark.usefixtures won't work on fixtures either).

[sanscore]

Those are valid constructive criticisms, and I thank you for them. I may have to advocate the usage of @pytest.mark.usefixtures. And, you are too right about the usage of auto-use and fixtures consuming fixtures. As I was writing the didactic code (in the "Expected Behavior" section), I had considerable difficulty coming up with a case where either (A) an auto-use fixture or (B) a fixture consuming a fixture would make better sense. The case I came up with is kind of 'meh'; I would probably chose the auto-use fixture option. But, as written, it's not an unreasonable solution; especially, for an older pytest codebase.

I'll side-step the winky comment with a 👍 .

[sanscore]

I've come up with an argument for having unused-arguments in a pytest scenario. Assume "unique_fixture" (A) is necessary to execute a test, and (B) it has some value that is only useful for _debug time_. Given that scenario, neither auto-use nor @pytest.mark.usefixtures will be useful.

[The-Compiler]

Why couldn't pylint just be more clever about pytest? It could keep track about every function name decorated with @pytest.fixture, and not complain about arguments named like that in other functions decorated that way, or in test_* functions. At that point, it could also stop complaining about redefined-outer-name for those.

[sanscore]

Then pylint would have to implement pytest's discovery algorithm. I have contemplated writing a plugin. Since there are lot of pytest-isms that pylint will complain about, maybe it makes sense to do that. Most of them can be ignored by handcrafting a pylintrc (i.e. using pytestmark causes an [invalid-name]), but there's likely enough common interest (between pytest users that want to check test code quality with pylint) to make it a worthwhile effort.

The initial thought was to leverage pytest before each run to collect all the available fixtures. But, I would argue that is not static analysis, proper linting. Though, I could make the case for treating all the discoverable conftest.py files as "magic", or implicit, import statements.

It's a tough call, and could be a bit of work.

[PCManticore]

I am not sure we should add support for ignoring an argument from a given function name, since this will soon turn into a slippery slope of every check having the need for the same feature. I would rather support a pylint-pytest plugin, which will know how to translate the idiosyncrasies of pytest into something that pylint can understand, something à la pylint-django, which knows how to modify some checkers in order to not emit for the idiosyncrasies of Django.

Mind this would not happen in the core of pylint though, I am opposed to supporting particular behaviours of frameworks or tools straight into pylint.

[sanscore]

That works for me. Thanks for recommending pylint-django, I'll see how they go about modifying checkers. My current investigation has been to (somehow) grab the discovered, available fixtures for a given module. Use ast to add references for all the fixtures in the function. Compiled, it would look like:

test_some_function(setup_fixture, configure_fixture, unused_fixture):
    _pytest_fixtures = [setup_fixture, configure_fixture, unused_fixture]
    ...

This way _pytest_fixtures will be ignored, and the otherwise unused-arguments should not cause an issue.

[dickreuter]

As pytest has become very popular, this seems to be a very common issue. Is there any other way to solve this? For example by listing the fixtures in .pylintrc?

[reverbc]

@dickreuter this bothered me a lot so a couple of weeks ago I made a pylint-pytest plugin to suppress the false-positive warnings. Maybe you want to take a look to see if that project can help you as well?

[dickreuter]

Great. Very useful.

On Sun, 31 May 2020 at 04:40, Reverb C. [e-mail] wrote:

@dickreuter https://github.com/dickreuter this bothered me a lot so a
couple of weeks ago I made a pylint-pytest
https://github.com/reverbc/pylint-pytest plugin to suppress the
false-positive warnings. Maybe you want to take a look to see if that
project can help you as well?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/PyCQA/pylint/issues/1057#issuecomment-636416012, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAJSW7RLMQIMNQFYJJGTKL3RUHGRXANCNFSM4CLLKPRA
.