unidiomatic-typecheck to check right-hand side too

[alexprabhat99]

Type of Changes

	Type
✓	🐛 Bug fix

Description

Closes #10217

[jacobtylerwalls]

Thanks for pitching in with this one. There are a couple test failures to look into. We will also want a simple addition to the functional test in unidiomatic_typecheck.py. There's details in the contributor docs, but for a small change like this one you should be able to just blend your changes in without much hassle. Thanks again.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.84%. Comparing base (12a8747) to head (d62a636).
Report is 123 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #10225   +/-   ##
=======================================
  Coverage   95.84%   95.84%           
=======================================
  Files         175      175           
  Lines       19056    19058    +2     
=======================================
+ Hits        18264    18266    +2     
  Misses        792      792           

Files with missing lines	Coverage Δ
pylint/checkers/base/comparison_checker.py	100.00% <100.00%> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Pierre-Sassoulas]

Looks great already ! I have a question :)

[Pierre-Sassoulas]

If I understand correctly the first condition is to avoid to analyse thing like "X" is type(...), right ? Why is this a problem ?

[alexprabhat99]

Some tests were failing without this condition. So, I have added it.

[Pierre-Sassoulas]

Ok ! I think the check still make sense in this case so it's better to disable unidiomatic-typecheck in the affected functional tests (add a disable comment at the top of the file, or preferably on an existing line so not everything is modified in the expected output), or add the new expected message on the affected line with # [unidiomatic-typecheck]. The functional test are not very independent this kind of things happen when we fix a false negative or introduce a new check.

[alexprabhat99]

All tests run fine now. So, I think it is better not to touch the functional tests even to disable unidiomatic-typecheck.

[jacobtylerwalls]

If I understand correctly the first condition is to avoid to analyse thing like "X" is type(...), right ?

I think it's not just string literals but almost any ast node. I think there's value in keeping this bugfix scoped small, esp as a "good first issue", but that's my 2c.

For string literals, I guess I'd expect any usage of is to have a lint warning, to resemble the runtime warning you get from python:

SyntaxWarning: "is" with 'str' literal.

[jacobtylerwalls]

Thanks for the first contribution, welcome aboard!