E 1003: bad-super-call with two-argument super() call

[yumasheta]

Bug description

"""Produces 'bad-super-call'."""


class ClassB:
    "Dummy B."
    
    def whoami(self):
        """Print Name"""
        print("I am B")


class ClassC:
    "Dummy C."

    def whoami(self):
        """Print Name"""
        print("hello? this is C.")


class ClassD(ClassB, ClassC):
    "Dummy D."

    def greet(self):
        """Print Name"""
        super(ClassB, self).whoami()

d = ClassD()
d.greet()
d.whoami()

Configuration

`super(ClassB, self).whoami()`

Command used

pylint test.py

Pylint output

...
test.py:25:8: E1003: Bad first argument 'ClassB' given to super() (bad-super-call)
...

Expected behavior

Hi!

This is similar to #2903, but the major difference is multiple inheritance. I hope the example makes sense from a code design perspective (if not just imagine that classB and classC inherit from an ABC that has the whoami method as an abstractmethod).

As far as I can tell, the flagged call to super is valid and the only way to actually call the whoami function of ClassC on ClassD instances.

Of course, in this example, switching the order of inheritance would be a simple solution, but just assume that based on other (not included) methods, the inheritance order has to be this way.

Thanks

Pylint version

pylint 2.10.3-dev0
astroid 2.7.2
Python 3.9.6 (default, Jun 30 2021, 10:22:16) 
[GCC 11.1.0]

OS / Environment

Linux 5.13.12-arch1-1 #1 SMP PREEMPT Wed, 18 Aug 2021 20:49:03 +0000 x86_64 GNU/Linux

Additional dependencies

No response

[NeilGirdhar]

@DanielNoord Why was this error removed?

This pattern of calling super with a different class seems just bad (even if it's a parent class), and I think this code should probably report some kind of error. Is there any realistic justification for this pattern?

Also, inheriting from both classes with eponymous methods that don't call super also seems bad (one method is hidden). This could be another error.

If both parent classes have a member that child classes need to call, then give them different names if possible. Otherwise, call them without super (ClassB.whoami(self), which is still not great). The case where super does come in handy is when you want many parents to extend the behavior of a method:

class HasWhoAmI:
    def whoami(self):
        pass

class ClassB(HasWhoAmI):
    "Dummy B."
    
    def whoami(self):
        """Print Name"""
        super().whoami()
        print("I am B")


class ClassC(HasWhoAmI):
    "Dummy C."

    def whoami(self):
        """Print Name"""
        super().whoami()
        print("hello? this is C.")


class ClassD(ClassB, ClassC):
    "Dummy D."

    def greet(self):
        """Print Name"""
        self.whoami()  # if you want all parents.

d = ClassD()
d.greet()
d.whoami()

If you agree, then my suggestion would be to keep the test added by #6956, but instead of checking that no error is returned, check that an error is always returned 😄

[DanielNoord]

Fixing this is a byproduct of fixing #2903.

We discussed the issue there and in the astroid repo here:
pylint-dev/astroid#1217 (comment)
pylint-dev/astroid#1217 (comment)

More specifically:

🤔 Not sure about that. What it comes done to is if we want to emit errors for valid Python code. Even if it's probably not what was intended, there are still situations where it's useful and has it's place, especially with multi-inheritance.

I think that rationale applies to this issue as well?

[NeilGirdhar]

I think that rationale applies to this issue as well?

super(Child, self).__init__() # will skip 'Child.__init__ and call 'Parent.init' instead`

This code in the first comment doesn't seem like a reasonable pattern to me. I can't imagine a good reason for this, and it will suffer huge problems if you ever try to inherit from Grandchild. E.g.,

class Y(Child): ...

class X(GrandChild, Y): ...

Now, Y's __init__ will never be called! You may want to skip Child.__init__, but did you want to skip Y.__init__?

What it comes done to is if we want to emit errors for valid Python code. E

I disagree with this argument in the second comment as well. There are plenty of errors that pylint gives for valid Python code (e.g., f-string should be used), and I think these are a huge benefit to the Python community by steering people to write good Python code rather than just valid Python code.

Even if it's probably not what was intended, there are still situations where it's useful and has it's place, especially with multi-inheritance.

This part of the second comment is also puzzling to me. In which situation is it useful? When does it have its place in multiple inheritance? I don't' think it does, so I humbly suggest switching the test added by #6956 around to ensuring that an error is always returned for this.

[Pierre-Sassoulas]

Pylint emits various level of message. Fatal, error, warning, convention, refactor... If something would exit with an error code at runtime we raise an error level message, otherwise it should be warning or below. It's not to say that convention message are useless but it's important that pylint is clear about the seriousness of its messages.

[DanielNoord]

@NeilGirdhar Based on the 👍 I think we can go ahead and merge #6956 then?

[NeilGirdhar]

@DanielNoord I was just agreeing that pylint emits various levels of message. I still think that pylint should actively steer users away from super(X,...) where X isn't the enclosing class. Maybe it should be warning or convention? I'm not sure.

[DanielNoord]

@DanielNoord I was just agreeing that pylint emits various levels of message. I still think that pylint should actively steer users away from super(X,...) where X isn't the enclosing class. Maybe it should be warning or convention? I'm not sure.

Yeah, I think it makes sense to have that as an Refactoring or Convention warning. It can certainly create unexpected behaviour, but I don't think it should emit the current message.

Perhaps we should open a new issue for this to discuss the level and name for the new message?

[NeilGirdhar]

Perhaps we should open a new issue for this to discuss the level and name for the new message?

Great idea.