Intelligently collect errors and warnings

tools/distrib/python/grpcio_tools/grpc_tools/_protoc_compiler.pyx

@@ -13,10 +13,27 @@
 # limitations under the License.
 
 from libc cimport stdlib
+from libcpp.map cimport map
+from libcpp.vector cimport vector
+from libcpp.string cimport string
+
+import warnings
 
 cdef extern from "grpc_tools/main.h":
+  cppclass cProtocError "ProtocError":
+    string filename
+    int line
+    int column
+    string message
+
+  cppclass cProtocWarning "ProtocWarning":
+    string filename
+    int line
+    int column
+    string message
+
   int protoc_main(int argc, char *argv[])
-  int protoc_in_memory(char* protobuf_path, char* include_path) except +
+  int protoc_in_memory(char* protobuf_path, char* include_path, map[string, string]* files_out, vector[cProtocError]* errors, vector[cProtocWarning]* wrnings) except +
 
 def run_main(list args not None):
   cdef char **argv = <char **>stdlib.malloc(len(args)*sizeof(char *))
@@ -24,6 +41,53 @@ def run_main(list args not None):
     argv[i] = args[i]
   return protoc_main(len(args), argv)
 
+class ProtocError(Exception):
+    def __init__(self, filename, line, column, message):
+        self.filename = filename
+        self.line = line
+        self.column = column
+        self.message = message
+
+    def __repr__(self):
+        return "ProtocError(filename=\"{}\", line={}, column={}, message=\"{}\")".format(self.filename, self.line, self.column, self.message)
+
+    # TODO: Maybe come up with something better than this
+    __str__ = __repr__
+
+class ProtocWarning(Warning):
+    def __init__(self, filename, line, column, message):
+        self.filename = filename
+        self.line = line
+        self.column = column
+        self.message = message
+
+    def __repr__(self):
+        return "ProtocWarning(filename=\"{}\", line={}, column={}, message=\"{}\")".format(self.filename, self.line, self.column, self.message)
+
+    # TODO: Maybe come up with something better than this
+    __str__ = __repr__
+
+cdef _c_protoc_error_to_protoc_error(cProtocError c_protoc_error):
+    return ProtocError(c_protoc_error.filename, c_protoc_error.line, c_protoc_error.column, c_protoc_error.message)
+
+cdef _c_protoc_warning_to_protoc_warning(cProtocWarning c_protoc_warning):
+    return ProtocWarning(c_protoc_warning.filename, c_protoc_warning.line, c_protoc_warning.column, c_protoc_warning.message)
+
 def run_protoc_in_memory(bytes protobuf_path, bytes include_path):
-  import sys; sys.stdout.write("cython run_protoc_in_memory\n"); sys.stdout.flush()
-  return protoc_in_memory(protobuf_path, include_path)
+  cdef map[string, string] files
+  cdef vector[cProtocError] errors
+  # NOTE: Abbreviated name used to shadowing of the module name.
+  cdef vector[cProtocWarning] wrnings
+  return_value = protoc_in_memory(protobuf_path, include_path, &files, &errors, &wrnings)
+  for warning in wrnings:
+      warnings.warn(_c_protoc_warning_to_protoc_warning(warning))
+  if return_value != 0:
+    if errors.size() != 0:
+       py_errors = [_c_protoc_error_to_protoc_error(c_error) for c_error in errors]
+       # TODO: Come up with a good system for printing multiple errors from
+       # protoc.
+       raise Exception(py_errors)
+    raise Exception("An unknown error occurred while compiling {}".format(protobuf_path))
+
+  return files
+

tools/distrib/python/grpcio_tools/grpc_tools/main.cc

@@ -57,12 +57,14 @@ namespace detail {
 // TODO: Separate declarations and definitions.
 class GeneratorContextImpl : public ::google::protobuf::compiler::GeneratorContext {
 public:
-  GeneratorContextImpl(const std::vector<const ::google::protobuf::FileDescriptor*>& parsed_files) :
-    parsed_files_(parsed_files){}
+  GeneratorContextImpl(const std::vector<const ::google::protobuf::FileDescriptor*>& parsed_files,
+                       std::map<std::string, std::string>* files_out) :
+    parsed_files_(parsed_files),
+    files_(files_out){}
 
   ::google::protobuf::io::ZeroCopyOutputStream* Open(const std::string& filename) {
     // TODO(rbellevi): Learn not to dream impossible dreams. :(
-    auto [iter, _] = files_.emplace(filename, "");
+    auto [iter, _] = files_->emplace(filename, "");
     return new ::google::protobuf::io::StringOutputStream(&(iter->second));
   }
 
@@ -81,59 +83,60 @@ public:
     *output = parsed_files_;
   }
 
-  // TODO: Figure out a method with less copying.
-  std::map<std::string, std::string>
-  GetFiles() const {
-    return files_;
-  }
-
 private:
-  std::map<std::string, std::string> files_;
+  std::map<std::string, std::string>* files_;
   const std::vector<const ::google::protobuf::FileDescriptor*>& parsed_files_;
 };
 
