Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential New Lint Check: Alphabetized Enums #2315

Open
traviscook21 opened this issue Jan 29, 2023 · 3 comments
Open

Potential New Lint Check: Alphabetized Enums #2315

traviscook21 opened this issue Jan 29, 2023 · 3 comments
Labels
rule Implementing or modifying a lint rule

Comments

@traviscook21
Copy link

Hi there - thanks for the great repo. I've been continually amazed by how many checks have been included and how fast ruff is. The work y'all are doing is amazing.

I'm interesting in adding a new set of linter checks around alphabetized enums. In the variety of corporate repos I work in, there are some use cases with very long lists of values in an enum, like log types, slack channel names, etc. We've been enforcing that these stay in alphabetical order via PR, but obviously enforcing via a lint check would be easier, especially if it can be auto fixed. I've tried to find flake8 extensions that have a similar behavior but I have yet to find anything that does this exactly. I'm a python developer without any Rust experience (yet) but would be interested in implementing in ruff if this is deemed appropriate as a new kind of check.

Some examples:

1️⃣ Alphabetize non-auto string values based on the enum attribute names.

Bad:

from enum import Enum

class Animals(Enum): 
     DOG = "dog"
     CAT = "cat"

Good:

from enum import Enum

class Animals(Enum): 
     CAT = "cat"
     DOG = "dog"

2️⃣ Order non-auto integer values based on the enum values

It's fairly common to use enums to handle priority order, so I think the clearest thing to do is to sort these on the values, not on the attribute names. This could also be sorted on the attribute names.

Bad:

from enum import Enum

class Priority(Enum):
    THIRD_PARTY = 3
    PUBLIC = 1
    INTERNAL = 2

Good:

from enum import Enum

class Priority(Enum):
    PUBLIC = 1    
    INTERNAL = 2
    THIRD_PARTY = 3

3️⃣ Alphabetize auto values based on the StrEnum attribute names.

When using a StrEnum, auto will set the values to name.lower() so changing the order will have no impact.

Bad:

from enum import StrEnum, auto

class Animals(StrEnum):
    DOG = auto()
    CAT = auto()

Good:

from enum import StrEnum, auto

class Animals(StrEnum):
    CAT = auto()
    DOG = auto()

4️⃣ Do Nothing with regular enums that use auto()
Because regular enum auto() assigns integer values, changing the order will change the integer values that get assigned. Assuming all of the uses are internal, this might not cause issues, but if the values are written to a database, used outside the application, or any comparisons happen directly to the integer values, this could cause problems. As a result, I think it's better to leave enums that look like this alone

from enum import Enum, auto

class Animals(Enum):
    DOG = auto()
    CAT = auto()
@charliermarsh
Copy link
Member

I think this is overall a reasonable suggestion. It'll take some care to preserve comments, if that's something that we care about.

The main blockers here, IMO, are: (1) we're a little behind on defining the criteria for inclusion new checks (#2257), and (2) we need better mechanisms for categorizing these novel rules (we can put them in RUF, but I'd rather they go in some more experimental section -- see #1774).

I'm hoping to write something up to kickstart the discussion on (1) by the end of the week.

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Jan 31, 2023
@ngnpope
Copy link
Contributor

ngnpope commented Jan 31, 2023

For this particular one it would be nice if it's possible to so something like the following:

class Animals(Enum):
    CAT = auto()
    DOG = auto()
    PIG = auto()

    UNIDENTIFIED = auto()  # noqa: ENU001

Or whatever... Sometimes there is a special value that we don't want to sort, but I guess I'd expect the violation to normally be raised against the class Animals(Enum): line so flagging this could be awkward.

@michaeloliverx
Copy link

michaeloliverx commented Feb 7, 2023

This is great suggestion! Something related would be the ability to sort collections that use enums as keys similar to the undocumented isort options to sort collections with string keys. I do this manually in many places which is always a pain.

Being able to opt out an entire enum rather than each member with a comment would be useful also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

4 participants