diff --git a/migrate_cpp/cpp_refactoring/BUILD b/migrate_cpp/cpp_refactoring/BUILD index 90155f4957ac..8c8bcddb31bc 100644 --- a/migrate_cpp/cpp_refactoring/BUILD +++ b/migrate_cpp/cpp_refactoring/BUILD @@ -19,7 +19,10 @@ cc_binary( cc_library( name = "matcher", srcs = ["matcher.cpp"], - hdrs = ["matcher.h"], + hdrs = [ + "matcher.h", + "matcher_manager.h", + ], deps = [ "@llvm-project//clang:ast_matchers", "@llvm-project//clang:tooling_core", @@ -29,9 +32,9 @@ cc_library( cc_library( name = "matcher_test_base", testonly = 1, - srcs = ["matcher_test_base.cpp"], hdrs = ["matcher_test_base.h"], deps = [ + ":matcher", "@llvm-project//clang:ast_matchers", "@llvm-project//clang:tooling", "@llvm-project//llvm:gmock", diff --git a/migrate_cpp/cpp_refactoring/fn_inserter.cpp b/migrate_cpp/cpp_refactoring/fn_inserter.cpp index d21e36d4c7d8..1a147372502b 100644 --- a/migrate_cpp/cpp_refactoring/fn_inserter.cpp +++ b/migrate_cpp/cpp_refactoring/fn_inserter.cpp @@ -5,52 +5,42 @@ #include "migrate_cpp/cpp_refactoring/fn_inserter.h" #include "clang/ASTMatchers/ASTMatchers.h" -#include "clang/Lex/Lexer.h" namespace cam = ::clang::ast_matchers; namespace Carbon { -FnInserter::FnInserter(std::map& in_replacements, - cam::MatchFinder* finder) - : Matcher(in_replacements) { - finder->addMatcher( - cam::functionDecl(cam::anyOf(cam::hasTrailingReturn(), - cam::returns(cam::asString("void"))), - cam::unless(cam::anyOf(cam::cxxConstructorDecl(), - cam::cxxDestructorDecl()))) - .bind(Label), - this); -} +static constexpr char Label[] = "FnInserter"; -void FnInserter::run(const cam::MatchFinder::MatchResult& result) { - const auto* decl = result.Nodes.getNodeAs(Label); - if (!decl) { - llvm::report_fatal_error(std::string("getNodeAs failed for ") + Label); - } - - auto& sm = *(result.SourceManager); - auto lang_opts = result.Context->getLangOpts(); +void FnInserter::Run() { + const auto& decl = GetNodeAsOrDie(Label); // For names like "Class::Method", replace up to "Class" not "Method". - clang::NestedNameSpecifierLoc qual_loc = decl->getQualifierLoc(); + clang::NestedNameSpecifierLoc qual_loc = decl.getQualifierLoc(); clang::SourceLocation name_begin_loc = - qual_loc.hasQualifier() ? qual_loc.getBeginLoc() : decl->getLocation(); + qual_loc.hasQualifier() ? qual_loc.getBeginLoc() : decl.getLocation(); auto range = - clang::CharSourceRange::getCharRange(decl->getBeginLoc(), name_begin_loc); + clang::CharSourceRange::getCharRange(decl.getBeginLoc(), name_begin_loc); // In order to handle keywords like "virtual" in "virtual auto Foo() -> ...", // scan the replaced text and only drop auto/void entries. llvm::SmallVector split; - clang::Lexer::getSourceText(range, sm, lang_opts) - .split(split, ' ', /*MaxSplit=*/-1, /*KeepEmpty=*/false); + GetSourceText(range).split(split, ' ', /*MaxSplit=*/-1, /*KeepEmpty=*/false); std::string new_text = "fn "; for (llvm::StringRef t : split) { if (t != "auto" && t != "void") { new_text += t.str() + " "; } } - AddReplacement(*(result.SourceManager), range, new_text); + AddReplacement(range, new_text); +} + +auto FnInserterFactory::GetAstMatcher() -> cam::DeclarationMatcher { + return cam::functionDecl(cam::anyOf(cam::hasTrailingReturn(), + cam::returns(cam::asString("void"))), + cam::unless(cam::anyOf(cam::cxxConstructorDecl(), + cam::cxxDestructorDecl()))) + .bind(Label); } } // namespace Carbon diff --git a/migrate_cpp/cpp_refactoring/fn_inserter.h b/migrate_cpp/cpp_refactoring/fn_inserter.h index 69cde11397c2..8d7c9bd0a68e 100644 --- a/migrate_cpp/cpp_refactoring/fn_inserter.h +++ b/migrate_cpp/cpp_refactoring/fn_inserter.h @@ -12,13 +12,13 @@ namespace Carbon { // Inserts `fn` for functions and methods. class FnInserter : public Matcher { public: - explicit FnInserter(std::map& in_replacements, - MatchFinder* finder); + using Matcher::Matcher; + void Run() override; +}; - void run(const MatchFinder::MatchResult& result) override; - - private: - static constexpr char Label[] = "FnInserter"; +class FnInserterFactory : public MatcherFactoryBase { + public: + auto GetAstMatcher() -> clang::ast_matchers::DeclarationMatcher override; }; } // namespace Carbon diff --git a/migrate_cpp/cpp_refactoring/fn_inserter_test.cpp b/migrate_cpp/cpp_refactoring/fn_inserter_test.cpp index 4f4fdd19d5ee..d08de18f68b1 100644 --- a/migrate_cpp/cpp_refactoring/fn_inserter_test.cpp +++ b/migrate_cpp/cpp_refactoring/fn_inserter_test.cpp @@ -9,12 +9,7 @@ namespace Carbon { namespace { -class FnInserterTest : public MatcherTestBase { - protected: - FnInserterTest() : fn_inserter(replacements, &finder) {} - - Carbon::FnInserter fn_inserter; -}; +class FnInserterTest : public MatcherTestBase {}; TEST_F(FnInserterTest, TrailingReturn) { constexpr char Before[] = "auto A() -> int;"; diff --git a/migrate_cpp/cpp_refactoring/main.cpp b/migrate_cpp/cpp_refactoring/main.cpp index 8ee8d2e1869d..623fbfc21394 100644 --- a/migrate_cpp/cpp_refactoring/main.cpp +++ b/migrate_cpp/cpp_refactoring/main.cpp @@ -5,18 +5,18 @@ #include "clang/Tooling/CommonOptionsParser.h" #include "clang/Tooling/Refactoring.h" #include "migrate_cpp/cpp_refactoring/fn_inserter.h" +#include "migrate_cpp/cpp_refactoring/matcher_manager.h" #include "migrate_cpp/cpp_refactoring/var_decl.h" -namespace cam = ::clang::ast_matchers; -namespace ct = ::clang::tooling; +using clang::tooling::RefactoringTool; // Initialize the files in replacements. Matcher will restrict replacements to // initialized files. -static void InitReplacements(ct::RefactoringTool* tool) { - auto& files = tool->getFiles(); - auto& repl = tool->getReplacements(); - for (const auto& path : tool->getSourcePaths()) { - auto file = files.getFile(path); +static void InitReplacements(RefactoringTool* tool) { + clang::FileManager& files = tool->getFiles(); + Carbon::Matcher::ReplacementMap& repl = tool->getReplacements(); + for (const std::string& path : tool->getSourcePaths()) { + llvm::ErrorOr file = files.getFile(path); if (file.getError()) { llvm::report_fatal_error("Error accessing `" + path + "`: " + file.getError().message() + "\n"); @@ -27,17 +27,16 @@ static void InitReplacements(ct::RefactoringTool* tool) { auto main(int argc, const char** argv) -> int { llvm::cl::OptionCategory category("C++ refactoring options"); - auto parser = ct::CommonOptionsParser::create(argc, argv, category); - ct::RefactoringTool tool(parser->getCompilations(), - parser->getSourcePathList()); + auto parser = + clang::tooling::CommonOptionsParser::create(argc, argv, category); + RefactoringTool tool(parser->getCompilations(), parser->getSourcePathList()); InitReplacements(&tool); // Set up AST matcher callbacks. - auto& repl = tool.getReplacements(); - cam::MatchFinder finder; - Carbon::FnInserter fn_inserter(repl, &finder); - Carbon::VarDecl var_decl(repl, &finder); + Carbon::MatcherManager matchers(&tool.getReplacements()); + matchers.Register(std::make_unique()); + matchers.Register(std::make_unique()); return tool.runAndSave( - clang::tooling::newFrontendActionFactory(&finder).get()); + clang::tooling::newFrontendActionFactory(matchers.GetFinder()).get()); } diff --git a/migrate_cpp/cpp_refactoring/matcher.cpp b/migrate_cpp/cpp_refactoring/matcher.cpp index 21c12bddc34c..177746a9ec33 100644 --- a/migrate_cpp/cpp_refactoring/matcher.cpp +++ b/migrate_cpp/cpp_refactoring/matcher.cpp @@ -4,28 +4,31 @@ #include "migrate_cpp/cpp_refactoring/matcher.h" -namespace ct = ::clang::tooling; +#include "clang/Basic/SourceManager.h" namespace Carbon { -void Matcher::AddReplacement(const clang::SourceManager& sm, - clang::CharSourceRange range, +void Matcher::AddReplacement(clang::CharSourceRange range, llvm::StringRef replacement_text) { if (!range.isValid()) { // Invalid range. return; } - if (sm.getDecomposedLoc(range.getBegin()).first != - sm.getDecomposedLoc(range.getEnd()).first) { + const auto& source_manager = GetSourceManager(); + if (source_manager.getDecomposedLoc(range.getBegin()).first != + source_manager.getDecomposedLoc(range.getEnd()).first) { // Range spans macro expansions. return; } - if (sm.getFileID(range.getBegin()) != sm.getFileID(range.getEnd())) { + if (source_manager.getFileID(range.getBegin()) != + source_manager.getFileID(range.getEnd())) { // Range spans files. return; } - auto rep = ct::Replacement(sm, sm.getExpansionRange(range), replacement_text); + auto rep = clang::tooling::Replacement( + source_manager, source_manager.getExpansionRange(range), + replacement_text); auto entry = replacements->find(std::string(rep.getFilePath())); if (entry == replacements->end()) { // The replacement was in a file which isn't being updated, such as a system @@ -33,7 +36,7 @@ void Matcher::AddReplacement(const clang::SourceManager& sm, return; } - auto err = entry->second.add(rep); + llvm::Error err = entry->second.add(rep); if (err) { llvm::errs() << "Error with replacement `" << rep.toString() << "`: " << llvm::toString(std::move(err)) << "\n"; diff --git a/migrate_cpp/cpp_refactoring/matcher.h b/migrate_cpp/cpp_refactoring/matcher.h index 31db93c12c5c..c61c55c9c850 100644 --- a/migrate_cpp/cpp_refactoring/matcher.h +++ b/migrate_cpp/cpp_refactoring/matcher.h @@ -6,28 +6,87 @@ #define MIGRATE_CPP_CPP_REFACTORING_MATCHER_H_ #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" #include "clang/Tooling/Core/Replacement.h" namespace Carbon { // This is an abstract class with helpers to make it easier to write matchers. -class Matcher : public clang::ast_matchers::MatchFinder::MatchCallback { +// Note a MatcherFactory (below) is also typically required. +class Matcher { public: - // Alias these to elide the namespaces in subclass headers. - using MatchFinder = clang::ast_matchers::MatchFinder; - using Replacements = clang::tooling::Replacements; + using ReplacementMap = std::map; - explicit Matcher(std::map& in_replacements) - : replacements(&in_replacements) {} + Matcher(const clang::ast_matchers::MatchFinder::MatchResult* in_match_result, + ReplacementMap* in_replacements) + : match_result(in_match_result), replacements(in_replacements) {} + virtual ~Matcher() = default; + + // Performs main execution of the matcher when a result is found. + virtual void Run() = 0; protected: // Replaces the given range with the specified text. - void AddReplacement(const clang::SourceManager& sm, - clang::CharSourceRange range, + void AddReplacement(clang::CharSourceRange range, llvm::StringRef replacement_text); + // Returns a matched node by ID, exiting if not present. + template + auto GetNodeAsOrDie(llvm::StringRef id) -> const NodeType& { + auto* node = match_result->Nodes.getNodeAs(id); + if (!node) { + llvm::report_fatal_error(std::string("getNodeAs failed for ") + id); + } + return *node; + } + + // Returns the language options. + auto GetLangOpts() -> const clang::LangOptions& { + return match_result->Context->getLangOpts(); + } + + // Returns the full source manager. + auto GetSourceManager() -> const clang::SourceManager& { + return *match_result->SourceManager; + } + + // Returns the source text for a given range. + auto GetSourceText(clang::CharSourceRange range) -> llvm::StringRef { + return clang::Lexer::getSourceText(range, GetSourceManager(), + GetLangOpts()); + } + private: - std::map* const replacements; + const clang::ast_matchers::MatchFinder::MatchResult* const match_result; + ReplacementMap* const replacements; +}; + +// A factory used to instantiate per-MatchResult Matchers, to be registered with +// the MatcherManager. +class MatcherFactory { + public: + virtual ~MatcherFactory() = default; + + virtual auto CreateMatcher( + const clang::ast_matchers::MatchFinder::MatchResult* match_result, + Matcher::ReplacementMap* replacements) -> std::unique_ptr = 0; + + // Returns the AST matcher which determines when the Matcher is instantiated + // and run. + virtual auto GetAstMatcher() -> clang::ast_matchers::DeclarationMatcher = 0; +}; + +// A convenience factory that implements CreateMatcher for Matchers that have a +// standard constructor. +template +class MatcherFactoryBase : public MatcherFactory { + public: + auto CreateMatcher( + const clang::ast_matchers::MatchFinder::MatchResult* match_result, + Matcher::ReplacementMap* replacements) + -> std::unique_ptr override { + return std::make_unique(match_result, replacements); + } }; } // namespace Carbon diff --git a/migrate_cpp/cpp_refactoring/matcher_manager.h b/migrate_cpp/cpp_refactoring/matcher_manager.h new file mode 100644 index 000000000000..683c2be6f57e --- /dev/null +++ b/migrate_cpp/cpp_refactoring/matcher_manager.h @@ -0,0 +1,58 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#ifndef MIGRATE_CPP_CPP_REFACTORING_MATCHER_MANAGER_H_ +#define MIGRATE_CPP_CPP_REFACTORING_MATCHER_MANAGER_H_ + +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Tooling/Core/Replacement.h" +#include "migrate_cpp/cpp_refactoring/matcher.h" + +namespace Carbon { + +// Manages registration of AST matchers. +class MatcherManager { + public: + explicit MatcherManager(Matcher::ReplacementMap* in_replacements) + : replacements(in_replacements) {} + + // Registers Matcher implementations. + void Register(std::unique_ptr factory) { + matchers.push_back(std::make_unique( + &finder, std::move(factory), replacements)); + } + + auto GetFinder() -> clang::ast_matchers::MatchFinder* { return &finder; } + + private: + // Adapts Matcher for use with MatchCallback. + class MatchCallbackWrapper + : public clang::ast_matchers::MatchFinder::MatchCallback { + public: + explicit MatchCallbackWrapper(clang::ast_matchers::MatchFinder* finder, + std::unique_ptr in_factory, + Matcher::ReplacementMap* in_replacements) + : factory(std::move(in_factory)), replacements(in_replacements) { + finder->addMatcher(factory->GetAstMatcher(), this); + } + + void run(const clang::ast_matchers::MatchFinder::MatchResult& match_result) + override { + factory->CreateMatcher(&match_result, replacements)->Run(); + } + + private: + std::unique_ptr factory; + Matcher::ReplacementMap* const replacements; + }; + + Matcher::ReplacementMap* const replacements; + clang::ast_matchers::MatchFinder finder; + std::vector> factories; + std::vector> matchers; +}; + +} // namespace Carbon + +#endif // MIGRATE_CPP_CPP_REFACTORING_MATCHER_MANAGER_H_ diff --git a/migrate_cpp/cpp_refactoring/matcher_test_base.cpp b/migrate_cpp/cpp_refactoring/matcher_test_base.cpp deleted file mode 100644 index edbea70f4362..000000000000 --- a/migrate_cpp/cpp_refactoring/matcher_test_base.cpp +++ /dev/null @@ -1,39 +0,0 @@ -// Part of the Carbon Language project, under the Apache License v2.0 with LLVM -// Exceptions. See /LICENSE for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception - -#include "migrate_cpp/cpp_refactoring/matcher_test_base.h" - -#include "clang/Tooling/Tooling.h" -#include "gmock/gmock.h" -#include "gtest/gtest.h" - -namespace ct = ::clang::tooling; - -namespace Carbon { - -void MatcherTestBase::ExpectReplacement(llvm::StringRef before, - llvm::StringRef after) { - auto factory = ct::newFrontendActionFactory(&finder); - constexpr char Filename[] = "test.cc"; - replacements.clear(); - replacements.insert({Filename, {}}); - ASSERT_TRUE(ct::runToolOnCodeWithArgs( - factory->create(), before, {}, Filename, "clang-tool", - std::make_shared(), - ct::FileContentMappings())); - EXPECT_THAT(replacements, testing::ElementsAre(testing::Key(Filename))); - auto actual = ct::applyAllReplacements(before, replacements[Filename]); - if (after.find('\n') == std::string::npos) { - EXPECT_THAT(*actual, testing::Eq(after.str())); - } else { - // Split lines to get gmock to get an easier-to-read error. - llvm::SmallVector actual_lines; - llvm::SplitString(*actual, actual_lines, "\n"); - llvm::SmallVector after_lines; - llvm::SplitString(after, after_lines, "\n"); - EXPECT_THAT(actual_lines, testing::ContainerEq(after_lines)); - } -} - -} // namespace Carbon diff --git a/migrate_cpp/cpp_refactoring/matcher_test_base.h b/migrate_cpp/cpp_refactoring/matcher_test_base.h index 74f974220467..d0f51ca0878f 100644 --- a/migrate_cpp/cpp_refactoring/matcher_test_base.h +++ b/migrate_cpp/cpp_refactoring/matcher_test_base.h @@ -7,19 +7,57 @@ #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Tooling/Core/Replacement.h" +#include "clang/Tooling/Tooling.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "migrate_cpp/cpp_refactoring/matcher_manager.h" namespace Carbon { +// Matcher test framework. +template class MatcherTestBase : public ::testing::Test { protected: + MatcherTestBase() : matchers(&replacements) { + matchers.Register(std::make_unique()); + } + // Expects that that the replacements produced by running the finder result in // the specified code transformation. - void ExpectReplacement(llvm::StringRef before, llvm::StringRef after); + void ExpectReplacement(llvm::StringRef before, llvm::StringRef after) { + auto factory = + clang::tooling::newFrontendActionFactory(matchers.GetFinder()); + constexpr char Filename[] = "test.cc"; + replacements.clear(); + replacements.insert({Filename, {}}); + ASSERT_TRUE(clang::tooling::runToolOnCodeWithArgs( + factory->create(), before, {}, Filename, "clang-tool", + std::make_shared(), + clang::tooling::FileContentMappings())); + EXPECT_THAT(replacements, testing::ElementsAre(testing::Key(Filename))); + llvm::Expected actual = + clang::tooling::applyAllReplacements(before, replacements[Filename]); - std::map replacements; - clang::ast_matchers::MatchFinder finder; + // Make a specific note if the matcher didn't make any changes. + std::string unchanged; + if (before == *actual) { + unchanged = "NOTE: Actual matches original text, no changes made."; + } + + if (after.find('\n') == std::string::npos) { + EXPECT_THAT(*actual, testing::Eq(after.str())) << unchanged; + } else { + // Split lines to get gmock to get an easier-to-read error. + llvm::SmallVector actual_lines; + llvm::SplitString(*actual, actual_lines, "\n"); + llvm::SmallVector after_lines; + llvm::SplitString(after, after_lines, "\n"); + EXPECT_THAT(actual_lines, testing::ContainerEq(after_lines)) << unchanged; + } + } + + Matcher::ReplacementMap replacements; + MatcherManager matchers; }; } // namespace Carbon diff --git a/migrate_cpp/cpp_refactoring/var_decl.cpp b/migrate_cpp/cpp_refactoring/var_decl.cpp index 6c33271d8168..f0d9d802c287 100644 --- a/migrate_cpp/cpp_refactoring/var_decl.cpp +++ b/migrate_cpp/cpp_refactoring/var_decl.cpp @@ -5,21 +5,12 @@ #include "migrate_cpp/cpp_refactoring/var_decl.h" #include "clang/ASTMatchers/ASTMatchers.h" -#include "clang/Lex/Lexer.h" namespace cam = ::clang::ast_matchers; namespace Carbon { -VarDecl::VarDecl(std::map& in_replacements, - cam::MatchFinder* finder) - : Matcher(in_replacements) { - // Skip range-based for loops in this var-inserter. - finder->addMatcher(cam::varDecl(cam::unless(cam::hasParent(cam::declStmt( - cam::hasParent(cam::cxxForRangeStmt()))))) - .bind(Label), - this); -} +static constexpr char Label[] = "VarDecl"; // Helper function for printing TypeLocClass. Useful for debugging. LLVM_ATTRIBUTE_UNUSED @@ -38,12 +29,10 @@ static auto TypeLocClassToString(clang::TypeLoc::TypeLocClass c) } // Returns a string for the type. -static auto GetTypeStr(const clang::VarDecl* decl, - const clang::SourceManager& sm, - const clang::LangOptions& lang_opts) -> std::string { +auto VarDecl::GetTypeStr(const clang::VarDecl& decl) -> std::string { // Built a vector of class information, because we'll be traversing reverse // order to construct the final type. - auto type_loc = decl->getTypeSourceInfo()->getTypeLoc(); + auto type_loc = decl.getTypeSourceInfo()->getTypeLoc(); std::vector> segments; while (!type_loc.isNull()) { std::string text; @@ -54,8 +43,7 @@ static auto GetTypeStr(const clang::VarDecl* decl, } auto range = clang::CharSourceRange::getTokenRange(type_loc.getLocalSourceRange()); - std::string range_str = - clang::Lexer::getSourceText(range, sm, lang_opts).str(); + std::string range_str = GetSourceText(range).str(); // Make a list of segments with their TypeLocClass for reconstruction of the // string. Locally, we will have a qualifier (such as `const`) and a type @@ -101,46 +89,44 @@ static auto GetTypeStr(const clang::VarDecl* decl, return type_str; } -void VarDecl::run(const cam::MatchFinder::MatchResult& result) { - const auto* decl = result.Nodes.getNodeAs(Label); - if (!decl) { - llvm::report_fatal_error(std::string("getNodeAs failed for ") + Label); - } - - if (decl->getTypeSourceInfo() == nullptr) { +void VarDecl::Run() { + const auto& decl = GetNodeAsOrDie(Label); + if (decl.getTypeSourceInfo() == nullptr) { // TODO: Need to understand what's happening in this case. Not sure if we // need to address it. return; } - auto& sm = *(result.SourceManager); - auto lang_opts = result.Context->getLangOpts(); - std::string after; - if (decl->getType().isConstQualified()) { + if (decl.getType().isConstQualified()) { after = "let "; - } else if (result.Nodes.getNodeAs(Label) == nullptr) { + } else if (!llvm::isa(&decl)) { // Start the replacement with "var" unless it's a parameter. after = "var "; } // Add "identifier: type" to the replacement. - after += decl->getNameAsString() + ": " + GetTypeStr(decl, sm, lang_opts); + after += decl.getNameAsString() + ": " + GetTypeStr(decl); // This decides the range to replace. Normally the entire decl is replaced, // but for code like `int i, j` we need to detect the comma between the // declared names. That case currently results in `var i: int, var j: int`. // If there's a comma, this range will be non-empty. - auto type_loc = decl->getTypeSourceInfo()->getTypeLoc(); - auto after_type_loc = - clang::Lexer::getLocForEndOfToken(type_loc.getEndLoc(), 0, sm, lang_opts); - auto comma_source_text = clang::Lexer::getSourceText( - clang::CharSourceRange::getCharRange(after_type_loc, decl->getLocation()), - sm, lang_opts); + auto type_loc = decl.getTypeSourceInfo()->getTypeLoc(); + clang::SourceLocation after_type_loc = clang::Lexer::getLocForEndOfToken( + type_loc.getEndLoc(), 0, GetSourceManager(), GetLangOpts()); + llvm::StringRef comma_source_text = GetSourceText( + clang::CharSourceRange::getCharRange(after_type_loc, decl.getLocation())); bool has_comma = !comma_source_text.trim().empty(); clang::CharSourceRange replace_range = clang::CharSourceRange::getTokenRange( - has_comma ? decl->getLocation() : decl->getBeginLoc(), decl->getEndLoc()); + has_comma ? decl.getLocation() : decl.getBeginLoc(), decl.getEndLoc()); - AddReplacement(sm, replace_range, after); + AddReplacement(replace_range, after); +} + +auto VarDeclFactory::GetAstMatcher() -> cam::DeclarationMatcher { + return cam::varDecl(cam::unless(cam::hasParent(cam::declStmt( + cam::hasParent(cam::cxxForRangeStmt()))))) + .bind(Label); } } // namespace Carbon diff --git a/migrate_cpp/cpp_refactoring/var_decl.h b/migrate_cpp/cpp_refactoring/var_decl.h index 86ac1ff9719e..4e89470e494e 100644 --- a/migrate_cpp/cpp_refactoring/var_decl.h +++ b/migrate_cpp/cpp_refactoring/var_decl.h @@ -12,13 +12,16 @@ namespace Carbon { // Updates variable declarations for `var name: Type`. class VarDecl : public Matcher { public: - explicit VarDecl(std::map& in_replacements, - MatchFinder* finder); - - void run(const MatchFinder::MatchResult& result) override; + using Matcher::Matcher; + void Run() override; private: - static constexpr char Label[] = "VarDecl"; + auto GetTypeStr(const clang::VarDecl& decl) -> std::string; +}; + +class VarDeclFactory : public MatcherFactoryBase { + public: + auto GetAstMatcher() -> clang::ast_matchers::DeclarationMatcher override; }; } // namespace Carbon diff --git a/migrate_cpp/cpp_refactoring/var_decl_test.cpp b/migrate_cpp/cpp_refactoring/var_decl_test.cpp index 48d346bd4914..a40091c36743 100644 --- a/migrate_cpp/cpp_refactoring/var_decl_test.cpp +++ b/migrate_cpp/cpp_refactoring/var_decl_test.cpp @@ -9,12 +9,7 @@ namespace Carbon { namespace { -class VarDeclTest : public MatcherTestBase { - protected: - VarDeclTest() : var_decl(replacements, &finder) {} - - Carbon::VarDecl var_decl; -}; +class VarDeclTest : public MatcherTestBase {}; TEST_F(VarDeclTest, Declaration) { constexpr char Before[] = "int i;";