+// TODO: Write a bunch of tests for exception propagation.
 class ErrorCollectorImpl : public ::google::protobuf::compiler::MultiFileErrorCollector {
  public:
-  ErrorCollectorImpl() {}
-  ~ErrorCollectorImpl() {}
+  ErrorCollectorImpl(std::vector<ProtocError>* errors,
+                     std::vector<ProtocWarning>* warnings) :
+                     errors_(errors),
+                     warnings_(warnings) {}
 
   // implements ErrorCollector ---------------------------------------
   void AddError(const std::string& filename, int line, int column,
                 const std::string& message) {
-    // TODO: Implement.
+    errors_->emplace_back(filename, line, column, message);
   }
 
   void AddWarning(const std::string& filename, int line, int column,
                   const std::string& message) {
-    // TODO: Implement.
+    warnings_->emplace_back(filename, line, column, message);
   }
+
+private:
+  std::vector<ProtocError>* errors_;
+  std::vector<ProtocWarning>* warnings_;
 };
 
 } // end namespace detail
 
-#include <iostream>
-
-int protoc_in_memory(char* protobuf_path, char* include_path) {
+int protoc_in_memory(char* protobuf_path,
+                     char* include_path,
+                     std::map<std::string, std::string>* files_out,
+                     std::vector<ProtocError>* errors,
+                     std::vector<ProtocWarning>* warnings)
+{
   std::cout << "C++ protoc_in_memory" << std::endl << std::flush;
   // TODO: Create parsed_files.
   std::string protobuf_filename(protobuf_path);
-  std::unique_ptr<detail::ErrorCollectorImpl> error_collector(new detail::ErrorCollectorImpl());
+  std::unique_ptr<detail::ErrorCollectorImpl> error_collector(new detail::ErrorCollectorImpl(errors, warnings));
   std::unique_ptr<::google::protobuf::compiler::DiskSourceTree> source_tree(new ::google::protobuf::compiler::DiskSourceTree());
   // NOTE: This is equivalent to "--proto_path=."
   source_tree->MapPath("", ".");
   // TODO: Figure out more advanced virtual path mapping.
   ::google::protobuf::compiler::Importer importer(source_tree.get(), error_collector.get());
   const ::google::protobuf::FileDescriptor* parsed_file = importer.Import(protobuf_filename);
-  detail::GeneratorContextImpl generator_context({parsed_file});
+  if (parsed_file == nullptr) {
+    return 1;
+  }
+  detail::GeneratorContextImpl generator_context({parsed_file}, files_out);
   std::string error;
   ::google::protobuf::compiler::python::Generator python_generator;
   python_generator.Generate(parsed_file, "", &generator_context, &error);
-  for (const auto& [filename, contents] : generator_context.GetFiles()) {
-    std::cout << "# File: " << filename << std::endl;
-    std::cout << contents << std::endl;
-    std::cout << std::endl;
-  }
-  std::cout << std::flush;
   // TODO: Come up with a better error reporting mechanism than this.
   return 0;
 }

tools/distrib/python/grpcio_tools/grpc_tools/main.h

@@ -16,4 +16,25 @@
 // We declare `protoc_main` here since we want access to it from Cython as an
 // extern but *without* triggering a dllimport declspec when on Windows.
 int protoc_main(int argc, char *argv[]);
-int protoc_in_memory(char* protobuf_path, char* include_path);
+
+struct ProtocError {
+  std::string filename;
+  int line;
+  int column;
+  std::string message;
+
+  ProtocError() {}
+  ProtocError(std::string filename, int line, int column, std::string message) :
+    filename(filename),
+    line(line),
+    column(column),
+    message(message) {}
+};
+
+typedef ProtocError ProtocWarning;
+
+int protoc_in_memory(char* protobuf_path,
+                     char* include_path,
+                     std::map<std::string, std::string>* files_out,
+                     std::vector<ProtocError>* errors,
+                     std::vector<ProtocWarning>* warnings);

tools/distrib/python/grpcio_tools/grpc_tools/protoc_test.py

@@ -21,16 +21,14 @@ class ProtocTest(unittest.TestCase):
     #     pass
 
     def test_protoc_in_memory(self):
-        print("Running test_protoc_in_memory")
         from grpc_tools import protoc
         import os
         original_dir = os.getcwd()
         # TODO: Completely get rid of this chdir stuff.
         os.chdir(os.path.join(original_dir, "tools/distrib/python/grpcio_tools/"))
-        protoc.run_protoc_in_memory("grpc_tools/simple.proto", "")
+        files = protoc.run_protoc_in_memory("grpc_tools/simple.proto", "")
         os.chdir(original_dir)
-        import sys; sys.stdout.flush()
-        print("Got to end of test.")
+        print("Files: {}".format(files))
 
 
 if __name__ == '__main__':