Skip to content

Refactor checker error messages #10959

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

Merged
merged 15 commits into from
Nov 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
81 changes: 38 additions & 43 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
is_literal_type_like,
)
from mypy import message_registry
from mypy.message_registry import ErrorMessage
from mypy.subtypes import (
is_subtype, is_equivalent, is_proper_subtype, is_more_precise,
restrict_subtype_away, is_subtype_ignoring_tvars, is_callable_compatible,
Expand Down Expand Up @@ -1014,10 +1015,9 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: Optional[str])
# entirely pass/Ellipsis/raise NotImplementedError.
if isinstance(return_type, UninhabitedType):
# This is a NoReturn function
self.msg.fail(message_registry.INVALID_IMPLICIT_RETURN, defn)
self.fail(message_registry.INVALID_IMPLICIT_RETURN, defn)
else:
self.msg.fail(message_registry.MISSING_RETURN_STATEMENT, defn,
code=codes.RETURN)
self.fail(message_registry.MISSING_RETURN_STATEMENT, defn)

self.return_types.pop()

Expand Down Expand Up @@ -1073,31 +1073,25 @@ def is_unannotated_any(t: Type) -> bool:
if fdef.type is None and self.options.disallow_untyped_defs:
if (not fdef.arguments or (len(fdef.arguments) == 1 and
(fdef.arg_names[0] == 'self' or fdef.arg_names[0] == 'cls'))):
self.fail(message_registry.RETURN_TYPE_EXPECTED, fdef,
code=codes.NO_UNTYPED_DEF)
self.fail(message_registry.RETURN_TYPE_EXPECTED, fdef)
if not has_return_statement(fdef) and not fdef.is_generator:
self.note('Use "-> None" if function does not return a value', fdef,
code=codes.NO_UNTYPED_DEF)
else:
self.fail(message_registry.FUNCTION_TYPE_EXPECTED, fdef,
code=codes.NO_UNTYPED_DEF)
self.fail(message_registry.FUNCTION_TYPE_EXPECTED, fdef)
elif isinstance(fdef.type, CallableType):
ret_type = get_proper_type(fdef.type.ret_type)
if is_unannotated_any(ret_type):
self.fail(message_registry.RETURN_TYPE_EXPECTED, fdef,
code=codes.NO_UNTYPED_DEF)
self.fail(message_registry.RETURN_TYPE_EXPECTED, fdef)
elif fdef.is_generator:
if is_unannotated_any(self.get_generator_return_type(ret_type,
fdef.is_coroutine)):
self.fail(message_registry.RETURN_TYPE_EXPECTED, fdef,
code=codes.NO_UNTYPED_DEF)
self.fail(message_registry.RETURN_TYPE_EXPECTED, fdef)
elif fdef.is_coroutine and isinstance(ret_type, Instance):
if is_unannotated_any(self.get_coroutine_return_type(ret_type)):
self.fail(message_registry.RETURN_TYPE_EXPECTED, fdef,
code=codes.NO_UNTYPED_DEF)
self.fail(message_registry.RETURN_TYPE_EXPECTED, fdef)
if any(is_unannotated_any(t) for t in fdef.type.arg_types):
self.fail(message_registry.ARGUMENT_TYPE_EXPECTED, fdef,
code=codes.NO_UNTYPED_DEF)
self.fail(message_registry.ARGUMENT_TYPE_EXPECTED, fdef)

def check___new___signature(self, fdef: FuncDef, typ: CallableType) -> None:
self_type = fill_typevars_with_any(fdef.info)
Expand Down Expand Up @@ -1372,7 +1366,7 @@ def check_getattr_method(self, typ: Type, context: Context, name: str) -> None:
if len(self.scope.stack) == 1:
# module scope
if name == '__getattribute__':
self.msg.fail(message_registry.MODULE_LEVEL_GETATTRIBUTE, context)
self.fail(message_registry.MODULE_LEVEL_GETATTRIBUTE, context)
return
# __getattr__ is fine at the module level as of Python 3.7 (PEP 562). We could
# show an error for Python < 3.7, but that would be annoying in code that supports
Expand Down Expand Up @@ -2593,7 +2587,7 @@ def check_assignment_to_slots(self, lvalue: Lvalue) -> None:
return

