False negative: ARG002 is not triggered for methods decorated with typing_extensions.override

[bryanforbes]

Ruff version: 0.0.261

Given the following file foo.py:

from __future__ import annotations

from typing_extensions import override


class Foo:
    def do_something(self, arg: int) -> None:
        print('here')


class Bar(Foo):
    @override
    def do_something(self, arg: int) -> None:
        print('here as well')

And the following invocation:

ruff --select ARG --isolated --no-cache foo.py

The following output is produced:

foo.py:7:28: ARG002 Unused method argument: `arg`

There should be another error reported at line 13. For reference, PEP-698 defines @override.

[charliermarsh]

I believe this is intentional, the reason being that if you're overriding a method, you can't really control the signature. For the same reason, we ignore unused arguments in abstract methods. It looks like these are configuration options in the originating plugin (flake8-unused-arguments).

What would you consider to be the right thing to do here, as a user? Underscore-prefix the variable name?

[Avasam]

To add to the discussion:
https://peps.python.org/pep-0698/#no-new-rules-for-override-compatibility explicitly doesn't mention anything about the overridden argument name. (yet, it may come in further PEPs)

Because it's in the context of overriding, there's a couple reasons to keep the arguments the same. Especially when it comes to static type-checking, passing keyword arguments, and having a consistent signature (which is what @override is for in the first place). My editor (VScode+Pylance) also marks unused arguments by graying them out.

Otherwise, prepending a variable with an underscore to mark unused variables is a convention I use as well in Python, TypeScript and C#. As long as it's an option I don't really mind because the current behaviour with @override is desirable.

[bryanforbes]

I believe this is intentional, the reason being that if you're overriding a method, you can't really control the signature.

This makes sense and I should have thought of that. Let's blame it on all the meetings I was in yesterday.

It looks like these are configuration options in the originating plugin (flake8-unused-arguments).

It might be nice to give users the option to do this, but after reading that first sentence, I'm not sure that's even needed.

What would you consider to be the right thing to do here, as a user? Underscore-prefix the variable name?

For clarity's sake, the original method I was working with had a positional-only argument. If we take a look at all of the types of arguments:

class Foo:
    def do_something(
        self, arg: int, /, arg2: str, *, arg3: str, arg4: str | None = None
    ) -> None:
        print('here')


class Bar(Foo):
    @override
    def do_something(
        self, arg: int, /, arg2: str, *, arg3: str, arg4: str | None = None
    ) -> None:
        print('here as well')

In this case, arg could be renamed since it can never be called as a named argument, but none of the rest can be since they can all be called as named arguments. Positional-only arguments are the only case I can think of where I could see ruff flagging an argument as unused in an overridden method since they can be renamed.

[bryanforbes]

@Avasam

Otherwise, prepending a variable with an underscore to mark unused variables is a convention I use as well in Python

As I pointed out above, the only arguments you can do that with are positional-only arguments. The rest technically can't be renamed since they have to match the name of the method you're overriding.

[bryanforbes]

To add to the discussion:
https://peps.python.org/pep-0698/#no-new-rules-for-override-compatibility explicitly doesn't mention anything about the overridden argument name. (yet, it may come in further PEPs)

@Avasam sorry for the second ping, but I forgot something in my last reply. The PEP doesn't need to mention anything about overridden argument names because that is covered by basic type checking for method overrides. If a type checker isn't already checking that before it implements PEP-698, it's not doing its job.

[charliermarsh]

Yeah this all makes sense -- I follow what you're saying RE positional-only arguments. I think the current behavior is reasonable, I think it would be surprising to users that positional-only arguments are flagged while all others aren't, even if it'd actually be totally fine to rename them. I'm gonna close but always open to follow-up questions :)