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.

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

Make strict Optional type system changes standard #3024

Merged
merged 7 commits into from
Mar 19, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
22 changes: 15 additions & 7 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1737,7 +1737,12 @@ def try_infer_partial_type_from_indexed_assignment(
del partial_types[var]

def visit_expression_stmt(self, s: ExpressionStmt) -> None:
self.expr_checker.accept(s.expr)
# Special-case call exprs so we can support warnings on return value of
# None-returning functions
if isinstance(s.expr, CallExpr):
self.expr_checker.accept(s.expr, allow_none_return=True)
else:
self.expr_checker.accept(s.expr)

def visit_return_stmt(self, s: ReturnStmt) -> None:
"""Type check a return statement."""
Expand All @@ -1758,13 +1763,16 @@ def check_return_stmt(self, s: ReturnStmt) -> None:
return

if s.expr:
is_lambda = isinstance(self.scope.top_function(), FuncExpr)
declared_none_return = isinstance(return_type, NoneTyp)
# Return with a value.
typ = self.expr_checker.accept(s.expr, return_type)
typ = self.expr_checker.accept(s.expr,
return_type,
allow_none_return=is_lambda or declared_none_return)

if defn.is_async_generator:
self.fail("'return' with value in async generator is not allowed", s)
return

# Returning a value of type Any is always fine.
if isinstance(typ, AnyType):
# (Unless you asked to be warned in that case, and the
Expand All @@ -1773,10 +1781,10 @@ def check_return_stmt(self, s: ReturnStmt) -> None:
self.warn(messages.RETURN_ANY.format(return_type), s)
return

if self.is_unusable_type(return_type):
# Lambdas are allowed to have a unusable returns.
if declared_none_return:
# Lambdas are allowed to have None returns.
# Functions returning a value of type None are allowed to have a None return.
if isinstance(self.scope.top_function(), FuncExpr) or isinstance(typ, NoneTyp):
if is_lambda or isinstance(typ, NoneTyp):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is unclear.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added some comments

return
self.fail(messages.NO_RETURN_VALUE_EXPECTED, s)
else:
Expand Down Expand Up @@ -2105,7 +2113,7 @@ def visit_del_stmt(self, s: DelStmt) -> None:
m.line = s.line
c = CallExpr(m, [e.index], [nodes.ARG_POS], [None])
c.line = s.line
c.accept(self.expr_checker)
self.expr_checker.accept(c, allow_none_return=True)
else:
s.expr.accept(self.expr_checker)
for elt in flatten(s.expr):
Expand Down
23 changes: 17 additions & 6 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def analyze_var_ref(self, var: Var, context: Context) -> Type:
else:
return val

def visit_call_expr(self, e: CallExpr) -> Type:
def visit_call_expr(self, e: CallExpr, allow_none_return: bool = False) -> Type:
"""Type check a call expression."""
if e.analyzed:
# It's really a special form that only looks like a call.
Expand All @@ -192,6 +192,8 @@ def visit_call_expr(self, e: CallExpr) -> Type:
ret_type = self.check_call_expr_with_callee_type(callee_type, e)
if isinstance(ret_type, UninhabitedType):
self.chk.binder.unreachable()
if not allow_none_return and isinstance(ret_type, NoneTyp):
self.chk.msg.does_not_return_value(callee_type, e)
return ret_type

def check_typeddict_call(self, callee: TypedDictType,
Expand Down Expand Up @@ -1516,13 +1518,12 @@ def visit_enum_index_expr(self, enum_type: TypeInfo, index: Expression,

def visit_cast_expr(self, expr: CastExpr) -> Type:
"""Type check a cast expression."""
source_type = self.accept(expr.expr, type_context=AnyType())
source_type = self.accept(expr.expr, type_context=AnyType(), allow_none_return=True)
target_type = expr.type
if self.chk.options.warn_redundant_casts and is_same_type(source_type, target_type):
self.msg.redundant_cast(target_type, expr)
return target_type


def visit_reveal_type_expr(self, expr: RevealTypeExpr) -> Type:
"""Type check a reveal_type expression."""
revealed_type = self.accept(expr.expr, type_context=self.type_context[-1])
Expand Down Expand Up @@ -2006,11 +2007,21 @@ def visit_backquote_expr(self, e: BackquoteExpr) -> Type:
# Helpers
#

def accept(self, node: Expression, type_context: Type = None) -> Type:
"""Type check a node in the given type context."""
def accept(self,
node: Expression,
type_context: Type = None,
allow_none_return: bool = False
) -> Type:
"""Type check a node in the given type context. If allow_none_return
is True and this expression is a call, allow it to return None. This
applies only to this expression and not any subexpressions.
"""
self.type_context.append(type_context)
try:
typ = node.accept(self)
if allow_none_return and isinstance(node, CallExpr):
typ = self.visit_call_expr(node, allow_none_return=True)
else:
typ = node.accept(self)
except Exception as err:
report_internal_error(err, self.chk.errors.file,
node.line, self.chk.errors, self.chk.options)
Expand Down
8 changes: 6 additions & 2 deletions mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -619,9 +619,13 @@ def duplicate_argument_value(self, callee: CallableType, index: int,
format(capitalize(callable_name(callee)),
callee.arg_names[index]), context)

def does_not_return_value(self, unusable_type: Type, context: Context) -> None:
def does_not_return_value(self, callee_type: Type, context: Context) -> None:
"""Report an error about use of an unusable type."""
self.fail('Function does not return a value', context)
if isinstance(callee_type, FunctionLike) and callee_type.get_name() is not None:
self.fail('{} does not return a value'.format(
capitalize(callee_type.get_name())), context)
else:
self.fail('Function does not return a value', context)

def deleted_as_rvalue(self, typ: DeletedType, context: Context) -> None:
"""Report an error about using an deleted type as an rvalue."""
Expand Down
11 changes: 10 additions & 1 deletion mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,9 @@ def items(self) -> List['CallableType']: pass
@abstractmethod
def with_name(self, name: str) -> 'FunctionLike': pass

@abstractmethod
def get_name(self) -> str: pass

# Corresponding instance type (e.g. builtins.type)
fallback = None # type: Instance

Expand Down Expand Up @@ -632,6 +635,9 @@ def with_name(self, name: str) -> 'CallableType':
"""Return a copy of this type with the specified name."""
return self.copy_modified(ret_type=self.ret_type, name=name)

def get_name(self) -> str:
return self.name

def max_fixed_args(self) -> int:
n = len(self.arg_types)
if self.is_var_arg:
Expand Down Expand Up @@ -778,7 +784,7 @@ def items(self) -> List[CallableType]:
return self._items

def name(self) -> str:
return self._items[0].name
return self.get_name()

def is_type_obj(self) -> bool:
# All the items must have the same type object status, so it's
Expand All @@ -796,6 +802,9 @@ def with_name(self, name: str) -> 'Overloaded':
ni.append(it.with_name(name))
return Overloaded(ni)

def get_name(self) -> str:
return self._items[0].name

def accept(self, visitor: 'TypeVisitor[T]') -> T:
return visitor.visit_overloaded(self)

Expand Down