self.fail(
'Trying to assign name "{}" that is not in "__slots__" of type "{}"'.format(
message_registry.NAME_NOT_IN_SLOTS.format(
lvalue.name, inst.type.fullname,
),
lvalue,
Expand Down Expand Up @@ -2637,16 +2631,16 @@ def check_assignment_to_multiple_lvalues(self, lvalues: List[Lvalue], rvalue: Ex
elif self.type_is_iterable(typs) and isinstance(typs, Instance):
if (iterable_type is not None
and iterable_type != self.iterable_item_type(typs)):
self.fail("Contiguous iterable with same type expected", context)
self.fail(message_registry.CONTIGUOUS_ITERABLE_EXPECTED, context)
else:
if last_idx is None or last_idx + 1 == idx_rval:
rvalues.append(rval)
last_idx = idx_rval
iterable_type = self.iterable_item_type(typs)
else:
self.fail("Contiguous iterable with same type expected", context)
self.fail(message_registry.CONTIGUOUS_ITERABLE_EXPECTED, context)
else:
self.fail("Invalid type '{}' for *expr (iterable expected)".format(typs),
self.fail(message_registry.ITERABLE_TYPE_EXPECTED.format(typs),
context)
else:
rvalues.append(rval)
Expand Down Expand Up @@ -3179,8 +3173,7 @@ def check_member_assignment(self, instance_type: Type, attribute_type: Type,

dunder_set = attribute_type.type.get_method('__set__')
if dunder_set is None:
self.msg.fail(message_registry.DESCRIPTOR_SET_NOT_CALLABLE.format(attribute_type),
context)
self.fail(message_registry.DESCRIPTOR_SET_NOT_CALLABLE.format(attribute_type), context)
return AnyType(TypeOfAny.from_error), get_type, False

function = function_type(dunder_set, self.named_type('builtins.function'))
Expand Down Expand Up @@ -3363,8 +3356,7 @@ def check_return_stmt(self, s: ReturnStmt) -> None:
# Functions returning a value of type None are allowed to have a None return.
if is_lambda or isinstance(typ, NoneType):
return
self.fail(message_registry.NO_RETURN_VALUE_EXPECTED, s,
code=codes.RETURN_VALUE)
self.fail(message_registry.NO_RETURN_VALUE_EXPECTED, s)
else:
self.check_subtype(
subtype_label='got',
Expand All @@ -3386,7 +3378,7 @@ def check_return_stmt(self, s: ReturnStmt) -> None:
return

if self.in_checked_function():
self.fail(message_registry.RETURN_VALUE_EXPECTED, s, code=codes.RETURN_VALUE)
self.fail(message_registry.RETURN_VALUE_EXPECTED, s)

def visit_if_stmt(self, s: IfStmt) -> None:
"""Type check an if statement."""
Expand Down Expand Up @@ -4236,23 +4228,16 @@ def format_expr_type() -> str:
return f'Expression has type "{t}"'

if isinstance(t, FunctionLike):
self.msg.fail(
f'Function "{t}" could always be true in boolean context', expr,
code=codes.TRUTHY_BOOL,
)
self.fail(message_registry.FUNCTION_ALWAYS_TRUE.format(t), expr)
elif isinstance(t, UnionType):
self.msg.fail(
f"{format_expr_type()} of which no members implement __bool__ or __len__ "
"so it could always be true in boolean context",
self.fail(
message_registry.TYPE_ALWAYS_TRUE_UNIONTYPE.format(format_expr_type()),
expr,
code=codes.TRUTHY_BOOL,
)
else:
self.msg.fail(
f'{format_expr_type()} which does not implement __bool__ or __len__ '
'so it could always be true in boolean context',
self.fail(
message_registry.TYPE_ALWAYS_TRUE.format(format_expr_type()),
expr,
code=codes.TRUTHY_BOOL,
)

def find_type_equals_check(self, node: ComparisonExpr, expr_indices: List[int]
Expand Down Expand Up @@ -4385,7 +4370,7 @@ def find_isinstance_check_helper(self, node: Expression) -> Tuple[TypeMap, TypeM
if node.callee.type_guard is not None:
# TODO: Follow keyword args or *args, **kwargs
if node.arg_kinds[0] != nodes.ARG_POS:
self.fail("Type guard requires positional argument", node)
self.fail(message_registry.TYPE_GUARD_POS_ARG_REQUIRED, node)
return {}, {}
if literal(expr) == LITERAL_TYPE:
# Note: we wrap the target type, so that we can special case later.
Expand Down Expand Up @@ -4931,7 +4916,7 @@ def check_subtype(self,
subtype: Type,
supertype: Type,
context: Context,
msg: str = message_registry.INCOMPATIBLE_TYPES,
msg: Union[str, ErrorMessage] = message_registry.INCOMPATIBLE_TYPES,
subtype_label: Optional[str] = None,
supertype_label: Optional[str] = None,
*,
Expand All @@ -4941,9 +4926,14 @@ def check_subtype(self,
if is_subtype(subtype, supertype):
return True

if isinstance(msg, ErrorMessage):
msg_text = msg.value
code = msg.code
else:
msg_text = msg
subtype = get_proper_type(subtype)
supertype = get_proper_type(supertype)
if self.msg.try_report_long_tuple_assignment_error(subtype, supertype, context, msg,
if self.msg.try_report_long_tuple_assignment_error(subtype, supertype, context, msg_text,
subtype_label, supertype_label, code=code):
return False
if self.should_suppress_optional_error([subtype]):
Expand All @@ -4962,8 +4952,9 @@ def check_subtype(self,
if isinstance(subtype, Instance) and isinstance(supertype, Instance):
notes = append_invariance_notes([], subtype, supertype)
if extra_info:
msg += ' (' + ', '.join(extra_info) + ')'
self.fail(msg, context, code=code)
msg_text += ' (' + ', '.join(extra_info) + ')'

self.fail(ErrorMessage(msg_text, code=code), context)
for note in notes:
self.msg.note(note, context, code=code)
if note_msg:
Expand Down Expand Up @@ -5234,8 +5225,12 @@ def temp_node(self, t: Type, context: Optional[Context] = None) -> TempNode:
"""Create a temporary node with the given, fixed type."""
return TempNode(t, context=context)

def fail(self, msg: str, context: Context, *, code: Optional[ErrorCode] = None) -> None:
def fail(self, msg: Union[str, ErrorMessage], context: Context, *,
code: Optional[ErrorCode] = None) -> None:
"""Produce an error message."""
if isinstance(msg, ErrorMessage):
self.msg.fail(msg.value, context, code=msg.code)
return
self.msg.fail(msg, context, code=code)

def note(self,
Expand Down
18 changes: 10 additions & 8 deletions mypy/errorcodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ def __str__(self) -> str:
OVERRIDE: Final = ErrorCode(
"override", "Check that method override is compatible with base class", "General"
)
RETURN: Final = ErrorCode("return", "Check that function always returns a value", "General")
RETURN_VALUE: Final = ErrorCode(
RETURN: Final[ErrorCode] = ErrorCode(
"return", "Check that function always returns a value", "General"
)
RETURN_VALUE: Final[ErrorCode] = ErrorCode(
"return-value", "Check that return value is compatible with signature", "General"
)
ASSIGNMENT: Final = ErrorCode(
Expand Down Expand Up @@ -94,11 +96,11 @@ def __str__(self) -> str:
)

# These error codes aren't enabled by default.
NO_UNTYPED_DEF: Final = ErrorCode(
NO_UNTYPED_DEF: Final[ErrorCode] = ErrorCode(
"no-untyped-def", "Check that every function has an annotation", "General"
)
NO_UNTYPED_CALL: Final = ErrorCode(
'no-untyped-call',
"no-untyped-call",
"Disallow calling functions without type annotations from annotated functions",
"General",
)
Expand All @@ -122,11 +124,11 @@ def __str__(self) -> str:
REDUNDANT_EXPR: Final = ErrorCode(
"redundant-expr", "Warn about redundant expressions", "General", default_enabled=False
)
TRUTHY_BOOL: Final = ErrorCode(
'truthy-bool',
TRUTHY_BOOL: Final[ErrorCode] = ErrorCode(
"truthy-bool",
"Warn about expressions that could always evaluate to true in boolean contexts",
'General',
default_enabled=False
"General",
default_enabled=False,
)
NAME_MATCH: Final = ErrorCode(
"name-match", "Check that type definition has consistent naming", "General"
Expand Down
8 changes: 7 additions & 1 deletion mypy/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from mypy.options import Options
from mypy.version import __version__ as mypy_version
from mypy.errorcodes import ErrorCode, IMPORT
from mypy.message_registry import ErrorMessage
from mypy import errorcodes as codes
from mypy.util import DEFAULT_SOURCE_OFFSET, is_typeshed_file

Expand Down Expand Up @@ -677,7 +678,12 @@ def render_messages(self,
result.append((file, -1, -1, 'note',
'In class "{}":'.format(e.type), e.allow_dups, None))

result.append((file, e.line, e.column, e.severity, e.message, e.allow_dups, e.code))
if isinstance(e.message, ErrorMessage):
result.append(
(file, e.line, e.column, e.severity, e.message.value, e.allow_dups, e.code))
else:
result.append(
(file, e.line, e.column, e.severity, e.message, e.allow_dups, e.code))

prev_import_context = e.import_ctx
prev_function_or_member = e.function_or_member
Expand Down
Loading