Skip to content

Commit

Permalink
Revert "Various fixups to warnings flags (#9344)" (#10037)
Browse files Browse the repository at this point in the history
This reverts commit 448d421.

Unfortunately we have to revert this because we're finding that it
introduces too much new build log spam for existing proto files that are
out of compliance with the warnings. We might be able to roll it forward
again if we can figure out a way to do so without so many new log
messages.
  • Loading branch information
acozzette authored May 24, 2022
1 parent 586b72c commit 5e7b709
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 33 deletions.
25 changes: 6 additions & 19 deletions src/google/protobuf/compiler/command_line_interface_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1217,8 +1217,7 @@ TEST_F(CommandLineInterfaceTest, InsertWithAnnotationFixup) {
"--plug_out=insert_endlines=test_generator,test_plugin:$tmpdir "
"--proto_path=$tmpdir foo.proto");

ExpectWarningSubstring(
"foo.proto:2:36: warning: Message name should be in UpperCamelCase.");
ExpectNoErrors();
CheckGeneratedAnnotations("test_generator", "foo.proto");
CheckGeneratedAnnotations("test_plugin", "foo.proto");
}
Expand Down Expand Up @@ -2372,21 +2371,6 @@ 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

Expand Down Expand Up @@ -2707,6 +2691,7 @@ TEST_P(EncodeDecodeTest, Encode) {
EXPECT_TRUE(Run(args + " --encode=protobuf_unittest.TestAllTypes"));
ExpectStdoutMatchesBinaryFile(TestUtil::GetTestDataPath(
"third_party/protobuf/testdata/golden_message_oneof_implemented"));
ExpectStderrMatchesText("");
}

TEST_P(EncodeDecodeTest, Decode) {
Expand All @@ -2718,6 +2703,7 @@ TEST_P(EncodeDecodeTest, Decode) {
ExpectStdoutMatchesTextFile(TestUtil::GetTestDataPath(
"third_party/protobuf/"
"testdata/text_format_unittest_data_oneof_implemented.txt"));
ExpectStderrMatchesText("");
}

TEST_P(EncodeDecodeTest, Partial) {
Expand All @@ -2726,7 +2712,7 @@ TEST_P(EncodeDecodeTest, Partial) {
Run(TestUtil::MaybeTranslatePath("net/proto2/internal/unittest.proto") +
" --encode=protobuf_unittest.TestRequired"));
ExpectStdoutMatchesText("");
ExpectStderrContainsText(
ExpectStderrMatchesText(
"warning: Input message is missing required fields: a, b, c\n");
}

Expand All @@ -2750,7 +2736,7 @@ TEST_P(EncodeDecodeTest, UnknownType) {
Run(TestUtil::MaybeTranslatePath("net/proto2/internal/unittest.proto") +
" --encode=NoSuchType"));
ExpectStdoutMatchesText("");
ExpectStderrContainsText("Type not defined: NoSuchType\n");
ExpectStderrMatchesText("Type not defined: NoSuchType\n");
}

TEST_P(EncodeDecodeTest, ProtoParseError) {
Expand All @@ -2775,6 +2761,7 @@ TEST_P(EncodeDecodeTest, EncodeDeterministicOutput) {
args + " --encode=protobuf_unittest.TestAllTypes --deterministic_output"));
ExpectStdoutMatchesBinaryFile(TestUtil::GetTestDataPath(
"third_party/protobuf/testdata/golden_message_oneof_implemented"));
ExpectStderrMatchesText("");
}

TEST_P(EncodeDecodeTest, DecodeDeterministicOutput) {
Expand Down
6 changes: 0 additions & 6 deletions src/google/protobuf/compiler/importer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,6 @@ 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_;
Expand Down
12 changes: 5 additions & 7 deletions src/google/protobuf/compiler/parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -643,11 +643,10 @@ 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_) {
AddWarning(
"No syntax specified. Please use 'syntax = \"proto2\";' or "
"'syntax = \"proto3\";' to specify a syntax version. "
"(Defaulted to proto2 syntax.)"
);
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.)";
syntax_identifier_ = "proto2";
}

Expand Down Expand Up @@ -1020,8 +1019,7 @@ bool Parser::ParseMessageFieldNoLabel(
location.RecordLegacyLocation(field, DescriptorPool::ErrorCollector::NAME);
DO(ConsumeIdentifier(field->mutable_name(), "Expected field name."));

if (field->type() != FieldDescriptorProto::TYPE_GROUP &&
!IsLowerUnderscore(field->name())) {
if (!IsLowerUnderscore(field->name())) {
AddWarning(
"Field name should be lowercase. Found: " + field->name() +
". See: https://developers.google.com/protocol-buffers/docs/style");
Expand Down
3 changes: 2 additions & 1 deletion src/google/protobuf/compiler/parser_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,9 @@ TEST_F(ParserTest, StopAfterSyntaxIdentifierWithErrors) {
TEST_F(ParserTest, WarnIfSyntaxIdentifierOmmitted) {
SetupParser("message A {}");
FileDescriptorProto file;
CaptureTestStderr();
EXPECT_TRUE(parser_->Parse(input_.get(), &file));
EXPECT_TRUE(error_collector_.warning_.find("No syntax specified") !=
EXPECT_TRUE(GetCapturedTestStderr().find("No syntax specified") !=
std::string::npos);
}

Expand Down

0 comments on commit 5e7b709

Please sign in to comment.