From 871688f02617921452a77f50aba33fd8c5b4dbe5 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Thu, 24 Jul 2025 16:01:57 +0100 Subject: [PATCH] Update docs --- .../IncorrectRaiseInSpecialMethod.py | 16 -------- .../IncorrectRaiseInSpecialMethod.qhelp | 40 +++++++++---------- .../IncorrectRaiseInSpecialMethod2.py | 15 ------- .../IncorrectRaiseInSpecialMethod3.py | 27 ------------- .../examples/IncorrectRaiseInSpecialMethod.py | 22 ++++++++++ .../IncorrectRaiseInSpecialMethod2.py | 7 ++++ .../IncorrectRaiseInSpecialMethod3.py | 4 ++ 7 files changed, 52 insertions(+), 79 deletions(-) delete mode 100644 python/ql/src/Functions/IncorrectRaiseInSpecialMethod.py delete mode 100644 python/ql/src/Functions/IncorrectRaiseInSpecialMethod2.py delete mode 100644 python/ql/src/Functions/IncorrectRaiseInSpecialMethod3.py create mode 100644 python/ql/src/Functions/examples/IncorrectRaiseInSpecialMethod.py create mode 100644 python/ql/src/Functions/examples/IncorrectRaiseInSpecialMethod2.py create mode 100644 python/ql/src/Functions/examples/IncorrectRaiseInSpecialMethod3.py diff --git a/python/ql/src/Functions/IncorrectRaiseInSpecialMethod.py b/python/ql/src/Functions/IncorrectRaiseInSpecialMethod.py deleted file mode 100644 index e76c27145db..00000000000 --- a/python/ql/src/Functions/IncorrectRaiseInSpecialMethod.py +++ /dev/null @@ -1,16 +0,0 @@ -#Incorrect unhashable class -class MyMutableThing(object): - - def __init__(self): - pass - - def __hash__(self): - raise NotImplementedError("%r is unhashable" % self) - -#Make class unhashable in the standard way -class MyCorrectMutableThing(object): - - def __init__(self): - pass - - __hash__ = None diff --git a/python/ql/src/Functions/IncorrectRaiseInSpecialMethod.qhelp b/python/ql/src/Functions/IncorrectRaiseInSpecialMethod.qhelp index f4f0cd6920a..a0c3463b9d1 100644 --- a/python/ql/src/Functions/IncorrectRaiseInSpecialMethod.qhelp +++ b/python/ql/src/Functions/IncorrectRaiseInSpecialMethod.qhelp @@ -9,7 +9,7 @@ When the expression a + b is evaluated the Python virtual machine w is not implemented it will call type(b).__radd__(b, a).

Since the virtual machine calls these special methods for common expressions, users of the class will expect these operations to raise standard exceptions. -For example, users would expect that the expression a.b might raise an AttributeError +For example, users would expect that the expression a.b may raise an AttributeError if the object a does not have an attribute b. If a KeyError were raised instead, then this would be unexpected and may break code that expected an AttributeError, but not a KeyError. @@ -20,18 +20,18 @@ Therefore, if a method is unable to perform the expected operation then its resp

-

If the method is meant to be abstract, then declare it so using the @abstractmethod decorator. +

If the method is intended to be abstract, then declare it so using the @abstractmethod decorator. Otherwise, either remove the method or ensure that the method raises an exception of the correct type.

@@ -39,31 +39,29 @@ Otherwise, either remove the method or ensure that the method raises an exceptio

-This example shows two unhashable classes. The first class is unhashable in a non-standard way which may cause maintenance problems. -The second, corrected, class uses the standard idiom for unhashable classes. +In the following example, the __add__ method of A raises a TypeError if other is of the wrong type. +However, it should return NotImplemented instead of rising an exception, to allow other classes to support adding to A. +This is demonstrated in the class B.

- +

-In this example, the first class is implicitly abstract; the __add__ method is unimplemented, -presumably with the expectation that it will be implemented by sub-classes. -The second class makes this explicit with an @abstractmethod decoration on the unimplemented __add__ method. +In the following example, the __getitem__ method of C raises a ValueError, rather than a KeyError or IndexError as expected.

- +

