`unidiomatic-typecheck` not flagged for type(x) is type(y)

[jacobtylerwalls]

The spirit of unidiomatic-typecheck is to flag unidiomatic typechecks.

This is unidiomatic:

type(x) is type(y)

It should be one of:

isinstance(x, type(y))
issubclass(type(x), type(y))

[DanielNoord]

None of those replacements deal with the case where you actually want to check the exact type, right? They would all pass if the type of x is a subclass of the type of y.

[jacobtylerwalls]

Exactly. The spirit of the message as I understand it is to discourage that kind of exact check.

[DanielNoord]

I have some use cases in my codebases where I need this, but would personally be fine with having to disable the check there. This is likely a case where we need input from the primer to determine how good/bad it would be?

[Pierre-Sassoulas]

The linked MR with a fix already exists and does not have a very good primer imo, see https://github.com/pylint-dev/pylint/pull/10372#issuecomment-2854435684. I think this is an opinionated check and my intuition is that it's been toned down in the past. I'm currently git blaming the code to see the reasoning for the way the code is right now and have a more informed opinion. (made the comment to point out that the fix/primer already exists)/

Edit: ff4e14 from https://github.com/pylint-dev/pylint/issues/299, the discussion from bitbucket is lost https://bitbucket.org/logilab/pylint/issues/299

[Pierre-Sassoulas]

Following up, there's not a lot of argument from the git blame. But I personally think the initial intention was to warn when isinstance should clearly be preferred (i.e. for is type([])). We should clarify the reasoning in the documentation, and either add a new separate message or an option if we want to warn on type(x) is type(y) or do nothing.

[jacobtylerwalls]

We should add an exception for uses inside __eq__(self, other). A lot of the primer cases are coming from traffic like that.