Add flake8-pie PIE796: prefer-unique-enum

[ljesparis]

I accept any suggestion. By the way, I have a doubt, I have checked and all flake8-pie plugins can be fixed by ruff, but is it necessary that this one is also fixed automatically ?

rel #1543

[charliermarsh]

Awesome, thank you -- I'll review this tonight!

...but is it necessary that this one is also fixed automatically ?

I think in this case we probably shouldn't autofix it (even though we could), since there's not a clearly-correct way to resolve the error. That is, it's not clear which enum member should be removed, so we should probably just leave it.

[charliermarsh]

Could we instead use...

let comparable_value: ComparableExpr = (&value).into();
if !seen_targets.insert(comparable_value) {
  ...
}

Not exact code, just pseudo code. But this would detect any duplicate values, not just constants.

If we do want to leave it as constants, can we change the ExprKind::Constant match to use ExprKind::Constant { value: Constant::Str(value), .. }? And remove the value.to_string()? That way, we'll only match on strings, which I think better matches the desired semantics.

[ljesparis]

good one, i didn't know about ComparableExpr 😅. It makes everything a bit more simple.

[charliermarsh]

I know this is copied over from flake8-pie but I think we can do something more descriptive...

Can we change pub struct PreferUniqueEnums to:

pub struct PreferUniqueEnums {
  pub value: String
}

Then, change this to, like: format!("Enum contains duplicate value: {value}").

In prefer_unique_enums, you can pass violations::PreferUniqueEnums(unparse_expr(&values[i], checker.stylist)) to get the formatted value.

Does that seem ok? Let me know if you have any questions!

[ljesparis]

it seems more explicit, which is a good thing. I've uploaded the review changes 👍🏿

[charliermarsh]

I tweaked this logic to use resolve_call_path, which validates that the expression is bound to enum.Enum (i.e., it tracks imports, follows aliases, and so on).

[charliermarsh]

(Tweaked this to use named fields which we're trying to prefer going forwards.)

[charliermarsh]

Thank you! Made a few small changes prior to merging (left notes).