diff --git a/python/ql/src/Exceptions/NotImplementedIsNotAnException.qhelp b/python/ql/src/Exceptions/NotImplementedIsNotAnException.qhelp index 3bf09bbfab07..a7930a8d2c21 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?" 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? |