Use double quotes in error messages for more consistent colors

[JukkaL]

Some messages use double quotes for quoting while other use single quotes. Maybe we should move to double quotes since the colored output only highlights double quoted strings.

Examples where this happens:

t.py:14: note: Revealed type is 'builtins.list[builtins.str*]'
t.py:27: error: Name 'asdf' is not defined

[ilevkivskyi]

Some more examples:

Cannot redefine 'a' as a NewType
Cannot redefine 'T' as a type variable
Unsupported type Type["Tuple[int]"]   # this one is especially weird
Cannot determine type of 'blah'
Cannot determine type of 'foo' in base class 'B'
Cannot instantiate abstract class 'A'
TypedDict key 'x' cannot be deleted  # and several other TypedDict related errors

[ilevkivskyi]

Also #5072 is kind of related because type representations can be unnecessary long.

[Patil2099]

@JukkaL Can I work on this issue? Also, Can you explain where I have to make the changes?
Thanks!

[JukkaL]

@Patil2099 Sure! Here are some hints:

• You'll need to update both the error message generation code and any tests that test for that error message.
• You can use grep to find where the error message is being generated/tested for.
• It's best to update one error message per PR, since the PRs may get quickly stale.
• Regular expressions are helpful.

[Sechan-BL]

I would like to work on this issue. So please can you guide me what to do and where to start my work?

[JelleZijlstra]

@Sechan-tkd you can look for places where we currently emit single-quoted strings, perhaps by reading through mypy/messages.py or through our test cases.

[Sechan-BL]

@Sechan-tkd you can look for places where we currently emit single-quoted strings, perhaps by reading through mypy/messages.py or through our test cases.

Should I need to update every single-quoted strings to double-quoted strings? Or else what should I do?

[JukkaL]

Should I need to update every single-quoted strings to double-quoted strings? Or else what should I do?

You can update one kind of message (or several messages) at a time, and fix any test failures that result from this. Even a single updated message is good enough for a PR -- no need to do everything at once. However, it's important to also update tests.

[Sechan-BL]

I dont know how to do . Can you please guide me with this? Like where to get the code ? What changes to make ?

[JukkaL]

@Sechan-tkd You start by reading the readme at the root of the repo and the docs linked to from it. They explain the basics.

[PrasanthChettri]

This is not taken right, I want to do this, seems doable even for a noob like me !

[PrasanthChettri]

Im getting an Invalid Ouput Assertion error during the automatic testing

Detail

Alignment of first line difference:
  E: ...note: Revealed type is 'Any'
  A: ...note: Revealed type is "Any"

From what I gather this is the intended output we want i.e. error messages with double quotes but the tests that ran in my pull request are failing because of it. If anybody would give a feedback it will be helpful

[JelleZijlstra]

@PrasanthChettri you'll have to update the test cases. They live in the test-data/ directory. (That, along with finding the code to change, is likely the biggest chunk of work for fixing this issue.)

[PrasanthChettri]

@JelleZijlstra So do I have to change these test cases for my branch or are those test cases running from the main branch.

[JelleZijlstra]

You'll have to change them on your branch.

[PrasanthChettri]

Do I have to manually change all the occurences of ' to " in test_data/*.*. There are 200+ files it's painfully hectic to replace them all, are there alternatives ?

[JelleZijlstra]

I'd recommend learning to use https://github.com/facebook/codemod . Once you get the PR ready and passing tests, I'm happy to review and merge quickly to avoid merge conflicts, since @JukkaL already approved the idea.

[PrasanthChettri]

@JelleZijlstra Hey could you review the branch. I guess the tests are passing.

[PrasanthChettri]

@JelleZijlstra @JukkaL I did the requested changes
Note : Not all of the messages could be changed to use single quotes in this PR most importantly because of large test cases. Also,
I'm thinking we could do this over a couple PRs as these messages are spread throughout the code and it's a big change for one PR.

[dixith]

@PrasanthChettri Are you working on other error messages as well? Can I make changes for object is not iterable error message?

[JelleZijlstra]

reopening since this isn't fully done yet

[dixith]

I'll get PR created for is not a valid TypedDict key. Looks like thats the only message left for correction. Do you guys see any other error message thats needs change?

[dixith]

@JelleZijlstra @JukkaL @hauntsaninja
Do you guys see any other error message thats needs change?

[JelleZijlstra]

Grepping for single quotes in test-data/unit/ still shows some instances in error messages. Some examples:

unit/check-abstract.test:A() # E: Cannot instantiate abstract class 'A' with abstract attribute 'f'
unit/check-annotated.test:x: Annotated[XXX, ...]  # E: Name 'XXX' is not defined
unit/check-async-await.test:main:3: error: 'yield from' in async function
unit/check-classes.test:x = C.x # E: Cannot determine type of 'x'
unit/check-errorcodes.test:x = 0  # type: x y  # E: syntax error in type comment 'x y'  [syntax]

One even uses backticks:

unit/check-attr.test:@attrs(cmp=False, eq=True)  # E: Don't mix `cmp` with `eq' and `order`

[JelleZijlstra]

Another interesting one is check-errorcodes.test:from m import think # E: Module 'm' has no attribute 'think'; maybe "thing"? [attr-defined], where we use both conventions in the same error. I got these with git grep "E:.*'".

[dixith]

Thank you @JelleZijlstra for the pointer. I'll get these corrected.

[dixith]

@JelleZijlstra My last PR covered all remaining messages. I have not updated below 4 messages becasue they are created in AST module

1. '(' was never closed
2. Missing parentheses in call to 'print'.
3. expected ':'
4. can't assign to literal here. Maybe you meant '==' instead of '='?

So this issue can be closed. If you find any other error that needs update, please let me know.

[JelleZijlstra]

Thanks for all your work here! Let's close this issue once your most recent PR is merged.