Migrate away from legacy Thrift compiler API

Summary:
`parse_and_get_program` is a legacy API that has a number of problems:

* It takes stringly-typed arguments, most of which are ignored.
* It may return an invalid AST (missing includes, unresolved types) which is not obvious at the call sites.
* It claims to not run mutators which doesn't make sense since mutation is an important part of semantic analysis and should be run e.g. for type inference.

Migrate two of its few uses to `parse_ast` in preparation for removal.

Reviewed By: rmakheja

Differential Revision: D68496448

fbshipit-source-id: 8479a1a566d3d037ead19448a8b06cfc4a088a71

Author: vitaut

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

@@ -17,6 +17,8 @@
 #include <fmt/core.h>
 #include <thrift/compiler/codemod/codemod.h>
 #include <thrift/compiler/compiler.h>
+#include <thrift/compiler/parse/parse_ast.h>
+#include <thrift/compiler/sema/sema_context.h>
 
 namespace apache::thrift::compiler {
 
@@ -29,12 +31,27 @@ int run_codemod(
     return 1;
   }
 
+  // Parse command-line arguments.
+  auto parsing_params = compiler::parsing_params();
+  auto sema_params = compiler::sema_params();
+  std::optional<std::string> filename = detail::parse_command_line_args(
+      {argv, argv + argc}, parsing_params, sema_params);
+  if (!filename) {
+    return 1;
+  }
+  parsing_params.allow_missing_includes = true;
+  sema_params.skip_lowering_annotations = true;
+
+  // Parse the Thrift file.
   auto source_mgr = source_manager();
-  auto program_bundle = parse_and_get_program(
-      source_mgr, std::vector<std::string>(argv, argv + argc));
+  auto diags = make_diagnostics_printer(source_mgr);
+  auto program_bundle =
+      parse_ast(source_mgr, diags, *filename, parsing_params, &sema_params);
   if (!program_bundle) {
     return 1;
   }
+
+  // Run the codemod.
   codemod(source_mgr, *program_bundle->root_program());
   return 0;
 }

third-party/thrift/src/thrift/compiler/codemod/test/file_manager_test.cc

@@ -19,39 +19,36 @@
 #include <gtest/gtest.h>
 #include <thrift/compiler/ast/t_program_bundle.h>
 #include <thrift/compiler/codemod/file_manager.h>
-#include <thrift/compiler/compiler.h>
+#include <thrift/compiler/parse/parse_ast.h>
 #include <thrift/compiler/source_location.h>
 
-namespace apache::thrift::compiler::codemod {
-
-namespace {
-
-using ::testing::Eq;
-using ::testing::IsTrue;
 using ::testing::NotNull;
 using ::testing::StrEq;
 using std::literals::string_view_literals::operator""sv;
 
-static const std::string kVirtualFileName("virtual/path/file1.thrift");
+namespace apache::thrift::compiler::codemod {
+namespace {
+
+const std::string test_file_name = "virtual/path/file1.thrift";
 
-static const std::string kVirtualFileContents(R"(
+const std::string test_file_contents = R"(
 package "test.module"
 
 namespace java test.module
 
 struct Foo {
   1: optional i32 bar;
 }
-)");
+)";
 } // namespace
 
 class FileManagerTest : public ::testing::Test {
  protected:
   void SetUp() override {
-    source_ = source_manager_.add_virtual_file(
-        kVirtualFileName, kVirtualFileContents);
-    program_bundle_ = parse_and_get_program(
-        source_manager_, {"<virtual compiler>", kVirtualFileName});
+    source_ =
+        source_manager_.add_virtual_file(test_file_name, test_file_contents);
+    auto diags = make_diagnostics_printer(source_manager_);
+    program_bundle_ = parse_ast(source_manager_, diags, test_file_name, {});
     ASSERT_THAT(program_bundle_, NotNull());
 
     file_manager_ = std::make_unique<file_manager>(
@@ -65,9 +62,9 @@ class FileManagerTest : public ::testing::Test {
 };
 
 TEST_F(FileManagerTest, old_content) {
-  // NOTE: comparison is done via underlying data() using STREQ because the text
+  // NOTE: comparison is done via underlying data() using StrEq because the text
   // in the source manager is explicitly null-terminated.
-  EXPECT_THAT(file_manager_->old_content().data(), StrEq(kVirtualFileContents));
+  EXPECT_THAT(file_manager_->old_content().data(), StrEq(test_file_contents));
 }
 
 TEST_F(FileManagerTest, add_include) {

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

@@ -839,6 +839,19 @@ std::unique_ptr<t_program_bundle> parse_and_mutate(
 
 } // namespace
 
+std::optional<std::string> detail::parse_command_line_args(
+    const std::vector<std::string>& args,
+    parsing_params& parsing_params,
+    sema_params& sema_params) {
+  gen_params gen_params = {};
+  gen_params.targets.emplace_back(); // Avoid the need to pass --gen.
+  diagnostic_params diag_params = {};
+  auto filename =
+      parse_args(args, parsing_params, gen_params, diag_params, sema_params);
+  return filename.empty() ? std::nullopt
+                          : std::make_optional(std::move(filename));
+}
+
 std::pair<std::unique_ptr<t_program_bundle>, diagnostic_results>
 parse_and_mutate_program(
     source_manager& sm,

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

@@ -17,13 +17,24 @@
 #pragma once
 
 #include <memory>
+#include <optional>
 #include <string>
 #include <vector>
 
 #include <thrift/compiler/diagnostic.h>
 #include <thrift/compiler/parse/parse_ast.h> // parsing_params
 
 namespace apache::thrift::compiler {
+namespace detail {
+
+// Parses command-line arguments and returns the input file name if successful;
+// otherwise returns an empty optional.
+[[nodiscard]] std::optional<std::string> parse_command_line_args(
+    const std::vector<std::string>& args,
+    parsing_params& parsing_params,
+    sema_params& sema_params);
+
+} // namespace detail
 
 class t_program_bundle;