Merge pull request #20086 from joefarebrother/python-qual-raise-not-implemented

Python: Modernise raise-not-implemented query

Author: joefarebrother

python/ql/src/Exceptions/NotImplementedIsNotAnException.qhelp

@@ -4,25 +4,25 @@
 <qhelp>
 <overview>
 
-<p><code>NotImplemented</code> is not an Exception, but is often mistakenly used in place of <code>NotImplementedError</code>.
-Executing <code>raise NotImplemented</code> or <code>raise NotImplemented()</code> will raise a <code>TypeError</code>.
-When <code>raise NotImplemented</code> is used to mark code that is genuinely never called, this mistake is benign.
-
-However, should it be called, then a <code>TypeError</code> will be raised rather than the expected <code>NotImplemented</code>,
-which might make debugging the issue difficult.
+<p>
+The constant <code>NotImplemented</code> is not an <code>Exception</code>, but is often confused for <code>NotImplementedError</code>.
+If it is used as an exception, such as in <code>raise NotImplemented</code> or <code>raise NotImplemented("message")</code>,
+a <code>TypeError</code> will be raised rather than the expected <code>NotImplemented</code>. This may make debugging more difficult.
 </p>
 
-<p>The correct use of <code>NotImplemented</code> is to implement binary operators.
+<p><code>NotImplemented</code> should only be used as a special return value for implementing special methods such as <code>__lt__</code>.
 Code that is not intended to be called should raise <code>NotImplementedError</code>.</p>
 
 </overview>
 <recommendation>
-<p>Replace uses of <code>NotImplemented</code> with <code>NotImplementedError</code>.</p>
+<p>If a <code>NotImplementedError</code> is intended to be raised, replace the use of <code>NotImplemented</code>
+with that. If <code>NotImplemented</code> is intended to be returned rather than raised, replace the <code>raise</code> with <code>return NotImplemented</code>.
+</p>
 </recommendation>
 <example>
 
 <p>
-In the example below, the method <code>wrong</code> will incorrectly raise a <code>TypeError</code> when called.
+In the following example, the method <code>wrong</code> will incorrectly raise a <code>TypeError</code> when called.
 The method <code>right</code> will raise a <code>NotImplementedError</code>.
 </p>
 
@@ -34,6 +34,7 @@ The method <code>right</code> will raise a <code>NotImplementedError</code>.
 <references>
 
     <li>Python Language Reference: <a href="https://docs.python.org/library/exceptions.html#NotImplementedError">The NotImplementedError exception</a>.</li>
+    <li>Python Language Reference: <a href="https://docs.python.org/3/library/constants.html#NotImplemented">The NotImplemented constant</a>.</li>
     <li>Python Language Reference: <a href="https://docs.python.org/3/reference/datamodel.html#emulating-numeric-types">Emulating numeric types</a>.</li>
 
 </references>

python/ql/src/Exceptions/NotImplementedIsNotAnException.ql

@@ -1,6 +1,6 @@
 /**
- * @name NotImplemented is not an Exception
- * @description Using 'NotImplemented' as an exception will result in a type error.
+ * @name Raising `NotImplemented`
+ * @description Using `NotImplemented` as an exception will result in a type error.
  * @kind problem
  * @problem.severity warning
  * @sub-severity high
@@ -12,8 +12,17 @@
  */
 
 import python
-import Exceptions.NotImplemented
+import semmle.python.ApiGraphs
+
+predicate raiseNotImplemented(Raise raise, Expr notImpl) {
+  exists(API::Node n | n = API::builtin("NotImplemented") |
+    notImpl = n.getACall().asExpr()
+    or
+    n.asSource().flowsTo(DataFlow::exprNode(notImpl))
+  ) and
+  notImpl = raise.getException()
+}
 
 from Expr notimpl
-where use_of_not_implemented_in_raise(_, notimpl)
+where raiseNotImplemented(_, notimpl)
 select notimpl, "NotImplemented is not an Exception. Did you mean NotImplementedError?"

python/ql/test/query-tests/Exceptions/general/NotImplementedIsNotAnException.expected

@@ -1,2 +1,2 @@
 | exceptions_test.py:170:11:170:24 | NotImplemented | NotImplemented is not an Exception. Did you mean NotImplementedError? |
-| exceptions_test.py:173:11:173:24 | NotImplemented | NotImplemented is not an Exception. Did you mean NotImplementedError? |
+| exceptions_test.py:173:11:173:26 | NotImplemented() | NotImplemented is not an Exception. Did you mean NotImplementedError? |