Makes DESCRIPTOR non nullable again

[asyks]

This change is a partial revert of 90f5422, specifically making the DESCRIPTOR class attribute of Message and EnumTypeWrapper non nullable again.

• I'm following up on - Add/fix types to a bunch of non-generated protobuf files #7017 (comment) on behalf of @neruson

What we're seeing is that when doing something like this:

from google.protobuf.message import Message
...
message: Message = some_protobuf_message
message.DESCRIPTOR.fields_by_name

The following mypy error occurs:
Item "None" of "Optional[Descriptor]" has no attribute "fields_by_name"

The Message abstract base class defines DESCRIPTOR as None, but there's a comment specifying the type for subclasses of Message: https://github.com/protocolbuffers/protobuf/blob/3be46483ba11605ec9d29ae439a16c4fc6bf02c3/python/google/protobuf/message.py#L77-L78
This seems to imply that None is not actually a valid value for the DESCRIPTOR attribute of subclasses of Message, and this seems to be reflected in the protoc generated python message types I've looked at (DESCRIPTOR is never None).

• It may be worth noting that although Message is called an abstract base class in its docstring, it doesn't inherit from python's abc.ABC.

Likewise The EnumTypeWrapper class defines DESCRIPTOR as None, but requires it be set in its init:
https://github.com/protocolbuffers/protobuf/blob/3be46483ba11605ec9d29ae439a16c4fc6bf02c3/python/google/protobuf/internal/enum_type_wrapper.py#L55-L58

[asyks]

cc: @nipunn1313

[nipunn1313]

Looks good! Thanks!

[Akuli]

I would appreciate a short comment here.

[asyks]

@Akuli - I'm new to mypy stubtests but I'm seeing the following locally (linux 5.13.0-28-generic, python 3.9.5).

Without these two allowlist entries:

python tests/stubtest_third_party.py protobuf
...
error: google.protobuf.internal.enum_type_wrapper.EnumTypeWrapper.DESCRIPTOR variable differs from runtime type None
Stub: at line 10
google.protobuf.descriptor.EnumDescriptor
Runtime:
None

error: google.protobuf.message.Message.DESCRIPTOR variable differs from runtime type None
Stub: at line 14
google.protobuf.descriptor.Descriptor
Runtime:
None

stubtest failed for protobuf

With these two allowlist entries:

python tests/stubtest_third_party.py protobuf
...
stubtest succeeded for protobuf

My interpretation of that is: while Message and Descriptor are both defined with a null DESCRIPTOR, sublcasses of Message and instances of EnumTypeWrapper require this value to be set, and since These type stubs are intended for use with protoc generated python it's more accurate to make them non-nullable.

[Akuli]

while Message and Descriptor are both defined with a null DESCRIPTOR, sublcasses of Message and instances of EnumTypeWrapper require this value to be set, and since These type stubs are intended for use with protoc generated python it's more accurate to make them non-nullable.

Add a comment that says this to the allowlist :)

[nipunn1313]

Yep! Your understanding is perfect.

Just need a comment in the allowlist.txt file so that someone in the future doesn't revert your change by accident.

[asyks]

Ah ok, got it, done.

[Akuli]

Thanks!