From 10cf9140c752614d6a95a09b600685bad1e85b81 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 18 May 2023 11:00:17 -0700 Subject: [PATCH] Fixes false negatives in unused dependency checker. Don't mark a dependency as used just because it defines a package that's used, since many files can define the same package. PiperOrigin-RevId: 533184254 --- src/google/protobuf/descriptor.cc | 2 +- src/google/protobuf/descriptor_unittest.cc | 36 ++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index 9a7e37adc2..acd65a44bf 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -4436,7 +4436,7 @@ Symbol DescriptorBuilder::FindSymbolNotEnforcingDeps(const std::string& name, // Only find symbols which were defined in this file or one of its // dependencies. const FileDescriptor* file = result.GetFile(); - if (file == file_ || dependencies_.contains(file)) { + if ((file == file_ || dependencies_.contains(file)) && !result.IsPackage()) { unused_dependency_.erase(file); } return result; diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index ded1321720..d5db1d2cc0 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -6204,6 +6204,42 @@ TEST_F(ValidationErrorTest, UnusedImportWarning) { "forward.proto: bar.proto: IMPORT: Import bar.proto is unused.\n"); } +// Verifies that the dependency checker isn't fooled by package symbols, +// which can be defined in multiple files. +TEST_F(ValidationErrorTest, SamePackageUnusedImportError) { + BuildFile(R"pb( + name: "unused_dependency.proto" + package: "protobuf_unittest.subpackage" + message_type { name: "Foo" } + )pb"); + + BuildFile(R"pb( + name: "used_dependency.proto" + package: "protobuf_unittest.subpackage" + message_type { name: "Bar" } + )pb"); + + pool_.AddUnusedImportTrackFile("import.proto", true); + BuildFileWithErrors(R"pb( + name: "import.proto" + package: "protobuf_unittest" + dependency: "unused_dependency.proto" + dependency: "used_dependency.proto" + message_type { + name: "Baz" + field { + name: "bar" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_MESSAGE + type_name: "subpackage.Bar" + } + } + )pb", + "import.proto: unused_dependency.proto: " + "IMPORT: Import unused_dependency.proto is unused.\n"); +} + namespace { void FillValidMapEntry(FileDescriptorProto* file_proto) { ASSERT_TRUE(TextFormat::ParseFromString(