Content-Length: 377699 | pFad | http://github.com/github/codeql/pull/20038

C6 Python: Modernize 3 quality queries for comparison methods by joefarebrother · Pull Request #20038 · github/codeql · GitHub
Skip to content

Python: Modernize 3 quality queries for comparison methods #20038

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

joefarebrother
Copy link
Contributor

Modernizes py/incomplete-ordering, py/inconsistent-equality, and py/equals-hash-mismatch
No longer uses pointsTo, removes python 2 specific cases, and updates documentation.

@Copilot Copilot AI review requested due to automatic review settings July 14, 2025 10:18
@joefarebrother joefarebrother requested a review from a team as a code owner July 14, 2025 10:18
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR modernizes three Python quality queries for comparison methods: py/incomplete-ordering, py/inconsistent-equality, and py/equals-hash-mismatch. The modernization removes dependency on the legacy pointsTo analysis, eliminates Python 2-specific cases to focus on Python 3, and updates documentation with clearer examples and explanations.

Key changes include:

  • Complete rewrite of the three comparison queries using modern CodeQL libraries
  • Migration of query files from Classes/ to Classes/Comparisons/ directory structure
  • Updated test cases with inline expectations format and comprehensive examples
  • Enhanced documentation with Python 3-focused guidance and improved examples

Reviewed Changes

Copilot reviewed 36 out of 37 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
python/ql/src/Classes/Comparisons/*.ql New modernized query implementations using current CodeQL libraries
python/ql/src/Classes/Comparisons/*.qhelp Updated documentation with Python 3 focus and clearer examples
python/ql/test/query-tests/Classes/*/**.py Enhanced test cases with comprehensive comparison method scenarios
python/ql/src/Classes/Equality.qll Extended DelegatingEqualityMethod to handle method call delegation patterns
python/ql/lib/semmle/python/Class.qll Added getMethod convenience predicate for retrieving methods by name

Copy link
Contributor

QHelp previews:

python/ql/src/Classes/Comparisons/EqualsOrHash.qhelp

Inconsistent equality and hashing

A hashable class has an __eq__ method, and a __hash__ method that agrees with equality. When a hash method is defined, an equality method should also be defined; otherwise object identity is used for equality comparisons which may not be intended.

Note that defining an __eq__ method without defining a __hash__ method automatically makes the class unhashable in Python 3. (even if a superclass defines a hash method).

Recommendation

If a __hash__ method is defined, ensure a compatible __eq__ method is also defined.

To explicitly declare a class as unhashable, set __hash__ = None, rather than defining a __hash__ method that always raises an exception. Otherwise, the class would be incorrectly identified as hashable by an isinstance(obj, collections.abc.Hashable) call.

Example

In the following example, the A class defines an hash method but no equality method. Equality will be determined by object identity, which may not be the expected behaviour.

class A:
    def __init__(self, a, b):
        self.a = a 
        self.b = b

    # No equality method is defined
    def __hash__(self):
        return hash((self.a, self.b))

References

python/ql/src/Classes/Comparisons/EqualsOrNotEquals.qhelp

Inconsistent equality and inequality

In order to ensure the == and != operators behave consistently as expected (i.e. they should be negations of each other), care should be taken when implementing the __eq__ and __ne__ special methods.

In Python 3, if the __eq__ method is defined in a class while the __ne__ is not, then the != operator will automatically delegate to the __eq__ method in the expected way.

However, if the __ne__ method is defined without a corresponding __eq__ method, the == operator will still default to object identity (equivalent to the is operator), while the != operator will use the __ne__ method, which may be inconsistent.

Additionally, if the __ne__ method is defined on a superclass, and the subclass defines its own __eq__ method without overriding the superclass __ne__ method, the != operator will use this superclass __ne__ method, rather than automatically delegating to __eq__, which may be incorrect.

Recommendation

Ensure that when an __ne__ method is defined, the __eq__ method is also defined, and their results are consistent. In most cases, the __ne__ method does not need to be defined at all, as the default behavior is to delegate to __eq__ and negate the result.

Example

In the following example, A defines a __ne__ method, but not an __eq__ method. This leads to inconsistent results between equality and inequality operators.

class A:
    def __init__(self, a):
        self.a = a 

    # BAD: ne is defined, but not eq.
    def __ne__(self, other):
        if not isinstance(other, A):
            return NotImplemented 
        return self.a != other.a

x = A(1)
y = A(1)

print(x == y) # Prints False (potentially unexpected - object identity is used)
print(x != y) # Prints False

In the following example, C defines an __eq__ method, but its __ne__ implementation is inherited from B, which is not consistent with the equality operation.

class B:
    def __init__(self, b):
        self.b = b 
    
    def __eq__(self, other):
        return self.b == other.b 
    
    def __ne__(self, other):
        return self.b != other.b 
    
class C(B):
    def __init__(self, b, c):
        super().__init__(b)
        self.c = c 

    # BAD: eq is defined, but != will use superclass ne method, which is not consistent
    def __eq__(self, other):
        return self.b == other.b and self.c == other.c 
    
print(C(1,2) == C(1,3)) # Prints False 
print(C(1,2) != C(1,3)) # Prints False (potentially unexpected)

References

python/ql/src/Classes/Comparisons/IncompleteOrdering.qhelp

Incomplete ordering

A class that implements the rich comparison operators (__lt__, __gt__, __le__, or __ge__) should ensure that all four comparison operations <, <=, >, and >= function as expected, consistent with expected mathematical rules. In Python 3, this is ensured by implementing one of __lt__ or __gt__, and one of __le__ or __ge__. If the ordering is not consistent with default equality, then __eq__ should also be implemented.

Recommendation

Ensure that at least one of __lt__ or __gt__ and at least one of __le__ or __ge__ is defined.

The functools.total_ordering class decorator can be used to automatically implement all four comparison methods from a single one, which is typically the cleanest way to ensure all necessary comparison methods are implemented consistently.

Example

In the following example, only the __lt__ operator has been implemented, which would lead to unexpected errors if the <= or >= operators are used on A instances. The __le__ method should also be defined, as well as __eq__ in this case.

class A:
    def __init__(self, i):
        self.i = i

    # BAD: le is not defined, so `A(1) <= A(2)` would result in an error.
    def __lt__(self, other):
        return self.i < other.i
    

References

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/github/codeql/pull/20038

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy