fix false negative when isinstance is provided too many or too few arguments

[rogersheu]

Type of Changes

	Type
✓	🐛 Bug fix
✓	📜 Docs

Description

In #9847, @nickdrozd pointed out that when the wrong number of arguments (specifically too many) are passed to isinstance, we see a false negative, where no pylint warning comes out despite the issue.

Though it has been previously pointed out that built-in functions do not provide relevant argument information, i.e., expected number of arguments, we can for now rely on isinstance taking exactly two arguments to provide the user with a too-many-arguments warning.

pylint/pylint/checkers/typecheck.py

Line 1455 in 466c671

# Built-in functions have no argument information.

A functional test has also been included to account for this case. The isinstance call makes a TypeError, so that must be caught in order for pylint to find the too-many-arguments issue successfully.

Closes #9847

[jacobtylerwalls]

Thanks for the contribution!

[nickdrozd]

Nice! Could you add a check for < 2 args too, as in isinstance(x)? That's the false negative that burned me in the real world.

[rogersheu]

Nice! Could you add a check for < 2 args too, as in isinstance(x)? That's the false negative that burned me in the real world.

@nickdrozd I've noticed that there isn't a pylint warning for too-few-arguments. Does it make sense to make a new message for this? (Is that within the scope of this PR?)

[nickdrozd]

I've noticed that there isn't a pylint warning for too-few-arguments. Does it make sense to make a new message for this? (Is that within the scope of this PR?)

For me the only thing that matters is that bad isinstance calls are flagged. I don't have any opinions about how exactly this should be arranged.

@jacobtylerwalls, any thoughts?

[rogersheu]

The test looks pretty fixable; I'll get to that tonight.

I've gotten the changelog error a number of times here. It looks like the test is not happy with seeing _template.rst for whatever reason?

[Pierre-Sassoulas]

Don't worry about the _template.rst issue, not related to this MR :)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.80%. Comparing base (cb6db06) to head (2d6db8a).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9868   +/-   ##
=======================================
  Coverage   95.80%   95.80%           
=======================================
  Files         174      174           
  Lines       18939    18942    +3     
=======================================
+ Hits        18144    18147    +3     
  Misses        795      795           

Files	Coverage Δ
pylint/checkers/typecheck.py	96.52% <100.00%> (+0.01%)	⬆️

[rogersheu]

I see that I needed to run the make html as mentioned on the Contributing page.

[rogersheu]

Also, it would appear that the Primer / Comment workflow bumped in Python version from 3.12.4 to 3.12.5 and so the Primer / Comment Action is not finding a cached venv (and does not build one itself).

https://github.com/pylint-dev/pylint/actions/runs/10382711912/job/28746257136

However, Primer / Run has found the Python 3.12.5 cache fine at around the same time, so it's mysterious.

https://github.com/pylint-dev/pylint/actions/runs/10382595367/job/28769681193

It would appear that they are looking at different cache keys, at least.

[jacobtylerwalls]

Thanks, I'll look into it. Speaking of the primer, I noticed there are warnings newly emitted (expand the "Echo Warnings" step).

[jacobtylerwalls]

But the warning just looks like #9138, which was supposed to have been solved, maybe there really is a bad cache key somewhere?

[jacobtylerwalls]

Looks really good, just some documentation nits and this should be g2g

[jacobtylerwalls]

Speaking of the primer, I noticed there are warnings newly emitted (expand the "Echo Warnings" step).

Oh, and you can forget this -- the warnings are happening on main now, too.

[jacobtylerwalls]

Wonderful, thanks!

[rogersheu]

@jacobtylerwalls Thanks a lot for providing some great feedback! 😄

[github-actions]

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit 2d6db8a

[nickdrozd]

Thanks for working on this @rogersheu! Looking forward to seeing this warning the next time I mess up some isinstance call 😆

[rogersheu]

No probs, @nickdrozd , thanks for making this suggestion in the first place!

Might be nice if this could someday be extended for other built-in functions 🤔 , but then again, maybe it'd be easier to expand on the too-few-function-args message.

[jacobtylerwalls]

Something minor for next time: in the pylint docs we tend to avoid "foo" and "bar" because junior programmers are often the target audience, and if they don't know the convention/backstory it has the potential to be distracting.

Also, "foo" is a literal just like 1 -- I was suggesting to change it for the reason that if we introduce a check later like literal-isinstance (in the spirit of using-constant-test), then we'll have to adjust this example. But let's not change it now, I'm not sure it's worth fleshing out this example more.