Skip to content
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 52 additions & 26 deletions pyflakes/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,10 +310,12 @@ class Binding(object):
the node that this binding was last used.
"""

def __init__(self, name, source):
def __init__(self, name, source, during_type_checking):
self.name = name
self.source = source
self.used = False
assert isinstance(during_type_checking, bool)
self.during_type_checking = during_type_checking
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe for_annotations is a more descriptive name here? (idk, I'm bikeshedding on this, feel free to ignore this suggestion, it just feels a little like the meaning of the name gets lost a bit)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine; takes a couple of seconds to change.


def __str__(self):
return self.name
Expand All @@ -338,7 +340,7 @@ class Builtin(Definition):
"""A definition created for all Python builtins."""

def __init__(self, name):
super(Builtin, self).__init__(name, None)
super(Builtin, self).__init__(name, None, during_type_checking=False)

def __repr__(self):
return '<%s object %r at 0x%x>' % (self.__class__.__name__,
Expand Down Expand Up @@ -380,10 +382,11 @@ class Importation(Definition):
@type fullName: C{str}
"""

def __init__(self, name, source, full_name=None):
def __init__(self, name, source, during_type_checking, full_name=None):
self.fullName = full_name or name
self.redefined = []
super(Importation, self).__init__(name, source)
super(Importation, self).__init__(name, source,
during_type_checking=during_type_checking)

def redefines(self, other):
if isinstance(other, SubmoduleImportation):
Expand Down Expand Up @@ -428,11 +431,12 @@ class SubmoduleImportation(Importation):
name is also the same, to avoid false positives.
"""

def __init__(self, name, source):
def __init__(self, name, source, during_type_checking):
# A dot should only appear in the name when it is a submodule import
assert '.' in name and (not source or isinstance(source, ast.Import))
package_name = name.split('.')[0]
super(SubmoduleImportation, self).__init__(package_name, source)
super(SubmoduleImportation, self).__init__(
package_name, source, during_type_checking=during_type_checking)
self.fullName = name

def redefines(self, other):
Expand All @@ -450,7 +454,7 @@ def source_statement(self):

class ImportationFrom(Importation):

def __init__(self, name, source, module, real_name=None):
def __init__(self, name, source, module, during_type_checking, real_name=None):
self.module = module
self.real_name = real_name or name

Expand All @@ -459,7 +463,8 @@ def __init__(self, name, source, module, real_name=None):
else:
full_name = module + '.' + self.real_name

super(ImportationFrom, self).__init__(name, source, full_name)
super(ImportationFrom, self).__init__(name, source, full_name=full_name,
during_type_checking=during_type_checking)

def __str__(self):
"""Return import full name with alias."""
Expand All @@ -481,8 +486,9 @@ def source_statement(self):
class StarImportation(Importation):
"""A binding created by a 'from x import *' statement."""

def __init__(self, name, source):
super(StarImportation, self).__init__('*', source)
def __init__(self, name, source, during_type_checking):
super(StarImportation, self).__init__('*', source,
during_type_checking=during_type_checking)
# Each star importation needs a unique name, and
# may not be the module name otherwise it will be deemed imported
self.name = name + '.*'
Expand All @@ -508,14 +514,17 @@ class FutureImportation(ImportationFrom):
"""

def __init__(self, name, source, scope):
super(FutureImportation, self).__init__(name, source, '__future__')
super(FutureImportation, self).__init__(name, source, '__future__',
during_type_checking=False)
self.used = (scope, source)


class Argument(Binding):
"""
Represents binding a name as an argument.
"""
def __init__(self, name, source):
super(Argument, self).__init__(name, source, during_type_checking=False)


