Make ARG002 compatible with EM101 when raising NotImplementedError

[Watercycle]

Summary

This pull request resolves some rule thrashing identified in #12427 by allowing for unused arguments when using NotImplementedError with a variable per this comment.

Note

This feels a little heavy-handed / edge-case-prone. So, to be clear, I'm happy to scrap this code and just update the docs to communicate that abstractmethod and friends should be used in this scenario (or similar). Just let me know what you'd like done!

fixes: #12427

Test Plan

I added a test-case to the existing ARG.py file and ran...

cargo run -p ruff -- check crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py --no-cache --preview --select ARG002

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[MichaReiser]

This looks good to me. Would you mind adding a test string with an f-string message?

[MichaReiser]

Nit:

Suggested change

	}) = exception.as_ref()
	}) = &**exception

[Watercycle]

I see mixed usages of &** and as_ref throughout the code base. Out of curiosity, when is one used over the other? The general heuristic I've seen is * and & are recommended for simple stuff; and, once you start de-referencing and re-borrowing, that as_ref is recommended if just to avoid needing to think about whatever container the value* is in.

[MichaReiser]

We used to use as_ref but started to prefer dereferencing (except for Option where you still need as_ref and as_deref) because it's possible that as_ref becomes ambiguous if a new AsRef implementation is added to a type. For example, String has many as_ref methods:

  = note: multiple `impl`s satisfying `String: AsRef<_>` found in the following crates: `alloc`, `std`:
          - impl AsRef<OsStr> for String;
          - impl AsRef<Path> for String;
          - impl AsRef<[u8]> for String;
          - impl AsRef<str> for String;

[Watercycle]

Oh interesting, that makes sense! Thank you!

[dhruvmanila]

Thanks for the PR!

I made a few final changes:

• We should match the assignment target and argument node exactly. Previously, if either of them isn't a name node, the function would return true.
• Added some more test cases where ARG002 should be raised

[dhruvmanila]

Going to wait for ecosystem checks and then merge.

[Watercycle]

I made a few final changes:

Thank you for fixing it up! I noticed the snapshot test didn't get updated after committing and thought "this can be Sunday's problem" 😅