Skip to content

Support for import inside class #2077

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
merged 4 commits into from
Sep 3, 2016
Merged

Support for import inside class #2077

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e16a77f
Add test (for now it fails).
Aug 18, 2016
61bdd77
Silence the crash, but leave an unsatisfying type (module).
Aug 10, 2016
afd4db1
Tentative fix.
Sep 1, 2016
a5639ce
Update comments in response to code review.
Sep 2, 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
6 changes: 5 additions & 1 deletion mypy/checkmember.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
Overloaded, TypeVarType, TypeTranslator, UnionType, PartialType,
DeletedType, NoneTyp, TypeType
)
from mypy.nodes import TypeInfo, FuncBase, Var, FuncDef, SymbolNode, Context
from mypy.nodes import TypeInfo, FuncBase, Var, FuncDef, SymbolNode, Context, MypyFile
from mypy.nodes import ARG_POS, ARG_STAR, ARG_STAR2, OpExpr, ComparisonExpr
from mypy.nodes import function_type, Decorator, OverloadedFuncDef
from mypy.messages import MessageBuilder
Expand Down Expand Up @@ -349,6 +349,10 @@ def analyze_class_attribute_access(itype: Instance,
if isinstance(node.node, TypeInfo):
return type_object_type(node.node, builtin_type)

if isinstance(node.node, MypyFile):
# Reference to a module object.
return builtin_type('builtins.module')

if is_decorated:
# TODO: Return type of decorated function. This is quick hack to work around #998.
return AnyType()
Expand Down
18 changes: 18 additions & 0 deletions mypy/semanal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2107,6 +2107,9 @@ def visit_member_expr(self, expr: MemberExpr) -> None:
base.accept(self)
# Bind references to module attributes.
if isinstance(base, RefExpr) and base.kind == MODULE_REF:
# This branch handles the case foo.bar where foo is a module.
# In this case base.node is the module's MypyFile and we look up
# bar in its namespace. This must be done for all types of bar.
file = cast(MypyFile, base.node)
n = file.names.get(expr.name, None) if file is not None else None
if n:
Expand All @@ -2128,6 +2131,21 @@ def visit_member_expr(self, expr: MemberExpr) -> None:
if full_name in obsolete_name_mapping:
self.fail("Module has no attribute %r (it's now called %r)" % (
expr.name, obsolete_name_mapping[full_name]), expr)
elif isinstance(base, RefExpr) and isinstance(base.node, TypeInfo):
Copy link
Collaborator

Choose a reason for hiding this comment

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

As far as I can understand, the if statement above seems to be looking for module members also. What's the difference?

Copy link
Member Author

Choose a reason for hiding this comment

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

The previous block looks for foo.bar where foo is a module (and bar is anything defined in foo.py). The new block here looks for C.bar where C is some class (C could also be a more complex expression, like foo.C), but only of bar is a module object. For other types of things in C (e.g. methods or class variables) other existing code paths do the right thing, but those other paths (in particular in checkmember.py) don't get it right if bar happens to be a module. Without my small change there, the test code would crash (see original issue, #1839). But the code here (semanal.py) is required to create the right symbol table entry so that looking up things in C.bar will actually work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool, that makes sense. Would you mind adding short comments to the two branches saying this?

# This branch handles the case C.bar where C is a class
# and bar is a module resulting from `import bar` inside
# class C. Here base.node is a TypeInfo, and again we
# look up the name in its namespace. This is done only
# when bar is a module; other things (e.g. methods)
# are handled by other code in checkmember.
n = base.node.names.get(expr.name)
if n is not None and n.kind == MODULE_REF:
n = self.normalize_type_alias(n, expr)
if not n:
return
expr.kind = n.kind
expr.fullname = n.fullname
expr.node = n.node

def visit_op_expr(self, expr: OpExpr) -> None:
expr.left.accept(self)
Expand Down
9 changes: 9 additions & 0 deletions test-data/unit/check-modules.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -974,6 +974,15 @@ from a import x
x = 0
[out]

[case testImportInClass]
class C:
import foo
reveal_type(C.foo.bar) # E: Revealed type is 'builtins.int'
[file foo.py]
bar = 0
[builtins fixtures/module.pyi]
[out]


-- Test stability under import cycles
-- ----------------------------------
Expand Down