MessageBuilder.format inconsistently surrounds types with quotes

[Nurdok]

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).

[ilevkivskyi]

I think this is on purpose. See this comment in format_simple:

                # No type arguments. Place the type name in quotes to avoid
                # potential for confusion: otherwise, the type name could be
                # interpreted as a normal word.

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#L255

[gnprice]

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:

Element Node[int] of "Union[Any, Node[int]]" has no attribute "foo"

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]

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.

I agree. @Nurdok woul you like to make a PR removing the check (and comment) I mentioned above?

[mistermocha]

@ilevkivskyi I tidied up the messages lib to put double quotes everywhere.