Skip to content

Provide hint when Union type with None may not have been narrowed #17178

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

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Provide hint when Union type with None may not have been narrowed #17178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
80f9788
added new circumstance of having NONE as the second object of a union…
Apr 23, 2024
0c28fad
added new circumstance of having NONE as the second object of a union…
Apr 23, 2024
30493ba
Add back new circumstance of having NONE as the second object of a union
angelawuuu Apr 24, 2024
7f4ea67
add note to use None guard when an an error complaining about None ty…
angelawuuu Apr 24, 2024
9b15594
uses actual variable name in note to use None guard when error compla…
angelawuuu Apr 24, 2024
b743fa1
remove extraneous parenthesis around if block
angelawuuu Apr 24, 2024
2a2d50c
fixed issue with getting variable name for None guard hint
angelawuuu Apr 24, 2024
f4b905a
fixed variable name when expression is a NameExpr for adding None gua…
angelawuuu Apr 24, 2024
70d5669
Fixed variable name for hint if context.expr is an instance of NameExpr
angelawuuu Apr 24, 2024
a65ac35
updated tests to provide note to use None guard when accessing attrib…
angelawuuu Apr 24, 2024
f31cf9c
Added test case that checks for note when a union type with None does…
angelawuuu Apr 24, 2024
ab28935
fixed linter issues
angelawuuu Apr 25, 2024
a102f95
Added checks that context is a MemberExpr before accessing its expr a…
angelawuuu Apr 25, 2024
310e619
Merge branch 'master' into union
angelawuuu Apr 29, 2024
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
16 changes: 16 additions & 0 deletions mypy/messages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
Expression,
FuncDef,
IndexExpr,
MemberExpr,
MypyFile,
NameExpr,
ReturnStmt,
Expand All @@ -48,6 +49,7 @@
SymbolTable,
TypeInfo,
Var,
get_member_expr_fullname,
reverse_builtin_aliases,
)
from mypy.operators import op_methods, op_methods_to_symbols
Expand Down Expand Up @@ -529,6 +531,20 @@ def has_no_attr(
context,
code=codes.UNION_ATTR,
)
if typ_format == '"None"':
if isinstance(context, NameExpr):
var_name = f" {context.name}"
elif isinstance(context, MemberExpr) and isinstance(context.expr, NameExpr):
var_name = f" {context.expr.name}"
elif isinstance(context, MemberExpr) and isinstance(context.expr, MemberExpr):
var_name = f" {get_member_expr_fullname(context.expr)}"
Copy link
Collaborator

@brianschubert brianschubert Sep 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This branch has some hiccups if the expression isn't strictly a dotted member expression:

class A:
    x: int | None

class B:
    a: A

[A()][0].x.bit_count()  # N: You can use "if None is not None" to guard against a None value
B().a.x.bit_count()     # N: You can use "if None.x is not None" to guard against a None value

I think two adjustments would be needed:

  1. check for get_member_expr_fullname returning None (and don't emit the note if it does)
  2. fix the case where get_member_expr_fullname returns a string with an embedded "None" (edit: fixed in #17848)

else:
var_name = " <variable name>"
self.note(
f'You can use "if{var_name} is not None" to guard against a None value',
context,
code=codes.UNION_ATTR,
)
return codes.UNION_ATTR
elif isinstance(original_type, TypeVarType):
bound = get_proper_type(original_type.upper_bound)
Expand Down
6 changes: 4 additions & 2 deletions test-data/unit/check-inference.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2828,15 +2828,17 @@ class C:
a = None # E: Need type annotation for "a" (hint: "a: Optional[<type>] = ...")

def f(self, x) -> None:
C.a.y # E: Item "None" of "Optional[Any]" has no attribute "y"
C.a.y # E: Item "None" of "Optional[Any]" has no attribute "y" \
# N: You can use "if C.a is not None" to guard against a None value

[case testLocalPartialTypesAccessPartialNoneAttribute2]
# flags: --local-partial-types
class C:
a = None # E: Need type annotation for "a" (hint: "a: Optional[<type>] = ...")

def f(self, x) -> None:
self.a.y # E: Item "None" of "Optional[Any]" has no attribute "y"
self.a.y # E: Item "None" of "Optional[Any]" has no attribute "y" \
# N: You can use "if self.a is not None" to guard against a None value

-- Special case for assignment to '_'
-- ----------------------------------
Expand Down
6 changes: 4 additions & 2 deletions test-data/unit/check-optional.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,8 @@ if int():
if int():
x = f('x') # E: Argument 1 to "f" has incompatible type "str"; expected "int"

x.x = 1 # E: Item "None" of "Optional[Node[int]]" has no attribute "x"
x.x = 1 # E: Item "None" of "Optional[Node[int]]" has no attribute "x" \
# N: You can use "if x is not None" to guard against a None value
if x is not None:
x.x = 1 # OK here

Expand Down Expand Up @@ -617,7 +618,8 @@ A = None # type: Any
class C(A):
pass
x = None # type: Optional[C]
x.foo() # E: Item "None" of "Optional[C]" has no attribute "foo"
x.foo() # E: Item "None" of "Optional[C]" has no attribute "foo" \
# N: You can use "if x is not None" to guard against a None value

[case testIsinstanceAndOptionalAndAnyBase]
from typing import Any, Optional
Expand Down
16 changes: 14 additions & 2 deletions test-data/unit/check-unions.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,16 @@ if int():

[builtins fixtures/isinstance.pyi]

[case testUnionAttributeAccessWithNoneItem]
from typing import Union

class A:
def foo(self) -> int: pass

def f(x: Union[A, None]) -> int:
return x.foo() # E: Item "None" of "Optional[A]" has no attribute "foo" \
# N: You can use "if x is not None" to guard against a None value

[case testUnionMethodCalls]
from typing import Union

Expand Down Expand Up @@ -935,7 +945,8 @@ a: Any
d: Dict[str, Tuple[List[Tuple[str, str]], str]]
x, _ = d.get(a, (None, None))

for y in x: pass # E: Item "None" of "Optional[List[Tuple[str, str]]]" has no attribute "__iter__" (not iterable)
for y in x: pass # E: Item "None" of "Optional[List[Tuple[str, str]]]" has no attribute "__iter__" (not iterable) \
# N: You can use "if x is not None" to guard against a None value
if x:
for s, t in x:
reveal_type(s) # N: Revealed type is "builtins.str"
Expand All @@ -950,7 +961,8 @@ x = None
d: Dict[str, Tuple[List[Tuple[str, str]], str]]
x, _ = d.get(a, (None, None))

for y in x: pass # E: Item "None" of "Optional[List[Tuple[str, str]]]" has no attribute "__iter__" (not iterable)
for y in x: pass # E: Item "None" of "Optional[List[Tuple[str, str]]]" has no attribute "__iter__" (not iterable) \
# N: You can use "if x is not None" to guard against a None value
if x:
for s, t in x:
reveal_type(s) # N: Revealed type is "builtins.str"
Expand Down
Loading