[pep8-naming] Avoid false positive for class Bar(type(foo)) (N804)

[ntBre]

Summary

Fixes #14675, which reports N804 (invalid-first-argument-name-for-class-method) for this code:

foo = {}

class Bar(type(foo)):
    def foo_method(self):
        pass

I don't really love having to capture maybe in the closure, but this was the best way I could come up with to avoid propagating the new IsMetaclass return type through any_base_class. Another option along these lines, which at least avoids changing the Fns to FnMut, is to let maybe be a RefCell<bool> and mutate it that way.

Test Plan

cargo test with the example above added to N804.py.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[AlexWaygood]

I don't really love having to capture maybe in the closure

hmm, yeah, this does feel a bit... icky 😄

I wonder if a better solution (though it's a bit more of a refactor) would be to split up any_base_class() into two functions: one that just handles iterating through all superclasses recursively (iter_base_classes() or iter_superclasses()), and one that checks if a condition holds true for any of the superclasses (that one could still be called any_base_class()). That way is_metaclass() could use the lower-level iter_base_classes() function directly rather than having to awkwardly work around the fact that the quite high-level any_base_class function doesn't really have quite the API we'd like it to have here.

WDYT?

[ntBre]

I think that's a great idea, I'll give it a try!

[MichaReiser]

Just some extra data point if it helps to avoid a lot of work: I don't mind that it it captures the variable

[ntBre]

This was a little harder than I thought. I've added an iter_base_classes function, but it's not truly an iterator now since I build up a Vec of base classes and then call into_iter. I'll keep thinking about this, but at least the API has the shape I was going for.

[MichaReiser]

I kind of prefer the old solution. It overall felt simpler and avoiding the extra allocation is nice too

[AlexWaygood]

Yeah, writing a recursive iterator seems annoyingly hard :(

Sorry for leading you down the wrong track here @ntBre. My bad -- your first solution probably was best!

[ntBre]

No worries, it was interesting to try! I've reverted to the original.

One other thing I just noticed, should maybe also be true for the subscript case? I only handled the type(foo) example directly from the issue, but should type[foo] work as well? That's just a one-line code change, but I can update the tests too.

[AlexWaygood]

should maybe also be true for the subscript case

I don't think so -- type[str], type[int], etc. always behaves the same as bare type in the context of a class's bases, and if a class inherits from type then it's always a metaclass

[AlexWaygood]

Even subscripts that are "invalid" from a typing perspective behave the same way:

>>> class bar(type[int, str]): pass
... 
>>> bar.__bases__
(<class 'type'>,)

[AlexWaygood]

Also thanks for jumping on this! :D

[AlexWaygood]

Thanks!

[alexhenman]

Thanks all for your very speedy work getting this fixed!