Skip to content

Support conditional import in functions/classes #2304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gvanrossum merged 5 commits into from
Oct 28, 2016
Merged

Support conditional import in functions/classes #2304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c0c8dd9
Support conditional import in functions/classes
drvink Oct 23, 2016
d43fbd2
Suggested cleanups
drvink Oct 24, 2016
c2ca2db
Simplify code
drvink Oct 24, 2016
09ecc92
Added a comment and call sem.errors.{push,pop}_function()
gvanrossum Oct 28, 2016
8e45bab
Fix variable name
drvink Oct 28, 2016
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
98 changes: 71 additions & 27 deletions mypy/semanal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2557,6 +2557,9 @@ def is_func_scope(self) -> bool:
def is_class_scope(self) -> bool:
return self.type is not None and not self.is_func_scope()

def is_module_scope(self) -> bool:
return not (self.is_class_scope() or self.is_func_scope())

def add_symbol(self, name: str, node: SymbolTableNode,
context: Context) -> None:
if self.is_func_scope():
Expand Down Expand Up @@ -2739,14 +2742,16 @@ def visit_block(self, b: Block) -> None:
self.sem.block_depth[-1] -= 1

def visit_assignment_stmt(self, s: AssignmentStmt) -> None:
for lval in s.lvalues:
self.analyze_lvalue(lval, explicit_type=s.type is not None)
if self.sem.is_module_scope():
for lval in s.lvalues:
self.analyze_lvalue(lval, explicit_type=s.type is not None)

def visit_func_def(self, func: FuncDef) -> None:
sem = self.sem
func.is_conditional = sem.block_depth[-1] > 0
func._fullname = sem.qualified_name(func.name())
if func.name() in sem.globals:
at_module = sem.is_module_scope()

@gvanrossum gvanrossum Oct 24, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think it's worth a separate local variable to shorten the condition on the next line (and notice our line length limit is 99).

@drvink drvink Oct 24, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh, oops. The separation was indeed to make the line shorter (which mattered in some WIP copy) but is now unnecessary. I'll remove it.

@drvink drvink Oct 24, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

(In the simplified version, at_module still eliminates some redundancy, so it has returned.)

if at_module and func.name() in sem.globals:
# Already defined in this module.
original_sym = sem.globals[func.name()]
if original_sym.kind == UNBOUND_IMPORTED:
Expand All @@ -2761,25 +2766,41 @@ def visit_func_def(self, func: FuncDef) -> None:
# Report error.
sem.check_no_global(func.name(), func)
else:
sem.globals[func.name()] = SymbolTableNode(GDEF, func, sem.cur_mod_id)
if at_module:
sem.globals[func.name()] = SymbolTableNode(GDEF, func, sem.cur_mod_id)
# Also analyze the function body (in case there are conditional imports).
sem.function_stack.append(func)

@gvanrossum gvanrossum Oct 27, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So this is the key part of this diff (processing function bodies in the FirstPass). Could you add a comment drawing attention to it? Also, I wonder if it would be prudent to add matching calls to self.errors.push_function() and self.errors.pop_function()?

sem.errors.push_function(func.name())
sem.enter()
func.body.accept(self)
sem.leave()
sem.errors.pop_function()
sem.function_stack.pop()

def visit_overloaded_func_def(self, func: OverloadedFuncDef) -> None:
self.sem.check_no_global(func.name(), func, True)
kind = self.kind_by_scope()
if kind == GDEF:
self.sem.check_no_global(func.name(), func, True)
func._fullname = self.sem.qualified_name(func.name())
self.sem.globals[func.name()] = SymbolTableNode(GDEF, func,
self.sem.cur_mod_id)
if kind == GDEF:
self.sem.globals[func.name()] = SymbolTableNode(kind, func, self.sem.cur_mod_id)

def visit_class_def(self, cdef: ClassDef) -> None:
self.sem.check_no_global(cdef.name, cdef)
kind = self.kind_by_scope()
if kind == LDEF:
return
elif kind == GDEF:
self.sem.check_no_global(cdef.name, cdef)
cdef.fullname = self.sem.qualified_name(cdef.name)
info = TypeInfo(SymbolTable(), cdef, self.sem.cur_mod_id)
info.set_line(cdef.line, cdef.column)
cdef.info = info
self.sem.globals[cdef.name] = SymbolTableNode(GDEF, info,
self.sem.cur_mod_id)
if kind == GDEF:
self.sem.globals[cdef.name] = SymbolTableNode(kind, info, self.sem.cur_mod_id)
self.process_nested_classes(cdef)

