From 2e9bdd5a919ec622c3a743186bdbf87d7b4c5d84 Mon Sep 17 00:00:00 2001 From: Asher Glick Date: Tue, 31 Oct 2023 22:00:06 -0500 Subject: [PATCH 1/2] Using the .proto type values in generated code The types of each field are now dynamically determined via reading the proto file and not assumed based on the markdown docs frontmatter data. --- xml_converter/doc/trigger/guid.md | 1 - .../cpp_templates/attribute_template.hpp | 10 +-- .../attribute_template_compoundvalue.cpp | 6 +- .../cpp_templates/attribute_template_enum.cpp | 10 +-- .../attribute_template_multiflagvalue.cpp | 6 +- .../cpp_templates/class_template.cpp | 8 +- xml_converter/generators/generate_cpp.py | 61 +++++++++++---- xml_converter/generators/main.py | 1 - xml_converter/generators/protobuf_types.py | 74 +++++++++++++++++-- 9 files changed, 132 insertions(+), 45 deletions(-) diff --git a/xml_converter/doc/trigger/guid.md b/xml_converter/doc/trigger/guid.md index a25d39f7..ad4a8094 100644 --- a/xml_converter/doc/trigger/guid.md +++ b/xml_converter/doc/trigger/guid.md @@ -5,7 +5,6 @@ class: UniqueId xml_fields: ["GUID"] applies_to: ["Icon", "Trail"] protobuf_field: guid -protobuf_type: String --- A globally unique identifier value to make sure this maker's trigger reset data is always assocaited with this marker and never lost or confused with other markers. diff --git a/xml_converter/generators/cpp_templates/attribute_template.hpp b/xml_converter/generators/cpp_templates/attribute_template.hpp index 33d219d7..08dfe844 100644 --- a/xml_converter/generators/cpp_templates/attribute_template.hpp +++ b/xml_converter/generators/cpp_templates/attribute_template.hpp @@ -16,9 +16,7 @@ }; {% else: %} class XMLError; - namespace waypoint { - class {{class_name}}; - } + {{proto_field_cpp_type_prototype}} class {{class_name}} { public: @@ -38,8 +36,8 @@ void xml_attribute_to_{{attribute_name}}( bool* is_set); std::string {{attribute_name}}_to_xml_attribute(const std::string& attribute_name, const {{class_name}}* value); {% if type == "Enum":%} - waypoint::{{class_name}} to_proto_{{attribute_name}}({{class_name}} attribute_value); + {{proto_field_cpp_type}} to_proto_{{attribute_name}}({{class_name}} attribute_value); {% else: %} - waypoint::{{class_name}}* to_proto_{{attribute_name}}({{class_name}} attribute_value); + {{proto_field_cpp_type}}* to_proto_{{attribute_name}}({{class_name}} attribute_value); {% endif %} -{{class_name}} from_proto_{{attribute_name}}(waypoint::{{class_name}} proto_{{attribute_name}}); +{{class_name}} from_proto_{{attribute_name}}({{proto_field_cpp_type}} proto_{{attribute_name}}); diff --git a/xml_converter/generators/cpp_templates/attribute_template_compoundvalue.cpp b/xml_converter/generators/cpp_templates/attribute_template_compoundvalue.cpp index aef45764..fbc95cb5 100644 --- a/xml_converter/generators/cpp_templates/attribute_template_compoundvalue.cpp +++ b/xml_converter/generators/cpp_templates/attribute_template_compoundvalue.cpp @@ -48,15 +48,15 @@ void xml_attribute_to_{{attribute_name}}( } {% endif %} -waypoint::{{class_name}}* to_proto_{{attribute_name}}({{class_name}} attribute_value) { - waypoint::{{class_name}}* proto_{{attribute_name}} = new waypoint::{{class_name}}(); +{{proto_field_cpp_type}}* to_proto_{{attribute_name}}({{class_name}} attribute_value) { + {{proto_field_cpp_type}}* proto_{{attribute_name}} = new {{proto_field_cpp_type}}(); {% for attribute_variable in attribute_variables %} proto_{{attribute_name}}->set_{{attribute_variable.protobuf_field}}(attribute_value.{{attribute_variable.attribute_name}}); {% endfor %} return proto_{{attribute_name}}; } -{{class_name}} from_proto_{{attribute_name}}(waypoint::{{class_name}} proto_{{attribute_name}}) { +{{class_name}} from_proto_{{attribute_name}}({{proto_field_cpp_type}} proto_{{attribute_name}}) { {{class_name}} {{attribute_name}}; {% for attribute_variable in attribute_variables: %} {{attribute_name}}.{{attribute_variable.attribute_name}} = proto_{{attribute_name}}.{{attribute_variable.protobuf_field}}(); diff --git a/xml_converter/generators/cpp_templates/attribute_template_enum.cpp b/xml_converter/generators/cpp_templates/attribute_template_enum.cpp index c094494f..86f2a1a7 100644 --- a/xml_converter/generators/cpp_templates/attribute_template_enum.cpp +++ b/xml_converter/generators/cpp_templates/attribute_template_enum.cpp @@ -57,21 +57,21 @@ string {{attribute_name}}_to_xml_attribute(const std::string& attribute_name, co } } -waypoint::{{class_name}} to_proto_{{attribute_name}}({{class_name}} attribute_value) { +{{proto_field_cpp_type}} to_proto_{{attribute_name}}({{class_name}} attribute_value) { switch (attribute_value) { {% for attribute_variable in attribute_variables %} case {{class_name}}::{{attribute_variable.attribute_name}}: - return waypoint::{{class_name}}::{{attribute_variable.attribute_name}}; + return {{proto_field_cpp_type}}::{{attribute_variable.attribute_name}}; {% endfor %} default: - return waypoint::{{class_name}}::{{attribute_variables[0].attribute_name}}; + return {{proto_field_cpp_type}}::{{attribute_variables[0].attribute_name}}; } } -{{class_name}} from_proto_{{attribute_name}}(waypoint::{{class_name}} proto_{{attribute_name}}) { +{{class_name}} from_proto_{{attribute_name}}({{proto_field_cpp_type}} proto_{{attribute_name}}) { switch (proto_{{attribute_name}}) { {% for attribute_variable in attribute_variables %} - case waypoint::{{class_name}}::{{attribute_variable.attribute_name}}: + case {{proto_field_cpp_type}}::{{attribute_variable.attribute_name}}: return {{class_name}}::{{attribute_variable.attribute_name}}; {% endfor %} default: diff --git a/xml_converter/generators/cpp_templates/attribute_template_multiflagvalue.cpp b/xml_converter/generators/cpp_templates/attribute_template_multiflagvalue.cpp index 5dd088e9..75efa357 100644 --- a/xml_converter/generators/cpp_templates/attribute_template_multiflagvalue.cpp +++ b/xml_converter/generators/cpp_templates/attribute_template_multiflagvalue.cpp @@ -59,15 +59,15 @@ string {{attribute_name}}_to_xml_attribute(const std::string& attribute_name, co return " " + attribute_name + "=\"" + output + "\""; } -waypoint::{{class_name}}* to_proto_{{attribute_name}}({{class_name}} attribute_value) { - waypoint::{{class_name}}* proto_{{attribute_name}} = new waypoint::{{class_name}}(); +{{proto_field_cpp_type}}* to_proto_{{attribute_name}}({{class_name}} attribute_value) { + {{proto_field_cpp_type}}* proto_{{attribute_name}} = new {{proto_field_cpp_type}}(); {% for n, attribute_variable in enumerate(attribute_variables)%} proto_{{attribute_name}}->set_{{attribute_variable.attribute_name}}(attribute_value.{{attribute_variable.attribute_name}}); {% endfor %} return proto_{{attribute_name}}; } -{{class_name}} from_proto_{{attribute_name}}(waypoint::{{class_name}} proto_{{attribute_name}}) { +{{class_name}} from_proto_{{attribute_name}}({{proto_field_cpp_type}} proto_{{attribute_name}}) { {{class_name}} {{attribute_name}}; {% for n, attribute_variable in enumerate(attribute_variables)%} {{attribute_name}}.{{attribute_variable.attribute_name}} = proto_{{attribute_name}}.{{attribute_variable.attribute_name}}(); diff --git a/xml_converter/generators/cpp_templates/class_template.cpp b/xml_converter/generators/cpp_templates/class_template.cpp index 2c3dda84..9b085dc4 100644 --- a/xml_converter/generators/cpp_templates/class_template.cpp +++ b/xml_converter/generators/cpp_templates/class_template.cpp @@ -98,7 +98,7 @@ waypoint::{{cpp_class}} {{cpp_class}}::as_protobuf() const { {% for attribute_variable in attribute_variables %} {% if attribute_variable.is_component == false %} if (this->{{attribute_variable.attribute_flag_name}}) { - {% if attribute_variable.protobuf_type in ["MultiflagValue", "CompoundValue", "Custom", "CompoundCustomClass"] %} + {% if not attribute_variable.is_proto_field_scalar %} proto_{{cpp_class_header}}.{{attribute_variable.mutable_proto_drilldown_calls}}set_allocated_{{attribute_variable.protobuf_field}}(to_proto_{{attribute_variable.class_name}}(this->{{attribute_variable.attribute_name}})); {% else %} proto_{{cpp_class_header}}.{{attribute_variable.mutable_proto_drilldown_calls}}set_{{attribute_variable.protobuf_field}}(to_proto_{{attribute_variable.class_name}}(this->{{attribute_variable.attribute_name}})); @@ -112,12 +112,10 @@ waypoint::{{cpp_class}} {{cpp_class}}::as_protobuf() const { void {{cpp_class}}::parse_protobuf(waypoint::{{cpp_class}} proto_{{cpp_class_header}}) { {% for attribute_variable in attribute_variables %} {% if attribute_variable.is_component == false %} - {% if attribute_variable.protobuf_type in ["MultiflagValue", "CompoundValue", "Custom", "CompoundCustomClass"] %} + {% if not attribute_variable.is_proto_field_scalar %} if (proto_{{cpp_class_header}}{{attribute_variable.proto_drilldown_calls}}.has_{{attribute_variable.protobuf_field}}()) { - {% elif attribute_variable.protobuf_type == "String" %} + {% elif attribute_variable.protobuf_cpp_type == "std::string" %} if (proto_{{cpp_class_header}}{{attribute_variable.proto_drilldown_calls}}.{{attribute_variable.protobuf_field}}() != "") { - {% elif attribute_variable.protobuf_type == "Enum" %} - if (proto_{{cpp_class_header}}{{attribute_variable.proto_drilldown_calls}}.{{attribute_variable.protobuf_field}}() != 0) { {% else %} if (proto_{{cpp_class_header}}{{attribute_variable.proto_drilldown_calls}}.{{attribute_variable.protobuf_field}}() != 0) { {% endif %} diff --git a/xml_converter/generators/generate_cpp.py b/xml_converter/generators/generate_cpp.py index 0bdb8a1f..af37fe7b 100644 --- a/xml_converter/generators/generate_cpp.py +++ b/xml_converter/generators/generate_cpp.py @@ -4,6 +4,7 @@ from jinja_helpers import UnindentBlocks from util import lowercase, capitalize, normalize, Document, SchemaType import os +from protobuf_types import is_proto_field_scalar, get_proto_field_cpp_type, get_proto_field_cpp_prototype XML_ATTRIBUTE_PARSER_DEFAULT_ARGUMENTS: Final[List[str]] = ["attribute", "errors"] @@ -53,7 +54,16 @@ class AttributeVariable: cpp_type: str class_name: str xml_fields: List[str] + + # The name of the field in the protobuf that this attribute corresponds to. protobuf_field: str + + # A the cpp type the protobuf library expects to receive. + protobuf_cpp_type: str + + # Protoc cares if a field is scalar or not in the case of "set" vs "set allocated". + is_proto_field_scalar: bool + args: List[str] = field(default_factory=list) default_xml_field: str = "" side_effects: List[str] = field(default_factory=list) @@ -72,13 +82,6 @@ class AttributeVariable: uses_file_path: bool = False is_component: bool = False - # A flag to override the type that should be used when writing or reading from a protobuf - protobuf_type: str = "" - - def __post_init__(self) -> None: - if self.protobuf_type == "": - self.protobuf_type = self.attribute_type - ################################################################################ # CPPInclude @@ -284,6 +287,8 @@ def generate_cpp_variable_data( xml_fields=component_xml_fields, default_xml_field=component_default_xml_field, protobuf_field=component["protobuf_field"], + protobuf_cpp_type=get_proto_field_cpp_type(doc_type, fieldval["protobuf_field"] + "." + component["protobuf_field"]), + is_proto_field_scalar=is_proto_field_scalar(doc_type, fieldval["protobuf_field"] + "." + component["protobuf_field"]), attribute_flag_name=attribute_name + "_is_set", write_to_xml=write_to_xml, is_component=True, @@ -294,10 +299,6 @@ def generate_cpp_variable_data( if fieldval['xml_bundled_components'] == []: write_to_xml = False - protobuf_type = "" - if "protobuf_type" in fieldval: - protobuf_type = fieldval["protobuf_type"] - attribute_variable = AttributeVariable( attribute_name=attribute_name, attribute_type=fieldval["type"], @@ -306,13 +307,14 @@ def generate_cpp_variable_data( xml_fields=xml_fields, default_xml_field=default_xml_field, protobuf_field=protobuf_field, + protobuf_cpp_type=get_proto_field_cpp_type(doc_type, fieldval["protobuf_field"]), + is_proto_field_scalar=is_proto_field_scalar(doc_type, fieldval["protobuf_field"]), proto_drilldown_calls=proto_drilldown_calls, mutable_proto_drilldown_calls=mutable_proto_drilldown_calls, args=args, write_to_xml=write_to_xml, attribute_flag_name=attribute_name + "_is_set", side_effects=side_effects, - protobuf_type=protobuf_type ) attribute_variables.append(attribute_variable) @@ -357,6 +359,19 @@ def write_attribute(output_directory: str, data: Dict[str, Document]) -> None: metadata[filepath] = data[filepath].metadata attribute_name = attribute_name_from_markdown_data(metadata[filepath]['name']) + proto_field_type: str = "" + proto_field_prototype: Optional[str] = None + for marker_type in metadata[filepath]["applies_to"]: + new_type = get_proto_field_cpp_type(marker_type, metadata[filepath]["protobuf_field"]) + if proto_field_type != "" and proto_field_type != new_type: + print("Proto Field type differes between different marker types for ", metadata[filepath]["protobuf_field"]) + proto_field_type = new_type + + new_prototype = get_proto_field_cpp_prototype(marker_type, metadata[filepath]["protobuf_field"]) + if proto_field_prototype != None and proto_field_prototype != new_prototype: + print("Proto Field prototype differes between different marker types for ", metadata[filepath]["protobuf_field"]) + proto_field_prototype = new_prototype + proto_drilldown_calls: str mutable_proto_drilldown_calls: str protobuf_field: str @@ -368,6 +383,7 @@ def write_attribute(output_directory: str, data: Dict[str, Document]) -> None: for item in metadata[filepath]['flags'][flag]: xml_fields.append(normalize(item)) + # TODO: Replace AttributeVariable with a more fitting dataclass attribute_variable = AttributeVariable( attribute_name=flag, attribute_type=metadata[filepath]['type'], @@ -376,7 +392,9 @@ def write_attribute(output_directory: str, data: Dict[str, Document]) -> None: xml_fields=xml_fields, protobuf_field=protobuf_field, proto_drilldown_calls=proto_drilldown_calls, - mutable_proto_drilldown_calls=mutable_proto_drilldown_calls + mutable_proto_drilldown_calls=mutable_proto_drilldown_calls, + protobuf_cpp_type="", + is_proto_field_scalar=False ) attribute_variables.append(attribute_variable) @@ -392,6 +410,7 @@ def write_attribute(output_directory: str, data: Dict[str, Document]) -> None: xml_fields.append(normalize(item)) if component['name'] in metadata[filepath]['xml_bundled_components']: xml_bundled_components.append(component_attribute_name) + # TODO: Replace AttributeVariable with a more fitting dataclass attribute_variable = AttributeVariable( attribute_name=component_attribute_name, attribute_type=metadata[filepath]['type'], @@ -400,7 +419,9 @@ def write_attribute(output_directory: str, data: Dict[str, Document]) -> None: xml_fields=xml_fields, protobuf_field=component["protobuf_field"], proto_drilldown_calls=proto_drilldown_calls, - mutable_proto_drilldown_calls=mutable_proto_drilldown_calls + mutable_proto_drilldown_calls=mutable_proto_drilldown_calls, + protobuf_cpp_type="", + is_proto_field_scalar=False, ) attribute_variables.append(attribute_variable) @@ -409,6 +430,7 @@ def write_attribute(output_directory: str, data: Dict[str, Document]) -> None: xml_fields = [] for item in metadata[filepath]['values'][value]: xml_fields.append(normalize(item)) + # TODO: Replace AttributeVariable with a more fitting dataclass attribute_variable = AttributeVariable( attribute_name=value, attribute_type=metadata[filepath]['type'], @@ -417,7 +439,9 @@ def write_attribute(output_directory: str, data: Dict[str, Document]) -> None: xml_fields=xml_fields, protobuf_field=protobuf_field, proto_drilldown_calls=proto_drilldown_calls, - mutable_proto_drilldown_calls=mutable_proto_drilldown_calls + mutable_proto_drilldown_calls=mutable_proto_drilldown_calls, + protobuf_cpp_type="", + is_proto_field_scalar=False, ) attribute_variables.append(attribute_variable) @@ -430,15 +454,20 @@ def write_attribute(output_directory: str, data: Dict[str, Document]) -> None: attribute_variables=sorted(attribute_variables, key=get_attribute_variable_key), class_name=capitalize(attribute_name, delimiter=""), type=metadata[filepath]['type'], + proto_field_cpp_type=proto_field_type, + proto_field_cpp_type_prototype=proto_field_prototype, )) with open(os.path.join(output_directory, attribute_name + "_gen.cpp"), 'w') as f: f.write(template[metadata[filepath]['type']].render( attribute_name=attribute_name, + # TODO: Should this attribute_variables list be sorted? The hpp one is. attribute_variables=attribute_variables, class_name=capitalize(attribute_name, delimiter=""), enumerate=enumerate, - xml_bundled_components=xml_bundled_components + xml_bundled_components=xml_bundled_components, + proto_field_cpp_type=proto_field_type, + proto_field_cpp_type_prototype=proto_field_prototype, )) diff --git a/xml_converter/generators/main.py b/xml_converter/generators/main.py index 71cc3927..c0dbb3b8 100644 --- a/xml_converter/generators/main.py +++ b/xml_converter/generators/main.py @@ -69,7 +69,6 @@ optional={ "side_effects": array_t(string_t()), "uses_file_path": boolean_t(), - "protobuf_type": enum_t(["Int32", "Fixed32", "Float32", "String"]), } ), }) diff --git a/xml_converter/generators/protobuf_types.py b/xml_converter/generators/protobuf_types.py index 69ecdce0..646acd53 100644 --- a/xml_converter/generators/protobuf_types.py +++ b/xml_converter/generators/protobuf_types.py @@ -1,6 +1,6 @@ from lark import Lark, Transformer from lark.lexer import Token -from typing import List +from typing import List, Dict, Optional ################################################################################ # This module parses a proto definition file with the goal of identifying the @@ -52,7 +52,17 @@ def start(self, items: List): # type: ignore messages = {} for item in items: if type(item) is dict: - messages.update(item) + for key, value in item.items(): + if key in messages: + # The enum organization is kinda a super hack. But we + # don't want to invest too much time here. Future us + # will make this better. + if key == "_enums_": + messages[key] |= value + else: + print("Duplicate Key", key) + else: + messages[key] = value elif item is None: pass else: @@ -60,7 +70,7 @@ def start(self, items: List): # type: ignore return messages def package_directive(self, items): # type: ignore - return {"__package__": items[0]} + return {"_package_": items[0]} def dotted_identifier(self, items): # type: ignore return items @@ -70,8 +80,9 @@ def syntax_directive(self, items) -> None: # type: ignore return None # Ignore enums - def enum(self, items) -> None: # type: ignore - return None + def enum(self, items): # type: ignore + name, body = items + return {"_enums_": {name}} def declaration(self, items): # type: ignore if len(items) == 0: @@ -136,3 +147,56 @@ def get_proto_field_type(message: str, field: str) -> str: field_type = proto_field_types[field_type][field] return field_type + + +PROTO_TO_CPP_TYPES: Dict[str, str] = { + "double": "double", + "float": "float", + "int32": "int", + "int64": "long", + "uint32": "int", + "uint64": "long", + "sint32": "int", + "sint64": "long", + "fixed32": "int", + "fixed64": "long", + "sfixed32": "int", + "sfixed64": "long", + "bool": "bool", + "string": "std::string", + "bytes": "std::string", +} + +def get_proto_field_cpp_type(message: str, field: str) -> str: + value = get_proto_field_type(message, field) + + # If this is a scalar type then return the cpp version of the scalar types + if value in PROTO_TO_CPP_TYPES: + return PROTO_TO_CPP_TYPES[value] + + # Otherwise assume this is a message or enum and return the qualified path to that instead. + return "waypoint::" + value + + +def get_proto_field_cpp_prototype(message: str, field: str) -> Optional[str]: + value = get_proto_field_type(message, field) + + if value in PROTO_TO_CPP_TYPES: + return None + + return "namespace {package} {{\nclass {proto_field_type};\n}}".format( + package="waypoint", + proto_field_type=value, + ) + +def is_proto_field_scalar(message: str, field: str) -> bool: + value = get_proto_field_type(message, field) + + if value in PROTO_TO_CPP_TYPES: + return True + + if value in proto_field_types["_enums_"]: + return True + + + return False From b9208b5c907bf1240ad3d48b4e913b5dac87c19e Mon Sep 17 00:00:00 2001 From: Asher Glick Date: Tue, 31 Oct 2023 23:35:43 -0500 Subject: [PATCH 2/2] fixing linter errors --- xml_converter/generators/generate_cpp.py | 2 +- xml_converter/generators/protobuf_types.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/xml_converter/generators/generate_cpp.py b/xml_converter/generators/generate_cpp.py index af37fe7b..d4a473c1 100644 --- a/xml_converter/generators/generate_cpp.py +++ b/xml_converter/generators/generate_cpp.py @@ -368,7 +368,7 @@ def write_attribute(output_directory: str, data: Dict[str, Document]) -> None: proto_field_type = new_type new_prototype = get_proto_field_cpp_prototype(marker_type, metadata[filepath]["protobuf_field"]) - if proto_field_prototype != None and proto_field_prototype != new_prototype: + if proto_field_prototype is not None and proto_field_prototype != new_prototype: print("Proto Field prototype differes between different marker types for ", metadata[filepath]["protobuf_field"]) proto_field_prototype = new_prototype diff --git a/xml_converter/generators/protobuf_types.py b/xml_converter/generators/protobuf_types.py index 646acd53..691470fb 100644 --- a/xml_converter/generators/protobuf_types.py +++ b/xml_converter/generators/protobuf_types.py @@ -49,7 +49,7 @@ # Define transformer class ProtoDictTransformer(Transformer): # type: ignore def start(self, items: List): # type: ignore - messages = {} + messages = {} # type: ignore for item in items: if type(item) is dict: for key, value in item.items(): @@ -167,6 +167,7 @@ def get_proto_field_type(message: str, field: str) -> str: "bytes": "std::string", } + def get_proto_field_cpp_type(message: str, field: str) -> str: value = get_proto_field_type(message, field) @@ -189,6 +190,7 @@ def get_proto_field_cpp_prototype(message: str, field: str) -> Optional[str]: proto_field_type=value, ) + def is_proto_field_scalar(message: str, field: str) -> bool: value = get_proto_field_type(message, field) @@ -198,5 +200,4 @@ def is_proto_field_scalar(message: str, field: str) -> bool: if value in proto_field_types["_enums_"]: return True - return False