Update docs

Author: joefarebrother

python/ql/src/Functions/IncorrectRaiseInSpecialMethod.py

@@ -9,7 +9,7 @@ When the expression <code>a + b</code> is evaluated the Python virtual machine w
 is not implemented it will call <code>type(b).__radd__(b, a)</code>.</p>
 <p>
 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 <code>a.b</code> might raise an <code>AttributeError</code>
+For example, users would expect that the expression <code>a.b</code> may raise an <code>AttributeError</code>
 if the object <code>a</code> does not have an attribute <code>b</code>.
 If a <code>KeyError</code> were raised instead,
 then this would be unexpected and may break code that expected an <code>AttributeError</code>, but not a <code>KeyError</code>.
@@ -20,50 +20,48 @@ Therefore, if a method is unable to perform the expected operation then its resp
 </p>
 
 <ul>
-    <li>Attribute access, <code>a.b</code>: Raise <code>AttributeError</code></li>
-    <li>Arithmetic operations, <code>a + b</code>: Do not raise an exception, return <code>NotImplemented</code> instead.</li>
-    <li>Indexing, <code>a[b]</code>: Raise <code>KeyError</code>.</li>
-    <li>Hashing, <code>hash(a)</code>: Use <code>__hash__ = None</code> to indicate that an object is unhashable.</li>
-    <li>Equality methods, <code>a != b</code>: Never raise an exception, always return <code>True</code> or <code>False</code>.</li>
-    <li>Ordering comparison methods, <code>a &lt; b</code>: Raise a <code>TypeError</code> if the objects cannot be ordered.</li>
+    <li>Attribute access, <code>a.b</code> (<code>__getattr__</code>): Raise <code>AttributeError</code></li>
+    <li>Arithmetic operations, <code>a + b</code> (<code>__add__</code>): Do not raise an exception, return <code>NotImplemented</code> instead.</li>
+    <li>Indexing, <code>a[b]</code> (<code>__getitem__</code>): Raise <code>KeyError</code> or <code>IndexError</code>.</li>
+    <li>Hashing, <code>hash(a)</code> (<code>__hash__</code>): Should not raise an exception. Use <code>__hash__ = None</code> to indicate that an object is unhashable rather than raising an exception.</li>
+    <li>Equality methods, <code>a == b</code> (<code>__eq__</code>): Never raise an exception, always return <code>True</code> or <code>False</code>.</li>
+    <li>Ordering comparison methods, <code>a &lt; b</code> (<code>__lt__</code>): Raise a <code>TypeError</code> if the objects cannot be ordered.</li>
     <li>Most others: Ideally, do not implement the method at all, otherwise raise <code>TypeError</code> to indicate that the operation is unsupported.</li>
 </ul>
 
 </overview>
 <recommendation>
-<p>If the method is meant to be abstract, then declare it so using the <code>@abstractmethod</code> decorator.
+<p>If the method is intended to be abstract, then declare it so using the <code>@abstractmethod</code> decorator.
 Otherwise, either remove the method or ensure that the method raises an exception of the correct type.
 </p>
 
 </recommendation>
 <example>
 
 <p>
-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 <code>__add__</code> method of <code>A</code> raises a <code>TypeError</code> if <code>other</code> is of the wrong type.
+However, it should return <code>NotImplemented</code> instead of rising an exception, to allow other classes to support adding to <code>A</code>.
+This is demonstrated in the class <code>B</code>.
 </p>
-<sample src="IncorrectRaiseInSpecialMethod.py" />
+<sample src="examples/IncorrectRaiseInSpecialMethod.py" />
 <p>
-In this example, the first class is implicitly abstract; the <code>__add__</code> method is unimplemented,
-presumably with the expectation that it will be implemented by sub-classes.
-The second class makes this explicit with an <code>@abstractmethod</code> decoration on the unimplemented  <code>__add__</code> method.
+In the following example, the <code>__getitem__</code> method of <code>C</code> raises a <code>ValueError</code>, rather than a <code>KeyError</code> or <code>IndexError</code> as expected.
 </p>
-<sample src="IncorrectRaiseInSpecialMethod2.py" />
+<sample src="examples/IncorrectRaiseInSpecialMethod2.py" />
 <p>
-In this last example, the first class implements a collection backed by the file store.
-However, should an <code>IOError</code> be raised in the <code>__getitem__</code> it will propagate to the caller.
-The second class handles any <code>IOError</code> by reraising a <code>KeyError</code> which is the standard exception for
-the <code>__getitem__</code> method.
+In the following example, the class <code>__hash__</code> method of <code>D</code> raises <code>TypeError</code>. 
+This causes <code>D</code> to be incorrectly identified as hashable by <code>isinstance(obj, collections.abc.Hashable)</code>; so the correct 
+way to make a class unhashable is to set <code>__hash__ = None</code>.
 </p>
 
-<sample src="IncorrectRaiseInSpecialMethod3.py" />
+<sample src="examples/IncorrectRaiseInSpecialMethod3.py" />
 
 
 </example>
 <references>
 
 <li>Python Language Reference: <a href="http://docs.python.org/dev/reference/datamodel.html#special-method-names">Special Method Names</a>.</li>
-<li>Python Library Reference: <a href="https://docs.python.org/2/library/exceptions.html">Exceptions</a>.</li>
+<li>Python Library Reference: <a href="https://docs.python.org/3/library/exceptions.html">Exceptions</a>.</li>
 
 
 

python/ql/src/Functions/IncorrectRaiseInSpecialMethod.qhelp

@@ -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)
+    
+
+        

python/ql/src/Functions/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)
+ 

python/ql/src/Functions/IncorrectRaiseInSpecialMethod3.py

@@ -0,0 +1,4 @@
+class D:
+    def __hash__(self):
+        # BAD: Use `__hash__ = None` instead.
+        raise NotImplementedError(f"{self.__type__} is unhashable.")