Add flag to tag known messages with an error code (take 2)

[chadrik]

Hi all,
I'm back with a new PR to replace #6152. I've tried to get satisfactory answers to the great questions that @JukkaL and @ilevkivskyi posed in the last PR, which I've reposted below with answers:

• How are new static messages defined?

This has been partially addressed by #6194, by moving many message constants to mypy.message_registry, however, we have a lot more to move.

• How are new runtime constructed messages defined?

Also in mypy.message_registry. String format args must be passed to fail() methods rather than formatting first, in order to preserve the message constant until it reaches MessageBuilder, so that an id can be looked up from the string literal. I haven't made that change throughout the code yet since it will be a lot of repetitive work and I wanted to get some feedback on the design of this PR first. That means in its current form, registered messages that are pre-formatted before being passed to fail() are still being marked as 'uncategorized_error' in mypy output.

• What would the error message ids look like in mypy output or in the config file (give examples for a few common messages)?

For output examples, you can see some examples in check-error-codes.txt.

Silencing error messages via a config file has been discussed here and here. This PR sets up all of the data necessary to implement filtering and the semantics of configuration and filtering messages can be solved in a separate PR.

• How would the message ids be shown in mypy output?

I'm assuming this is about listing message ids. This is not in the tests yet, but here's an example of what it looks like:

$ mypy --list-error-codes
mypy.plugins.default:auto_attrib_unsupported  'auto_attribs is not supported in Python 2'
mypy.plugins.default:cannot_determine_init_from_converter  'Cannot determine __init__ type from converter'
mypy.plugins.default:cant_pass_convert_and_converter  "Can't pass both `convert` and `converter`."
mypy.plugins.default:cant_pass_default_and_factory  "Can't pass both `default` and `factory`."
mypy.plugins.default:convert_is_deprecated  'convert is deprecated, use converter'
mypy.plugins.default:invalid_arg_to_type  'Invalid argument to type'
mypy.plugins.default:kw_only_unsupported  'kw_only is not supported in Python 2'
mypy.plugins.default:non_default_attr_not_allowed_after_default  'Non-default attributes not allowed after default attributes.'
mypy.plugins.default:non_kw_only_attr_not_allowed_after_kw_only  'Non keyword-only attributes are not allowed after a keyword-only attribute.'
mypy.plugins.default:old_style_classes_unsupported  'attrs only works with new-style classes'
mypy.plugins.default:too_many_names_for_attr  'Too many names for one attribute'
mypy.plugins.default:unsupported_converter  'Unsupported converter, only named functions and types are currently supported'
mypy:all_must_be_seq_str  'Type of __all__ must be {}, not {}'
mypy:argument_type_expected  'Function is missing a type annotation for one or more arguments'
mypy:bare_generic  'Missing type parameters for generic type'
mypy:cannot_access_final_instance_attr  'Cannot access final instance attribute "{}" on class object'
mypy:cannot_access_init  'Cannot access "__init__" directly'
...

• At what granularity should the message ids be (i.e., should we group similar messages under one id)?

This is a great question. My current thinking is that message grouping can be added using a separate dictionary at a later point.

• How well does this the approach work with mypyc? In particular, mypyc is bad at compiling certain Python idioms such as decorated methods and enums. We should perhaps either use those idioms sparingly or first improve how mypyc handles them. This can be important since there are many error messages, so whatever approach we choose it will probably have many instances in the code.

I've simplified my approach from the original PR, so I think we should be in the clear, but I have not run any tests against mypyc yet.

• How can user plugins define new error messages?

I provided an example for attrs, and if we like it I can propagate it to the other builtin plugins.
That said, since I'm using the plugin module as a unique namespace to prefix the error codes, this PR would benefit from the default plugin being broken up into a series of plugins: mypy.plugins.attrs, mypy.plugins.ctypes, and mypy.plugins.dataclasses.

In addition to improving error labels for this PR, breaking up the default plugin has some other advantages:

• it's more intuitive in general
• it will make it easier for these plugins to be migrated into their respective projects (thinking especially of attrs here)

If you agree, I'm happy to do that in a separate PR.

• Do we need to do something to avoid conflicts between different plugins (e.g. both define a similar message)?

Covered above: the name of the plugin module is used. For the core error codes I use the namespace 'mypy'.

I decided to provide a default namespace for 3 reasons:

• better sorting of messages in --list-error-codes
• consistent number of : in mypy output with --show-error-codes makes for easier parsing by 3rd party scripts
• we may decide we want to breakup the core error codes into phases, such as mypy.parse mypy.analysis, mypy.check, etc.

• Does the proposed approach slow down mypy startup? This is probably not an issue, but in case we construct some complex data structures this might plausibly have a measurable effect.

TBD

[ilevkivskyi]

Thanks for continuing to work on this! Sorry for a long delay, we are now in process of a big refactoring, so we don't have much time for reviews. One of us (most likely @JukkaL since he reviewed previous related PR) will review this as soon as we are more or less done with the refactoring.

[chadrik]

No worries. Is there an issue or PR where I ca read about the goals of the refactor? Sounds exciting ;)

…

On Mon, Mar 4, 2019 at 2:55 AM Ivan Levkivskyi ***@***.***> wrote: Thanks for continuing to work on this! Sorry for a long delay, we are now in process of a big refactoring, so we don't have much time for reviews. One of us (most likely @JukkaL <https://github.com/JukkaL> since he reviewed previous related PR) will review this as soon as we are more or less done with the refactoring. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6472 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD3ExB_e__Ih5y6UG6s0LgP1IQ0yoATks5vTPu2gaJpZM4bOOHv> .

[ilevkivskyi]

It looks like there is no main issue, but we have this label to track progress https://github.com/python/mypy/labels/new-semantic-analyzer.

[chadrik]

I have time to work on feedback now, if you all can find the time for a review!

[ilevkivskyi]

@JukkaL Will you have time to review this PR? It looks pretty important.
@chadrik There are few merge conflicts now.

[chadrik]

I’m happy to resolve the conflicts when you all are ready to review. Just let me know.

…

On Wed, May 8, 2019 at 9:37 AM Ivan Levkivskyi ***@***.***> wrote: @JukkaL <https://github.com/JukkaL> Will you have time to review this PR? It looks pretty important. @chadrik <https://github.com/chadrik> There are few merge conflicts now. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6472 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAPOEZ6LFBHP5SELR3IAGLPUL6UXANCNFSM4GZY4HXQ> .

[ilevkivskyi]

IIUC this is superseded by #7334 (now merged). Thank you for your efforts!