Skip to content

Commit

Permalink
Ignore haskell annotations
Browse files Browse the repository at this point in the history
Summary: The hs2 compiler has its own frontend, so we can't lower the catch-all annotation to support its unstructured annotations. Instead, strip them during semantic analysis and ignore them in the codemod.

Reviewed By: vitaut

Differential Revision: D68231281

fbshipit-source-id: 94fd95eb9709826aaf117a7a0f5b0f1da4b8b827
  • Loading branch information
iahs authored and facebook-github-bot committed Jan 16, 2025
1 parent 938ff15 commit 3977301
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 8 deletions.
13 changes: 12 additions & 1 deletion thrift/compiler/codemod/structure_annotations.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions thrift/compiler/codemod/structure_annotations_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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}
Expand Down
36 changes: 31 additions & 5 deletions thrift/compiler/sema/sema.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string, std::string> 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()) {
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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<t_type&>(*node_type));
}
return;
}

const t_type* node_type = node.get_type();
if (node_type->is_container() ||
(node_type->is_typedef() &&
static_cast<const t_typedef*>(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<t_type*>(node_type)->set_annotation(pair.first, pair.second);
}
update_annotations(
const_cast<t_type&>(*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();
Expand Down
13 changes: 13 additions & 0 deletions thrift/compiler/test/compiler_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
)");
}

0 comments on commit 3977301

Please sign in to comment.