Fix Improper Method Call: Return NotImplemented

[fazledyn-or]

Description

While triaging your project, our bug fixing tool generated the following message(s)-

In file: [ipaddress.py], class: _TotalOrderingMixin, there is a special method __lt__ that raises a NotImplementedError. If a special method supporting a binary operation is not implemented it should return NotImplemented. On the other hand, NotImplementedError should be raised from abstract methods inside user defined base classes to indicate that derived classes should override those methods. iCR suggested that the special method __lt__ should return NotImplemented instead of raising an exception. An example of how NotImplemented helps the interpreter support a binary operation is here.

As we can see, the _TotalOrderingMixin class is-

class _TotalOrderingMixin(object):
    # ...
    def __eq__(self, other):
        raise NotImplementedError

    def __ne__(self, other):
        equal = self.__eq__(other)
        if equal is NotImplemented:
            return NotImplemented
        return not equal

    def __lt__(self, other):
        raise NotImplementedError

    def __le__(self, other):
        less = self.__lt__(other)
        if less is NotImplemented or not less:
            return self.__eq__(other)
        return less

    def __gt__(self, other):
        less = self.__lt__(other)
        if less is NotImplemented:
            return NotImplemented
        equal = self.__eq__(other)
        if equal is NotImplemented:
            return NotImplemented
        return not (less or equal)

    def __ge__(self, other):
        less = self.__lt__(other)
        if less is NotImplemented:
            return NotImplemented
        return not less

Here, we can see that the methods __ne__, __le__, __gt__, and __ge__ are dependent on method __eq__ and __lt__. However, since both of these methods raise an exception instead of returning NotImplemented, the four prior methods aren't going to have any meaningful impact at all. So what can we do about it?

1. Use NotImplemented in methods __eq__ and __lt__

Since four of these methods don't work the way they are written to do, we can modify the two methods to make them work. I understand that _BaseAddress is the class that implements _IPAddressBase class, which implements _TotalOrderingMixin class.

2. Convert _TotalOrderingMixin to an ABC

According to the documentation -

This exception is derived from RuntimeError. In user defined base classes, abstract methods should raise this exception when they require derived classes to override the method, or while the class is being developed to indicate that the real implementation still needs to be added.

As a result, the class should be converted to an abstract class and all these methods need to raise NotImplementedError. It shouldn't affect any other classes since the _BaseAddress class is overriding those methods manually. This approach is more like tidying up/refactoring the codebase.

Please suggest your opinion on this matter. If you're willing, I can create and submit a PR accordingly.

CLA Requirements

This section is only relevant if your project requires contributors to sign a Contributor License Agreement (CLA) for external contributions.

All contributed commits are already automatically signed off.

The meaning of a signoff depends on the project, but it typically certifies that committer has the rights to submit this work under the same license and agrees to a Developer Certificate of Origin (see https://developercertificate.org/ for more information).
- Git Commit SignOff documentation

Sponsorship and Support

This work is done by the security researchers from OpenRefactory and is supported by the Open Source Security Foundation (OpenSSF): Project Alpha-Omega. Alpha-Omega is a project partnering with open source software project maintainers to systematically find new, as-yet-undiscovered vulnerabilities in open source code - and get them fixed – to improve global software supply chain security.

The bug is found by running the Intelligent Code Repair (iCR) tool by OpenRefactory and then manually triaging the results.