Add option to forbid annotations on field type

Summary:
Introduces a mechanism for gating validators to allow gradual rollout of new
restrictions. Uses that mechanism to gate field type annotations.

Reviewed By: aristidisp

Differential Revision: D51334853

fbshipit-source-id: de9095340e382e127c8cb2f85702806dc85639f6

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

@@ -135,6 +135,9 @@ void usage() {
   fprintf(
       stderr,
       "  --inject-schema-const  Inject generated schema constant (must use thrift2ast)\n");
+  fprintf(
+      stderr,
+      "  --extra-validation  Comma-separated list of opt-in validators to run\n");
   fprintf(stderr, "\n");
   fprintf(stderr, "Available generators (and options):\n");
 
@@ -277,7 +280,8 @@ std::string parse_args(
     const std::vector<std::string>& arguments,
     parsing_params& pparams,
     gen_params& gparams,
-    diagnostic_params& dparams) {
+    diagnostic_params& dparams,
+    sema_params& sparams) {
   // Check for necessary arguments, you gotta have at least a filename and
   // an output language flag.
   if (arguments.size() < 3 && gparams.targets.empty()) {
@@ -374,6 +378,24 @@ std::string parse_args(
       gparams.add_gen_dir = (flag == "o");
     } else if (flag == "inject-schema-const") {
       gparams.inject_schema_const = true;
+    } else if (flag == "extra-validation") {
+      const std::string* arg = consume_arg("extra validation");
+      if (arg == nullptr) {
+        return {};
+      }
+      std::vector<std::string> validators;
+      boost::algorithm::split(
+          validators, *arg, [](char c) { return c == ','; });
+      for (const auto& validator : validators) {
+        if (validator == "unstructured_annotations_on_field_type") {
+          sparams.forbid_unstructured_annotations_on_field_types = true;
+        } else {
+          fprintf(
+              stderr, "!!! Unrecognized validator: %s\n\n", validator.c_str());
+          usage();
+          return {};
+        }
+      }
     } else {
       fprintf(
           stderr, "!!! Unrecognized option: %s\n\n", arguments[arg_i].c_str());
@@ -825,8 +847,10 @@ std::unique_ptr<t_program_bundle> parse_and_get_program(
   pparams.allow_missing_includes = true;
   gen_params gparams;
   diagnostic_params dparams;
+  sema_params sparams;
   gparams.targets.push_back(""); // Avoid needing to pass --gen
-  std::string filename = parse_args(arguments, pparams, gparams, dparams);
+  std::string filename =
+      parse_args(arguments, pparams, gparams, dparams, sparams);
   if (filename.empty()) {
     return {};
   }
@@ -835,6 +859,7 @@ std::unique_ptr<t_program_bundle> parse_and_get_program(
       sm,
       [](const diagnostic& d) { fmt::print(stderr, "{}\n", d); },
       diagnostic_params::only_errors());
+  ctx.sema_parameters() = std::move(sparams);
   return parse_ast(sm, ctx, filename, std::move(pparams));
 }
 
@@ -846,11 +871,14 @@ compile_result compile(
   parsing_params pparams;
   gen_params gparams;
   diagnostic_params dparams;
-  std::string input_filename = parse_args(arguments, pparams, gparams, dparams);
+  sema_params sparams;
+  std::string input_filename =
+      parse_args(arguments, pparams, gparams, dparams, sparams);
   if (input_filename.empty()) {
     return result;
   }
   sema_context ctx(source_mgr, result.detail, std::move(dparams));
+  ctx.sema_parameters() = std::move(sparams);
 
   std::unique_ptr<t_program_bundle> program_bundle =
       parse_and_mutate(source_mgr, ctx, input_filename, pparams, gparams);

third-party/thrift/src/thrift/compiler/sema/sema_context.h

@@ -90,6 +90,10 @@ class node_metadata_cache {
   }
 };
 
+struct sema_params {
+  bool forbid_unstructured_annotations_on_field_types = false;
+};
+
 // An AST visitor context for semantic analysis. It combines diagnostics
 // reporting and node metadata cache.
 class sema_context : public diagnostics_engine, public const_visitor_context {
@@ -125,8 +129,12 @@ class sema_context : public diagnostics_engine, public const_visitor_context {
 
   using diagnostics_engine::report;
 
+  const sema_params& sema_parameters() const { return params_; }
+  sema_params& sema_parameters() { return params_; }
+
  private:
   node_metadata_cache cache_;
+  sema_params params_;
 };
 
 } // namespace compiler

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

@@ -1189,6 +1189,18 @@ struct ValidateAnnotationPositions {
         assert(false && "Unknown container type");
     }
   }
+  void operator()(sema_context& ctx, const t_field& node) {
+    if (ctx.sema_parameters().forbid_unstructured_annotations_on_field_types &&
+        owns_annotations(node.type()) &&
+        std::any_of(
+            node.type()->annotations().begin(),
+            node.type()->annotations().end(),
+            [](const auto& pair) {
+              return pair.second.src_range.begin != source_location{};
+            })) {
+      err(ctx);
+    }
+  }
 
  private:
   static void err(sema_context& ctx) {
@@ -1375,6 +1387,7 @@ ast_validator standard_validator() {
   validator.add_field_visitor(&validate_required_field);
   validator.add_field_visitor(&validate_cpp_type_annotation<t_field>);
   validator.add_field_visitor(&validate_field_name);
+  validator.add_field_visitor(ValidateAnnotationPositions{});
 
   validator.add_enum_visitor(&validate_enum_value_name_uniqueness);
   validator.add_enum_visitor(&validate_enum_value_uniqueness);

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

@@ -693,15 +693,32 @@ TEST(CompilerTest, mixin_field_name_uniqueness) {
 
 TEST(CompilerTest, annotation_positions) {
   check_compile(R"(
+    struct Type {1: string name} (thrift.uri = "facebook.com/thrift/annotation/Type") # expected-warning: The annotation thrift.uri is deprecated. Please use @thrift.Uri instead.
     typedef set<set<i32> (annot)> T # expected-error: Annotations are not allowed in this position. Extract the type into a named typedef instead.
     const i32 (annot) C = 42 # expected-error: Annotations are not allowed in this position. Extract the type into a named typedef instead.
     service S {
       i32 (annot) foo() # expected-error: Annotations are not allowed in this position. Extract the type into a named typedef instead.
       void bar(1: i32 (annot) p) # expected-error: Annotations are not allowed in this position. Extract the type into a named typedef instead.
     }
+    struct Foo {
+      1: i32 (annot) f
+      @Type{name="foo"}
+      2: i32 g
+    }
   )");
 }
 
+TEST(CompilerTest, annotation_positions_field) {
+  check_compile(
+      R"(
+    struct Foo {
+      1: i32 (annot) f # expected-error: Annotations are not allowed in this position. Extract the type into a named typedef instead.
+      2: i32 g
+    }
+  )",
+      {"--extra-validation", "unstructured_annotations_on_field_type"});
+}
+
 TEST(CompilerTest, performs_in_interaction) {
   check_compile(R"(
     interaction J {}