Expand skip_lowering_type_annotations to all annotations

Summary: This is the intention when enabling the option, and is especially important since the API that enables it is currently advertised as not doing any mutation.

Reviewed By: thedavekwon

Differential Revision: D68281792

fbshipit-source-id: 430870bd7bb8bf481c3c8e4abeb4e6fda6632206

thrift/compiler/codemod/structure_annotations_test.py

@@ -478,6 +478,8 @@ def test_remaining(self):
             "foo.thrift",
             textwrap.dedent(
                 """\
+                include "thrift.thrift"
+
                 struct S {
                     1: i32 field1 (foo);
                 }(foo, bar = "baz")
@@ -486,6 +488,24 @@ def test_remaining(self):
 
                 enum E {QUX = 1} (foo, bar = "baz")
 
+                @DeprecatedUnvalidatedAnnotations{items = {"bar": "baz", "foo": "1"}}
+                struct AlreadyVisited {
+                    @DeprecatedUnvalidatedAnnotations{items = {"bar": "baz", "foo": "1"}}
+                    1: i32 f;
+                }
+
+                """
+            ),
+        )
+        write_file(
+            "thrift.thrift",
+            textwrap.dedent(
+                """\
+                package "facebook.com/thrift/annotation"
+
+                struct DeprecatedUnvalidatedAnnotations {
+                    1: map<string, string> items;
+                }
                 """
             ),
         )
@@ -497,6 +517,7 @@ def test_remaining(self):
             self.trim(read_file("foo.thrift")),
             self.trim(
                 """\
+                include "thrift.thrift"
                 include "thrift/annotation/thrift.thrift"
 
                 @thrift.DeprecatedUnvalidatedAnnotations{items = {"bar": "baz", "foo": "1"}}
@@ -506,10 +527,16 @@ def test_remaining(self):
                 }
 
                 @thrift.DeprecatedUnvalidatedAnnotations{items = {"bar": "baz", "foo": "1"}}
-                typedef i32 (hs.type = "hs") T (hs.category = "value")
+                typedef i32 ( hs.type = "hs") T ( hs.category = "value")
 
                 @thrift.DeprecatedUnvalidatedAnnotations{items = {"bar": "baz", "foo": "1"}}
                 enum E {QUX = 1}
+
+                @DeprecatedUnvalidatedAnnotations{items = {"bar": "baz", "foo": "1"}}
+                struct AlreadyVisited {
+                    @DeprecatedUnvalidatedAnnotations{items = {"bar": "baz", "foo": "1"}}
+                    1: i32 f;
+                }
                 """
             ),
         )

thrift/compiler/compiler.cc

@@ -883,7 +883,7 @@ std::unique_ptr<t_program_bundle> parse_and_get_program(
       sm,
       [](const diagnostic& d) { fmt::print(stderr, "{}\n", d); },
       diagnostic_params::only_errors());
-  sparams.skip_lowering_type_annotations = true;
+  sparams.skip_lowering_annotations = true;
   return parse_ast(sm, diags, filename, std::move(pparams), &sparams);
 }
 

thrift/compiler/sema/sema.cc

@@ -451,11 +451,15 @@ void lower_deprecated_annotations(
           "Must specify at least one item in @thrift.DeprecatedUnvalidatedAnnotations.");
       return;
     }
-    deprecated_annotation_map map;
-    for (auto& [k, v] : val->get_map()) {
-      map[k->get_string()] = {{}, v->get_string()};
+    if (!ctx.sema_parameters().skip_lowering_annotations) {
+      deprecated_annotation_map map;
+      for (auto& [k, v] : val->get_map()) {
+        map[k->get_string()] = {{}, v->get_string()};
+      }
+      node.reset_annotations(std::move(map));
+    } else {
+      update_annotations(node);
     }
-    node.reset_annotations(std::move(map));
   } else {
     update_annotations(node);
   }
@@ -484,7 +488,7 @@ void lower_type_annotations(
     sema_context& ctx, mutator_context& mctx, Node& node) {
   std::map<std::string, std::string> unstructured;
 
-  if (!ctx.sema_parameters().skip_lowering_type_annotations) {
+  if (!ctx.sema_parameters().skip_lowering_annotations) {
     if (const t_const* annot =
             node.find_structured_annotation_or_null(kCppTypeUri)) {
       if (auto type =

thrift/compiler/sema/sema_context.h

@@ -90,7 +90,7 @@ class node_metadata_cache {
 
 struct sema_params {
   bool forbid_unstructured_annotations_on_field_types = true;
-  bool skip_lowering_type_annotations = false;
+  bool skip_lowering_annotations = false;
 
   // DO_BEFORE(aristidis,20250201): Remove forbid_non_optional_cpp_ref_fields
   // option when always true.