diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index f48135ecb3481..e88489753d3f3 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -1217,7 +1217,8 @@ TEST_F(CommandLineInterfaceTest, InsertWithAnnotationFixup) { "--plug_out=insert_endlines=test_generator,test_plugin:$tmpdir " "--proto_path=$tmpdir foo.proto"); - ExpectNoErrors(); + ExpectWarningSubstring( + "foo.proto:2:36: warning: Message name should be in UpperCamelCase."); CheckGeneratedAnnotations("test_generator", "foo.proto"); CheckGeneratedAnnotations("test_plugin", "foo.proto"); } @@ -2371,6 +2372,21 @@ TEST_F(CommandLineInterfaceTest, Warnings) { ExpectErrorSubstring("foo.proto:2:1: warning: Import bar.proto is unused."); } +TEST_F(CommandLineInterfaceTest, ParserWarnings) { + // Test that parser warnings are propagated. See #9343. + + CreateTempFile("foo.proto", + "syntax = \"proto2\";\n" + "message bad_to_the_bone {};\n"); + + Run("protocol_compiler --test_out=$tmpdir " + "--proto_path=$tmpdir foo.proto"); + ExpectCapturedStderrSubstringWithZeroReturnCode( + "foo.proto:2:25: warning: Message name should be in UpperCamelCase. " + "Found: bad_to_the_bone. " + "See https://developers.google.com/protocol-buffers/docs/style"); +} + // ------------------------------------------------------------------- // Flag parsing tests @@ -2691,7 +2707,6 @@ TEST_P(EncodeDecodeTest, Encode) { EXPECT_TRUE(Run(args + " --encode=protobuf_unittest.TestAllTypes")); ExpectStdoutMatchesBinaryFile(TestUtil::GetTestDataPath( "net/proto2/internal/testdata/golden_message_oneof_implemented")); - ExpectStderrMatchesText(""); } TEST_P(EncodeDecodeTest, Decode) { @@ -2703,7 +2718,6 @@ TEST_P(EncodeDecodeTest, Decode) { ExpectStdoutMatchesTextFile(TestUtil::GetTestDataPath( "net/proto2/internal/" "testdata/text_format_unittest_data_oneof_implemented.txt")); - ExpectStderrMatchesText(""); } TEST_P(EncodeDecodeTest, Partial) { @@ -2712,7 +2726,7 @@ TEST_P(EncodeDecodeTest, Partial) { Run(TestUtil::MaybeTranslatePath("net/proto2/internal/unittest.proto") + " --encode=protobuf_unittest.TestRequired")); ExpectStdoutMatchesText(""); - ExpectStderrMatchesText( + ExpectStderrContainsText( "warning: Input message is missing required fields: a, b, c\n"); } @@ -2736,7 +2750,7 @@ TEST_P(EncodeDecodeTest, UnknownType) { Run(TestUtil::MaybeTranslatePath("net/proto2/internal/unittest.proto") + " --encode=NoSuchType")); ExpectStdoutMatchesText(""); - ExpectStderrMatchesText("Type not defined: NoSuchType\n"); + ExpectStderrContainsText("Type not defined: NoSuchType\n"); } TEST_P(EncodeDecodeTest, ProtoParseError) { @@ -2761,7 +2775,6 @@ TEST_P(EncodeDecodeTest, EncodeDeterministicOutput) { args + " --encode=protobuf_unittest.TestAllTypes --deterministic_output")); ExpectStdoutMatchesBinaryFile(TestUtil::GetTestDataPath( "net/proto2/internal/testdata/golden_message_oneof_implemented")); - ExpectStderrMatchesText(""); } TEST_P(EncodeDecodeTest, DecodeDeterministicOutput) { diff --git a/src/google/protobuf/compiler/importer.cc b/src/google/protobuf/compiler/importer.cc index f1e26f8bdd1d3..595bcd1276376 100644 --- a/src/google/protobuf/compiler/importer.cc +++ b/src/google/protobuf/compiler/importer.cc @@ -105,6 +105,12 @@ class SourceTreeDescriptorDatabase::SingleFileErrorCollector had_errors_ = true; } + void AddWarning(int line, int column, const std::string& message) override { + if (multi_file_error_collector_ != NULL) { + multi_file_error_collector_->AddWarning(filename_, line, column, message); + } + } + private: std::string filename_; MultiFileErrorCollector* multi_file_error_collector_; diff --git a/src/google/protobuf/compiler/parser.cc b/src/google/protobuf/compiler/parser.cc index 5bd37d147bc44..472394dbba04d 100644 --- a/src/google/protobuf/compiler/parser.cc +++ b/src/google/protobuf/compiler/parser.cc @@ -643,10 +643,11 @@ bool Parser::Parse(io::Tokenizer* input, FileDescriptorProto* file) { // Store the syntax into the file. if (file != nullptr) file->set_syntax(syntax_identifier_); } else if (!stop_after_syntax_identifier_) { - GOOGLE_LOG(WARNING) << "No syntax specified for the proto file: " << file->name() - << ". Please use 'syntax = \"proto2\";' " - << "or 'syntax = \"proto3\";' to specify a syntax " - << "version. (Defaulted to proto2 syntax.)"; + AddWarning( + "No syntax specified. Please use 'syntax = \"proto2\";' or " + "'syntax = \"proto3\";' to specify a syntax version. " + "(Defaulted to proto2 syntax.)" + ); syntax_identifier_ = "proto2"; } @@ -1019,7 +1020,8 @@ bool Parser::ParseMessageFieldNoLabel( location.RecordLegacyLocation(field, DescriptorPool::ErrorCollector::NAME); DO(ConsumeIdentifier(field->mutable_name(), "Expected field name.")); - if (!IsLowerUnderscore(field->name())) { + if (field->type() != FieldDescriptorProto::TYPE_GROUP && + !IsLowerUnderscore(field->name())) { AddWarning( "Field name should be lowercase. Found: " + field->name() + ". See: https://developers.google.com/protocol-buffers/docs/style"); diff --git a/src/google/protobuf/compiler/parser_unittest.cc b/src/google/protobuf/compiler/parser_unittest.cc index 2d681d957f7ee..83f591dc3c74f 100644 --- a/src/google/protobuf/compiler/parser_unittest.cc +++ b/src/google/protobuf/compiler/parser_unittest.cc @@ -221,9 +221,8 @@ TEST_F(ParserTest, StopAfterSyntaxIdentifierWithErrors) { TEST_F(ParserTest, WarnIfSyntaxIdentifierOmmitted) { SetupParser("message A {}"); FileDescriptorProto file; - CaptureTestStderr(); EXPECT_TRUE(parser_->Parse(input_.get(), &file)); - EXPECT_TRUE(GetCapturedTestStderr().find("No syntax specified") != + EXPECT_TRUE(error_collector_.warning_.find("No syntax specified") != std::string::npos); }