N804 and N805 apply ignore-names to function, rather than parameter names (self, cls)

[njzjz]

I want to use N804 to correct def setUpClass(self): in my repository. However, I found that when the class method name is setUpClass, no error is reported.

ruff v0.5.4
https://play.ruff.rs/f96de0de-0667-4e6b-88c1-fe229ad7ca5a

import unittest

class TestXXX(unittest.TestCase):
    @classmethod
    def setUpClass(self):
        # no error reported
        pass


class TestXXX:
    @classmethod
    def setUpClss(self):
        # N804: First argument of a class method should be named `cls`
        pass

I don't understand why N804 ignores setUpClass. There is no documentation for this behavior.

[njzjz]

Oh, I just understand that the default ignore-names contains setUpClass. However, N804 does not rename setUpClass. It just renames self to cls.

[charliermarsh]

Is the request that setUpClass be renamed to set_up_class?

[njzjz]

Is the request that setUpClass be renamed to set_up_class?

No, N804 should just rename

@classmethod
def setUpClass(self):

to

@classmethod
def setUpClass(cls):

[charliermarsh]

Ok, I think I understand what you're saying: setUpClass should not be ignored by N804 despite being in ignore-names, because the rule is about the parameter (self) rather than the function (setUpClass). We've had this behavior from the start (#2855), so it may not even be intentional.

It'd be reasonable to change, but it also means that there's no way to ignore methods that do deviate from the pattern in that way.

I'd be nice to get more opinions on this. Maybe @AlexWaygood? What behavior would you expect here?

[AlexWaygood]

This is an interesting question! I'd be curious to see the ecosystem results on an experimental PR that ignored ignore-names for N804. I don't feel like I have any sense of how many users might be overriding ignore-names in their config to systematically ignore all N804 violations for all methods that have a specific name across a codebase.

It does feel a bit silly to me that we ignore N804 violations by default if the method is called setUpClass, because that's obviously not the motivation for including that entry in the ignore-names default list. But I can also see @charliermarsh's point that if N804 just ignores the ignore-names setting entirely then there's no setting available to users that they can use to tweak how N804 works.

Possibly we could start ignoring the setting for N804 in preview mode only, and see if anybody complains. If we do get any complaints, we could add a new dedicated setting that could be separately configured for determining the behaviour of rules like N804 that complain about parameter names rather than class or method names.

[charliermarsh]

Yeah the current behavior is strange. Though it's also strange to imagine putting self or cls in ignore-names, since it's effectively disabling the rules. We should change it though and see if the ecosystem checks look good.

[AlexWaygood]

Right, I don't think it makes sense to have N804 be disabled if N804 sees cls or self in ignore-names either. What would make sense to me would be if we had a separate setting -- ignore-weird-parameter-names-for-these-methods (bikeshedding permitted, I thought about it for 0 seconds ;) -- that could be used to configure N804 separately to the N* rules that are configured via ignore-names. But I just don't really know if users actually need to configure N804 in the first place.