From 5351a2d766e0ac8f5aeb22caa17c5fef3707a527 Mon Sep 17 00:00:00 2001
From: Shai Szulanski <iahs@meta.com>
Date: Mon, 13 Jan 2025 12:29:17 -0800
Subject: [PATCH] Structure annotations on typedef types

Summary:
Code that can find an annotation on the unnamed typedef in the typedef
type can also find it directly on the named typedef

Reviewed By: yoney

Differential Revision: D68113088

fbshipit-source-id: 887ef6704c2d4ad6a33e4965d5b25dbdd7e423ac
---
 .../compiler/codemod/structure_annotations.cc | 29 ++++++++++++-------
 .../codemod/structure_annotations_test.py     |  4 +--
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/third-party/thrift/src/thrift/compiler/codemod/structure_annotations.cc b/third-party/thrift/src/thrift/compiler/codemod/structure_annotations.cc
index 8e079602ed43c4..231d0d894b0120 100644
--- a/third-party/thrift/src/thrift/compiler/codemod/structure_annotations.cc
+++ b/third-party/thrift/src/thrift/compiler/codemod/structure_annotations.cc
@@ -39,7 +39,12 @@ class structure_annotations {
   structure_annotations(source_manager& sm, t_program& program)
       : fm_(sm, program), sm_(sm), prog_(program) {}
 
-  std::set<std::string> visit_type(t_type_ref typeRef, const t_named& node) {
+  // if annotations_for_catch_all is non-null, type annotations will be removed
+  // and added to that map. (This only makes sense for typedefs).
+  std::set<std::string> visit_type(
+      t_type_ref typeRef,
+      const t_named& node,
+      std::map<std::string, std::string>* annotations_for_catch_all) {
     std::set<std::string> to_add;
     if (!typeRef.resolve() || typeRef->is_primitive_type() ||
         typeRef->is_container() ||
@@ -69,6 +74,9 @@ 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) {
+          to_remove.emplace_back(name, data);
+          annotations_for_catch_all->emplace(name, data.value);
         }
       }
 
@@ -86,13 +94,13 @@ class structure_annotations {
   }
 
   void visit_def(const t_named& node) {
+    std::map<std::string, std::string> annotations_for_catch_all;
     std::set<std::string> to_add;
     if (auto typedf = dynamic_cast<const t_typedef*>(&node)) {
-      to_add = visit_type(typedf->type(), node);
+      to_add = visit_type(typedf->type(), node, &annotations_for_catch_all);
     } else if (auto field = dynamic_cast<const t_field*>(&node)) {
-      to_add = visit_type(field->type(), node);
+      to_add = visit_type(field->type(), node, nullptr);
     }
-    std::map<std::string, std::string> remaining;
 
     std::vector<t_annotation> to_remove;
     bool has_cpp_type = node.find_structured_annotation_or_null(kCppTypeUri);
@@ -463,7 +471,7 @@ class structure_annotations {
       // catch-all
       else {
         to_remove.emplace_back(name, data);
-        remaining.emplace(name, data.value);
+        annotations_for_catch_all.emplace(name, data.value);
       }
     }
 
@@ -475,14 +483,15 @@ class structure_annotations {
       }
     }
 
-    if (!remaining.empty()) {
-      std::vector<std::string> remaining_strs;
-      for (const auto& [name, value] : remaining) {
-        remaining_strs.push_back(fmt::format(R"("{}": "{}")", name, value));
+    if (!annotations_for_catch_all.empty()) {
+      std::vector<std::string> annotations_for_catch_all_strs;
+      for (const auto& [name, value] : annotations_for_catch_all) {
+        annotations_for_catch_all_strs.push_back(
+            fmt::format(R"("{}": "{}")", name, value));
       }
       to_add.insert(fmt::format(
           "@thrift.DeprecatedUnvalidatedAnnotations{{items = {{{}}}}}",
-          fmt::join(remaining_strs, ", ")));
+          fmt::join(annotations_for_catch_all_strs, ", ")));
       fm_.add_include("thrift/annotation/thrift.thrift");
     }
 
diff --git a/third-party/thrift/src/thrift/compiler/codemod/structure_annotations_test.py b/third-party/thrift/src/thrift/compiler/codemod/structure_annotations_test.py
index e68337e90550ee..5b50b4b6d52cd9 100644
--- a/third-party/thrift/src/thrift/compiler/codemod/structure_annotations_test.py
+++ b/third-party/thrift/src/thrift/compiler/codemod/structure_annotations_test.py
@@ -482,7 +482,7 @@ def test_remaining(self):
                     1: i32 field1 (foo);
                 }(foo, bar = "baz")
 
-                typedef i32 T (foo, bar = "baz")
+                typedef i32 (foo) T (bar = "baz")
 
                 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  T
 
                 @thrift.DeprecatedUnvalidatedAnnotations{items = {"bar": "baz", "foo": "1"}}
                 enum E {QUX = 1}