Fix an invalid comparison of UserPresenceState to str

[anoadragon453]

This PR fixes an invalid comparison of types. The comparison was included in #12213 and effectively meant that the fix did not apply when calling /sync or /events on a worker process. Presumably the included unit tests didn't catch the bug as they don't run in worker mode.

How come mypy didn't catch this?

[babolivier]

Can you add a regression test please?

[babolivier]

How come mypy didn't catch this?

It looks like this is the check that should take care of this, and that it only runs if --strict-equality is set (which is implied by --strict fwiw). So it sounds like we want to add strict_equality = true to mypy.ini (hopefully setting it doesn't unearth more badness).

Here's some example code attempting to reproduce the issue:

import attr


@attr.s(slots=True, frozen=True, auto_attribs=True)
class Foo:
    state: str


f = Foo(state="busy")

print(f == "busy")

No strictness

mypy.ini

[mypy]
strict = false

mypy test.py

Success: no issues found in 1 source file

Maximum strictness

mypy.ini

[mypy]
strict = true
show_error_codes = true

mypy test.py

test.py:25: error: Non-overlapping equality check (left operand type: "Foo", right operand type: "Literal['busy']")  [comparison-overlap]
Found 1 error in 1 file (checked 1 source file)

Strict equality checks enabled

mypy.ini

[mypy]
strict = false
show_error_codes = true
strict_equality = true

mypy test.py

test.py:25: error: Non-overlapping equality check (left operand type: "Foo", right operand type: "Literal['busy']")  [comparison-overlap]
Found 1 error in 1 file (checked 1 source file)

[anoadragon453]

@babolivier Thanks for the thorough investigation on mypy! Setting strict_equality = true in Synapse's mypy.ini for fun reveals some other problematic areas:

$ mypy
synapse/storage/database.py:575: error: Non-overlapping container check (element type: "str", container item type: "Dict[str, Any]")  [comparison-overlap]
synapse/handlers/presence.py:481: error: Non-overlapping equality check (left operand type: "UserPresenceState", right operand type: "Literal['org.matrix.msc3026.busy']")  [comparison-overlap]
synapse/replication/tcp/protocol.py:248: error: Non-overlapping equality check (left operand type: "bytes", right operand type: "Literal['']")  [comparison-overlap]
synapse/push/bulk_push_rule_evaluator.py:380: error: Non-overlapping container check (element type: "str", container item type: "Dict[Any, Any]")  [comparison-overlap]
Found 4 errors in 4 files (checked 742 source files)

The first is just mypy complaining about redefining the type of an existing variable. The second was the cause of this bug. The third will indeed render its check to be always False. And the fourth may indeed be another bug (though I'm not entirely familiar with how the Rust -> Python is working here to confirm).

I'll make a separate issue tackling these and recommending that we enable strict_equality = true in our mypy.ini.

[anoadragon453]

Can you add a regression test please?

Done!

[anoadragon453]

The failing tests are different across runs, so it appears that CI is just flaky.

[anoadragon453]

flakes, flakes, flakes, flakes, flakes.