class Assignment(Binding):
Expand Down Expand Up @@ -551,7 +560,7 @@ class ExportBinding(Binding):
C{__all__} will not have an unused import warning reported for them.
"""

def __init__(self, name, source, scope):
def __init__(self, name, source, scope, during_type_checking):
if '__all__' in scope and isinstance(source, ast.AugAssign):
self.names = list(scope['__all__'].names)
else:
Expand Down Expand Up @@ -582,7 +591,7 @@ def _add_to_names(container):
# If not list concatenation
else:
break
super(ExportBinding, self).__init__(name, source)
super(ExportBinding, self).__init__(name, source, during_type_checking)


class Scope(dict):
Expand Down Expand Up @@ -829,6 +838,7 @@ class Checker(object):
_in_annotation = False
_in_typing_literal = False
_in_deferred = False
_in_type_checking = False

builtIns = set(builtin_vars).union(_MAGIC_GLOBALS)
_customBuiltIns = os.environ.get('PYFLAKES_BUILTINS')
Expand Down Expand Up @@ -1165,6 +1175,10 @@ def handleNodeLoad(self, node):
scope[n.fullName].used = (self.scope, node)
except KeyError:
pass
if n.during_type_checking and not self._in_annotation:
# Only defined during type-checking; this does not count. Real code
# (not an annotation) using this binding will not work.
continue
except KeyError:
pass
else:
Expand Down Expand Up @@ -1225,13 +1239,14 @@ def handleNodeStore(self, node):
if isinstance(parent_stmt, (FOR_TYPES, ast.comprehension)) or (
parent_stmt != node._pyflakes_parent and
not self.isLiteralTupleUnpacking(parent_stmt)):
binding = Binding(name, node)
binding = Binding(name, node, during_type_checking=self._in_type_checking)
elif name == '__all__' and isinstance(self.scope, ModuleScope):
binding = ExportBinding(name, node._pyflakes_parent, self.scope)
binding = ExportBinding(name, node._pyflakes_parent, self.scope,
during_type_checking=self._in_type_checking)
elif PY2 and isinstance(getattr(node, 'ctx', None), ast.Param):
binding = Argument(name, self.getScopeNode(node))
else:
binding = Assignment(name, node)
binding = Assignment(name, node, during_type_checking=self._in_type_checking)
self.addBinding(node, binding)

def handleNodeDelete(self, node):
Expand Down Expand Up @@ -1833,7 +1848,12 @@ def DICT(self, node):
def IF(self, node):
if isinstance(node.test, ast.Tuple) and node.test.elts != []:
self.report(messages.IfTuple, node)

prev = self._in_type_checking
if _is_typing(node.test, 'TYPE_CHECKING', self.scopeStack):
self._in_type_checking = True
self.handleChildren(node)
self._in_type_checking = prev

IFEXP = IF

Expand All @@ -1854,7 +1874,8 @@ def GLOBAL(self, node):

# One 'global' statement can bind multiple (comma-delimited) names.
for node_name in node.names:
node_value = Assignment(node_name, node)
node_value = Assignment(node_name, node,
during_type_checking=self._in_type_checking)

# Remove UndefinedName messages already reported for this name.
# TODO: if the global is not used in this scope, it does not
Expand Down Expand Up @@ -1956,7 +1977,8 @@ def FUNCTIONDEF(self, node):
for deco in node.decorator_list:
self.handleNode(deco, node)
self.LAMBDA(node)
self.addBinding(node, FunctionDefinition(node.name, node))
self.addBinding(node, FunctionDefinition(
node.name, node, during_type_checking=self._in_type_checking))
# doctest does not process doctest within a doctest,
# or in nested functions.
if (self.withDoctest and
Expand Down Expand Up @@ -2081,7 +2103,8 @@ def CLASSDEF(self, node):
for stmt in node.body:
self.handleNode(stmt, node)
self.popScope()
self.addBinding(node, ClassDefinition(node.name, node))
self.addBinding(node, ClassDefinition(
node.name, node, during_type_checking=self._in_type_checking))

def AUGASSIGN(self, node):
self.handleNodeLoad(node.target)
Expand Down Expand Up @@ -2116,10 +2139,12 @@ def TUPLE(self, node):
def IMPORT(self, node):
for alias in node.names:
if '.' in alias.name and not alias.asname:
importation = SubmoduleImportation(alias.name, node)
importation = SubmoduleImportation(
alias.name, node, during_type_checking=self._in_type_checking)
else:
name = alias.asname or alias.name
importation = Importation(name, node, alias.name)
importation = Importation(name, node, full_name=alias.name,
during_type_checking=self._in_type_checking)
self.addBinding(node, importation)

def IMPORTFROM(self, node):
Expand All @@ -2144,16 +2169,17 @@ def IMPORTFROM(self, node):
elif alias.name == '*':
# Only Python 2, local import * is a SyntaxWarning
if not PY2 and not isinstance(self.scope, ModuleScope):
self.report(messages.ImportStarNotPermitted,
node, module)
self.report(messages.ImportStarNotPermitted, node, module)
continue

self.scope.importStarred = True
self.report(messages.ImportStarUsed, node, module)
importation = StarImportation(module, node)
importation = StarImportation(
module, node, during_type_checking=self._in_type_checking)
else:
importation = ImportationFrom(name, node,
module, alias.name)
importation = ImportationFrom(
name, node, module, real_name=alias.name,
during_type_checking=self._in_type_checking)
self.addBinding(node, importation)

def TRY(self, node):
Expand Down
68 changes: 53 additions & 15 deletions pyflakes/test/test_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,78 +14,86 @@
class TestImportationObject(TestCase):

def test_import_basic(self):
binding = Importation('a', None, 'a')
binding = Importation('a', None, during_type_checking=False, full_name='a')
assert binding.source_statement == 'import a'
assert str(binding) == 'a'

def test_import_as(self):
binding = Importation('c', None, 'a')
binding = Importation('c', None, during_type_checking=False, full_name='a')
assert binding.source_statement == 'import a as c'
assert str(binding) == 'a as c'

def test_import_submodule(self):
binding = SubmoduleImportation('a.b', None)
binding = SubmoduleImportation('a.b', None, during_type_checking=False)
assert binding.source_statement == 'import a.b'
assert str(binding) == 'a.b'

def test_import_submodule_as(self):
# A submodule import with an as clause is not a SubmoduleImportation
binding = Importation('c', None, 'a.b')
binding = Importation('c', None, during_type_checking=False, full_name='a.b')
assert binding.source_statement == 'import a.b as c'
assert str(binding) == 'a.b as c'

def test_import_submodule_as_source_name(self):
binding = Importation('a', None, 'a.b')
binding = Importation('a', None, during_type_checking=False, full_name='a.b')
assert binding.source_statement == 'import a.b as a'
assert str(binding) == 'a.b as a'

def test_importfrom_relative(self):
binding = ImportationFrom('a', None, '.', 'a')
binding = ImportationFrom('a', None, '.',
during_type_checking=False, real_name='a')
assert binding.source_statement == 'from . import a'
assert str(binding) == '.a'

def test_importfrom_relative_parent(self):
binding = ImportationFrom('a', None, '..', 'a')
binding = ImportationFrom('a', None, '..',
during_type_checking=False, real_name='a')
assert binding.source_statement == 'from .. import a'
assert str(binding) == '..a'

def test_importfrom_relative_with_module(self):
binding = ImportationFrom('b', None, '..a', 'b')
binding = ImportationFrom('b', None, '..a',
during_type_checking=False, real_name='b')
assert binding.source_statement == 'from ..a import b'
assert str(binding) == '..a.b'

def test_importfrom_relative_with_module_as(self):
binding = ImportationFrom('c', None, '..a', 'b')
binding = ImportationFrom('c', None, '..a',
during_type_checking=False, real_name='b')
assert binding.source_statement == 'from ..a import b as c'
assert str(binding) == '..a.b as c'

def test_importfrom_member(self):
binding = ImportationFrom('b', None, 'a', 'b')
binding = ImportationFrom('b', None, 'a',
during_type_checking=False, real_name='b')
assert binding.source_statement == 'from a import b'
assert str(binding) == 'a.b'

def test_importfrom_submodule_member(self):
binding = ImportationFrom('c', None, 'a.b', 'c')
binding = ImportationFrom('c', None, 'a.b',
during_type_checking=False, real_name='c')
assert binding.source_statement == 'from a.b import c'
assert str(binding) == 'a.b.c'

def test_importfrom_member_as(self):
binding = ImportationFrom('c', None, 'a', 'b')
binding = ImportationFrom('c', None, 'a',
during_type_checking=False, real_name='b')
assert binding.source_statement == 'from a import b as c'
assert str(binding) == 'a.b as c'

def test_importfrom_submodule_member_as(self):
binding = ImportationFrom('d', None, 'a.b', 'c')
binding = ImportationFrom('d', None, 'a.b',
during_type_checking=False, real_name='c')
assert binding.source_statement == 'from a.b import c as d'
assert str(binding) == 'a.b.c as d'

def test_importfrom_star(self):
binding = StarImportation('a.b', None)
binding = StarImportation('a.b', None, during_type_checking=False)
assert binding.source_statement == 'from a.b import *'
assert str(binding) == 'a.b.*'

def test_importfrom_star_relative(self):
binding = StarImportation('.b', None)
binding = StarImportation('.b', None, during_type_checking=False)
assert binding.source_statement == 'from .b import *'
assert str(binding) == '.b.*'

Expand Down Expand Up @@ -1031,6 +1039,36 @@ def test_futureImportStar(self):
from __future__ import *
''', m.FutureFeatureNotDefined)

def test_ignoresTypingImports(self):
"""Ignores imports within 'if TYPE_CHECKING' checking normal code."""
self.flakes('''
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from a import b
b()
''', m.UndefinedName)

def test_ignoresTypingClassDefinition(self):
"""Ignores definitions within 'if TYPE_CHECKING' checking normal code."""
self.flakes('''
from typing import TYPE_CHECKING
if TYPE_CHECKING:
class T:
pass
t = T()
''', m.UndefinedName)

@skipIf(version_info < (3,), 'has type annotations')
def test_usesTypingImportsForAnnotations(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw, the new tests are using snake_case

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

"""Uses imports within 'if TYPE_CHECKING' checking annotations."""
self.flakes('''
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from a import b
def f() -> "b":
pass
''')


class TestSpecialAll(TestCase):
"""
Expand Down