Skip to content

Commit 8a71f98

Browse files
iahsfacebook-github-bot
authored andcommitted
Ignore haskell annotations
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
1 parent b024a70 commit 8a71f98

File tree

4 files changed

+58
-8
lines changed

4 files changed

+58
-8
lines changed

third-party/thrift/src/thrift/compiler/codemod/structure_annotations.cc

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,14 @@ class structure_annotations {
7474
name == "cpp.ref" || name == "cpp2.ref" || name == "cpp.ref_type" ||
7575
name == "cpp2.ref_type") {
7676
to_remove.emplace_back(name, data);
77-
} else if (annotations_for_catch_all) {
77+
}
78+
79+
// haskell annotations are ignored by the main compiler
80+
else if (name.find("hs.") == 0) {
81+
}
82+
83+
// catch-all (if typedef)
84+
else if (annotations_for_catch_all) {
7885
to_remove.emplace_back(name, data);
7986
annotations_for_catch_all->emplace(name, data.value);
8087
}
@@ -471,6 +478,10 @@ class structure_annotations {
471478
fm_.add_include("thrift/annotation/rust.thrift");
472479
}
473480

481+
// haskell annotations are ignored by the main compiler
482+
else if (name.find("hs.") == 0) {
483+
}
484+
474485
// catch-all
475486
else {
476487
to_remove.emplace_back(name, data);

third-party/thrift/src/thrift/compiler/codemod/structure_annotations_test.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ def test_remaining(self):
482482
1: i32 field1 (foo);
483483
}(foo, bar = "baz")
484484
485-
typedef i32 (foo) T (bar = "baz")
485+
typedef i32 (foo, hs.type = "hs") T (bar = "baz", hs.category = "value")
486486
487487
enum E {QUX = 1} (foo, bar = "baz")
488488
@@ -506,7 +506,7 @@ def test_remaining(self):
506506
}
507507
508508
@thrift.DeprecatedUnvalidatedAnnotations{items = {"bar": "baz", "foo": "1"}}
509-
typedef i32 T
509+
typedef i32 (hs.type = "hs") T (hs.category = "value")
510510
511511
@thrift.DeprecatedUnvalidatedAnnotations{items = {"bar": "baz", "foo": "1"}}
512512
enum E {QUX = 1}

third-party/thrift/src/thrift/compiler/sema/sema.cc

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -417,12 +417,33 @@ void mutate_inject_metadata_fields(
417417
}
418418
}
419419

420+
// Strips haskell annotations and optionally inserts new annotations.
421+
void update_annotations(
422+
t_named& node, std::map<std::string, std::string> new_annotations = {}) {
423+
auto annotations = node.annotations();
424+
// First strip any haskell annotations
425+
for (auto it = annotations.begin(); it != annotations.end();) {
426+
if (it->first.find("hs.") == 0) {
427+
it = annotations.erase(it);
428+
} else {
429+
++it;
430+
}
431+
}
432+
for (auto& [k, v] : new_annotations) {
433+
annotations[k] = {source_range{}, v};
434+
}
435+
node.reset_annotations(std::move(annotations));
436+
}
437+
420438
void lower_deprecated_annotations(
421439
sema_context& ctx, mutator_context&, t_named& node) {
422440
if (auto cnst = node.find_structured_annotation_or_null(
423441
kDeprecatedUnvalidatedAnnotationsUri)) {
424442
ctx.check(
425-
node.annotations().empty(),
443+
std::all_of(
444+
node.annotations().begin(),
445+
node.annotations().end(),
446+
[](const auto& pair) { return pair.first.find("hs.") == 0; }),
426447
"Cannot combine @thrift.DeprecatedUnvalidatedAnnotations with legacy annotation syntax.");
427448
auto val = cnst->get_value_from_structured_annotation_or_null("items");
428449
if (!val || val->get_map().empty()) {
@@ -435,6 +456,8 @@ void lower_deprecated_annotations(
435456
map[k->get_string()] = {{}, v->get_string()};
436457
}
437458
node.reset_annotations(std::move(map));
459+
} else {
460+
update_annotations(node);
438461
}
439462
}
440463

@@ -475,20 +498,23 @@ void lower_type_annotations(
475498
}
476499
}
477500

501+
const t_type* node_type = node.get_type();
502+
478503
if (unstructured.empty()) {
504+
if (!node_type->annotations().empty()) {
505+
update_annotations(const_cast<t_type&>(*node_type));
506+
}
479507
return;
480508
}
481509

482-
const t_type* node_type = node.get_type();
483510
if (node_type->is_container() ||
484511
(node_type->is_typedef() &&
485512
static_cast<const t_typedef*>(node_type)->typedef_kind() !=
486513
t_typedef::kind::defined) ||
487514
(node_type->is_primitive_type() && !node_type->annotations().empty())) {
488515
// This is a new type we can modify in place
489-
for (auto& pair : unstructured) {
490-
const_cast<t_type*>(node_type)->set_annotation(pair.first, pair.second);
491-
}
516+
update_annotations(
517+
const_cast<t_type&>(*node_type), std::move(unstructured));
492518
} else if (node_type->is_primitive_type()) {
493519
// Copy type as we don't handle unnamed typedefs to base types :(
494520
auto& program = mctx.program();

third-party/thrift/src/thrift/compiler/test/compiler_test.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2291,3 +2291,16 @@ TEST(CompilerTest, circular_typedef) {
22912291
typedef Bar Foo
22922292
)");
22932293
}
2294+
2295+
TEST(CompilerTest, combining_unstructured_annotations) {
2296+
check_compile(R"(
2297+
include "thrift/annotation/thrift.thrift"
2298+
2299+
@thrift.DeprecatedUnvalidatedAnnotations{items = {"foo": "bar"}}
2300+
struct Foo {} (hs.foo)
2301+
2302+
@thrift.DeprecatedUnvalidatedAnnotations{items = {"foo": "bar"}}
2303+
struct Bar {} (rust.foo)
2304+
# expected-error@-2: Cannot combine @thrift.DeprecatedUnvalidatedAnnotations with legacy annotation syntax.
2305+
)");
2306+
}

0 commit comments

Comments
 (0)