From 2a698f87c02a43d4489e30481e9def14ed4b4431 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Thu, 10 Mar 2016 22:00:57 -0800 Subject: [PATCH] Warn against reusing exception names after the except: block on Python 3 --- pyflakes/checker.py | 31 ++++- pyflakes/test/test_undefined_names.py | 169 ++++++++++++++++++++++++++ 2 files changed, 195 insertions(+), 5 deletions(-) diff --git a/pyflakes/checker.py b/pyflakes/checker.py index 76dc137d..5ff14800 100644 --- a/pyflakes/checker.py +++ b/pyflakes/checker.py @@ -358,7 +358,7 @@ def getNodeName(node): # Returns node.id, or node.name, or None if hasattr(node, 'id'): # One of the many nodes with an id return node.id - if hasattr(node, 'name'): # a ExceptHandler node + if hasattr(node, 'name'): # an ExceptHandler node return node.name @@ -1173,8 +1173,29 @@ def TRY(self, node): TRYEXCEPT = TRY def EXCEPTHANDLER(self, node): - # 3.x: in addition to handling children, we must handle the name of - # the exception, which is not a Name node, but a simple string. - if isinstance(node.name, str): - self.handleNodeStore(node) + if PY2 or node.name is None: + self.handleChildren(node) + return + + # 3.x: the name of the exception, which is not a Name node, but + # a simple string, creates a local that is only bound within the scope + # of the except: block. + + for scope in self.scopeStack[::-1]: + if node.name in scope: + is_name_previously_defined = True + break + else: + is_name_previously_defined = False + + self.handleNodeStore(node) self.handleChildren(node) + if not is_name_previously_defined: + # See discussion on https://github.com/pyflakes/pyflakes/pull/59. + + # We're removing the local name since it's being unbound + # after leaving the except: block and it's always unbound + # if the except: block is never entered. This will cause an + # "undefined name" error raised if the checked code tries to + # use the name afterwards. + del self.scope[node.name] diff --git a/pyflakes/test/test_undefined_names.py b/pyflakes/test/test_undefined_names.py index 5653b915..9adc4c1f 100644 --- a/pyflakes/test/test_undefined_names.py +++ b/pyflakes/test/test_undefined_names.py @@ -22,6 +22,175 @@ def test_undefinedInListComp(self): ''', m.UndefinedName) + @skipIf(version_info < (3,), + 'in Python 2 exception names stay bound after the except: block') + def test_undefinedExceptionName(self): + """Exception names can't be used after the except: block.""" + self.flakes(''' + try: + raise ValueError('ve') + except ValueError as exc: + pass + exc + ''', + m.UndefinedName) + + def test_namesDeclaredInExceptBlocks(self): + """Locals declared in except: blocks can be used after the block. + + This shows the example in test_undefinedExceptionName is + different.""" + self.flakes(''' + try: + raise ValueError('ve') + except ValueError as exc: + e = exc + e + ''') + + @skip('error reporting disabled due to false positives below') + def test_undefinedExceptionNameObscuringLocalVariable(self): + """Exception names obscure locals, can't be used after. + + Last line will raise UnboundLocalError on Python 3 after exiting + the except: block. Note next two examples for false positives to + watch out for.""" + self.flakes(''' + exc = 'Original value' + try: + raise ValueError('ve') + except ValueError as exc: + pass + exc + ''', + m.UndefinedName) + + @skipIf(version_info < (3,), + 'in Python 2 exception names stay bound after the except: block') + def test_undefinedExceptionNameObscuringLocalVariable2(self): + """Exception names are unbound after the `except:` block. + + Last line will raise UnboundLocalError on Python 3 but would print out + 've' on Python 2.""" + self.flakes(''' + try: + raise ValueError('ve') + except ValueError as exc: + pass + print(exc) + exc = 'Original value' + ''', + m.UndefinedName) + + def test_undefinedExceptionNameObscuringLocalVariableFalsePositive1(self): + """Exception names obscure locals, can't be used after. Unless. + + Last line will never raise UnboundLocalError because it's only + entered if no exception was raised.""" + self.flakes(''' + exc = 'Original value' + try: + raise ValueError('ve') + except ValueError as exc: + print('exception logged') + raise + exc + ''') + + def test_undefinedExceptionNameObscuringLocalVariableFalsePositive2(self): + """Exception names obscure locals, can't be used after. Unless. + + Last line will never raise UnboundLocalError because `error` is + only falsy if the `except:` block has not been entered.""" + self.flakes(''' + exc = 'Original value' + error = None + try: + raise ValueError('ve') + except ValueError as exc: + error = 'exception logged' + if error: + print(error) + else: + exc + ''') + + @skip('error reporting disabled due to false positives below') + def test_undefinedExceptionNameObscuringGlobalVariable(self): + """Exception names obscure globals, can't be used after. + + Last line will raise UnboundLocalError on both Python 2 and + Python 3 because the existence of that exception name creates + a local scope placeholder for it, obscuring any globals, etc.""" + self.flakes(''' + exc = 'Original value' + def func(): + try: + pass # nothing is raised + except ValueError as exc: + pass # block never entered, exc stays unbound + exc + ''', + m.UndefinedLocal) + + @skip('error reporting disabled due to false positives below') + def test_undefinedExceptionNameObscuringGlobalVariable2(self): + """Exception names obscure globals, can't be used after. + + Last line will raise NameError on Python 3 because the name is + locally unbound after the `except:` block, even if it's + nonlocal. We should issue an error in this case because code + only working correctly if an exception isn't raised, is invalid. + Unless it's explicitly silenced, see false positives below.""" + self.flakes(''' + exc = 'Original value' + def func(): + global exc + try: + raise ValueError('ve') + except ValueError as exc: + pass # block never entered, exc stays unbound + exc + ''', + m.UndefinedLocal) + + def test_undefinedExceptionNameObscuringGlobalVariableFalsePositive1(self): + """Exception names obscure globals, can't be used after. Unless. + + Last line will never raise NameError because it's only entered + if no exception was raised.""" + self.flakes(''' + exc = 'Original value' + def func(): + global exc + try: + raise ValueError('ve') + except ValueError as exc: + print('exception logged') + raise + exc + ''') + + def test_undefinedExceptionNameObscuringGlobalVariableFalsePositive2(self): + """Exception names obscure globals, can't be used after. Unless. + + Last line will never raise NameError because `error` is only + falsy if the `except:` block has not been entered.""" + self.flakes(''' + exc = 'Original value' + def func(): + global exc + error = None + try: + raise ValueError('ve') + except ValueError as exc: + error = 'exception logged' + if error: + print(error) + else: + exc + ''') + def test_functionsNeedGlobalScope(self): self.flakes(''' class a: