F601: Repeated int keys not detected with booleans

[addoolit]

The multi-value-repeated-key-literal (F601) rule doesn't detect when a key has been duplicated for True and False boolean when they duplicate a matching int literal. This is also true for the reverse where an int literal duplicates an existing bool key. This was found with ruff 0.5.7 and Python 3.12.2.

mydict = {
    1: "abc",
    1: "def",
    True: "ghi",
    0: "foo",
    0: "bar",
    False: "baz",
}

Ruff detects the int literal repetition, but misses the bool repetition.

> ruff.exe check ruff_repro.py --isolated
ruff_repro.py:3:5: F601 Dictionary key literal `1` repeated
  |
1 | mydict = {
2 |     1: "abc",
3 |     1: "def",
  |     ^ F601
4 |     True: "ghi",
5 |     0: "foo",
  |
  = help: Remove repeated key literal `1`

ruff_repro.py:6:5: F601 Dictionary key literal `0` repeated
  |
4 |     True: "ghi",
5 |     0: "foo",
6 |     0: "bar",
  |     ^ F601
7 |     False: "baz",
8 | }
  |
  = help: Remove repeated key literal `0`

Found 2 errors.

This state of this dict is ultimately {1: 'ghi', 0: 'baz'}. I haven't found a stated guarantee that True is always 1, and False is always 0, but it appears that they are. The only thing I've seen related is this note in Boolean type - bool:

In many numeric contexts, False and True behave like the integers 0 and 1, respectively. However, relying on this is discouraged; explicitly convert using int() instead.

[dhruvmanila]

I think this makes sense because bool is a subtype of int. We might want to change this within ComparableExpr such that True == 1 and False == 0. Note that int is a subtype of float which means that True == 1.0 and False == 0.0 is also valid.

I'd be wary of any side effects of this change because a lot of rules utilize the ComparableExpr, including the HashableExpr type.

[charliermarsh]

I'm really surprised by this. @AlexWaygood or @carljm is this specified anywhere?

[AlexWaygood]

Yeah, this is a really common footgun. 1, 1.0 and True compare and hash the same. There's an FAQ about it IIRC.

(I think everybody agrees that it's terrible, but it's because early versions of Python didn't have a bool type, so it was implemented as a backwards-compatible subclass of int.)

[AlexWaygood]

Okay maybe not an FAQ, but there's some specification of this here: https://docs.python.org/3/library/stdtypes.html#mapping-types-dict

[charliermarsh]

Ok thanks!

[dylwil3]

Just to clarify, is the decision that we should include this identification as part of ComparableExpr or that we should make a more localized change?

I could imagine both situations where we would want to distinguish between True and 1 and situations where we wouldn't. (From a strict type theoretic point of view I would think that these two objects shouldn't even be comparable and one could only ask about whether bool(1)==True or int(True)==1 - but such is Python.)

What about making a custom equivalence method on the ComparableExpr struct for this weaker equivalence relation? Like expr1.iso(expr2) (where "iso" is short for "isomorphic").

[MichaReiser]

Yeah, not quiet sure to be honest. Maybe @charliermarsh has a better sense for it. Overall, the idea of ComparableExpr is to compare two ASTs. I do believe we also use it in the formatter to detect if we made "unwanted" AST changes. Here it would be unwanted if True == 1 would be true.

[charliermarsh]

Yeah, I think it needs to be a more localized change. I don’t think those AST nodes should compare as equal.