Why didn't you add tests for the new flags? I would be great to see how they work.

I wanted to get tentative approval of the idea first. Sounds like I have that, so I'll do that shortly.

SystemExit is never used in build.py and main.py. I think it can cause bad interference with mypy.api etc.

Yes, I was wary of doing it, but without it we need a way of communicating back to main that it should exit there without doing anything. I figured SystemExit behavior for this flag is the same behavior as with --version.

The return of process_options is Tuple[List[BuildSource], Options]. I can make List[BuildSource] optional, and then check for that in main. What do you think?

I addressed this by making List[BuildSource] optional.

I think this should accept an optional pattern: mypy --list-error-codes return would show only error codes that have a "return" substring.

sounds good

Converting all static messages to this format may cause lots of code churn, but in principle I am fine with this.

Another possible option is to have a global dictionary (or JSON file) mapping error message to error code:

message_to_id = {
    NO_RETURN_VALUE_EXPECTED: 'unexpected_return',
    MUST_HAVE_NONE_RETURN_TYPE: 'unexpected_return',
    MISSING_RETURN_STATEMENT: 'missing_return',
    ...
}

Another possible option is to have a global dictionary (or JSON file) mapping error message to error code

I think that could be a good compromise, but it doesn't work for messages that require formatting so we'd have to be very clear about this. A developer might expect this to work with the message_to_id dict:

self.fail(messages.MUST_HAVE_NONE_RETURN_TYPE.format(fdef.name()), item)

We can protect against this in a few ways:

• clear comments near message_to_id documenting that this is only for static messages
• validate message_to_id keys and raise an internal error if formatting operations {} are found.
• add a variation of MessageBuilder.fail, or a flag, that raises an exception if the message is not in message_to_id (I'm assuming here that we're not going to get to 100% message id coverage in the near term)

I just pushed a commit inspired your message_to_id idea, which addresses most of my concerns about type safety. I used an Enum, where the names are ids and the values are the messages. This is used for messages that are static or require only basic formatting (though I haven transitioned all of the code yet). If you like it, I'll finish it up.

@ilevkivskyi Sorry to bug you. Can I get a sign off on the idea of using Enum? If that sounds like a good idea to you I think I can take this PR to completion.

NP, I will think more about this tomorrow and let you know, I was really busy at work last two days.

Everything you've written makes sense. The main problem is that I can't get over the notion that the message_registry dictionary is going to be awfully redundant and tedious to maintain, since it's mostly going to be the message constant followed by a lowercase string of the message constant's name (depending on how broad vs granular we are with grouping) :

message_registry = {
    NO_RETURN_VALUE_EXPECTED: 'no_return_value_expected',
    MISSING_RETURN_STATEMENT: 'missing_return_statement',
    INVALID_IMPLICIT_RETURN: 'invalid_implicit_return',
    INCOMPATIBLE_RETURN_VALUE_TYPE: 'incompatible_return_type',
    RETURN_VALUE_EXPECTED: 'return_value_expected',
    NO_RETURN_EXPECTED: 'no_return_expected',
    INVALID_EXCEPTION: 'invalid_exception',
    INVALID_EXCEPTION_TYPE: 'invalid_exception',
    RETURN_IN_ASYNC_GENERATOR: 'return_in_async_generator',
}

I also don't like that the message and the id are stored in different locations. I think making the constants into basic data classes would be an improvement:

NO_RETURN_VALUE_EXPECTED = Msg('No return value expected', 'no_return_value_expected')  # type: Final
MISSING_RETURN_STATEMENT = Msg('Missing return statement', 'missing_return_statement')  # type: Final

But I'd really love to design a system where the constant doubles as the id, unless it's overridden by a group definition. If enums won't work, the only other way I can think of to do this is with a descriptor. Should I just give up on this idea?

But I'd really love to design a system where the constant doubles as the id

Why not move all the constants into a dedicated module, and iterate over the module namespace and calculate the id based on the attribute name? This sort of metaprogramming should be okay with mypyc. Something like this:

...
FOO_BAR = Msg('Foo bar')
MSG_B = Msg('Another message')

for name, val in list(globals().items()):
    if isinstance(val, Msg) and val.id is None:
        val.id = name.lower()

@chadrik I like what Jukka proposed. We should probably still support passing bare string to self.fail(), this will allow:

• Not breaking existing plugins
• Gradual transition to the new scheme for existing error messages (I strongly prefer two-three smaller PRs, than one large).

Passing a bare string would turn it into Msg(string, 'uncategorized_error').

Btw, the module may be named message_registry.py :-)

Also as I said above it should be clearly documented (probably both in the new module docstring and in docstring for self.register()) that there are two ways to register an error message: simple static and format strings should just go as constants in the module (and nothing else), error messages with complex formatting logic can be manually registered with @register().

Gradual transition to the new scheme for existing error messages (I strongly prefer two-three smaller PRs, than one large)

How about I start with a new PR that moves all of the existing constants into mypy.message_registry with no other changes? That seems to be one part of the design we've all come to agreement on.

TBH, I don't see how this is important now. Anyway, we agreed that @JukkaL will follow on this and related PRs.