From db9ec3577517c1de55fd7491800b4e6780c3177d Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Wed, 22 Jan 2025 10:59:49 -0800 Subject: [PATCH] Simplify codemods Summary: Reduce the amount of boilerplate in Thrift codemods by moving the top-level code needed to run a codemod to a separate function, imaginatively named `run_codemod`. It handles command-line arguments (which we don't actually use except for the file name and includes), parses the Thrift files and invokes the provided function that does the actual work on a parsed AST. Before: ``` int main(int argc, char** argv) { auto source_mgr = source_manager(); auto program_bundle = parse_and_get_program( source_mgr, std::vector(argv, argv + argc)); if (!program_bundle) { return 1; } auto program = program_bundle->root_program(); add_package(source_mgr, *program).run(); return 0; } ``` After: ``` int main(int argc, char** argv) { return apache::thrift::compiler::run_codemod( argc, argv, [](source_manager& sm, t_program& p) { add_package(sm, p).run(); }); } ``` Fix a few minor issues such as wrong includes. Reviewed By: yukselakinci Differential Revision: D68466951 fbshipit-source-id: c8f79b1ac93a72197003350548c59bc8e6af41ce --- .../thrift/compiler/codemod/add_package.cc | 18 ++--- .../annotate_non_optional_cpp_ref_fields.cc | 27 +++----- .../src/thrift/compiler/codemod/codemod.cc | 42 +++++++++++ .../src/thrift/compiler/codemod/codemod.h | 33 +++++++++ .../compiler/codemod/cppref_to_structured.cc | 28 +++----- .../compiler/codemod/hoist_annotated_types.cc | 20 ++---- .../codemod/remove_cpp_noexcept_move.cc | 38 ++++------ ...e_redundant_field_custom_default_values.cc | 30 +++----- .../codemod/specify_implicit_field_id.cc | 69 ++++++------------- .../compiler/codemod/structure_annotations.cc | 31 ++++----- 10 files changed, 161 insertions(+), 175 deletions(-) create mode 100644 third-party/thrift/src/thrift/compiler/codemod/codemod.cc create mode 100644 third-party/thrift/src/thrift/compiler/codemod/codemod.h diff --git a/third-party/thrift/src/thrift/compiler/codemod/add_package.cc b/third-party/thrift/src/thrift/compiler/codemod/add_package.cc index 9086589283cfd9..4e69072bf10e75 100644 --- a/third-party/thrift/src/thrift/compiler/codemod/add_package.cc +++ b/third-party/thrift/src/thrift/compiler/codemod/add_package.cc @@ -14,12 +14,10 @@ * limitations under the License. */ -#include - #include +#include #include #include -#include using apache::thrift::compiler::source_manager; using apache::thrift::compiler::t_program; @@ -143,14 +141,8 @@ class add_package { } // namespace int main(int argc, char** argv) { - auto source_mgr = source_manager(); - auto program_bundle = parse_and_get_program( - source_mgr, std::vector(argv, argv + argc)); - if (!program_bundle) { - return 1; - } - auto program = program_bundle->root_program(); - add_package(source_mgr, *program).run(); - - return 0; + return apache::thrift::compiler::run_codemod( + argc, argv, [](source_manager& sm, t_program& p) { + add_package(sm, p).run(); + }); } diff --git a/third-party/thrift/src/thrift/compiler/codemod/annotate_non_optional_cpp_ref_fields.cc b/third-party/thrift/src/thrift/compiler/codemod/annotate_non_optional_cpp_ref_fields.cc index 568db5771821e7..dbfd769c3ca52b 100644 --- a/third-party/thrift/src/thrift/compiler/codemod/annotate_non_optional_cpp_ref_fields.cc +++ b/third-party/thrift/src/thrift/compiler/codemod/annotate_non_optional_cpp_ref_fields.cc @@ -14,17 +14,19 @@ * limitations under the License. */ -#include #include #include #include #include #include #include +#include #include -#include #include +using apache::thrift::compiler::source_manager; +using apache::thrift::compiler::t_program; + namespace apache::thrift::compiler { namespace { @@ -83,7 +85,6 @@ class AnnonateNonOptionalCppRefFields final { private: source_manager& source_manager_; t_program& program_; - std::unique_ptr program_bundle_; codemod::file_manager file_manager_; bool maybe_annotate_field(const t_field& field) { @@ -115,24 +116,12 @@ class AnnonateNonOptionalCppRefFields final { } }; -int run_main(int argc, char** argv) { - source_manager src_manager; - const std::unique_ptr program_bundle = - parse_and_get_program( - src_manager, std::vector(argv, argv + argc)); - - if (program_bundle == nullptr) { - return 1; - } - - AnnonateNonOptionalCppRefFields(src_manager, *program_bundle->root_program()) - .run(); - return 0; -} - } // namespace } // namespace apache::thrift::compiler int main(int argc, char** argv) { - return apache::thrift::compiler::run_main(argc, argv); + return apache::thrift::compiler::run_codemod( + argc, argv, [](source_manager& sm, t_program& p) { + apache::thrift::compiler::AnnonateNonOptionalCppRefFields(sm, p).run(); + }); } diff --git a/third-party/thrift/src/thrift/compiler/codemod/codemod.cc b/third-party/thrift/src/thrift/compiler/codemod/codemod.cc new file mode 100644 index 00000000000000..495befc695bf4f --- /dev/null +++ b/third-party/thrift/src/thrift/compiler/codemod/codemod.cc @@ -0,0 +1,42 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include + +namespace apache::thrift::compiler { + +int run_codemod( + int argc, + char** argv, + std::function codemod) { + if (argc <= 1) { + fmt::print(stderr, "Usage: {} \n", argv[0]); + return 1; + } + + auto source_mgr = source_manager(); + auto program_bundle = parse_and_get_program( + source_mgr, std::vector(argv, argv + argc)); + if (!program_bundle) { + return 1; + } + codemod(source_mgr, *program_bundle->root_program()); + return 0; +} + +} // namespace apache::thrift::compiler diff --git a/third-party/thrift/src/thrift/compiler/codemod/codemod.h b/third-party/thrift/src/thrift/compiler/codemod/codemod.h new file mode 100644 index 00000000000000..1e1747928e30ba --- /dev/null +++ b/third-party/thrift/src/thrift/compiler/codemod/codemod.h @@ -0,0 +1,33 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include + +namespace apache::thrift::compiler { + +// Parses a Thrift file specified in the command-line arguments and runs +// `codemod`, a function implementing the codemod, on the AST representation of +// this file. +[[nodiscard]] int run_codemod( + int argc, + char** argv, + std::function codemod); + +} // namespace apache::thrift::compiler diff --git a/third-party/thrift/src/thrift/compiler/codemod/cppref_to_structured.cc b/third-party/thrift/src/thrift/compiler/codemod/cppref_to_structured.cc index b672c7c41a3357..b8c043a4e48a68 100644 --- a/third-party/thrift/src/thrift/compiler/codemod/cppref_to_structured.cc +++ b/third-party/thrift/src/thrift/compiler/codemod/cppref_to_structured.cc @@ -19,8 +19,8 @@ #include #include #include +#include #include -#include #include using namespace apache::thrift::compiler; @@ -75,23 +75,15 @@ static void cppref_to_structured( } int main(int argc, char** argv) { - auto source_mgr = source_manager(); - auto program_bundle = parse_and_get_program( - source_mgr, std::vector(argv, argv + argc)); + return apache::thrift::compiler::run_codemod( + argc, argv, [](source_manager& sm, t_program& p) { + codemod::file_manager fm(sm, p); - if (!program_bundle) { - return 0; - } - - auto program = program_bundle->root_program(); - - codemod::file_manager fm(source_mgr, *program); - - const_ast_visitor visitor; - visitor.add_field_visitor(folly::partial(cppref_to_structured, std::ref(fm))); - visitor(*program); - - fm.apply_replacements(); + const_ast_visitor visitor; + visitor.add_field_visitor( + folly::partial(cppref_to_structured, std::ref(fm))); + visitor(p); - return 0; + fm.apply_replacements(); + }); } diff --git a/third-party/thrift/src/thrift/compiler/codemod/hoist_annotated_types.cc b/third-party/thrift/src/thrift/compiler/codemod/hoist_annotated_types.cc index ad8fd3ab68c261..1b76cfa4248aca 100644 --- a/third-party/thrift/src/thrift/compiler/codemod/hoist_annotated_types.cc +++ b/third-party/thrift/src/thrift/compiler/codemod/hoist_annotated_types.cc @@ -24,8 +24,8 @@ #include #include #include +#include #include -#include #include using namespace apache::thrift::compiler; @@ -350,12 +350,12 @@ class hoist_annotated_types { } private: - struct Typedef { + struct typedef_info { std::string type; t_typedef* ptr; std::string structured = ""; }; - std::map typedefs_; + std::map typedefs_; codemod::file_manager fm_; source_manager sm_; t_program& prog_; @@ -363,14 +363,8 @@ class hoist_annotated_types { } // namespace int main(int argc, char** argv) { - auto source_mgr = source_manager(); - auto program_bundle = parse_and_get_program( - source_mgr, std::vector(argv, argv + argc)); - if (!program_bundle) { - return 1; - } - auto program = program_bundle->root_program(); - hoist_annotated_types(source_mgr, *program).run(); - - return 0; + return apache::thrift::compiler::run_codemod( + argc, argv, [](source_manager& sm, t_program& p) { + hoist_annotated_types(sm, p).run(); + }); } diff --git a/third-party/thrift/src/thrift/compiler/codemod/remove_cpp_noexcept_move.cc b/third-party/thrift/src/thrift/compiler/codemod/remove_cpp_noexcept_move.cc index 15a0dd786b2293..141dac223cbe17 100644 --- a/third-party/thrift/src/thrift/compiler/codemod/remove_cpp_noexcept_move.cc +++ b/third-party/thrift/src/thrift/compiler/codemod/remove_cpp_noexcept_move.cc @@ -17,10 +17,9 @@ #include #include -#include #include +#include #include -#include using namespace apache::thrift::compiler; @@ -35,26 +34,17 @@ static void remove_cpp_noexcept_move( } int main(int argc, char** argv) { - auto source_mgr = source_manager(); - auto program_bundle = parse_and_get_program( - source_mgr, std::vector(argv, argv + argc)); - - if (!program_bundle) { - return 0; - } - - auto program = program_bundle->root_program(); - - codemod::file_manager fm(source_mgr, *program); - - const_ast_visitor visitor; - visitor.add_struct_visitor( - folly::partial(remove_cpp_noexcept_move, std::ref(fm))); - visitor.add_union_visitor( - folly::partial(remove_cpp_noexcept_move, std::ref(fm))); - visitor(*program); - - fm.apply_replacements(); - - return 0; + return apache::thrift::compiler::run_codemod( + argc, argv, [](source_manager& sm, t_program& p) { + codemod::file_manager fm(sm, p); + + const_ast_visitor visitor; + visitor.add_struct_visitor( + folly::partial(remove_cpp_noexcept_move, std::ref(fm))); + visitor.add_union_visitor( + folly::partial(remove_cpp_noexcept_move, std::ref(fm))); + visitor(p); + + fm.apply_replacements(); + }); } diff --git a/third-party/thrift/src/thrift/compiler/codemod/remove_redundant_field_custom_default_values.cc b/third-party/thrift/src/thrift/compiler/codemod/remove_redundant_field_custom_default_values.cc index ded49273463451..6f2d833633d3af 100644 --- a/third-party/thrift/src/thrift/compiler/codemod/remove_redundant_field_custom_default_values.cc +++ b/third-party/thrift/src/thrift/compiler/codemod/remove_redundant_field_custom_default_values.cc @@ -14,19 +14,19 @@ * limitations under the License. */ -#include -#include -#include #include #include #include +#include #include -#include #include #include #include #include +using apache::thrift::compiler::source_manager; +using apache::thrift::compiler::t_program; + namespace apache::thrift::compiler { namespace { @@ -117,25 +117,13 @@ class RemoveRedundantFieldCustomDefaultValues final { } }; -int run_main(int argc, char** argv) { - source_manager src_manager; - const std::unique_ptr program_bundle = - parse_and_get_program( - src_manager, std::vector(argv, argv + argc)); - - if (program_bundle == nullptr) { - return 1; - } - - RemoveRedundantFieldCustomDefaultValues( - src_manager, *program_bundle->root_program()) - .run(); - return 0; -} - } // namespace } // namespace apache::thrift::compiler int main(int argc, char** argv) { - return apache::thrift::compiler::run_main(argc, argv); + return apache::thrift::compiler::run_codemod( + argc, argv, [](source_manager& sm, t_program& p) { + apache::thrift::compiler::RemoveRedundantFieldCustomDefaultValues(sm, p) + .run(); + }); } diff --git a/third-party/thrift/src/thrift/compiler/codemod/specify_implicit_field_id.cc b/third-party/thrift/src/thrift/compiler/codemod/specify_implicit_field_id.cc index 188dc85b091b57..ee9efd090df7af 100644 --- a/third-party/thrift/src/thrift/compiler/codemod/specify_implicit_field_id.cc +++ b/third-party/thrift/src/thrift/compiler/codemod/specify_implicit_field_id.cc @@ -14,68 +14,41 @@ * limitations under the License. */ -#include -#include -#include - #include #include -#include -#include +#include #include -#include -#include using namespace apache::thrift::compiler; namespace { -class specify_implicit_field_id { - public: - specify_implicit_field_id(source_manager& sm, t_program& program) - : fm_(sm, program), sm_(sm), prog_(program) {} - - void run() { - const_ast_visitor visitor; - visitor.add_field_visitor([=](const auto& f) { visit_field(f); }); +const auto comment = + "// @lint-ignore thrift-compiler-warning Negative field id is deprecated, " + "don't add new ones."; - visitor(prog_); - - fm_.apply_replacements(); +void specify_implicit_field_id(codemod::file_manager& fm, const t_field& f) { + if (f.explicit_id()) { + return; } - - void visit_field(const t_field& f) { - if (f.explicit_id()) { - return; - } - auto loc = f.type().src_range().begin.offset(); - if (f.qualifier() == t_field_qualifier::optional || - f.qualifier() == t_field_qualifier::required) { - loc -= std::string_view("optional ").size(); - } - fm_.add({loc, loc, fmt::format("{}\n {}: ", kComment, f.id())}); + auto loc = f.type().src_range().begin.offset(); + if (f.qualifier() == t_field_qualifier::optional || + f.qualifier() == t_field_qualifier::required) { + loc -= std::string_view("optional ").size(); } + fm.add({loc, loc, fmt::format("{}\n {}: ", comment, f.id())}); +} - private: - codemod::file_manager fm_; - source_manager sm_; - t_program& prog_; - - constexpr static std::string_view kComment = - "// @lint-ignore thrift-compiler-warning Negative field id is deprecated, don't add new ones."; -}; } // namespace int main(int argc, char** argv) { - auto source_mgr = source_manager(); - auto program_bundle = parse_and_get_program( - source_mgr, std::vector(argv, argv + argc)); - if (!program_bundle) { - return 1; - } - auto program = program_bundle->root_program(); - specify_implicit_field_id(source_mgr, *program).run(); - - return 0; + return apache::thrift::compiler::run_codemod( + argc, argv, [](source_manager& sm, t_program& p) { + basic_ast_visitor visitor; + visitor.add_field_visitor(specify_implicit_field_id); + codemod::file_manager fm(sm, p); + visitor(fm, p); + fm.apply_replacements(); + }); } 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 5bce18e1f7b6a6..4acdec5658819e 100644 --- a/third-party/thrift/src/thrift/compiler/codemod/structure_annotations.cc +++ b/third-party/thrift/src/thrift/compiler/codemod/structure_annotations.cc @@ -20,10 +20,9 @@ #include #include -#include -#include +#include +#include #include -#include #include using namespace apache::thrift::compiler; @@ -42,16 +41,16 @@ class structure_annotations { // 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 visit_type( - t_type_ref typeRef, + t_type_ref type_ref, const t_named& node, std::map* annotations_for_catch_all) { std::set to_add; - if (!typeRef.resolve() || typeRef->is_primitive_type() || - typeRef->is_container() || - (typeRef->is_typedef() && - static_cast(*typeRef).typedef_kind() != + if (!type_ref.resolve() || type_ref->is_primitive_type() || + type_ref->is_container() || + (type_ref->is_typedef() && + static_cast(*type_ref).typedef_kind() != t_typedef::kind::defined)) { - auto type = typeRef.get_type(); + auto type = type_ref.get_type(); std::vector to_remove; bool has_cpp_type = node.find_structured_annotation_or_null(kCppTypeUri); for (const auto& [name, data] : type->annotations()) { @@ -538,14 +537,8 @@ class structure_annotations { } // namespace int main(int argc, char** argv) { - auto source_mgr = source_manager(); - auto program_bundle = parse_and_get_program( - source_mgr, std::vector(argv, argv + argc)); - if (!program_bundle) { - return 1; - } - auto program = program_bundle->root_program(); - structure_annotations(source_mgr, *program).run(); - - return 0; + return apache::thrift::compiler::run_codemod( + argc, argv, [](source_manager& sm, t_program& p) { + structure_annotations(sm, p).run(); + }); }