From af45b41f52ec1727c2a84777af0c2e55d6b8d912 Mon Sep 17 00:00:00 2001 From: Scott Sanderson Date: Sun, 18 Feb 2018 18:21:26 -0500 Subject: Warn on raise NotImplemented(...) This is a common mistake for `raise NotImplementedError`. Since `NotImplemented` isn't an exception, it's never valid to raise it (barring strange circumstances where you've aliased `NotImplemented` in `__builtins__`). --- pyflakes/checker.py | 26 +++++++++++++++++++++++++- pyflakes/messages.py | 4 ++++ pyflakes/test/test_other.py | 17 +++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/pyflakes/checker.py b/pyflakes/checker.py index 9b39e6c..0ee6189 100644 --- a/pyflakes/checker.py +++ b/pyflakes/checker.py @@ -38,10 +38,17 @@ if PY2: def getNodeType(node_class): # workaround str.upper() which is locale-dependent return str(unicode(node_class.__name__).upper()) + + def get_raise_argument(node): + return node.type + else: def getNodeType(node_class): return node_class.__name__.upper() + def get_raise_argument(node): + return node.exc + # Silence `pyflakes` from reporting `undefined name 'unicode'` in Python 3. unicode = str @@ -138,6 +145,10 @@ def convert_to_value(item): return UnhandledKeyType() +def is_notimplemented_name_node(node): + return isinstance(node, ast.Name) and getNodeName(node) == 'NotImplemented' + + class Binding(object): """ Represents the binding of a value to a name. @@ -965,7 +976,7 @@ class Checker(object): # "stmt" type nodes DELETE = PRINT = FOR = ASYNCFOR = WHILE = IF = WITH = WITHITEM = \ - ASYNCWITH = ASYNCWITHITEM = RAISE = TRYFINALLY = EXEC = \ + ASYNCWITH = ASYNCWITHITEM = TRYFINALLY = EXEC = \ EXPR = ASSIGN = handleChildren PASS = ignore @@ -989,6 +1000,19 @@ class Checker(object): EQ = NOTEQ = LT = LTE = GT = GTE = IS = ISNOT = IN = NOTIN = \ MATMULT = ignore + def RAISE(self, node): + self.handleChildren(node) + + arg = get_raise_argument(node) + + if isinstance(arg, ast.Call): + if is_notimplemented_name_node(arg.func): + # Handle "raise NotImplemented(...)" + self.report(messages.RaiseNotImplemented, node) + elif is_notimplemented_name_node(arg): + # Handle "raise NotImplemented" + self.report(messages.RaiseNotImplemented, node) + # additional node types COMPREHENSION = KEYWORD = FORMATTEDVALUE = JOINEDSTR = handleChildren diff --git a/pyflakes/messages.py b/pyflakes/messages.py index 670f95f..eb87a72 100644 --- a/pyflakes/messages.py +++ b/pyflakes/messages.py @@ -239,3 +239,7 @@ class ForwardAnnotationSyntaxError(Message): def __init__(self, filename, loc, annotation): Message.__init__(self, filename, loc) self.message_args = (annotation,) + + +class RaiseNotImplemented(Message): + message = "'raise NotImplemented' should be 'raise NotImplementedError'" diff --git a/pyflakes/test/test_other.py b/pyflakes/test/test_other.py index 5bae7a0..c771ef6 100644 --- a/pyflakes/test/test_other.py +++ b/pyflakes/test/test_other.py @@ -1983,3 +1983,20 @@ class TestAsyncStatements(TestCase): self.flakes(''' a: 'a: "A"' ''', m.ForwardAnnotationSyntaxError) + + def test_raise_notimplemented(self): + self.flakes(''' + raise NotImplementedError("This is fine") + ''') + + self.flakes(''' + raise NotImplementedError + ''') + + self.flakes(''' + raise NotImplemented("This isn't gonna work") + ''', m.RaiseNotImplemented) + + self.flakes(''' + raise NotImplemented + ''', m.RaiseNotImplemented) -- cgit v1.2.1