diff --git a/thrift/compiler/codemod/structure_annotations.cc b/thrift/compiler/codemod/structure_annotations.cc index 1cd8e3506a2..bc70ccbabdf 100644 --- a/thrift/compiler/codemod/structure_annotations.cc +++ b/thrift/compiler/codemod/structure_annotations.cc @@ -74,7 +74,14 @@ class structure_annotations { name == "cpp.ref" || name == "cpp2.ref" || name == "cpp.ref_type" || name == "cpp2.ref_type") { to_remove.emplace_back(name, data); - } else if (annotations_for_catch_all) { + } + + // haskell annotations are ignored by the main compiler + else if (name.find("hs.") == 0) { + } + + // catch-all (if typedef) + else if (annotations_for_catch_all) { to_remove.emplace_back(name, data); annotations_for_catch_all->emplace(name, data.value); } @@ -471,6 +478,10 @@ class structure_annotations { fm_.add_include("thrift/annotation/rust.thrift"); } + // haskell annotations are ignored by the main compiler + else if (name.find("hs.") == 0) { + } + // catch-all else { to_remove.emplace_back(name, data); diff --git a/thrift/compiler/codemod/structure_annotations_test.py b/thrift/compiler/codemod/structure_annotations_test.py index 5b50b4b6d52..8153cd6218c 100644 --- a/thrift/compiler/codemod/structure_annotations_test.py +++ b/thrift/compiler/codemod/structure_annotations_test.py @@ -482,7 +482,7 @@ def test_remaining(self): 1: i32 field1 (foo); }(foo, bar = "baz") - typedef i32 (foo) T (bar = "baz") + typedef i32 (foo, hs.type = "hs") T (bar = "baz", hs.category = "value") enum E {QUX = 1} (foo, bar = "baz") @@ -506,7 +506,7 @@ def test_remaining(self): } @thrift.DeprecatedUnvalidatedAnnotations{items = {"bar": "baz", "foo": "1"}} - typedef i32 T + typedef i32 (hs.type = "hs") T (hs.category = "value") @thrift.DeprecatedUnvalidatedAnnotations{items = {"bar": "baz", "foo": "1"}} enum E {QUX = 1} diff --git a/thrift/compiler/sema/sema.cc b/thrift/compiler/sema/sema.cc index 16532956e9a..e10c371ee0a 100644 --- a/thrift/compiler/sema/sema.cc +++ b/thrift/compiler/sema/sema.cc @@ -417,12 +417,33 @@ void mutate_inject_metadata_fields( } } +// Strips haskell annotations and optionally inserts new annotations. +void update_annotations( + t_named& node, std::map new_annotations = {}) { + auto annotations = node.annotations(); + // First strip any haskell annotations + for (auto it = annotations.begin(); it != annotations.end();) { + if (it->first.find("hs.") == 0) { + it = annotations.erase(it); + } else { + ++it; + } + } + for (auto& [k, v] : new_annotations) { + annotations[k] = {source_range{}, v}; + } + node.reset_annotations(std::move(annotations)); +} + void lower_deprecated_annotations( sema_context& ctx, mutator_context&, t_named& node) { if (auto cnst = node.find_structured_annotation_or_null( kDeprecatedUnvalidatedAnnotationsUri)) { ctx.check( - node.annotations().empty(), + std::all_of( + node.annotations().begin(), + node.annotations().end(), + [](const auto& pair) { return pair.first.find("hs.") == 0; }), "Cannot combine @thrift.DeprecatedUnvalidatedAnnotations with legacy annotation syntax."); auto val = cnst->get_value_from_structured_annotation_or_null("items"); if (!val || val->get_map().empty()) { @@ -435,6 +456,8 @@ void lower_deprecated_annotations( map[k->get_string()] = {{}, v->get_string()}; } node.reset_annotations(std::move(map)); + } else { + update_annotations(node); } } @@ -475,20 +498,23 @@ void lower_type_annotations( } } + const t_type* node_type = node.get_type(); + if (unstructured.empty()) { + if (!node_type->annotations().empty()) { + update_annotations(const_cast(*node_type)); + } return; } - const t_type* node_type = node.get_type(); if (node_type->is_container() || (node_type->is_typedef() && static_cast(node_type)->typedef_kind() != t_typedef::kind::defined) || (node_type->is_primitive_type() && !node_type->annotations().empty())) { // This is a new type we can modify in place - for (auto& pair : unstructured) { - const_cast(node_type)->set_annotation(pair.first, pair.second); - } + update_annotations( + const_cast(*node_type), std::move(unstructured)); } else if (node_type->is_primitive_type()) { // Copy type as we don't handle unnamed typedefs to base types :( auto& program = mctx.program(); diff --git a/thrift/compiler/test/compiler_test.cc b/thrift/compiler/test/compiler_test.cc index f13db12d344..a51d69db3de 100644 --- a/thrift/compiler/test/compiler_test.cc +++ b/thrift/compiler/test/compiler_test.cc @@ -2291,3 +2291,16 @@ TEST(CompilerTest, circular_typedef) { typedef Bar Foo )"); } + +TEST(CompilerTest, combining_unstructured_annotations) { + check_compile(R"( + include "thrift/annotation/thrift.thrift" + + @thrift.DeprecatedUnvalidatedAnnotations{items = {"foo": "bar"}} + struct Foo {} (hs.foo) + + @thrift.DeprecatedUnvalidatedAnnotations{items = {"foo": "bar"}} + struct Bar {} (rust.foo) + # expected-error@-2: Cannot combine @thrift.DeprecatedUnvalidatedAnnotations with legacy annotation syntax. + )"); +}