new rule - detect when decorators are in the wrong order

[DetachHead]

some examples:

from collections.abc import Iterator
from contextlib import contextmanager


class Foo:
    @contextmanager
    @staticmethod
    def foo() -> Iterator[None]:
        yield None

    def bar(self):
        with self.foo():
            ...


Foo().bar() # TypeError: Foo.foo() takes 0 positional arguments but 1 was given

from abc import abstractmethod

class Foo:
    @abstractmethod # AttributeError: attribute '__isabstractmethod__' of 'property' objects is not writable
    @property
    def foo(self) -> int:
        ...

both of these errors are fixed by swapping the order of the decorators

[charliermarsh]

Would probably require that we maintain a set of known, proper orderings for the standard library decorators (like @property, etc.), but that's not too hard...

[namurphy]

Thank you for raising this issue!

I'm wondering if this rule could have a configuration option so that users could add their own decorator ordering rules. Even if not needed for correctness, this could help improve consistency and readability.

There's also a chance that users might want to specify the order of ≥3 decorators...though this would quickly add complexity to the rule. For ≥3 stacked decorators, a possible edge case would be when two decorators have to be in a certain order, but there might be a middle intervening decorator:

from abc import abstractmethod

class Foo:
    @abstractmethod
    @here_be_dragons
    @property
    def foo(self) -> int:
        ...

I don't know if this happens often in the pythoniverse (hopefully not!), but I'm not sure what would be best to do in this case. Perhaps it would be best for the rule to not be applied to cases with an intervening decorator.

[zanieb]

I think adding configuration should generally be carefully considered only once we have a reasonable first implementation. I often get confused about the correct order of property / method decorators as well, I think this would be pretty useful even in a limited scope.

I'm not sure we should handle cases with interleaved decorators.

[charliermarsh]

I like the idea of handling the known, common, invalid permutations from the standard library.

[namurphy]

Agreed on all of the above! Perhaps my suggestion for a configuration option for this rule would be best brought up in a follow-up issue after this rule gets implemented for decorators in the standard library.

For anyone curious about the use case motivating my suggestions, we use two decorators in PlasmaPy to process and validate arguments to functions and methods. The order of the decorators matters when they are applied to instance or class methods. When applied to class methods (which is admittedly rare), the @classmethod decorator would also be present. Right now, we handle the situation by noting this in admonitions in the docstring of @particle_input. This use case motivates a configuration option, but I have no idea if it's a common enough use case to warrant adding the capability.

Thank you!

[evanrittenhouse]

Feel free to assign this to me, I can take a look. Only major hurdle I see is the ordering of stdlib decorators. How would I even go about determining that?

[zanieb]

@evanrittenhouse basically by using them and seeing if they behave correctly at runtime.

The potentially relevant ones I can think of are:

• abc.abstractmethod
• classmethod
• staticmethod
• property
• contextlib.contextmanager
• contextlib.asyncontextmanager
• functools.wraps
• functools.cache
• functools.cached_property (maybe helpful context)
• functools.lru_cache

[DetachHead]

also property.setter

[evanrittenhouse]

@zanieb Thanks for the list - was hoping there were some docs somewhere, but off to REPL I go... 😄