diff --git a/include/flatbuffers/idl.h b/include/flatbuffers/idl.h index 9e58d4be95..5aa274def2 100644 --- a/include/flatbuffers/idl.h +++ b/include/flatbuffers/idl.h @@ -650,7 +650,9 @@ struct IDLOptions { // field case style options for C++ enum CaseStyle { CaseStyle_Unchanged = 0, CaseStyle_Upper, CaseStyle_Lower }; - enum class ProtoIdGapAction { NO_OP, WARNING, ERROR }; + enum class ProtoAction { NO_OP, WARNING, ERROR }; + static bool ParseProtoAction(const char* str, ProtoAction& action); + bool gen_jvmstatic; // Use flexbuffers instead for binary and text generation bool use_flexbuffers; @@ -744,7 +746,10 @@ struct IDLOptions { bool ts_omit_entrypoint; bool ts_undefined_for_optionals; - ProtoIdGapAction proto_id_gap_action; + ProtoAction proto_id_gap_action; + ProtoAction proto_option_action; + ProtoAction proto_service_action; + ProtoAction proto_extensions_action; // Possible options for the more general generator below. enum Language { @@ -871,7 +876,10 @@ struct IDLOptions { python_gen_numpy(true), ts_omit_entrypoint(false), ts_undefined_for_optionals(false), - proto_id_gap_action(ProtoIdGapAction::WARNING), + proto_id_gap_action(ProtoAction::WARNING), + proto_option_action(ProtoAction::NO_OP), + proto_service_action(ProtoAction::NO_OP), + proto_extensions_action(ProtoAction::NO_OP), mini_reflect(IDLOptions::kNone), require_explicit_ids(false), rust_serialize(false), @@ -1180,6 +1188,27 @@ class Parser : public ParserState { EnumDef** dest); FLATBUFFERS_CHECKED_ERROR ParseDecl(const char* filename); FLATBUFFERS_CHECKED_ERROR ParseService(const char* filename); + struct ProtoIgnoredInfo { + enum class Keyword : uint8_t { + kOption, + kService, + kExtensions, + }; + + enum class Scope : uint8_t { + kTopLevel, + kMessage, + }; + + static const char* KeywordName(Keyword keyword); + static const char* ScopeName(Scope scope); + }; + + FLATBUFFERS_CHECKED_ERROR HandleIgnoredProtoKeyword( + ProtoIgnoredInfo::Keyword keyword, ProtoIgnoredInfo::Scope scope); + IDLOptions::ProtoAction GetProtoIgnoredAction( + ProtoIgnoredInfo::Keyword keyword) const; + FLATBUFFERS_CHECKED_ERROR ParseProtoFields(StructDef* struct_def, bool isextend, bool inside_oneof); FLATBUFFERS_CHECKED_ERROR ParseProtoMapField(StructDef* struct_def); diff --git a/src/flatc.cpp b/src/flatc.cpp index ee34044fdd..8fd0ceff4a 100644 --- a/src/flatc.cpp +++ b/src/flatc.cpp @@ -31,6 +31,22 @@ namespace flatbuffers { +bool IDLOptions::ParseProtoAction(const char* str, ProtoAction& action) { + if (!strcmp(str, "nop")) { + action = ProtoAction::NO_OP; + return true; + } + if (!strcmp(str, "warn")) { + action = ProtoAction::WARNING; + return true; + } + if (!strcmp(str, "error")) { + action = ProtoAction::ERROR; + return true; + } + return false; +} + static const char* FLATC_VERSION() { return FLATBUFFERS_VERSION(); } void FlatCompiler::ParseFile( @@ -186,6 +202,15 @@ const static FlatCOption flatc_options[] = { "* 'error' - An error message will be shown and the fbs generation will " "be " "interrupted."}, + {"", "proto-option", "", + "Action when a protobuf `option` keyword is ignored during conversion. " + "Supported values: 'nop' (default), 'warn', 'error'."}, + {"", "proto-service", "", + "Action when a protobuf `service` keyword is ignored during conversion. " + "Supported values: 'nop' (default), 'warn', 'error'."}, + {"", "proto-extensions", "", + "Action when a protobuf `extensions` keyword is ignored during conversion. " + "Supported values: 'nop' (default), 'warn', 'error'."}, {"", "grpc", "", "Generate GRPC interfaces for the specified languages."}, {"", "schema", "", "Serialize schemas instead of JSON (use with -b)."}, {"", "bfbs-filenames", "PATH", @@ -602,15 +627,23 @@ FlatCOptions FlatCompiler::ParseFromCommandLineArguments(int argc, } else if (arg == "--keep-proto-id") { opts.keep_proto_id = true; } else if (arg == "--proto-id-gap") { - if (++argi >= argc) Error("missing case style following: " + arg, true); - if (!strcmp(argv[argi], "nop")) - opts.proto_id_gap_action = IDLOptions::ProtoIdGapAction::NO_OP; - else if (!strcmp(argv[argi], "warn")) - opts.proto_id_gap_action = IDLOptions::ProtoIdGapAction::WARNING; - else if (!strcmp(argv[argi], "error")) - opts.proto_id_gap_action = IDLOptions::ProtoIdGapAction::ERROR; - else - Error("unknown case style: " + std::string(argv[argi]), true); + if (++argi >= argc) Error("missing action following: " + arg, true); + if (!IDLOptions::ParseProtoAction(argv[argi], opts.proto_id_gap_action)) + Error("unknown action: " + std::string(argv[argi]), true); + } else if (arg == "--proto-option") { + if (++argi >= argc) Error("missing action following: " + arg, true); + if (!IDLOptions::ParseProtoAction(argv[argi], opts.proto_option_action)) + Error("unknown action: " + std::string(argv[argi]), true); + } else if (arg == "--proto-service") { + if (++argi >= argc) Error("missing action following: " + arg, true); + if (!IDLOptions::ParseProtoAction(argv[argi], + opts.proto_service_action)) + Error("unknown action: " + std::string(argv[argi]), true); + } else if (arg == "--proto-extensions") { + if (++argi >= argc) Error("missing action following: " + arg, true); + if (!IDLOptions::ParseProtoAction(argv[argi], + opts.proto_extensions_action)) + Error("unknown action: " + std::string(argv[argi]), true); } else if (arg == "--schema") { options.schema_binary = true; } else if (arg == "-M") { diff --git a/src/idl_gen_fbs.cpp b/src/idl_gen_fbs.cpp index 0acd7e49a4..264c4c66eb 100644 --- a/src/idl_gen_fbs.cpp +++ b/src/idl_gen_fbs.cpp @@ -143,7 +143,7 @@ static bool HasGapInProtoId(const std::vector& fields) { } static bool ProtobufIdSanityCheck(const StructDef& struct_def, - IDLOptions::ProtoIdGapAction gap_action, + IDLOptions::ProtoAction gap_action, bool no_log = false) { const auto& fields = struct_def.fields.vec; if (HasNonPositiveFieldId(fields)) { @@ -173,14 +173,14 @@ static bool ProtobufIdSanityCheck(const StructDef& struct_def, return false; } - if (gap_action != IDLOptions::ProtoIdGapAction::NO_OP) { + if (gap_action != IDLOptions::ProtoAction::NO_OP) { if (HasGapInProtoId(fields)) { // TODO: Use LogCompilerWarn if (!no_log) { fprintf(stderr, "Fields in struct %s have gap between ids\n", struct_def.name.c_str()); } - if (gap_action == IDLOptions::ProtoIdGapAction::ERROR) { + if (gap_action == IDLOptions::ProtoAction::ERROR) { return false; } } @@ -199,7 +199,7 @@ struct ProtobufToFbsIdMap { }; static ProtobufToFbsIdMap MapProtoIdsToFieldsId( - const StructDef& struct_def, IDLOptions::ProtoIdGapAction gap_action, + const StructDef& struct_def, IDLOptions::ProtoAction gap_action, bool no_log) { const auto& fields = struct_def.fields.vec; diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index b1bdffa014..1a0c284b08 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -3049,6 +3049,67 @@ CheckedError Parser::ParseNamespace() { return NoError(); } +const char* Parser::ProtoIgnoredInfo::KeywordName(Keyword keyword) { + switch (keyword) { + case Keyword::kOption: + return "option"; + case Keyword::kService: + return "service"; + case Keyword::kExtensions: + return "extensions"; + } + FLATBUFFERS_ASSERT(false); + return "unknown"; +} + +const char* Parser::ProtoIgnoredInfo::ScopeName(Scope scope) { + switch (scope) { + case Scope::kTopLevel: + return "top-level scope"; + case Scope::kMessage: + return "message scope"; + } + FLATBUFFERS_ASSERT(false); + return "unknown"; +} + +IDLOptions::ProtoAction Parser::GetProtoIgnoredAction( + ProtoIgnoredInfo::Keyword keyword) const { + switch (keyword) { + case ProtoIgnoredInfo::Keyword::kOption: + return opts.proto_option_action; + case ProtoIgnoredInfo::Keyword::kService: + return opts.proto_service_action; + case ProtoIgnoredInfo::Keyword::kExtensions: + return opts.proto_extensions_action; + } + FLATBUFFERS_ASSERT(false); + return IDLOptions::ProtoAction::NO_OP; +} + +CheckedError Parser::HandleIgnoredProtoKeyword( + ProtoIgnoredInfo::Keyword keyword, ProtoIgnoredInfo::Scope scope) { + const auto action = GetProtoIgnoredAction(keyword); + + const std::string message = + "ignoring unsupported protobuf keyword `" + + std::string(ProtoIgnoredInfo::KeywordName(keyword)) + "` in " + + ProtoIgnoredInfo::ScopeName(scope); + + switch (action) { + case IDLOptions::ProtoAction::NO_OP: + return NoError(); + case IDLOptions::ProtoAction::WARNING: + Warning(message); + return NoError(); + case IDLOptions::ProtoAction::ERROR: + return Error(message); + } + + FLATBUFFERS_ASSERT(false); + return Error("internal error: unknown proto ignored action"); +} + // Best effort parsing of .proto declarations, with the aim to turn them // in the closest corresponding FlatBuffer equivalent. // We parse everything as identifiers instead of keywords, since we don't @@ -3103,10 +3164,14 @@ CheckedError Parser::ParseProtoDecl() { EXPECT('='); EXPECT(kTokenStringConstant); EXPECT(';'); - } else if (IsIdent("option")) { // Skip these. + } else if (IsIdent("option")) { + ECHECK(HandleIgnoredProtoKeyword(ProtoIgnoredInfo::Keyword::kOption, + ProtoIgnoredInfo::Scope::kTopLevel)); ECHECK(ParseProtoOption()); EXPECT(';'); - } else if (IsIdent("service")) { // Skip these. + } else if (IsIdent("service")) { + ECHECK(HandleIgnoredProtoKeyword(ProtoIgnoredInfo::Keyword::kService, + ProtoIgnoredInfo::Scope::kTopLevel)); NEXT(); EXPECT(kTokenIdentifier); ECHECK(ParseProtoCurliesOrIdent()); @@ -3142,7 +3207,9 @@ CheckedError Parser::ParseProtoFields(StructDef* struct_def, bool isextend, if (IsIdent("message") || IsIdent("extend") || IsIdent("enum")) { // Nested declarations. ECHECK(ParseProtoDecl()); - } else if (IsIdent("extensions")) { // Skip these. + } else if (IsIdent("extensions")) { + ECHECK(HandleIgnoredProtoKeyword(ProtoIgnoredInfo::Keyword::kExtensions, + ProtoIgnoredInfo::Scope::kMessage)); NEXT(); EXPECT(kTokenIntegerConstant); if (Is(kTokenIdentifier)) { @@ -3150,7 +3217,9 @@ CheckedError Parser::ParseProtoFields(StructDef* struct_def, bool isextend, NEXT(); // num } EXPECT(';'); - } else if (IsIdent("option")) { // Skip these. + } else if (IsIdent("option")) { + ECHECK(HandleIgnoredProtoKeyword(ProtoIgnoredInfo::Keyword::kOption, + ProtoIgnoredInfo::Scope::kMessage)); ECHECK(ParseProtoOption()); EXPECT(';'); } else if (IsIdent("reserved")) { // Skip these. diff --git a/tests/proto_test.cpp b/tests/proto_test.cpp index 7a6f163db4..6aa94bf9c1 100644 --- a/tests/proto_test.cpp +++ b/tests/proto_test.cpp @@ -52,7 +52,7 @@ void proto_test(const std::string& proto_path, const std::string& proto_file) { flatbuffers::IDLOptions opts; opts.include_dependence_headers = false; opts.proto_mode = true; - opts.proto_id_gap_action = IDLOptions::ProtoIdGapAction::NO_OP; + opts.proto_id_gap_action = IDLOptions::ProtoAction::NO_OP; // load the .proto and the golden file from disk std::string golden_file; @@ -69,7 +69,7 @@ void proto_test_id(const std::string& proto_path, opts.include_dependence_headers = false; opts.proto_mode = true; opts.keep_proto_id = true; - opts.proto_id_gap_action = IDLOptions::ProtoIdGapAction::NO_OP; + opts.proto_id_gap_action = IDLOptions::ProtoAction::NO_OP; // load the .proto and the golden file from disk std::string golden_file; @@ -87,7 +87,7 @@ void proto_test_union(const std::string& proto_path, opts.include_dependence_headers = false; opts.proto_mode = true; opts.proto_oneof_union = true; - opts.proto_id_gap_action = IDLOptions::ProtoIdGapAction::NO_OP; + opts.proto_id_gap_action = IDLOptions::ProtoAction::NO_OP; std::string golden_file; TEST_EQ(flatbuffers::LoadFile((proto_path + "test_union.golden.fbs").c_str(), @@ -104,7 +104,7 @@ void proto_test_union_id(const std::string& proto_path, opts.proto_mode = true; opts.proto_oneof_union = true; opts.keep_proto_id = true; - opts.proto_id_gap_action = IDLOptions::ProtoIdGapAction::NO_OP; + opts.proto_id_gap_action = IDLOptions::ProtoAction::NO_OP; std::string golden_file; TEST_EQ( @@ -121,7 +121,7 @@ void proto_test_union_suffix(const std::string& proto_path, opts.proto_mode = true; opts.proto_namespace_suffix = "test_namespace_suffix"; opts.proto_oneof_union = true; - opts.proto_id_gap_action = IDLOptions::ProtoIdGapAction::NO_OP; + opts.proto_id_gap_action = IDLOptions::ProtoAction::NO_OP; std::string golden_file; TEST_EQ(flatbuffers::LoadFile( @@ -139,7 +139,7 @@ void proto_test_union_suffix_id(const std::string& proto_path, opts.proto_namespace_suffix = "test_namespace_suffix"; opts.proto_oneof_union = true; opts.keep_proto_id = true; - opts.proto_id_gap_action = IDLOptions::ProtoIdGapAction::NO_OP; + opts.proto_id_gap_action = IDLOptions::ProtoAction::NO_OP; std::string golden_file; TEST_EQ(flatbuffers::LoadFile( @@ -155,7 +155,7 @@ void proto_test_include(const std::string& proto_path, flatbuffers::IDLOptions opts; opts.include_dependence_headers = true; opts.proto_mode = true; - opts.proto_id_gap_action = IDLOptions::ProtoIdGapAction::NO_OP; + opts.proto_id_gap_action = IDLOptions::ProtoAction::NO_OP; std::string golden_file; TEST_EQ( @@ -173,7 +173,7 @@ void proto_test_include_id(const std::string& proto_path, opts.include_dependence_headers = true; opts.proto_mode = true; opts.keep_proto_id = true; - opts.proto_id_gap_action = IDLOptions::ProtoIdGapAction::NO_OP; + opts.proto_id_gap_action = IDLOptions::ProtoAction::NO_OP; std::string golden_file; TEST_EQ( @@ -191,7 +191,7 @@ void proto_test_include_union(const std::string& proto_path, opts.include_dependence_headers = true; opts.proto_mode = true; opts.proto_oneof_union = true; - opts.proto_id_gap_action = IDLOptions::ProtoIdGapAction::NO_OP; + opts.proto_id_gap_action = IDLOptions::ProtoAction::NO_OP; std::string golden_file; TEST_EQ(flatbuffers::LoadFile( @@ -210,7 +210,7 @@ void proto_test_include_union_id(const std::string& proto_path, opts.proto_mode = true; opts.proto_oneof_union = true; opts.keep_proto_id = true; - opts.proto_id_gap_action = IDLOptions::ProtoIdGapAction::NO_OP; + opts.proto_id_gap_action = IDLOptions::ProtoAction::NO_OP; std::string golden_file; TEST_EQ(flatbuffers::LoadFile( @@ -269,7 +269,7 @@ void ParseCorruptedProto(const std::string& proto_path) { // Parse proto with error on gap. { - opts.proto_id_gap_action = IDLOptions::ProtoIdGapAction::ERROR; + opts.proto_id_gap_action = IDLOptions::ProtoAction::ERROR; flatbuffers::Parser parser(opts); TEST_EQ(flatbuffers::LoadFile((proto_path + "test.proto").c_str(), false, &proto_file),