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
pull/12851/head
Protobuf Team Bot 2 years ago committed by Copybara-Service
parent 2a1ee6939e
commit 10cf9140c7
  1. 2
      src/google/protobuf/descriptor.cc
  2. 36
      src/google/protobuf/descriptor_unittest.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;

@ -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(

Loading…
Cancel
Save