From 80f9788a4087e469e53d51102fcfefe86cbe7c50 Mon Sep 17 00:00:00 2001 From: kr321 Date: Mon, 22 Apr 2024 21:14:58 -0400 Subject: [PATCH 01/13] added new circumstance of having NONE as the second object of a union with warning message --- mypy/messages.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/mypy/messages.py b/mypy/messages.py index 199b7c42b11b..eac8f29f8150 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -522,12 +522,23 @@ def has_no_attr( type(item) == NoneType for item in original_type.items ): typ_format = '"None"' - self.fail( - 'Item {} of {} has no attribute "{}"{}'.format( - typ_format, orig_type_format, member, extra - ), - context, - code=codes.UNION_ATTR, + if typ_format == '"None"':( + self.fail( + 'Item {} of {} has no attribute "{}"{}. ADD HINT HERE'.format( + typ_format, orig_type_format, member, extra + ), + context, + code=codes.UNION_ATTR, + ) + ) + else:( + self.fail( + 'Item {} of {} has no attribute "{}"{}'.format( + typ_format, orig_type_format, member, extra + ), + context, + code=codes.UNION_ATTR, + ) ) return codes.UNION_ATTR elif isinstance(original_type, TypeVarType): From 0c28fadd7dccd3c9fdbf3014b42aec1d6f95d3d4 Mon Sep 17 00:00:00 2001 From: kr321 Date: Mon, 22 Apr 2024 21:21:57 -0400 Subject: [PATCH 02/13] added new circumstance of having NONE as the second object of a union with warning message --- mypy/messages.py | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/mypy/messages.py b/mypy/messages.py index eac8f29f8150..199b7c42b11b 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -522,23 +522,12 @@ def has_no_attr( type(item) == NoneType for item in original_type.items ): typ_format = '"None"' - if typ_format == '"None"':( - self.fail( - 'Item {} of {} has no attribute "{}"{}. ADD HINT HERE'.format( - typ_format, orig_type_format, member, extra - ), - context, - code=codes.UNION_ATTR, - ) - ) - else:( - self.fail( - 'Item {} of {} has no attribute "{}"{}'.format( - typ_format, orig_type_format, member, extra - ), - context, - code=codes.UNION_ATTR, - ) + self.fail( + 'Item {} of {} has no attribute "{}"{}'.format( + typ_format, orig_type_format, member, extra + ), + context, + code=codes.UNION_ATTR, ) return codes.UNION_ATTR elif isinstance(original_type, TypeVarType): From 30493ba1a574899585679f0c2feb0c21a43b852d Mon Sep 17 00:00:00 2001 From: Angela Wu Date: Tue, 23 Apr 2024 21:17:48 -0400 Subject: [PATCH 03/13] Add back new circumstance of having NONE as the second object of a union This reverts commit 0c28fadd7dccd3c9fdbf3014b42aec1d6f95d3d4. --- mypy/messages.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/mypy/messages.py b/mypy/messages.py index 199b7c42b11b..eac8f29f8150 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -522,12 +522,23 @@ def has_no_attr( type(item) == NoneType for item in original_type.items ): typ_format = '"None"' - self.fail( - 'Item {} of {} has no attribute "{}"{}'.format( - typ_format, orig_type_format, member, extra - ), - context, - code=codes.UNION_ATTR, + if typ_format == '"None"':( + self.fail( + 'Item {} of {} has no attribute "{}"{}. ADD HINT HERE'.format( + typ_format, orig_type_format, member, extra + ), + context, + code=codes.UNION_ATTR, + ) + ) + else:( + self.fail( + 'Item {} of {} has no attribute "{}"{}'.format( + typ_format, orig_type_format, member, extra + ), + context, + code=codes.UNION_ATTR, + ) ) return codes.UNION_ATTR elif isinstance(original_type, TypeVarType): From 7f4ea67d8233e6e15643c3cc9552d9b49f36890b Mon Sep 17 00:00:00 2001 From: Angela Wu Date: Wed, 24 Apr 2024 11:34:38 -0400 Subject: [PATCH 04/13] add note to use None guard when an an error complaining about None types missing an attribute is generated --- mypy/messages.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/mypy/messages.py b/mypy/messages.py index eac8f29f8150..79f706ca22b2 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -522,22 +522,16 @@ def has_no_attr( type(item) == NoneType for item in original_type.items ): typ_format = '"None"' - if typ_format == '"None"':( - self.fail( - 'Item {} of {} has no attribute "{}"{}. ADD HINT HERE'.format( - typ_format, orig_type_format, member, extra - ), - context, - code=codes.UNION_ATTR, - ) - ) - else:( - self.fail( + self.fail( 'Item {} of {} has no attribute "{}"{}'.format( typ_format, orig_type_format, member, extra ), context, code=codes.UNION_ATTR, + ) + if typ_format == '"None"':( + self.note( + 'You can use "if is not None" to guard against a None value' ) ) return codes.UNION_ATTR From 9b1559401f0e760a62bf64b1be9d5d4544089ada Mon Sep 17 00:00:00 2001 From: Angela Wu Date: Wed, 24 Apr 2024 11:36:16 -0400 Subject: [PATCH 05/13] uses actual variable name in note to use None guard when error complaining about None type missing an attribute is generated --- mypy/messages.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/mypy/messages.py b/mypy/messages.py index 79f706ca22b2..fe888cbe30f4 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -530,8 +530,12 @@ def has_no_attr( code=codes.UNION_ATTR, ) if typ_format == '"None"':( + if typ.source is None: + s = "" + else: + s = f' "{typ.source}"' self.note( - 'You can use "if is not None" to guard against a None value' + f'You can use "if{s} is not None" to guard against a None value' ) ) return codes.UNION_ATTR From b743fa1261999a403119fe1dff56de4a236687b7 Mon Sep 17 00:00:00 2001 From: Angela Wu Date: Wed, 24 Apr 2024 11:43:42 -0400 Subject: [PATCH 06/13] remove extraneous parenthesis around if block --- mypy/messages.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/mypy/messages.py b/mypy/messages.py index fe888cbe30f4..1cbcb0521b07 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -523,21 +523,18 @@ def has_no_attr( ): typ_format = '"None"' self.fail( - 'Item {} of {} has no attribute "{}"{}'.format( - typ_format, orig_type_format, member, extra - ), - context, - code=codes.UNION_ATTR, + 'Item {} of {} has no attribute "{}"{}'.format( + typ_format, orig_type_format, member, extra + ), + context, + code=codes.UNION_ATTR, ) - if typ_format == '"None"':( + if typ_format == '"None"': if typ.source is None: s = "" else: s = f' "{typ.source}"' - self.note( - f'You can use "if{s} is not None" to guard against a None value' - ) - ) + self.note(f'You can use "if{s} is not None" to guard against a None value') return codes.UNION_ATTR elif isinstance(original_type, TypeVarType): bound = get_proper_type(original_type.upper_bound) From 2a2d50c4a5d7393e177b6f72226040577bccfb81 Mon Sep 17 00:00:00 2001 From: Angela Wu Date: Wed, 24 Apr 2024 14:46:26 -0400 Subject: [PATCH 07/13] fixed issue with getting variable name for None guard hint --- mypy/messages.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/mypy/messages.py b/mypy/messages.py index 1cbcb0521b07..f721541550c0 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -42,6 +42,7 @@ IndexExpr, MypyFile, NameExpr, + MemberExpr, ReturnStmt, StrExpr, SymbolNode, @@ -49,6 +50,7 @@ TypeInfo, Var, reverse_builtin_aliases, + get_member_expr_fullname, ) from mypy.operators import op_methods, op_methods_to_symbols from mypy.options import Options @@ -530,11 +532,8 @@ def has_no_attr( code=codes.UNION_ATTR, ) if typ_format == '"None"': - if typ.source is None: - s = "" - else: - s = f' "{typ.source}"' - self.note(f'You can use "if{s} is not None" to guard against a None value') + var_name = f' {get_member_expr_fullname(context.expr)}' + 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) From f4b905ad9b4cc0cfa017f115e453371de08475a6 Mon Sep 17 00:00:00 2001 From: Angela Wu Date: Wed, 24 Apr 2024 15:00:58 -0400 Subject: [PATCH 08/13] fixed variable name when expression is a NameExpr for adding None guard hint --- mypy/messages.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/mypy/messages.py b/mypy/messages.py index f721541550c0..459d8cc79f97 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -532,7 +532,12 @@ def has_no_attr( code=codes.UNION_ATTR, ) if typ_format == '"None"': - var_name = f' {get_member_expr_fullname(context.expr)}' + if isinstance(context, NameExpr): + var_name = f' {context.name}' + elif isinstance(context.expr, MemberExpr): + var_name = f' {get_member_expr_fullname(context.expr)}' + else: + var_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): From 70d5669410eb1e7b5dfac5deaf8fb9e454e67f45 Mon Sep 17 00:00:00 2001 From: Angela Wu Date: Wed, 24 Apr 2024 16:03:02 -0400 Subject: [PATCH 09/13] Fixed variable name for hint if context.expr is an instance of NameExpr --- mypy/messages.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mypy/messages.py b/mypy/messages.py index 459d8cc79f97..bd464226e902 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -534,6 +534,8 @@ def has_no_attr( if typ_format == '"None"': if isinstance(context, NameExpr): var_name = f' {context.name}' + elif isinstance(context.expr, NameExpr): + var_name = f' {context.expr.name}' elif isinstance(context.expr, MemberExpr): var_name = f' {get_member_expr_fullname(context.expr)}' else: From a65ac35b1571407037c5f290df9c4a8b3846c3dd Mon Sep 17 00:00:00 2001 From: Angela Wu Date: Wed, 24 Apr 2024 16:18:54 -0400 Subject: [PATCH 10/13] updated tests to provide note to use None guard when accessing attributes on union types --- test-data/unit/check-inference.test | 6 ++++-- test-data/unit/check-optional.test | 6 ++++-- test-data/unit/check-unions.test | 6 ++++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/test-data/unit/check-inference.test b/test-data/unit/check-inference.test index 08b53ab16972..98b5663cfc34 100644 --- a/test-data/unit/check-inference.test +++ b/test-data/unit/check-inference.test @@ -2828,7 +2828,8 @@ class C: a = None # E: Need type annotation for "a" (hint: "a: Optional[] = ...") 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 @@ -2836,7 +2837,8 @@ class C: a = None # E: Need type annotation for "a" (hint: "a: Optional[] = ...") 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 '_' -- ---------------------------------- diff --git a/test-data/unit/check-optional.test b/test-data/unit/check-optional.test index 70f3c4486e14..bb0b4ed2d3bf 100644 --- a/test-data/unit/check-optional.test +++ b/test-data/unit/check-optional.test @@ -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 @@ -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 diff --git a/test-data/unit/check-unions.test b/test-data/unit/check-unions.test index d79ab14184c6..bd622f4b34d0 100644 --- a/test-data/unit/check-unions.test +++ b/test-data/unit/check-unions.test @@ -935,7 +935,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" @@ -950,7 +951,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" From f31cf9c3ed9a8490785fb998332530b95c7f71ec Mon Sep 17 00:00:00 2001 From: Angela Wu Date: Wed, 24 Apr 2024 16:55:35 -0400 Subject: [PATCH 11/13] Added test case that checks for note when a union type with None doesn't have an attribute --- test-data/unit/check-unions.test | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test-data/unit/check-unions.test b/test-data/unit/check-unions.test index bd622f4b34d0..92c37e8a0f8e 100644 --- a/test-data/unit/check-unions.test +++ b/test-data/unit/check-unions.test @@ -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 From ab289356fb8861928b383e62297aec495817dd27 Mon Sep 17 00:00:00 2001 From: Angela Wu Date: Wed, 24 Apr 2024 23:29:11 -0400 Subject: [PATCH 12/13] fixed linter issues --- mypy/messages.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/mypy/messages.py b/mypy/messages.py index bd464226e902..ac071e9de2af 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -40,17 +40,17 @@ Expression, FuncDef, IndexExpr, + MemberExpr, MypyFile, NameExpr, - MemberExpr, ReturnStmt, StrExpr, SymbolNode, SymbolTable, TypeInfo, Var, - reverse_builtin_aliases, get_member_expr_fullname, + reverse_builtin_aliases, ) from mypy.operators import op_methods, op_methods_to_symbols from mypy.options import Options @@ -533,14 +533,18 @@ def has_no_attr( ) if typ_format == '"None"': if isinstance(context, NameExpr): - var_name = f' {context.name}' + var_name = f" {context.name}" elif isinstance(context.expr, NameExpr): - var_name = f' {context.expr.name}' + var_name = f" {context.expr.name}" elif isinstance(context.expr, MemberExpr): - var_name = f' {get_member_expr_fullname(context.expr)}' + var_name = f" {get_member_expr_fullname(context.expr)}" else: - var_name = ' ' - self.note(f'You can use "if{var_name} is not None" to guard against a None value', context, code=codes.UNION_ATTR) + var_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) From a102f95bc57535a5fd5dd8663f5da71d2d817f69 Mon Sep 17 00:00:00 2001 From: Angela Wu Date: Wed, 24 Apr 2024 23:41:28 -0400 Subject: [PATCH 13/13] Added checks that context is a MemberExpr before accessing its expr attribute --- mypy/messages.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mypy/messages.py b/mypy/messages.py index ac071e9de2af..8075789b348b 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -534,9 +534,9 @@ def has_no_attr( if typ_format == '"None"': if isinstance(context, NameExpr): var_name = f" {context.name}" - elif isinstance(context.expr, NameExpr): + elif isinstance(context, MemberExpr) and isinstance(context.expr, NameExpr): var_name = f" {context.expr.name}" - elif isinstance(context.expr, MemberExpr): + elif isinstance(context, MemberExpr) and isinstance(context.expr, MemberExpr): var_name = f" {get_member_expr_fullname(context.expr)}" else: var_name = " "