diff --git a/NEWS.txt b/NEWS.txt index 776fb6f0..2d507b71 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -1,5 +1,7 @@ UNRELEASED: - Detect the declared encoding in Python 3. + - Do not report redefinition of import in a local scope, if the + global name is used elsewhere in the module. 0.8.0 (2014-03-22): - Adapt for the AST in Python 3.4. diff --git a/pyflakes/checker.py b/pyflakes/checker.py index 6e1f1d18..91661d41 100644 --- a/pyflakes/checker.py +++ b/pyflakes/checker.py @@ -82,8 +82,17 @@ def __repr__(self): self.source.lineno, id(self)) + def redefines(self, other): + return isinstance(other, Definition) and self.name == other.name -class Importation(Binding): + +class Definition(Binding): + """ + A binding that defines a function or a class. + """ + + +class Importation(Definition): """ A binding created by an import statement. @@ -93,9 +102,15 @@ class Importation(Binding): """ def __init__(self, name, source): self.fullName = name + self.redefined = [] name = name.split('.')[0] super(Importation, self).__init__(name, source) + def redefines(self, other): + if isinstance(other, Importation): + return self.fullName == other.fullName + return isinstance(other, Definition) and self.name == other.name + class Argument(Binding): """ @@ -103,12 +118,6 @@ class Argument(Binding): """ -class Definition(Binding): - """ - A binding that defines a function or a class. - """ - - class Assignment(Binding): """ Represents binding a name with an explicit assignment. @@ -323,6 +332,9 @@ def checkDeadScopes(self): if (isinstance(importation, Importation) and not importation.used and importation.name not in all_names): + for node in importation.redefined: + self.report(messages.RedefinedWhileUnused, + node, importation.name, importation.source) self.report(messages.UnusedImport, importation.source, importation.name) @@ -381,45 +393,33 @@ def differentForks(self, lnode, rnode): return True return False - def addBinding(self, node, value, reportRedef=True): + def addBinding(self, node, value): """ Called when a binding is altered. - `node` is the statement responsible for the change - - `value` is the optional new value, a Binding instance, associated - with the binding; if None, the binding is deleted if it exists. - - if `reportRedef` is True (default), rebinding while unused will be - reported. + - `value` is the new value, a Binding instance """ - redefinedWhileUnused = False - if not isinstance(self.scope, ClassScope): - for scope in self.scopeStack[::-1]: - existing = scope.get(value.name) - if (isinstance(existing, Importation) - and not existing.used - and (not isinstance(value, Importation) or - value.fullName == existing.fullName) - and reportRedef - and not self.differentForks(node, existing.source)): - redefinedWhileUnused = True + for scope in self.scopeStack[::-1]: + if value.name in scope: + break + existing = scope.get(value.name) + + if existing and not self.differentForks(node, existing.source): + + if scope is self.scope: + if (self.hasParent(value.source, ast.ListComp) and + not self.hasParent(existing.source, (ast.For, ast.ListComp))): + self.report(messages.RedefinedInListComp, + node, value.name, existing.source) + elif not existing.used and value.redefines(existing): self.report(messages.RedefinedWhileUnused, node, value.name, existing.source) - existing = self.scope.get(value.name) - if not redefinedWhileUnused and self.hasParent(value.source, ast.ListComp): - if (existing and reportRedef - and not self.hasParent(existing.source, (ast.For, ast.ListComp)) - and not self.differentForks(node, existing.source)): - self.report(messages.RedefinedInListComp, - node, value.name, existing.source) - - if (isinstance(existing, Definition) - and not existing.used - and not self.differentForks(node, existing.source)): - self.report(messages.RedefinedWhileUnused, - node, value.name, existing.source) - else: - self.scope[value.name] = value + elif isinstance(existing, Importation) and value.redefines(existing): + existing.redefined.append(node) + + self.scope[value.name] = value def getNodeHandler(self, node_class): try: @@ -766,7 +766,7 @@ def runFunction(): self.pushScope() for name in args: - self.addBinding(node, Argument(name, node), reportRedef=False) + self.addBinding(node, Argument(name, node)) if isinstance(node.body, list): # case for FunctionDefs for stmt in node.body: diff --git a/pyflakes/test/test_imports.py b/pyflakes/test/test_imports.py index 76853c7a..515def75 100644 --- a/pyflakes/test/test_imports.py +++ b/pyflakes/test/test_imports.py @@ -172,6 +172,39 @@ def fu(): pass ''', m.RedefinedWhileUnused, m.UnusedImport) + def test_redefinedInNestedFunctionTwice(self): + """ + Test that shadowing a global name with a nested function definition + generates a warning. + """ + self.flakes(''' + import fu + def bar(): + import fu + def baz(): + def fu(): + pass + ''', m.RedefinedWhileUnused, m.RedefinedWhileUnused, + m.UnusedImport, m.UnusedImport) + + def test_redefinedButUsedLater(self): + """ + Test that a global import which is redefined locally, + but used later in another scope does not generate a warning. + """ + self.flakes(''' + import unittest, transport + + class GetTransportTestCase(unittest.TestCase): + def test_get_transport(self): + transport = 'transport' + self.assertIsNotNone(transport) + + class TestTransportMethodArgs(unittest.TestCase): + def test_send_defaults(self): + transport.Transport() + ''') + def test_redefinedByClass(self): self.flakes(''' import fu @@ -214,7 +247,7 @@ def test_shadowedByParameter(self): import fu def fun(fu): print(fu) - ''', m.UnusedImport) + ''', m.UnusedImport, m.RedefinedWhileUnused) self.flakes(''' import fu @@ -433,7 +466,8 @@ def test_usedInListComp(self): self.flakes('import fu; [1 for _ in range(1) if fu]') def test_redefinedByListComp(self): - self.flakes('import fu; [1 for fu in range(1)]', m.RedefinedWhileUnused) + self.flakes('import fu; [1 for fu in range(1)]', + m.RedefinedInListComp) def test_usedInTryFinally(self): self.flakes(''' @@ -481,7 +515,9 @@ def test_usedInLambda(self): self.flakes('import fu; lambda: fu') def test_shadowedByLambda(self): - self.flakes('import fu; lambda fu: fu', m.UnusedImport) + self.flakes('import fu; lambda fu: fu', + m.UnusedImport, m.RedefinedWhileUnused) + self.flakes('import fu; lambda fu: fu\nfu()') def test_usedInSliceObj(self): self.flakes('import fu; "meow"[::fu]')