Fixes to make the --fatal_warnings flag work with current master

protocolbuffers · Mar 9, 2021 · 0092910 · 0092910
1 parent 7110293
commit 0092910
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 15 deletions.
diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc
@@ -2054,6 +2054,10 @@ Parse PROTO_FILES and generate output based on the options given:
   --error_format=FORMAT       Set the format in which to print errors.
                               FORMAT may be 'gcc' (the default) or 'msvs'
                               (Microsoft Visual Studio format).
+  --fatal_warnings            Make warnings be fatal (similar to -Werr in
+                              gcc). This flag will make protoc return
+                              with a non-zero exit code if any warnings
+                              are generated.
   --print_free_field_numbers  Print the free field numbers of the messages
                               defined in the given proto files. Groups share
                               the same field number space with the parent

diff --git a/src/google/protobuf/compiler/command_line_interface.h b/src/google/protobuf/compiler/command_line_interface.h
@@ -393,7 +393,7 @@ class PROTOC_EXPORT CommandLineInterface {
   ErrorFormat error_format_ = ERROR_FORMAT_GCC;
 
   // True if we should treat warnings as errors that fail the compilation.
-  bool fatal_warnings_;
+  bool fatal_warnings_ = false;
 
   std::vector<std::pair<std::string, std::string> >
       proto_path_;                        // Search path for proto files.

diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc
@@ -132,9 +132,6 @@ class CommandLineInterfaceTest : public testing::Test {
   // -----------------------------------------------------------------
   // Methods to check the test results (called after Run()).
 
-  // Checks that Run() returned code r.
-  void ExpectReturnCode(int r);
-
   // Checks that no text was written to stderr during Run(), and Run()
   // returned 0.
   void ExpectNoErrors();
@@ -160,6 +157,11 @@ class CommandLineInterfaceTest : public testing::Test {
   void ExpectCapturedStdoutSubstringWithZeroReturnCode(
       const std::string& expected_substring);
 
+  // Checks that Run() returned zero and the stderr contains the given
+  // substring.
+  void ExpectCapturedStderrSubstringWithZeroReturnCode(
+      const std::string& expected_substring);
+
 #if defined(_WIN32) && !defined(__CYGWIN__)
   // Returns true if ExpectErrorSubstring(expected_substring) would pass, but
   // does not fail otherwise.
@@ -409,10 +411,6 @@ void CommandLineInterfaceTest::CreateTempDir(const std::string& name) {
 
 // -------------------------------------------------------------------
 
-void CommandLineInterfaceTest::ExpectReturnCode(int r) {
-  EXPECT_EQ(r, return_code_);
-}
-
 void CommandLineInterfaceTest::ExpectNoErrors() {
   EXPECT_EQ(0, return_code_);
   EXPECT_EQ("", error_text_);
@@ -433,8 +431,8 @@ void CommandLineInterfaceTest::ExpectErrorSubstring(
 
 void CommandLineInterfaceTest::ExpectWarningSubstring(
     const std::string& expected_substring) {
-  EXPECT_EQ(0, return_code_);
   EXPECT_PRED_FORMAT2(testing::IsSubstring, expected_substring, error_text_);
+  EXPECT_EQ(0, return_code_);
 }
 
 #if defined(_WIN32) && !defined(__CYGWIN__)
@@ -522,6 +520,13 @@ void CommandLineInterfaceTest::ExpectCapturedStdoutSubstringWithZeroReturnCode(
                       captured_stdout_);
 }
 
+void CommandLineInterfaceTest::ExpectCapturedStderrSubstringWithZeroReturnCode(
+    const std::string& expected_substring) {
+  EXPECT_EQ(0, return_code_);
+  EXPECT_PRED_FORMAT2(testing::IsSubstring, expected_substring,
+                      error_text_);
+}
+
 void CommandLineInterfaceTest::ExpectFileContent(const std::string& filename,
                                                  const std::string& content) {
   std::string path = temp_directory_ + "/" + filename;
@@ -2310,13 +2315,14 @@ TEST_F(CommandLineInterfaceTest, MsvsFormatErrors) {
 }
 
 TEST_F(CommandLineInterfaceTest, InvalidErrorFormat) {
-  // Test --error_format=msvs
+  // Test invalid --error_format
 
   CreateTempFile("foo.proto",
                  "syntax = \"proto2\";\n"
                  "badsyntax\n");
 
-  Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto");
+  Run("protocol_compiler --test_out=$tmpdir "
+      "--proto_path=$tmpdir --error_format=invalid foo.proto");
 
   ExpectErrorText("Unknown error format: invalid\n");
 }
@@ -2332,14 +2338,13 @@ TEST_F(CommandLineInterfaceTest, Warnings) {
 
   Run("protocol_compiler --test_out=$tmpdir "
     "--proto_path=$tmpdir foo.proto");
-  ExpectReturnCode(0);
-  ExpectErrorSubstringWithZeroReturnCode(
-    "foo.proto: warning: Import bar.proto but not used.");
+  ExpectCapturedStderrSubstringWithZeroReturnCode(
+    "foo.proto:2:1: warning: Import bar.proto is unused.");
 
   Run("protocol_compiler --test_out=$tmpdir --fatal_warnings "
     "--proto_path=$tmpdir foo.proto");
   ExpectErrorSubstring(
-    "foo.proto: warning: Import bar.proto but not used.");
+    "foo.proto:2:1: warning: Import bar.proto is unused.");
 }
 
 // -------------------------------------------------------------------