When raising NotImplementedError , ARG002 and EM101 are incompatible.

[DaniBodor]

ARG002 is usually not raised in methods that immediately raise a NotImplementedError, but does if there is some other code in the method. However, EM101 enforces adding the error message into a separate variable, which is then seen as "code".

The result of this is that there is no way to write such a method without raising one or the other issue. See example below.

class EM101_found:
    def test_method(self, **kwargs):
        raise NotImplementedError("error message.")


class ARG002_found:
    def test_method(self, **kwargs):
        msg = "error message"
        raise NotImplementedError(msg)

running ruff check test_file.py --select ARG002 --select EM101 on the code snippet above results in

test_file.py:3:35: EM101 Exception must not use a string literal, assign to variable first
test_file.py:7:29: ARG002 Unused method argument: kwargs
Found 2 errors.

[ast0815]

It's a bit of potentially unnecessary faff, but you can write these methods like this:

    def method(self, x):
        _ = x
        msg = "Something"
        raise NotImplementedError(msg)

[DaniBodor]

It's a bit of potentially unnecessary faff, but you can write these methods like this:

    def method(self, x):
        _ = x
        msg = "Something"
        raise NotImplementedError(msg)

I think rather than this, I would prefer suppressing one or the other of the rules (which is what I am doing now).

[ast0815]

I switched to abc.ABC and the @abstractmethod decorator. Yet another way around this.

[Rupt]

Google's style guide recommends deling the unused argument.

    def method(self, x):
        del x  # unused
        msg = "Something"
        raise NotImplementedError(msg)

[DaniBodor]

Interesting. I guess this should be at least mentioned in the ruff documentation of these errors as well.
For the example I gave above, I think this would require looping through the kwargs to delete them all, which also feels clunky.

Also, these all feel like workarounds for an issue that I feel doesn't really needs to be flagged in the first place.

[zanieb]

It seems reasonable to avoid this false positive in ARG002. I'm not sure how hard it will be.

[Watercycle]

Here's the rule, here's the original Python is_stub_function, and here's Ruff's is_stub.

It seems like a simple fix would be expanding the definition of is_stub to allow primitive string assignment with something along the lines of the below code.

But, going down that route, it's likely worth documenting what the cut-off for a "stub" is. That if you need things like str + str, f-string, and/or other functional calls, you should really just use alternatives like ABC (added in 3.4) or # noqa to disable one of the rules.

Stmt::Assign(StmtAssign { value, .. }) => {
    matches!(value.as_ref(), Expr::StringLiteral(_))
}

Does that sound reasonable?

[dhruvmanila]

It seems like a simple fix would be expanding the definition of is_stub to allow primitive string assignment with something along the lines of the below code.

I don't think this will work completely because it'll start giving false negatives. What we need to do is to add a new condition which will be checked if the first condition is false. This will check whether there are exactly two statements and they match the following pattern:

variable = <string | f-string>
raise NotImplementedError(variable)

Note that this condition should only be checked locally to ARG002 implementation and not be added to is_stub function as that's being used in other places as well. So, a new function can be added in unused_arguments.rs and that can be used in ARG002 implementation. Does this make sense?

[Watercycle]

Yes, that's fair! My inclination to modify is_stub is mostly because of naming.

But, to your point, it's probably better not to pollute the definition of is_stub until enough rules have common logic to warrant abstracting that out.

How does having a very specific is_implementation_stub function in unused_arguments.rs for now sound?

[dhruvmanila]

How does having a very specific is_implementation_stub function in unused_arguments.rs for now sound?

We can discuss the naming in the PR but yeah that sounds about right