Skip to content
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

Jump to bottom

Preserve implicitly exported types via attribute access #16129

Merged
merged 5 commits into from Sep 19, 2023
Merged

Preserve implicitly exported types via attribute access #16129

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
64157f1
Preserve implicitly exported types via attribute access
hauntsaninja Sep 16, 2023
3cd1252
.
hauntsaninja Sep 17, 2023
6fa6d29
Merge remote-tracking branch 'upstream/master' into jeopardy
hauntsaninja Sep 17, 2023
6b9b503
better fixture
hauntsaninja Sep 17, 2023
f8b1f0a
getattr test
hauntsaninja Sep 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion mypy/checkmember.py
View file
Open in desktop
Expand Up @@ -24,6 +24,7 @@
FuncDef,
IndexExpr,
MypyFile,
NameExpr,
OverloadedFuncDef,
SymbolNode,
SymbolTable,
Expand Down Expand Up @@ -608,7 +609,19 @@ def analyze_member_var_access(
mx.msg.undefined_in_superclass(name, mx.context)
return AnyType(TypeOfAny.from_error)
else:
return report_missing_attribute(mx.original_type, itype, name, mx)
ret = report_missing_attribute(mx.original_type, itype, name, mx)
# Avoid paying double jeopardy if we can't find the member due to --no-implicit-reexport
if (
mx.module_symbol_table is not None
and name in mx.module_symbol_table
and not mx.module_symbol_table[name].module_public
):
v = mx.module_symbol_table[name].node
e = NameExpr(name)
e.set_line(mx.context)
e.node = v
return mx.chk.expr_checker.analyze_ref_expr(e, lvalue=mx.is_lvalue)
return ret
Copy link
Member

Choose a reason for hiding this comment

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

Ideally I would put all this logic in checkexpr.py. We should try to start making checkmember.py more self-consistent. I am still hoping to refactor everything to resolve #7724

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, doesn't moving this out of checkmember.py mean we're more likely to not handle hidden attributes elsewhere?

i.e. is the following diff what you meant?

diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py
index f46c8cb15..ac4e29469 100644
--- a/mypy/checkexpr.py
+++ b/mypy/checkexpr.py
@@ -3178,14 +3178,28 @@ class ExpressionChecker(ExpressionVisitor[Type]):
             else:
                 is_self = False
 
+            if (
+                module_symbol_table is not None
+                and e.name in module_symbol_table
+                and not module_symbol_table[e.name].module_public
+            ):
+                # If we wouldn't be able to find the member due to --no-implicit-reexport,
+                # report an error, but try to get the correct type anyway.
+                v = module_symbol_table[e.name].node
+                ref = NameExpr(e.name)
+                ref.set_line(e)
+                ref.node = v
+                self.msg.has_no_attr(original_type, original_type, e.name, e, module_symbol_table)
+                return self.analyze_ref_expr(ref, lvalue=is_lvalue)
+
             member_type = analyze_member_access(
                 e.name,
                 original_type,
-                e,
-                is_lvalue,
-                False,
-                False,
-                self.msg,
+                context=e,
+                is_lvalue=is_lvalue,
+                is_super=False,
+                is_operator=False,
+                msg=self.msg,
                 original_type=original_type,
                 chk=self.chk,
                 in_literal_context=self.is_literal_context(),
diff --git a/mypy/checkmember.py b/mypy/checkmember.py
index 4316a5928..03c8f77ee 100644
--- a/mypy/checkmember.py
+++ b/mypy/checkmember.py
@@ -609,19 +609,13 @@ def analyze_member_var_access(
         mx.msg.undefined_in_superclass(name, mx.context)
         return AnyType(TypeOfAny.from_error)
     else:
-        ret = report_missing_attribute(mx.original_type, itype, name, mx)
-        # Avoid paying double jeopardy if we can't find the member due to --no-implicit-reexport
-        if (
-            mx.module_symbol_table is not None
-            and name in mx.module_symbol_table
-            and not mx.module_symbol_table[name].module_public
-        ):
-            v = mx.module_symbol_table[name].node
-            e = NameExpr(name)
-            e.set_line(mx.context)
-            e.node = v
-            return mx.chk.expr_checker.analyze_ref_expr(e, lvalue=mx.is_lvalue)
-        return ret
+        # We should have handled --no-implicit-reexport cases already
+        assert (
+            mx.module_symbol_table is None
+            or name not in mx.module_symbol_table
+            or mx.module_symbol_table[name].module_public
+        )
+        return report_missing_attribute(mx.original_type, itype, name, mx)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we are using analyze_member_access() in many places, so maybe it is not worth it. You can go ahead as is.



def check_final_member(name: str, info: TypeInfo, msg: MessageBuilder, ctx: Context) -> None:
Expand Down
13 changes: 10 additions & 3 deletions test-data/unit/check-flags.test
View file
Open in desktop
Expand Up @@ -1611,14 +1611,21 @@ from other_module_2 import a # E: Module "other_module_2" does not explicitly e
reveal_type(a) # N: Revealed type is "builtins.int"

import other_module_2
# TODO: this should also reveal builtins.int, see #13965
reveal_type(other_module_2.a) # E: "object" does not explicitly export attribute "a" [attr-defined] \
# N: Revealed type is "Any"
# N: Revealed type is "builtins.int"

from other_module_2 import b # E: Module "other_module_2" does not explicitly export attribute "b" [attr-defined]
reveal_type(b) # N: Revealed type is "def (a: builtins.int) -> builtins.str"

import other_module_2
reveal_type(other_module_2.b) # E: "object" does not explicitly export attribute "b" [attr-defined] \
hauntsaninja marked this conversation as resolved.
Show resolved Hide resolved
# N: Revealed type is "def (a: builtins.int) -> builtins.str"

[file other_module_1.py]
a = 5
def b(a: int) -> str: ...
[file other_module_2.py]
from other_module_1 import a
from other_module_1 import a, b

[case testNoImplicitReexportRespectsAll]
# flags: --no-implicit-reexport
Expand Down
3 changes: 2 additions & 1 deletion test-data/unit/check-modules.test
View file
Open in desktop
Expand Up @@ -1862,7 +1862,8 @@ import stub

reveal_type(stub.y) # N: Revealed type is "builtins.int"
reveal_type(stub.z) # E: Module "stub" does not explicitly export attribute "z" \
# N: Revealed type is "Any"
# N: Revealed type is "builtins.int"


[file stub.pyi]
from substub import y as y
Expand Down