From ae2f22dce2c38d3d0421d32bece435ee0e2f8a6f Mon Sep 17 00:00:00 2001 From: Konrad Kujawa Date: Fri, 24 Mar 2023 10:30:23 +0100 Subject: [PATCH] Generate enum for each Oneof field in the .proto message Generate separate from the QtProtobufFieldEnum enum for each Oneof field. Use that enum in the Oneof field number getter. Fix formatting in protobuf generator code. Fixes: QTBUG-109126 Change-Id: Ib889a499b9abdc39ea7416cea008d157881a15fc Reviewed-by: Qt CI Bot Reviewed-by: Alexey Edelev --- .../messagedeclarationprinter.cpp | 48 ++++++++++++++----- .../qtprotobufgen/messagedeclarationprinter.h | 4 +- src/tools/qtprotoccommon/commontemplates.cpp | 11 +++-- src/tools/qtprotoccommon/commontemplates.h | 1 + src/tools/qtprotoccommon/generatorcommon.cpp | 1 + .../basic/tst_protobuf_basictypes.cpp | 10 ++-- .../tests/oneofmessages.qpb.cpp | 12 ++--- .../tests/oneofmessages.qpb.h | 29 +++++++++-- .../no-options/oneofmessages.qpb.cpp | 12 ++--- .../no-options/oneofmessages.qpb.h | 29 +++++++++-- 10 files changed, 118 insertions(+), 39 deletions(-) diff --git a/src/tools/qtprotobufgen/messagedeclarationprinter.cpp b/src/tools/qtprotobufgen/messagedeclarationprinter.cpp index 6c0e2ed1..e17540bf 100644 --- a/src/tools/qtprotobufgen/messagedeclarationprinter.cpp +++ b/src/tools/qtprotobufgen/messagedeclarationprinter.cpp @@ -335,16 +335,20 @@ void MessageDeclarationPrinter::printPrivateMethods() Outdent(); } +void MessageDeclarationPrinter::printEnums() +{ + Indent(); + if (Options::instance().generateFieldEnum()) + printFieldEnum(); + if (m_descriptor->enum_type_count() > 0) + printQEnums(); + if (m_descriptor->oneof_decl_count() > 0) + printOneofEnums(); + Outdent(); +} + void MessageDeclarationPrinter::printQEnums() { - if (Options::instance().generateFieldEnum()) { - printFieldEnum(); - } - - if (m_descriptor->enum_type_count() <= 0) - return; - - Indent(); for (int i = 0; i < m_descriptor->enum_type_count(); ++i) { const auto *enumDescr = m_descriptor->enum_type(i); auto typeMap = common::produceEnumTypeMap(enumDescr, m_descriptor); @@ -367,7 +371,29 @@ void MessageDeclarationPrinter::printQEnums() auto typeMap = common::produceEnumTypeMap(enumDescr, m_descriptor); m_printer->Print(typeMap, CommonTemplates::UsingRepeatedEnumTemplate()); } - Outdent(); +} + +void MessageDeclarationPrinter::printOneofEnums() +{ + common::iterateOneofFields( + m_descriptor, [this](const OneofDescriptor *oneofDescr, PropertyMap &typeMap) { + m_printer->Print(typeMap, CommonTemplates::EnumClassDefinitionTemplate()); + Indent(); + m_printer->Print({ { "enumvalue", "UninitializedField" }, + { "value", "QtProtobuf::InvalidFieldNumber" } }, + CommonTemplates::EnumFieldTemplate()); + for (int j = 0; j < oneofDescr->field_count(); ++j) { + const auto *valueDescr = oneofDescr->field(j); + m_printer->Print({ { "enumvalue", + utils::capitalizeAsciiName(valueDescr->name()) }, + { "value", std::to_string(valueDescr->number()) } }, + CommonTemplates::EnumFieldTemplate()); + } + Outdent(); + m_printer->Print(CommonTemplates::SemicolonBlockEnclosureTemplate()); + m_printer->Print(typeMap, CommonTemplates::QEnumTemplate()); + m_printer->Print("\n"); + }); } void MessageDeclarationPrinter::printClassBody() @@ -375,7 +401,7 @@ void MessageDeclarationPrinter::printClassBody() printProperties(); printPublicBlock(); - printQEnums(); + printEnums(); printNested(); printMaps(); @@ -436,7 +462,6 @@ void MessageDeclarationPrinter::printDestructor() void MessageDeclarationPrinter::printFieldEnum() { if (m_descriptor->field_count() > 0) { - Indent(); m_printer->Print(CommonTemplates::FieldEnumTemplate()); Indent(); common::iterateMessageFields(m_descriptor, @@ -448,7 +473,6 @@ void MessageDeclarationPrinter::printFieldEnum() m_printer->Print(CommonTemplates::SemicolonBlockEnclosureTemplate()); m_printer->Print({ { "type", CommonTemplates::QtProtobufFieldEnum() } }, CommonTemplates::QEnumTemplate()); - Outdent(); m_printer->Print("\n"); } } diff --git a/src/tools/qtprotobufgen/messagedeclarationprinter.h b/src/tools/qtprotobufgen/messagedeclarationprinter.h index f2fb3d57..a59f4943 100644 --- a/src/tools/qtprotobufgen/messagedeclarationprinter.h +++ b/src/tools/qtprotobufgen/messagedeclarationprinter.h @@ -38,9 +38,11 @@ private: void printMaps(); void printNested(); void printClassDeclarationBegin(); - void printFieldEnum(); + void printEnums(); + void printFieldEnum(); void printQEnums(); + void printOneofEnums(); //Recursive functionality void printClassDeclarationPrivate(); diff --git a/src/tools/qtprotoccommon/commontemplates.cpp b/src/tools/qtprotoccommon/commontemplates.cpp index 8b81df82..8fa6473e 100644 --- a/src/tools/qtprotoccommon/commontemplates.cpp +++ b/src/tools/qtprotoccommon/commontemplates.cpp @@ -267,6 +267,10 @@ const char *CommonTemplates::EnumDefinitionTemplate() { return "enum $type$ {\n"; } +const char *CommonTemplates::EnumClassDefinitionTemplate() +{ + return "enum class $type$ {\n"; +} const char *CommonTemplates::EnumFieldTemplate() { return "$enumvalue$ = $value$,\n"; @@ -494,12 +498,13 @@ const char *CommonTemplates::PrivateGetterOneofMessageDefinitionTemplate() const char *CommonTemplates::GetterOneofFieldNumberDeclarationTemplate() { - return "int $optional_property_name$Field() const;\n"; + return "$type$ $optional_property_name$Field() const;\n"; } const char *CommonTemplates::GetterOneofFieldNumberDefinitionTemplate() { - return "int $classname$::$optional_property_name$Field() const\n{\n" - " return m_$optional_property_name$.fieldNumber();\n" + return "$classname$::$type$ $classname$::$optional_property_name$Field() const\n{\n" + " return " + "static_cast<$type$>(m_$optional_property_name$.fieldNumber());\n" "}\n"; } diff --git a/src/tools/qtprotoccommon/commontemplates.h b/src/tools/qtprotoccommon/commontemplates.h index bc695841..e4cdb0be 100644 --- a/src/tools/qtprotoccommon/commontemplates.h +++ b/src/tools/qtprotoccommon/commontemplates.h @@ -68,6 +68,7 @@ public: static const char *PublicBlockTemplate(); static const char *PrivateBlockTemplate(); static const char *EnumDefinitionTemplate(); + static const char *EnumClassDefinitionTemplate(); static const char *EnumFieldTemplate(); static const char *CopyConstructorDeclarationTemplate(); static const char *MoveConstructorDeclarationTemplate(); diff --git a/src/tools/qtprotoccommon/generatorcommon.cpp b/src/tools/qtprotoccommon/generatorcommon.cpp index 4c1dfb4b..4502b38a 100644 --- a/src/tools/qtprotoccommon/generatorcommon.cpp +++ b/src/tools/qtprotoccommon/generatorcommon.cpp @@ -462,6 +462,7 @@ PropertyMap common::producePropertyMap(const OneofDescriptor *oneof, const Descr propertyMap["optional_property_name_cap"] = utils::capitalizeAsciiName(oneof->name()); auto scopeTypeMap = produceMessageTypeMap(scope, nullptr); propertyMap["classname"] = scope != nullptr ? scopeTypeMap["classname"] : ""; + propertyMap["type"] = propertyMap["optional_property_name_cap"] + "Fields"; return propertyMap; } diff --git a/tests/auto/protobuf/basic/tst_protobuf_basictypes.cpp b/tests/auto/protobuf/basic/tst_protobuf_basictypes.cpp index ffe41005..d8095092 100644 --- a/tests/auto/protobuf/basic/tst_protobuf_basictypes.cpp +++ b/tests/auto/protobuf/basic/tst_protobuf_basictypes.cpp @@ -333,7 +333,7 @@ void QtProtobufTypesGenerationTest::OneofMessageEmptyTest() OneofMessage test; QVERIFY(!test.hasTestOneofFieldInt()); QVERIFY(!test.hasTestOneofComplexField()); - QCOMPARE(test.testOneofField(), QtProtobuf::InvalidFieldNumber); + QCOMPARE(test.testOneofField(), OneofMessage::TestOneofFields::UninitializedField); } void QtProtobufTypesGenerationTest::OneofMessageTest() @@ -345,7 +345,7 @@ void QtProtobufTypesGenerationTest::OneofMessageTest() QVERIFY(!test.hasTestOneofComplexField()); QVERIFY(!test.hasTestOneofSecondComplexField()); QCOMPARE(test.testOneofFieldInt(), 10); - QCOMPARE(test.testOneofField(), OneofMessage::TestOneofFieldIntProtoFieldNumber); + QCOMPARE(test.testOneofField(), OneofMessage::TestOneofFields::TestOneofFieldInt); ComplexMessage complexField; SimpleStringMessage stringField; @@ -357,7 +357,7 @@ void QtProtobufTypesGenerationTest::OneofMessageTest() QVERIFY(test.hasTestOneofComplexField()); QVERIFY(!test.hasTestOneofSecondComplexField()); QCOMPARE(test.testOneofComplexField(), complexField); - QCOMPARE(test.testOneofField(), OneofMessage::TestOneofComplexFieldProtoFieldNumber); + QCOMPARE(test.testOneofField(), OneofMessage::TestOneofFields::TestOneofComplexField); } void QtProtobufTypesGenerationTest::OneofMessageCopyComplexValueTest() @@ -372,13 +372,13 @@ void QtProtobufTypesGenerationTest::OneofMessageCopyComplexValueTest() QVERIFY(test.hasTestOneofComplexField()); QVERIFY(!test.hasTestOneofSecondComplexField()); QCOMPARE(test.testOneofComplexField(), complexField); - QCOMPARE(test.testOneofField(), OneofMessage::TestOneofComplexFieldProtoFieldNumber); + QCOMPARE(test.testOneofField(), OneofMessage::TestOneofFields::TestOneofComplexField); test.setTestOneofSecondComplexField(test.testOneofComplexField()); QVERIFY(!test.hasTestOneofComplexField()); QVERIFY(test.hasTestOneofSecondComplexField()); QCOMPARE(test.testOneofSecondComplexField(), complexField); - QCOMPARE(test.testOneofField(), OneofMessage::TestOneofSecondComplexFieldProtoFieldNumber); + QCOMPARE(test.testOneofField(), OneofMessage::TestOneofFields::TestOneofSecondComplexField); } void QtProtobufTypesGenerationTest::AssignmentOperatorTest() diff --git a/tests/auto/protobufgen/data/expected_result/folder/qtprotobufnamespace/tests/oneofmessages.qpb.cpp b/tests/auto/protobufgen/data/expected_result/folder/qtprotobufnamespace/tests/oneofmessages.qpb.cpp index 678ba83b..fc427907 100644 --- a/tests/auto/protobufgen/data/expected_result/folder/qtprotobufnamespace/tests/oneofmessages.qpb.cpp +++ b/tests/auto/protobufgen/data/expected_result/folder/qtprotobufnamespace/tests/oneofmessages.qpb.cpp @@ -155,9 +155,9 @@ void OneofSimpleMessage::setTestOneofFieldSecondInt_p(QtProtobuf::int32 testOneo m_testOneof.setValue(testOneofFieldSecondInt, 2); } -int OneofSimpleMessage::testOneofField() const +OneofSimpleMessage::TestOneofFields OneofSimpleMessage::testOneofField() const { - return m_testOneof.fieldNumber(); + return static_cast(m_testOneof.fieldNumber()); } void OneofSimpleMessage::clearTestOneof() { @@ -464,17 +464,17 @@ void OneofComplexMessage::setSecondSecondComplexField_p(ComplexMessage *secondSe m_secondOneof.setValue(value, 6); } -int OneofComplexMessage::testOneofField() const +OneofComplexMessage::TestOneofFields OneofComplexMessage::testOneofField() const { - return m_testOneof.fieldNumber(); + return static_cast(m_testOneof.fieldNumber()); } void OneofComplexMessage::clearTestOneof() { m_testOneof = QtProtobufPrivate::QProtobufOneof(); } -int OneofComplexMessage::secondOneofField() const +OneofComplexMessage::SecondOneofFields OneofComplexMessage::secondOneofField() const { - return m_secondOneof.fieldNumber(); + return static_cast(m_secondOneof.fieldNumber()); } void OneofComplexMessage::clearSecondOneof() { diff --git a/tests/auto/protobufgen/data/expected_result/folder/qtprotobufnamespace/tests/oneofmessages.qpb.h b/tests/auto/protobufgen/data/expected_result/folder/qtprotobufnamespace/tests/oneofmessages.qpb.h index e000c68c..09408f45 100644 --- a/tests/auto/protobufgen/data/expected_result/folder/qtprotobufnamespace/tests/oneofmessages.qpb.h +++ b/tests/auto/protobufgen/data/expected_result/folder/qtprotobufnamespace/tests/oneofmessages.qpb.h @@ -41,6 +41,13 @@ public: }; Q_ENUM(QtProtobufFieldEnum) + enum class TestOneofFields { + UninitializedField = QtProtobuf::InvalidFieldNumber, + TestOneofFieldInt = 1, + TestOneofFieldSecondInt = 2, + }; + Q_ENUM(TestOneofFields) + OneofSimpleMessage(); ~OneofSimpleMessage(); OneofSimpleMessage(const OneofSimpleMessage &other); @@ -55,7 +62,7 @@ public: bool hasTestOneofFieldSecondInt() const; QtProtobuf::int32 testOneofFieldSecondInt() const; - int testOneofField() const; + TestOneofFields testOneofField() const; void setTestOneofFieldInt(const QtProtobuf::int32 &testOneofFieldInt); void setTestOneofFieldSecondInt(const QtProtobuf::int32 &testOneofFieldSecondInt); void clearTestOneof(); @@ -100,6 +107,22 @@ public: }; Q_ENUM(QtProtobufFieldEnum) + enum class TestOneofFields { + UninitializedField = QtProtobuf::InvalidFieldNumber, + TestOneofFieldInt = 42, + TestOneofComplexField = 3, + TestOneofSecondComplexField = 4, + }; + Q_ENUM(TestOneofFields) + + enum class SecondOneofFields { + UninitializedField = QtProtobuf::InvalidFieldNumber, + SecondFieldInt = 43, + SecondComplexField = 5, + SecondSecondComplexField = 6, + }; + Q_ENUM(SecondOneofFields) + OneofComplexMessage(); ~OneofComplexMessage(); OneofComplexMessage(const OneofComplexMessage &other); @@ -132,8 +155,8 @@ public: bool hasSecondSecondComplexField() const; ComplexMessage &secondSecondComplexField() const; - int testOneofField() const; - int secondOneofField() const; + TestOneofFields testOneofField() const; + SecondOneofFields secondOneofField() const; void setTestFieldInt(const QtProtobuf::int32 &testFieldInt) { if (m_testFieldInt != testFieldInt) diff --git a/tests/auto/protobufgen/data/expected_result/no-options/oneofmessages.qpb.cpp b/tests/auto/protobufgen/data/expected_result/no-options/oneofmessages.qpb.cpp index 76b77cd7..b42897db 100644 --- a/tests/auto/protobufgen/data/expected_result/no-options/oneofmessages.qpb.cpp +++ b/tests/auto/protobufgen/data/expected_result/no-options/oneofmessages.qpb.cpp @@ -155,9 +155,9 @@ void OneofSimpleMessage::setTestOneofFieldSecondInt_p(QtProtobuf::int32 testOneo m_testOneof.setValue(testOneofFieldSecondInt, 2); } -int OneofSimpleMessage::testOneofField() const +OneofSimpleMessage::TestOneofFields OneofSimpleMessage::testOneofField() const { - return m_testOneof.fieldNumber(); + return static_cast(m_testOneof.fieldNumber()); } void OneofSimpleMessage::clearTestOneof() { @@ -464,17 +464,17 @@ void OneofComplexMessage::setSecondSecondComplexField_p(ComplexMessage *secondSe m_secondOneof.setValue(value, 6); } -int OneofComplexMessage::testOneofField() const +OneofComplexMessage::TestOneofFields OneofComplexMessage::testOneofField() const { - return m_testOneof.fieldNumber(); + return static_cast(m_testOneof.fieldNumber()); } void OneofComplexMessage::clearTestOneof() { m_testOneof = QtProtobufPrivate::QProtobufOneof(); } -int OneofComplexMessage::secondOneofField() const +OneofComplexMessage::SecondOneofFields OneofComplexMessage::secondOneofField() const { - return m_secondOneof.fieldNumber(); + return static_cast(m_secondOneof.fieldNumber()); } void OneofComplexMessage::clearSecondOneof() { diff --git a/tests/auto/protobufgen/data/expected_result/no-options/oneofmessages.qpb.h b/tests/auto/protobufgen/data/expected_result/no-options/oneofmessages.qpb.h index e000c68c..09408f45 100644 --- a/tests/auto/protobufgen/data/expected_result/no-options/oneofmessages.qpb.h +++ b/tests/auto/protobufgen/data/expected_result/no-options/oneofmessages.qpb.h @@ -41,6 +41,13 @@ public: }; Q_ENUM(QtProtobufFieldEnum) + enum class TestOneofFields { + UninitializedField = QtProtobuf::InvalidFieldNumber, + TestOneofFieldInt = 1, + TestOneofFieldSecondInt = 2, + }; + Q_ENUM(TestOneofFields) + OneofSimpleMessage(); ~OneofSimpleMessage(); OneofSimpleMessage(const OneofSimpleMessage &other); @@ -55,7 +62,7 @@ public: bool hasTestOneofFieldSecondInt() const; QtProtobuf::int32 testOneofFieldSecondInt() const; - int testOneofField() const; + TestOneofFields testOneofField() const; void setTestOneofFieldInt(const QtProtobuf::int32 &testOneofFieldInt); void setTestOneofFieldSecondInt(const QtProtobuf::int32 &testOneofFieldSecondInt); void clearTestOneof(); @@ -100,6 +107,22 @@ public: }; Q_ENUM(QtProtobufFieldEnum) + enum class TestOneofFields { + UninitializedField = QtProtobuf::InvalidFieldNumber, + TestOneofFieldInt = 42, + TestOneofComplexField = 3, + TestOneofSecondComplexField = 4, + }; + Q_ENUM(TestOneofFields) + + enum class SecondOneofFields { + UninitializedField = QtProtobuf::InvalidFieldNumber, + SecondFieldInt = 43, + SecondComplexField = 5, + SecondSecondComplexField = 6, + }; + Q_ENUM(SecondOneofFields) + OneofComplexMessage(); ~OneofComplexMessage(); OneofComplexMessage(const OneofComplexMessage &other); @@ -132,8 +155,8 @@ public: bool hasSecondSecondComplexField() const; ComplexMessage &secondSecondComplexField() const; - int testOneofField() const; - int secondOneofField() const; + TestOneofFields testOneofField() const; + SecondOneofFields secondOneofField() const; void setTestFieldInt(const QtProtobuf::int32 &testFieldInt) { if (m_testFieldInt != testFieldInt)