-In this last example, the first class implements a collection backed by the file store. -However, should an IOError be raised in the __getitem__ it will propagate to the caller. -The second class handles any IOError by reraising a KeyError which is the standard exception for -the __getitem__ method. +In the following example, the class __hash__ method of D raises TypeError. +This causes D to be incorrectly identified as hashable by isinstance(obj, collections.abc.Hashable); so the correct +way to make a class unhashable is to set __hash__ = None.

- +
  • Python Language Reference: Special Method Names.
    • -
  • Python Library Reference: Exceptions.
    • +
  • Python Library Reference: Exceptions.
    • diff --git a/python/ql/src/Functions/IncorrectRaiseInSpecialMethod2.py b/python/ql/src/Functions/IncorrectRaiseInSpecialMethod2.py deleted file mode 100644 index 405400bfe61..00000000000 --- a/python/ql/src/Functions/IncorrectRaiseInSpecialMethod2.py +++ /dev/null @@ -1,15 +0,0 @@ - -#Abstract base class, but don't declare it. -class ImplicitAbstractClass(object): - - def __add__(self, other): - raise NotImplementedError() - -#Make abstractness explicit. -class ExplicitAbstractClass: - __metaclass__ = ABCMeta - - @abstractmethod - def __add__(self, other): - raise NotImplementedError() - diff --git a/python/ql/src/Functions/IncorrectRaiseInSpecialMethod3.py b/python/ql/src/Functions/IncorrectRaiseInSpecialMethod3.py deleted file mode 100644 index 048d5043b4d..00000000000 --- a/python/ql/src/Functions/IncorrectRaiseInSpecialMethod3.py +++ /dev/null @@ -1,27 +0,0 @@ - -#Incorrect file-backed table -class FileBackedTable(object): - - def __getitem__(self, key): - if key not in self.index: - raise IOError("Key '%s' not in table" % key) - else: - #May raise an IOError - return self.backing.get_row(key) - -#Correct by transforming exception -class ObjectLikeFileBackedTable(object): - - def get_from_key(self, key): - if key not in self.index: - raise IOError("Key '%s' not in table" % key) - else: - #May raise an IOError - return self.backing.get_row(key) - - def __getitem__(self, key): - try: - return self.get_from_key(key) - except IOError: - raise KeyError(key) - diff --git a/python/ql/src/Functions/examples/IncorrectRaiseInSpecialMethod.py b/python/ql/src/Functions/examples/IncorrectRaiseInSpecialMethod.py new file mode 100644 index 00000000000..77c623bef79 --- /dev/null +++ b/python/ql/src/Functions/examples/IncorrectRaiseInSpecialMethod.py @@ -0,0 +1,22 @@ +class A: + def __init__(self, a): + self.a = a + + def __add__(self, other): + # BAD: Should return NotImplemented instead of raising + if not isinstance(other,A): + raise TypeError(f"Cannot add A to {other.__type__}") + return A(self.a + other.a) + +class B: + def __init__(self, a): + self.a = a + + def __add__(self, other): + # GOOD: Returning NotImplemented allows for other classes to support adding do B. + if not isinstance(other,B): + return NotImplemented + return B(self.a + other.a) + + + diff --git a/python/ql/src/Functions/examples/IncorrectRaiseInSpecialMethod2.py b/python/ql/src/Functions/examples/IncorrectRaiseInSpecialMethod2.py new file mode 100644 index 00000000000..ba5f90f4670 --- /dev/null +++ b/python/ql/src/Functions/examples/IncorrectRaiseInSpecialMethod2.py @@ -0,0 +1,7 @@ +class C: + def __getitem__(self, idx): + if self.idx < 0: + # BAD: Should raise a KeyError or IndexError instead. + raise ValueError("Invalid index") + return self.lookup(idx) + diff --git a/python/ql/src/Functions/examples/IncorrectRaiseInSpecialMethod3.py b/python/ql/src/Functions/examples/IncorrectRaiseInSpecialMethod3.py new file mode 100644 index 00000000000..84ce9d18d27 --- /dev/null +++ b/python/ql/src/Functions/examples/IncorrectRaiseInSpecialMethod3.py @@ -0,0 +1,4 @@ +class D: + def __hash__(self): + # BAD: Use `__hash__ = None` instead. + raise NotImplementedError(f"{self.__type__} is unhashable.") \ No newline at end of file