Fix detecting of unused nested messages

Previously if nested message was never used by the nesting class it
was ignored to be generated. It was the result if invalid filtering
of map messages as nested types.
Use the explicit map_key() based check instead of weird and tricky
one based on nesting class fields.

Keep the fallback solution that uses MessageOptions when building
with libprotoc versions less than 3.12.

Fixes: QTBUG-115175
Pick-to: 6.5 6.6
Change-Id: I4aa238dac3375bdcdf85e41ba9daef74474ed0fb
Reviewed-by: Tatiana Borisova <tatiana.borisova@qt.io>

src/tools/qtprotoccommon/CMakeLists.txt

@@ -2,6 +2,7 @@
 # SPDX-License-Identifier: BSD-3-Clause
 
 qt_add_library(QtProtocCommon STATIC
+    qtprotocdefs.h
     baseprinter.cpp baseprinter.h
     descriptorprinterbase.h
     generatorbase.cpp generatorbase.h

src/tools/qtprotoccommon/generatorbase.h

@@ -6,7 +6,7 @@
 #define GENERATORBASE_H
 
 #include <google/protobuf/compiler/code_generator.h>
-#include <google/protobuf/port_def.inc>
+#include "qtprotocdefs.h"
 
 #include <string>
 #include <functional>
@@ -41,7 +41,7 @@ public:
 // TODO: This suppresses the build issue with old protobuf versions. Since we don't have the
 // strict protobuf versions that we support this work around will be here for a while, since
 // yocto builds quite old protobuf by now. See QTBUG-115702.
-#if PROTOBUF_VERSION < 3012000
+#ifndef HAVE_PROTOBUF_SYNC_PIPER
     uint64_t GetSupportedFeatures() const { return 0; };
 #else
     uint64_t GetSupportedFeatures() const override
@@ -62,6 +62,4 @@ protected:
 };
 } // namespace qtprotoccommon
 
-#include <google/protobuf/port_undef.inc>
-
 #endif // GENERATORBASE_H

src/tools/qtprotoccommon/generatorcommon.cpp

@@ -7,6 +7,12 @@
 #include "utils.h"
 #include "commontemplates.h"
 
+#include "qtprotocdefs.h"
+
+#ifndef HAVE_PROTOBUF_SYNC_PIPER
+#  include <google/protobuf/descriptor.pb.h>
+#endif
+
 #include <cassert>
 #include <algorithm>
 #include <string_view>
@@ -622,41 +628,31 @@ void common::iterateNestedMessages(const Descriptor *message,
     int numNestedTypes = message->nested_type_count();
     for (int i = 0; i < numNestedTypes; ++i) {
         const Descriptor *nestedMessage = message->nested_type(i);
-        if (message->field_count() <= 0) {
+#ifdef HAVE_PROTOBUF_SYNC_PIPER
+        if (nestedMessage->map_key() == nullptr) {
+#else
+        if (!nestedMessage->options().map_entry()) {
+#endif
             callback(nestedMessage);
             continue;
         }
-        int numFields = message->field_count();
-        for (int j = 0; j < numFields; ++j) {
-            const FieldDescriptor *field = message->field(j);
-            // Probably there is more correct way to detect map in
-            // nested messages.
-            // TODO: Have idea to make maps nested classes instead of typedefs.
-            if (!field->is_map() && field->message_type() == nestedMessage) {
-                callback(nestedMessage);
-                break;
-            }
-        }
     }
 }
 
 bool common::hasNestedMessages(const Descriptor *message)
 {
     int numNestedTypes = message->nested_type_count();
-    int numFields = message->field_count();
-    if (numNestedTypes > 0 && numFields <= 0)
+    if (numNestedTypes > 0 && message->field_count() == 0)
         return true;
 
     for (int i = 0; i < numNestedTypes; ++i) {
         const Descriptor *nestedMessage = message->nested_type(i);
-        for (int j = 0; j < numFields; ++j) {
-            const FieldDescriptor *field = message->field(j);
-            // Probably there is more correct way to detect map in
-            // nested messages.
-            // TODO: Have idea to make maps nested classes instead of typedefs.
-            if (!field->is_map() && field->message_type() == nestedMessage)
-                return true;
-        }
+#ifdef HAVE_PROTOBUF_SYNC_PIPER
+        if (nestedMessage->map_key() == nullptr)
+#else
+        if (!nestedMessage->options().map_entry())
+#endif
+            return true;
     }
 
     return false;

src/tools/qtprotoccommon/qtprotocdefs.h

@@ -0,0 +1,14 @@
+// Copyright (C) 2023 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+#ifndef QTPROTOCDEFS_H
+#define QTPROTOCDEFS_H
+
+#include <google/protobuf/port_def.inc>
+#if PROTOBUF_VERSION >= 3012000
+// Idicates the major libprotoc API update that happened in version 3.12
+#  define HAVE_PROTOBUF_SYNC_PIPER
+#endif
+#include <google/protobuf/port_undef.inc>
+
+#endif // QTPROTOCDEFS_H

tests/auto/protobuf/nested/proto/nestedmessages.proto

@@ -10,6 +10,9 @@ message NestedFieldMessage {
     message NestedMessage {
         sint32 testFieldInt = 1;
     }
+    message UnusedNestedMessage {
+        sint32 testFieldInt = 1;
+    }
     NestedMessage nested = 2;
 }
 

tests/auto/protobuf/nested/tst_protobuf_nestedtypes.cpp

@@ -27,6 +27,7 @@ private slots:
     void NeighborTest();
     void NestedNoFieldsTest();
     void NestedCyclingTest();
+    void UnusedNestedMessageTest();
 private:
     std::unique_ptr<QProtobufSerializer> m_serializer;
 };
@@ -253,5 +254,12 @@ void QtProtobufNestedTest::NestedCyclingTest()
     test2.setTestField(test);
 }
 
+void QtProtobufNestedTest::UnusedNestedMessageTest()
+{
+    qProtobufAssertMessagePropertyRegistered<NestedFieldMessage::UnusedNestedMessage,
+                                             QtProtobuf::sint32>(1, "QtProtobuf::sint32",
+                                                                 "testFieldInt");
+}
+
 QTEST_MAIN(QtProtobufNestedTest)
 #include "tst_protobuf_nestedtypes.moc"