From fe1046083588921194f47eae518ef4a23993fcf8 Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Mon, 1 Jul 2019 08:36:08 -0700 Subject: [PATCH] Remove some "if type is None" style checks This pull request is a continuation of my quest to make mypy cleanly type-check under the `--warn-unreachable` flag. It removes some unnecessary checks we're performing against Type-related variables. Specifically: 1. We had some old code that checked if `t.accept(TypeJoinVisitor(s))` would return None. I believe this check was added back in 2014: after auditing our type visitors, it no longer seems relevant today. 2. We had a few unnecessary `if some_instance.type is not None` style checks. I believe we always set the TypeInfo when constructing Instance classes these days, so this check also seems unnecessary. 3. Finally, we had a "if TypeInfo.bases does not actually contain Instances" style check around some protocol-related code. I'm not really sure why, but TypeInfo.bases is annotated to be of type `List[Instance]` so it should be fine to remove these checks as well. --- mypy/join.py | 8 -------- mypy/newsemanal/semanal_classprop.py | 9 ++++----- mypy/semanal_pass3.py | 5 ++--- mypy/suggestions.py | 5 +---- mypy/types.py | 14 +++----------- 5 files changed, 10 insertions(+), 31 deletions(-) diff --git a/mypy/join.py b/mypy/join.py index 31266827e17d..f651f43203c5 100644 --- a/mypy/join.py +++ b/mypy/join.py @@ -48,14 +48,6 @@ def join_simple(declaration: Optional[Type], s: Type, t: Type) -> Type: s, t = t, s value = t.accept(TypeJoinVisitor(s)) - - if value is None: - # XXX this code path probably should be avoided. - # It seems to happen when a line (x = y) is a type error, and - # it's not clear that assuming that x is arbitrary afterward - # is a good idea. - return declaration - if declaration is None or is_subtype(value, declaration): return value diff --git a/mypy/newsemanal/semanal_classprop.py b/mypy/newsemanal/semanal_classprop.py index b10daae471e1..8f8ccd91fc6b 100644 --- a/mypy/newsemanal/semanal_classprop.py +++ b/mypy/newsemanal/semanal_classprop.py @@ -113,11 +113,10 @@ def check_protocol_status(info: TypeInfo, errors: Errors) -> None: """Check that all classes in MRO of a protocol are protocols""" if info.is_protocol: for type in info.bases: - if not isinstance(type, Instance) or not type.type.is_protocol: - if type.type.fullname() != 'builtins.object': - def report(message: str, severity: str) -> None: - errors.report(info.line, info.column, message, severity=severity) - report('All bases of a protocol must be protocols', 'error') + if not type.type.is_protocol and type.type.fullname() != 'builtins.object': + def report(message: str, severity: str) -> None: + errors.report(info.line, info.column, message, severity=severity) + report('All bases of a protocol must be protocols', 'error') def calculate_class_vars(info: TypeInfo) -> None: diff --git a/mypy/semanal_pass3.py b/mypy/semanal_pass3.py index 9d139eada4d1..3af1b2e5faa1 100644 --- a/mypy/semanal_pass3.py +++ b/mypy/semanal_pass3.py @@ -176,9 +176,8 @@ def visit_class_def(self, tdef: ClassDef) -> None: self.analyze_types(types, tdef.info) for type in tdef.info.bases: if tdef.info.is_protocol: - if not isinstance(type, Instance) or not type.type.is_protocol: - if type.type.fullname() != 'builtins.object': - self.fail('All bases of a protocol must be protocols', tdef) + if not type.type.is_protocol and type.type.fullname() != 'builtins.object': + self.fail('All bases of a protocol must be protocols', tdef) # Recompute MRO now that we have analyzed all modules, to pick # up superclasses of bases imported from other modules in an # import loop. (Only do so if we succeeded the first time.) diff --git a/mypy/suggestions.py b/mypy/suggestions.py index d4fa50308638..c0b2540b30e5 100644 --- a/mypy/suggestions.py +++ b/mypy/suggestions.py @@ -542,10 +542,7 @@ def __init__(self, module: Optional[str], graph: Graph) -> None: self.graph = graph def visit_instance(self, t: Instance) -> str: - if t.type is not None: - s = t.type.fullname() or t.type.name() or None - else: - s = None + s = t.type.fullname() or t.type.name() or None if s is None: return '' if s in reverse_builtin_aliases: diff --git a/mypy/types.py b/mypy/types.py index ca2f4e085bc4..c6f8b0e687dc 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -1947,10 +1947,7 @@ def visit_deleted_type(self, t: DeletedType) -> str: return "".format(t.source) def visit_instance(self, t: Instance) -> str: - if t.type is not None: - s = t.type.fullname() or t.type.name() or '' - else: - s = '' + s = t.type.fullname() or t.type.name() or '' if t.erased: s += '*' if t.args != []: @@ -2081,10 +2078,7 @@ def list_str(self, a: List[Type]) -> str: """ res = [] for t in a: - if isinstance(t, Type): - res.append(t.accept(self)) - else: - res.append(str(t)) + res.append(t.accept(self)) return ', '.join(res) @@ -2101,9 +2095,7 @@ def strip_type(typ: Type) -> Type: def is_named_instance(t: Type, fullname: str) -> bool: - return (isinstance(t, Instance) and - t.type is not None and - t.type.fullname() == fullname) + return isinstance(t, Instance) and t.type.fullname() == fullname def copy_type(t: Type) -> Type: