Correctly report warnings in cases where they have been silently supp…

…ressed.

SingleFileErrorCollector delegates to a MultiFileErrorCollector; before this change it delegates the errors but not the warnings, which meant many warnings were silently discarded.

PiperOrigin-RevId: 709785563

src/google/protobuf/compiler/command_line_interface_tester.cc

@@ -80,7 +80,7 @@ void CommandLineInterfaceTester::RunProtocWithArgs(
 
   return_code_ = cli_.Run(static_cast<int>(args.size()), argv.data());
 
-  error_text_ = GetCapturedTestStderr();
+  captured_stderr_ = GetCapturedTestStderr();
 #if !defined(__CYGWIN__)
   captured_stdout_ = GetCapturedTestStdout();
 #endif
@@ -116,26 +116,31 @@ void CommandLineInterfaceTester::CreateTempDir(absl::string_view name) {
 
 void CommandLineInterfaceTester::ExpectNoErrors() {
   EXPECT_EQ(0, return_code_);
-  EXPECT_EQ("", error_text_);
+  // Every line printed to stderr should be a warning, not an error.
+  for (const auto& line :
+       absl::StrSplit(captured_stderr_, '\n', absl::SkipEmpty())) {
+    EXPECT_THAT(line, HasSubstr("warning:"));
+  }
 }
 
 void CommandLineInterfaceTester::ExpectErrorText(
     absl::string_view expected_text) {
   EXPECT_NE(0, return_code_);
-  EXPECT_THAT(error_text_, HasSubstr(absl::StrReplaceAll(
-                               expected_text, {{"$tmpdir", temp_directory_}})));
+  EXPECT_THAT(captured_stderr_,
+              HasSubstr(absl::StrReplaceAll(expected_text,
+                                            {{"$tmpdir", temp_directory_}})));
 }
 
 void CommandLineInterfaceTester::ExpectErrorSubstring(
     absl::string_view expected_substring) {
   EXPECT_NE(0, return_code_);
-  EXPECT_THAT(error_text_, HasSubstr(expected_substring));
+  EXPECT_THAT(captured_stderr_, HasSubstr(expected_substring));
 }
 
 void CommandLineInterfaceTester::ExpectWarningSubstring(
     absl::string_view expected_substring) {
   EXPECT_EQ(0, return_code_);
-  EXPECT_THAT(error_text_, HasSubstr(expected_substring));
+  EXPECT_THAT(captured_stderr_, HasSubstr(expected_substring));
 }
 
 #if defined(_WIN32) && !defined(__CYGWIN__)
@@ -162,7 +167,7 @@ void CommandLineInterfaceTester::
     ExpectCapturedStderrSubstringWithZeroReturnCode(
         absl::string_view expected_substring) {
   EXPECT_EQ(0, return_code_);
-  EXPECT_THAT(error_text_, HasSubstr(expected_substring));
+  EXPECT_THAT(captured_stderr_, HasSubstr(expected_substring));
 }
 
 void CommandLineInterfaceTester::ExpectFileContent(absl::string_view filename,

src/google/protobuf/compiler/command_line_interface_tester.h

@@ -136,10 +136,8 @@ class CommandLineInterfaceTester : public testing::Test {
   // The result of Run().
   int return_code_;
 
-  // The captured stderr output.
-  std::string error_text_;
+  std::string captured_stderr_;
 
-  // The captured stdout.
   std::string captured_stdout_;
 
   std::vector<std::unique_ptr<CodeGenerator>> generators_;

src/google/protobuf/compiler/command_line_interface_unittest.cc

@@ -19,6 +19,7 @@
 #include <gmock/gmock.h>
 #include "absl/log/absl_check.h"
 #include "absl/strings/escaping.h"
+#include "absl/strings/match.h"
 #include "absl/strings/str_cat.h"
 #include "absl/types/span.h"
 #include "google/protobuf/compiler/command_line_interface_tester.h"
@@ -4204,6 +4205,15 @@ class EncodeDecodeTest : public testing::TestWithParam<EncodeDecodeTestMode> {
     captured_stdout_ = GetCapturedTestStdout();
     captured_stderr_ = GetCapturedTestStderr();
 
+    for (const auto& line :
+         absl::StrSplit(StripCR(captured_stderr_), '\n', absl::SkipEmpty())) {
+      if (absl::StrContains(line, "warning:")) {
+        captured_warnings_.push_back(std::string(line));
+      } else {
+        captured_errors_.push_back(std::string(line));
+      }
+    }
+
     return result == 0;
   }
 
@@ -4225,6 +4235,30 @@ class EncodeDecodeTest : public testing::TestWithParam<EncodeDecodeTestMode> {
     ExpectStdoutMatchesText(expected_output);
   }
 
+  void ExpectNoErrors() { EXPECT_THAT(captured_errors_, testing::IsEmpty()); }
+
+  void ExpectNoWarnings() {
+    EXPECT_THAT(captured_warnings_, testing::IsEmpty());
+  }
+
+  void ExpectError(absl::string_view expected_text) {
+    EXPECT_THAT(captured_errors_, testing::Contains(expected_text));
+  }
+
+  void ExpectErrorSubstring(absl::string_view expected_substring) {
+    EXPECT_THAT(captured_errors_,
+                testing::Contains(testing::HasSubstr(expected_substring)));
+  }
+
+  void ExpectWarning(absl::string_view expected_text) {
+    EXPECT_THAT(captured_warnings_, testing::Contains(expected_text));
+  }
+
+  void ExpectWarningSubstring(absl::string_view expected_substring) {
+    EXPECT_THAT(captured_warnings_,
+                testing::Contains(testing::HasSubstr(expected_substring)));
+  }
+
   void ExpectStdoutMatchesText(const std::string& expected_text) {
     EXPECT_EQ(StripCR(expected_text), StripCR(captured_stdout_));
   }
@@ -4263,6 +4297,9 @@ class EncodeDecodeTest : public testing::TestWithParam<EncodeDecodeTestMode> {
   int duped_stdin_;
   std::string captured_stdout_;
   std::string captured_stderr_;
+  std::vector<std::string> captured_warnings_;
+  std::vector<std::string> captured_errors_;
+
   std::string unittest_proto_descriptor_set_filename_;
 };
 
@@ -4286,7 +4323,7 @@ TEST_P(EncodeDecodeTest, Encode) {
   EXPECT_TRUE(
       Run(absl::StrCat(args, " --encode=protobuf_unittest.TestAllTypes")));
   ExpectStdoutMatchesBinaryFile(golden_path);
-  ExpectStderrMatchesText("");
+  ExpectNoErrors();
 }
 
 TEST_P(EncodeDecodeTest, Decode) {
@@ -4299,7 +4336,7 @@ TEST_P(EncodeDecodeTest, Decode) {
   ExpectStdoutMatchesTextFile(TestUtil::GetTestDataPath(
       "google/protobuf/"
       "testdata/text_format_unittest_data_oneof_implemented.txt"));
-  ExpectStderrMatchesText("");
+  ExpectNoErrors();
 }
 
 TEST_P(EncodeDecodeTest, Partial) {
@@ -4308,8 +4345,7 @@ TEST_P(EncodeDecodeTest, Partial) {
       Run("google/protobuf/unittest.proto"
           " --encode=protobuf_unittest.TestRequired"));
   ExpectStdoutMatchesText("");
-  ExpectStderrMatchesText(
-      "warning:  Input message is missing required fields:  a, b, c\n");
+  ExpectWarning("warning:  Input message is missing required fields:  a, b, c");
 }
 
 TEST_P(EncodeDecodeTest, DecodeRaw) {
@@ -4324,24 +4360,25 @@ TEST_P(EncodeDecodeTest, DecodeRaw) {
   ExpectStdoutMatchesText(
       "1: 123\n"
       "14: \"foo\"\n");
-  ExpectStderrMatchesText("");
+  ExpectNoErrors();
 }
 
 TEST_P(EncodeDecodeTest, UnknownType) {
   EXPECT_FALSE(
       Run("google/protobuf/unittest.proto"
           " --encode=NoSuchType"));
   ExpectStdoutMatchesText("");
-  ExpectStderrMatchesText("Type not defined: NoSuchType\n");
+  ExpectError("Type not defined: NoSuchType");
 }
 
 TEST_P(EncodeDecodeTest, ProtoParseError) {
   EXPECT_FALSE(
       Run("net/proto2/internal/no_such_file.proto "
           "--encode=NoSuchType"));
   ExpectStdoutMatchesText("");
-  ExpectStderrContainsText(
-      "net/proto2/internal/no_such_file.proto: No such file or directory\n");
+  ExpectErrorSubstring(
+      "net/proto2/internal/no_such_file.proto: "
+      "No such file or directory");
 }
 
 TEST_P(EncodeDecodeTest, EncodeDeterministicOutput) {
@@ -4357,7 +4394,7 @@ TEST_P(EncodeDecodeTest, EncodeDeterministicOutput) {
   EXPECT_TRUE(Run(absl::StrCat(
       args, " --encode=protobuf_unittest.TestAllTypes --deterministic_output")));
   ExpectStdoutMatchesBinaryFile(golden_path);
-  ExpectStderrMatchesText("");
+  ExpectNoErrors();
 }
 
 TEST_P(EncodeDecodeTest, DecodeDeterministicOutput) {
@@ -4367,8 +4404,7 @@ TEST_P(EncodeDecodeTest, DecodeDeterministicOutput) {
   EXPECT_FALSE(
       Run("google/protobuf/unittest.proto"
           " --decode=protobuf_unittest.TestAllTypes --deterministic_output"));
-  ExpectStderrMatchesText(
-      "Can only use --deterministic_output with --encode.\n");
+  ExpectError("Can only use --deterministic_output with --encode.");
 }
 
 INSTANTIATE_TEST_SUITE_P(FileDescriptorSetSource, EncodeDecodeTest,

src/google/protobuf/compiler/importer.cc

@@ -11,6 +11,8 @@
 
 #include "google/protobuf/compiler/importer.h"
 
+#include <string>
+
 #ifdef _MSC_VER
 #include <direct.h>
 #else
@@ -21,18 +23,16 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 
-#include <algorithm>
 #include <memory>
 #include <vector>
 
 #include "absl/strings/match.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/str_join.h"
-#include "absl/strings/str_replace.h"
 #include "absl/strings/str_split.h"
 #include "absl/strings/string_view.h"
 #include "google/protobuf/compiler/parser.h"
-#include "google/protobuf/io/io_win32.h"
+#include "google/protobuf/io/io_win32.h"  // IWYU pragma: keep
 #include "google/protobuf/io/tokenizer.h"
 #include "google/protobuf/io/zero_copy_stream_impl.h"
 
@@ -49,6 +49,7 @@ using google::protobuf::io::win32::open;
 
 #if defined(_WIN32) || defined(__CYGWIN__)
 #include "absl/strings/ascii.h"
+#include "absl/strings/str_replace.h"
 #endif
 
 // Returns true if the text looks like a Windows-style absolute path, starting
@@ -90,6 +91,13 @@ class SourceTreeDescriptorDatabase::SingleFileErrorCollector
     had_errors_ = true;
   }
 
+  void RecordWarning(int line, int column, absl::string_view message) override {
+    if (multi_file_error_collector_ != nullptr) {
+      multi_file_error_collector_->RecordWarning(filename_, line, column,
+                                                 message);
+    }
+  }
+
  private:
   std::string filename_;
   MultiFileErrorCollector* multi_file_error_collector_;

src/google/protobuf/io/tokenizer.cc

@@ -67,6 +67,9 @@
 
 #include "google/protobuf/io/tokenizer.h"
 
+#include <cstddef>
+#include <string>
+
 #include "google/protobuf/stubs/common.h"
 #include "absl/log/absl_check.h"
 #include "absl/log/absl_log.h"