Skip to content

Python: Modernise raise-not-implemented query #20086

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 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions python/ql/src/Exceptions/NotImplementedIsNotAnException.qhelp
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand All @@ -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>
Expand Down
17 changes: 13 additions & 4 deletions python/ql/src/Exceptions/NotImplementedIsNotAnException.ql
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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?"
Original file line number Diff line number Diff line change
@@ -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? |