def process_nested_classes(self, outer_def: ClassDef) -> None:
self.sem.enter_class(outer_def)
for node in outer_def.defs.body:
if isinstance(node, ClassDef):
node.info = TypeInfo(SymbolTable(), node, self.sem.cur_mod_id)
Expand All @@ -2791,20 +2812,28 @@ def process_nested_classes(self, outer_def: ClassDef) -> None:
symbol = SymbolTableNode(MDEF, node.info)
outer_def.info.names[node.name] = symbol
self.process_nested_classes(node)
elif isinstance(node, (ImportFrom, Import, ImportAll, IfStmt)):
node.accept(self)
self.sem.leave_class()

def visit_import_from(self, node: ImportFrom) -> None:
# We can't bind module names during the first pass, as the target module might be
# unprocessed. However, we add dummy unbound imported names to the symbol table so
# that we at least know that the name refers to a module.
node.is_top_level = True
at_module = self.sem.is_module_scope()
node.is_top_level = at_module
if not at_module:
return
for name, as_name in node.names:
imported_name = as_name or name
if imported_name not in self.sem.globals:
self.sem.add_symbol(imported_name, SymbolTableNode(UNBOUND_IMPORTED, None), node)

def visit_import(self, node: Import) -> None:
node.is_top_level = True
node.is_top_level = self.sem.is_module_scope()
# This is similar to visit_import_from -- see the comment there.
if not self.sem.is_module_scope():
return
for id, as_id in node.ids:
imported_id = as_id or id
if imported_id not in self.sem.globals:
Expand All @@ -2814,28 +2843,31 @@ def visit_import(self, node: Import) -> None:
self.sem.globals[imported_id] = SymbolTableNode(UNBOUND_IMPORTED, None)

def visit_import_all(self, node: ImportAll) -> None:
node.is_top_level = True
node.is_top_level = self.sem.is_module_scope()

def visit_while_stmt(self, s: WhileStmt) -> None:
s.body.accept(self)
if s.else_body:
s.else_body.accept(self)
if self.sem.is_module_scope():
s.body.accept(self)
if s.else_body:
s.else_body.accept(self)

def visit_for_stmt(self, s: ForStmt) -> None:
self.analyze_lvalue(s.index)
s.body.accept(self)
if s.else_body:
s.else_body.accept(self)
if self.sem.is_module_scope():
self.analyze_lvalue(s.index)
s.body.accept(self)
if s.else_body:
s.else_body.accept(self)

def visit_with_stmt(self, s: WithStmt) -> None:
for n in s.target:
if n:
self.analyze_lvalue(n)
s.body.accept(self)
if self.sem.is_module_scope():
for n in s.target:
if n:
self.analyze_lvalue(n)
s.body.accept(self)

def visit_decorator(self, d: Decorator) -> None:
d.var._fullname = self.sem.qualified_name(d.var.name())
self.sem.add_symbol(d.var.name(), SymbolTableNode(GDEF, d.var), d)
self.sem.add_symbol(d.var.name(), SymbolTableNode(self.kind_by_scope(), d.var), d)

def visit_if_stmt(self, s: IfStmt) -> None:
infer_reachability_of_if_statement(s, pyversion=self.pyversion, platform=self.platform)
Expand All @@ -2845,10 +2877,22 @@ def visit_if_stmt(self, s: IfStmt) -> None:
s.else_body.accept(self)

def visit_try_stmt(self, s: TryStmt) -> None:
self.sem.analyze_try_stmt(s, self, add_global=True)
if self.sem.is_module_scope():
self.sem.analyze_try_stmt(s, self, add_global=self.sem.is_module_scope())

def analyze_lvalue(self, lvalue: Lvalue, explicit_type: bool = False) -> None:
self.sem.analyze_lvalue(lvalue, add_global=True, explicit_type=explicit_type)
self.sem.analyze_lvalue(lvalue, add_global=self.sem.is_module_scope(),
explicit_type=explicit_type)

def kind_by_scope(self) -> int:
if self.sem.is_module_scope():
return GDEF
elif self.sem.is_class_scope():
return MDEF
elif self.sem.is_func_scope():
return LDEF
else:
assert False, "Couldn't determine scope"


class ThirdPass(TraverserVisitor):
Expand Down
15 changes: 14 additions & 1 deletion test-data/unit/check-unreachable-code.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ def foo() -> None:
[out]
main: note: In function "foo":

[case testSysVersionInfoInMethod]
[case testSysPlatformInMethod]
import sys
class C:
def foo(self) -> None:
Expand All @@ -385,6 +385,19 @@ class C:
[out]
main: note: In member "foo" of class "C":

[case testSysPlatformInFunctionImport]
import sys
def foo() -> None:
if sys.platform != 'fictional':
import a
else:
import b as a
a.x
[file a.py]
x = 1
[builtins fixtures/ops.pyi]
[out]

[case testCustomSysVersionInfo]
# flags: --python-version 3.2
import sys
Expand Down