Repository navigation
MessageBuilder.format inconsistently surrounds types with quotes #3409
Description
When working on #3402, using msg.format for types usually surrounded them with quotes, e.g., in test testUnionAttributeAccess, calling msg.format while printing generates the following:
Element "C" of "Union[A, C]" has no attribute "y"
However, in one test case, ``, the type is not surrounded by quotes (see last line, I attached the entire test case for context):
[case testGenericTypeAliasesUnion]
from typing import TypeVar, Generic, Union, Any
T = TypeVar('T')
class Node(Generic[T]):
def __init__(self, x: T) -> None:
self.x = x
UNode = Union[int, Node[T]]
x = 1 # type: UNode[int]
x + 1 # E: Unsupported left operand type for + (some union)
if not isinstance(x, Node):
x + 1
if not isinstance(x, int):
x.x = 1
x.x = 'a' # E: Incompatible types in assignment (expression has type "str", variable has type "int")
def f(x: T) -> UNode[T]:
if 1:
return Node(x)
else:
return 1
reveal_type(f(1)) # E: Revealed type is 'Union[builtins.int, __main__.Node[builtins.int*]]'
TNode = Union[T, Node[int]]
s = 1 # type: TNode[str] # E: Incompatible types in assignment (expression has type "int", variable has type "Union[str, Node[int]]")
if not isinstance(s, str):
s.x = 1
z = None # type: TNode # Same as TNode[Any]
z.x
z.foo() # E: Element Node[int] of "Union[Any, Node[int]]" has no attribute "foo"
Notice how Node[int] is not surrounded by quotes. The relevant format function is defined in mypy/messages.py.
(Sorry for only reproducing this in an unrelated test, I don't have enough knowledge on the code base to create the relevant type object directly).
Activity
ilevkivskyi commentedon May 23, 2017
I think this is on purpose. See this comment in
format_simple:The idea is that type with arguments (i.e. with
[...]) is already visible enough. If you think it is not, then you could just remove this check https://github.com/python/mypy/blob/master/mypy/messages.py#L255gnprice commentedon May 23, 2017
The reasoning in that comment could be a good idea, but I think it'd be hard to do consistently -- and certainly we don't do it consistently today. For example this error message in the quoted test:
I took a quick look at how C compilers format types in error messages. Both Clang and GCC seem to consistently quote all types, with single quotes, both simple and complicated: https://gcc.gnu.org/wiki/ClangDiagnosticsComparison
So I think we should be consistent too. We're also inconsistent on single vs. double quotes (see again the quoted test), which would also be good to fix but I think is a lower level of issue.
ilevkivskyi commentedon May 23, 2017
I agree. @Nurdok woul you like to make a PR removing the check (and comment) I mentioned above?
Consistent quote wrapping around MessageBuilder.format as per issue p…
mistermocha commentedon May 23, 2017
@ilevkivskyi I tidied up the messages lib to put double quotes everywhere.
Consistent quote wrapping around MessageBuilder.format as per issue p…
Consistent quote wrapping around MessageBuilder.format (#3873)