From 8dd1f3a3944e4c5aa791b491d492318ae1797187 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Tue, 26 Nov 2019 16:23:28 -0800 Subject: [PATCH] Intelligently collect errors and warnings --- .../grpc_tools/_protoc_compiler.pyx | 70 ++++++++++++++++++- .../python/grpcio_tools/grpc_tools/main.cc | 53 +++++++------- .../python/grpcio_tools/grpc_tools/main.h | 23 +++++- .../grpcio_tools/grpc_tools/protoc_test.py | 6 +- 4 files changed, 119 insertions(+), 33 deletions(-) diff --git a/tools/distrib/python/grpcio_tools/grpc_tools/_protoc_compiler.pyx b/tools/distrib/python/grpcio_tools/grpc_tools/_protoc_compiler.pyx index d94d650d214..9a631d22936 100644 --- a/tools/distrib/python/grpcio_tools/grpc_tools/_protoc_compiler.pyx +++ b/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 = 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 + diff --git a/tools/distrib/python/grpcio_tools/grpc_tools/main.cc b/tools/distrib/python/grpcio_tools/grpc_tools/main.cc index 072db21d415..03dea75e737 100644 --- a/tools/distrib/python/grpcio_tools/grpc_tools/main.cc +++ b/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& parsed_files) : - parsed_files_(parsed_files){} + GeneratorContextImpl(const std::vector& parsed_files, + std::map* 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 - GetFiles() const { - return files_; - } - private: - std::map files_; + std::map* files_; const std::vector& parsed_files_; }; +// TODO: Write a bunch of tests for exception propagation. class ErrorCollectorImpl : public ::google::protobuf::compiler::MultiFileErrorCollector { public: - ErrorCollectorImpl() {} - ~ErrorCollectorImpl() {} + ErrorCollectorImpl(std::vector* errors, + std::vector* 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* errors_; + std::vector* warnings_; }; } // end namespace detail -#include - -int protoc_in_memory(char* protobuf_path, char* include_path) { +int protoc_in_memory(char* protobuf_path, + char* include_path, + std::map* files_out, + std::vector* errors, + std::vector* warnings) +{ std::cout << "C++ protoc_in_memory" << std::endl << std::flush; // TODO: Create parsed_files. std::string protobuf_filename(protobuf_path); - std::unique_ptr error_collector(new detail::ErrorCollectorImpl()); + std::unique_ptr 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; } diff --git a/tools/distrib/python/grpcio_tools/grpc_tools/main.h b/tools/distrib/python/grpcio_tools/grpc_tools/main.h index 22e286826f6..4acc8fd933a 100644 --- a/tools/distrib/python/grpcio_tools/grpc_tools/main.h +++ b/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* files_out, + std::vector* errors, + std::vector* warnings); diff --git a/tools/distrib/python/grpcio_tools/grpc_tools/protoc_test.py b/tools/distrib/python/grpcio_tools/grpc_tools/protoc_test.py index b227c66ccfd..d3e4ab23410 100644 --- a/tools/distrib/python/grpcio_tools/grpc_tools/protoc_test.py +++ b/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__':