From 680e31dc48f1486f1e32a840f98128309fa3b377 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Thu, 17 Jul 2025 10:02:00 +0100 Subject: [PATCH 1/4] Modernize raise-not-implemented --- python/ql/src/Exceptions/NotImplemented.qll | 2 ++ .../NotImplementedIsNotAnException.qhelp | 19 ++++++++++--------- .../NotImplementedIsNotAnException.ql | 17 +++++++++++++---- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/python/ql/src/Exceptions/NotImplemented.qll b/python/ql/src/Exceptions/NotImplemented.qll index 2186a7b5f30b..184b7429a9f5 100644 --- a/python/ql/src/Exceptions/NotImplemented.qll +++ b/python/ql/src/Exceptions/NotImplemented.qll @@ -1,3 +1,5 @@ +deprecated module; + import python /** Holds if `notimpl` refers to `NotImplemented` or `NotImplemented()` in the `raise` statement */ diff --git a/python/ql/src/Exceptions/NotImplementedIsNotAnException.qhelp b/python/ql/src/Exceptions/NotImplementedIsNotAnException.qhelp index 3bf09bbfab07..c89a8806dea6 100644 --- a/python/ql/src/Exceptions/NotImplementedIsNotAnException.qhelp +++ b/python/ql/src/Exceptions/NotImplementedIsNotAnException.qhelp @@ -4,25 +4,25 @@ -

NotImplemented is not an Exception, but is often mistakenly used in place of NotImplementedError. -Executing raise NotImplemented or raise NotImplemented() will raise a TypeError. -When raise NotImplemented is used to mark code that is genuinely never called, this mistake is benign. - -However, should it be called, then a TypeError will be raised rather than the expected NotImplemented, -which might make debugging the issue difficult. +

+The constant NotImplemented is not an Exception, but is often confused for NotImplementedError. +If it is used as an exception, such as in raise NotImplemented or raise NotImplemented("message"), +a TypeError will be raised rather than the expected NotImplemented. This may make debugging more difficult.

-

The correct use of NotImplemented is to implement binary operators. +

NotImplemented should only be used as a special return value for implementing special methods such as __lt__. Code that is not intended to be called should raise NotImplementedError.

 -

Replace uses of NotImplemented with NotImplementedError.

+

If a NotImplementedError is intended to be raised, replace the use of NotImplemented +with that. If NotImplemented is intended to be returned rather than raised, replace the raise with return NotImplemented +

-In the example below, the method wrong will incorrectly raise a TypeError when called. +In the following example, the method wrong will incorrectly raise a TypeError when called. The method right will raise a NotImplementedError.

@@ -34,6 +34,7 @@ The method right will raise a NotImplementedError.
  • Python Language Reference: The NotImplementedError exception.
    • +
  • Python Language Reference: The NotImplemented constant.
  • Python Language Reference: Emulating numeric types.
    •  diff --git a/python/ql/src/Exceptions/NotImplementedIsNotAnException.ql b/python/ql/src/Exceptions/NotImplementedIsNotAnException.ql index 80dcd6f0dbea..36bf992b51a7 100644 --- a/python/ql/src/Exceptions/NotImplementedIsNotAnException.ql +++ b/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?" From 57f1d07b2b7b09de7cb82df4ee0f863540e23c86 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Thu, 17 Jul 2025 21:54:55 +0100 Subject: [PATCH 2/4] Undo module deprecation (used by another quality query) --- python/ql/src/Exceptions/NotImplemented.qll | 2 -- 1 file changed, 2 deletions(-) diff --git a/python/ql/src/Exceptions/NotImplemented.qll b/python/ql/src/Exceptions/NotImplemented.qll index 184b7429a9f5..2186a7b5f30b 100644 --- a/python/ql/src/Exceptions/NotImplemented.qll +++ b/python/ql/src/Exceptions/NotImplemented.qll @@ -1,5 +1,3 @@ -deprecated module; - import python /** Holds if `notimpl` refers to `NotImplemented` or `NotImplemented()` in the `raise` statement */ From f2dd96ecf4bb1daec8593f83697c1615e395419b Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Thu, 17 Jul 2025 22:08:01 +0100 Subject: [PATCH 3/4] Update python/ql/src/Exceptions/NotImplementedIsNotAnException.qhelp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- python/ql/src/Exceptions/NotImplementedIsNotAnException.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ql/src/Exceptions/NotImplementedIsNotAnException.qhelp b/python/ql/src/Exceptions/NotImplementedIsNotAnException.qhelp index c89a8806dea6..53bf6d51746a 100644 --- a/python/ql/src/Exceptions/NotImplementedIsNotAnException.qhelp +++ b/python/ql/src/Exceptions/NotImplementedIsNotAnException.qhelp @@ -16,7 +16,7 @@ Code that is not intended to be called should raise NotImplementedError

    If a NotImplementedError is intended to be raised, replace the use of NotImplemented -with that. If NotImplemented is intended to be returned rather than raised, replace the raise with return NotImplemented +with that. If NotImplemented is intended to be returned rather than raised, replace the raise with return NotImplemented

    From 6d33a7ec709acf414b9678b57ddad938651ec956 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Thu, 17 Jul 2025 22:25:18 +0100 Subject: [PATCH 4/4] Update test output --- .../Exceptions/general/NotImplementedIsNotAnException.expected | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ql/test/query-tests/Exceptions/general/NotImplementedIsNotAnException.expected b/python/ql/test/query-tests/Exceptions/general/NotImplementedIsNotAnException.expected index c9fa73e7c127..7c5ee490b4e6 100644 --- a/python/ql/test/query-tests/Exceptions/general/NotImplementedIsNotAnException.expected +++ b/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? |