Issue 9519 super init with non self arg #10190
Conversation
|
the test file lines 11-13 Running which is this: so the function actually exits earlier than what was suggested in the original issue. |
|
Current debug cruft: |
|
Just prior to turning off the debug cruft, here is the output: |
jzohrab
force-pushed
the
issue_9519_super_init_with_non_self_arg
branch
from
c4f2c85 to
e29a7c8
Compare
|
There's a bunch of cruft in the commits and the code, but the idea is there, so I'm going to mark this as ready for review. I'm sure the actual ast checking for the |
Pierre-Sassoulas
added
Work in progress
False Negative 🦋
|
As you can probably tell from my hacking, I'm very new at ast hacking, so there may be a far better way to do it than what I have. At least the test case should be good as a starting point. |
|
There are some test failures, but I'll wait for feedback/notes before I try to address them, as there will likely be changes needed. |
|
Hey, thanks for tackling this one! What do you think about something like this? diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py
index edef5188b..afed2d3fe 100644
--- a/pylint/checkers/typecheck.py
+++ b/pylint/checkers/typecheck.py
@@ -1453,8 +1453,19 @@ accessed. Python regular expressions are accepted.",
"""Check that called functions/methods are inferred to callable objects,
and that passed arguments match the parameters in the inferred function.
"""
- called = safe_infer(node.func, compare_constructors=True)
+ is_super = False
+ if (
+ isinstance(node.func, nodes.Attribute)
+ and node.func.attrname == "__init__"
+ and isinstance(node.func.expr, nodes.Call)
+ and isinstance(node.func.expr.func, nodes.Name)
+ and node.func.expr.func.name == "super"
+ ):
+ inferred = safe_infer(node.func.expr)
+ if isinstance(inferred, astroid.objects.Super):
+ is_super = True
+ called = safe_infer(node.func, compare_constructors=True)
self._check_not_callable(node, called)
try:
@@ -1468,7 +1479,21 @@ accessed. Python regular expressions are accepted.",
if called.name == "isinstance":
# Verify whether second argument of isinstance is a valid type
self._check_isinstance_args(node, callable_name)
- # Built-in functions have no argument information.
+ # Built-in functions have no argument information, but we know
+ # super() only takes self.
+ if is_super and utils.is_builtin_object(called):
+ if (
+ (call_site := astroid.arguments.CallSite.from_call(node))
+ and call_site.positional_arguments
+ and (first_arg := call_site.positional_arguments[0])
+ and not (isinstance(first_arg, nodes.Name) and first_arg.name == "self")
+ ):
+ self.add_message(
+ "too-many-function-args",
+ node=node,
+ args=(callable_name,),
+ )
+
return
if len(called.argnames()) != len(set(called.argnames())): |
|
Hi @jacobtylerwalls - thanks very much. I confess that this is out of my range of experience and so I can't say that your suggestion is the right way to handle it! It looks right to the uninformed (aka me), but I'm flying blind. If you'd like, I can make the change you suggested in the branch, remove the cruft code comments, and push (without squashing). |
|
Hey @jzohrab that would be great. Once you get a clean test run (I think I remember seeing there were a couple functional tests that needed updating?) the CI will run a "primer" job showing results of making this change on lint runs on several python packages. That might help us see if my suggestions is missing any edge cases. All part of the fun! (I'm not saying my sketch is perfect, but I was trying to rely less on asserting on the str()ingified value of a node and things like that.) |
Work-in-progress for issue #9519, to see if I can crack it.
This is going to contain many junk commits that I'll rebase and get rid of later, just doing this so that people can see it and maybe comment.
The first commit adds a failing test case, then there is some debug crap just so I can understand the code as I go.