diff --git a/clang/include/clang/Tooling/Refactoring/Extract/Extract.h b/clang/include/clang/Tooling/Refactoring/Extract/Extract.h new file mode 100644 index 0000000000000..2fd76d252c62b --- /dev/null +++ b/clang/include/clang/Tooling/Refactoring/Extract/Extract.h @@ -0,0 +1,53 @@ +//===--- Extract.h - Clang refactoring library ----------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLING_REFACTOR_EXTRACT_EXTRACT_H +#define LLVM_CLANG_TOOLING_REFACTOR_EXTRACT_EXTRACT_H + +#include "clang/Tooling/Refactoring/ASTSelection.h" +#include "clang/Tooling/Refactoring/RefactoringActionRules.h" + +namespace clang { +namespace tooling { + +/// An "Extract Function" refactoring moves code into a new function that's +/// then called from the place where the original code was. +class ExtractFunction final : public SourceChangeRefactoringRule { +public: + /// Initiates the extract function refactoring operation. + /// + /// \param Code The selected set of statements. + /// \param DeclName The name name of the extract function. If None, + /// "extracted" is used. + static Expected initiate(RefactoringRuleContext &Context, + CodeRangeASTSelection Code, + Optional DeclName); + + static const RefactoringDescriptor &describe(); + +private: + ExtractFunction(CodeRangeASTSelection Code, Optional DeclName) + : Code(std::move(Code)), + DeclName(DeclName ? std::move(*DeclName) : "extracted") {} + + Expected + createSourceReplacements(RefactoringRuleContext &Context) override; + + CodeRangeASTSelection Code; + + // FIXME: Account for naming collisions: + // - error when name is specified by user. + // - rename to "extractedN" when name is implicit. + std::string DeclName; +}; + +} // end namespace tooling +} // end namespace clang + +#endif // LLVM_CLANG_TOOLING_REFACTOR_EXTRACT_EXTRACT_H diff --git a/clang/include/clang/Tooling/Refactoring/RefactoringActionRegistry.def b/clang/include/clang/Tooling/Refactoring/RefactoringActionRegistry.def deleted file mode 100644 index 9aeead41489fe..0000000000000 --- a/clang/include/clang/Tooling/Refactoring/RefactoringActionRegistry.def +++ /dev/null @@ -1,8 +0,0 @@ -#ifndef REFACTORING_ACTION -#define REFACTORING_ACTION(Name) -#endif - -REFACTORING_ACTION(LocalRename) -REFACTORING_ACTION(Extract) - -#undef REFACTORING_ACTION diff --git a/clang/include/clang/Tooling/Refactoring/RefactoringActionRule.h b/clang/include/clang/Tooling/Refactoring/RefactoringActionRule.h index 294ccc381b5cb..4130e29d8dea5 100644 --- a/clang/include/clang/Tooling/Refactoring/RefactoringActionRule.h +++ b/clang/include/clang/Tooling/Refactoring/RefactoringActionRule.h @@ -11,6 +11,8 @@ #define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_H #include "clang/Basic/LLVM.h" +#include "llvm/ADT/Optional.h" +#include "llvm/ADT/StringRef.h" #include namespace clang { @@ -20,6 +22,15 @@ class RefactoringOptionVisitor; class RefactoringResultConsumer; class RefactoringRuleContext; +struct RefactoringDescriptor { + /// A unique identifier for the specific refactoring. + StringRef Name; + /// A human readable title for the refactoring. + StringRef Title; + /// A human readable description of what the refactoring does. + StringRef Description; +}; + /// A common refactoring action rule interface that defines the 'invoke' /// function that performs the refactoring operation (either fully or /// partially). @@ -33,6 +44,9 @@ class RefactoringActionRuleBase { /// consumer to propagate the result of the refactoring action. virtual void invoke(RefactoringResultConsumer &Consumer, RefactoringRuleContext &Context) = 0; + + /// Returns the structure that describes the refactoring. + // static const RefactoringDescriptor &describe() = 0; }; /// A refactoring action rule is a wrapper class around a specific refactoring diff --git a/clang/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h b/clang/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h index 443c7f86df812..75b6c8f70d173 100644 --- a/clang/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h +++ b/clang/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h @@ -57,7 +57,11 @@ void invokeRuleAfterValidatingRequirements( return Consumer.handleError(std::move(Err)); // Construct the target action rule by extracting the evaluated // requirements from Expected<> wrappers and then run it. - RuleType(std::move((*std::get(Values)))...).invoke(Consumer, Context); + auto Rule = + RuleType::initiate(Context, std::move((*std::get(Values)))...); + if (!Rule) + return Consumer.handleError(Rule.takeError()); + Rule->invoke(Consumer, Context); } inline void visitRefactoringOptionsImpl(RefactoringOptionVisitor &) {} @@ -141,7 +145,6 @@ createRefactoringActionRule(const RequirementTypes &... Requirements) { Visitor, Requirements, llvm::index_sequence_for()); } - private: std::tuple Requirements; }; diff --git a/clang/include/clang/Tooling/Refactoring/Rename/RenamingAction.h b/clang/include/clang/Tooling/Refactoring/Rename/RenamingAction.h index f2d9a7bb4d9a8..d9ed7d3a1f3cb 100644 --- a/clang/include/clang/Tooling/Refactoring/Rename/RenamingAction.h +++ b/clang/include/clang/Tooling/Refactoring/Rename/RenamingAction.h @@ -17,6 +17,7 @@ #include "clang/Tooling/Refactoring.h" #include "clang/Tooling/Refactoring/AtomicChange.h" +#include "clang/Tooling/Refactoring/RefactoringActionRules.h" #include "clang/Tooling/Refactoring/RefactoringOptions.h" #include "clang/Tooling/Refactoring/Rename/SymbolOccurrences.h" #include "llvm/Support/Error.h" @@ -46,12 +47,23 @@ class RenamingAction { bool PrintLocations; }; -class NewNameOption : public RequiredRefactoringOption { +class RenameOccurrences final : public SourceChangeRefactoringRule { public: - StringRef getName() const override { return "new-name"; } - StringRef getDescription() const override { - return "The new name to change the symbol to"; - } + static Expected initiate(RefactoringRuleContext &Context, + SourceRange SelectionRange, + std::string NewName); + + static const RefactoringDescriptor &describe(); + +private: + RenameOccurrences(const NamedDecl *ND, std::string NewName) + : ND(ND), NewName(std::move(NewName)) {} + + Expected + createSourceReplacements(RefactoringRuleContext &Context) override; + + const NamedDecl *ND; + std::string NewName; }; /// Returns source replacements that correspond to the rename of the given diff --git a/clang/include/clang/module.modulemap b/clang/include/clang/module.modulemap index 1e3262188f996..41cf73d910d89 100644 --- a/clang/include/clang/module.modulemap +++ b/clang/include/clang/module.modulemap @@ -146,8 +146,6 @@ module Clang_Tooling { // importing the AST matchers library gives a link dependency on the AST // matchers (and thus the AST), which clang-format should not have. exclude header "Tooling/RefactoringCallbacks.h" - - textual header "Tooling/Refactoring/RefactoringActionRegistry.def" } module Clang_ToolingCore { diff --git a/clang/lib/Tooling/Refactoring/Extract.cpp b/clang/lib/Tooling/Refactoring/Extract.cpp index 616900c1819eb..b1000b60ee008 100644 --- a/clang/lib/Tooling/Refactoring/Extract.cpp +++ b/clang/lib/Tooling/Refactoring/Extract.cpp @@ -13,12 +13,10 @@ /// //===----------------------------------------------------------------------===// +#include "clang/Tooling/Refactoring/Extract/Extract.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Expr.h" #include "clang/Rewrite/Core/Rewriter.h" -#include "clang/Tooling/Refactoring/RefactoringAction.h" -#include "clang/Tooling/Refactoring/RefactoringActionRules.h" -#include "clang/Tooling/Refactoring/RefactoringOptions.h" namespace clang { namespace tooling { @@ -44,58 +42,43 @@ bool isSimpleExpression(const Expr *E) { } } -class ExtractableCodeSelectionRequirement final - : public CodeRangeASTSelectionRequirement { -public: - Expected - evaluate(RefactoringRuleContext &Context) const { - Expected Selection = - CodeRangeASTSelectionRequirement::evaluate(Context); - if (!Selection) - return Selection.takeError(); - CodeRangeASTSelection &Code = *Selection; - - // We would like to extract code out of functions/methods/blocks. - // Prohibit extraction from things like global variable / field - // initializers and other top-level expressions. - if (!Code.isInFunctionLikeBodyOfCode()) - return Context.createDiagnosticError( - diag::err_refactor_code_outside_of_function); - - // Avoid extraction of simple literals and references. - if (Code.size() == 1 && isSimpleExpression(dyn_cast(Code[0]))) - return Context.createDiagnosticError( - diag::err_refactor_extract_simple_expression); - - // FIXME (Alex L): Prohibit extraction of Objective-C property setters. - return Selection; - } -}; - -class ExtractFunction final : public SourceChangeRefactoringRule { -public: - ExtractFunction(CodeRangeASTSelection Code, Optional DeclName) - : Code(std::move(Code)), - DeclName(DeclName ? std::move(*DeclName) : "extracted") {} - - Expected - createSourceReplacements(RefactoringRuleContext &Context) override; - -private: - CodeRangeASTSelection Code; - - // FIXME: Account for naming collisions: - // - error when name is specified by user. - // - rename to "extractedN" when name is implicit. - std::string DeclName; -}; - SourceLocation computeFunctionExtractionLocation(const Decl *D) { // FIXME (Alex L): Method -> function extraction should place function before // C++ record if the method is defined inside the record. return D->getLocStart(); } +} // end anonymous namespace + +const RefactoringDescriptor &ExtractFunction::describe() { + static const RefactoringDescriptor Descriptor = { + "extract-function", + "Extract Function", + "(WIP action; use with caution!) Extracts code into a new function", + }; + return Descriptor; +} + +Expected +ExtractFunction::initiate(RefactoringRuleContext &Context, + CodeRangeASTSelection Code, + Optional DeclName) { + // We would like to extract code out of functions/methods/blocks. + // Prohibit extraction from things like global variable / field + // initializers and other top-level expressions. + if (!Code.isInFunctionLikeBodyOfCode()) + return Context.createDiagnosticError( + diag::err_refactor_code_outside_of_function); + + // Avoid extraction of simple literals and references. + if (Code.size() == 1 && isSimpleExpression(dyn_cast(Code[0]))) + return Context.createDiagnosticError( + diag::err_refactor_extract_simple_expression); + + // FIXME (Alex L): Prohibit extraction of Objective-C property setters. + return ExtractFunction(std::move(Code), DeclName); +} + // FIXME: Support C++ method extraction. // FIXME: Support Objective-C method extraction. Expected @@ -194,39 +177,5 @@ ExtractFunction::createSourceReplacements(RefactoringRuleContext &Context) { return AtomicChanges{std::move(Change)}; } -class DeclNameOption final : public OptionalRefactoringOption { -public: - StringRef getName() const { return "name"; } - StringRef getDescription() const { - return "Name of the extracted declaration"; - } -}; - -class ExtractRefactoring final : public RefactoringAction { -public: - StringRef getCommand() const override { return "extract"; } - - StringRef getDescription() const override { - return "(WIP action; use with caution!) Extracts code into a new function " - "/ method / variable"; - } - - /// Returns a set of refactoring actions rules that are defined by this - /// action. - RefactoringActionRules createActionRules() const override { - RefactoringActionRules Rules; - Rules.push_back(createRefactoringActionRule( - ExtractableCodeSelectionRequirement(), - OptionRequirement())); - return Rules; - } -}; - -} // end anonymous namespace - -std::unique_ptr createExtractAction() { - return llvm::make_unique(); -} - } // end namespace tooling } // end namespace clang diff --git a/clang/lib/Tooling/Refactoring/RefactoringActions.cpp b/clang/lib/Tooling/Refactoring/RefactoringActions.cpp index 25f055b7270b8..73a3118396208 100644 --- a/clang/lib/Tooling/Refactoring/RefactoringActions.cpp +++ b/clang/lib/Tooling/Refactoring/RefactoringActions.cpp @@ -7,21 +7,80 @@ // //===----------------------------------------------------------------------===// +#include "clang/Tooling/Refactoring/Extract/Extract.h" #include "clang/Tooling/Refactoring/RefactoringAction.h" +#include "clang/Tooling/Refactoring/RefactoringOptions.h" +#include "clang/Tooling/Refactoring/Rename/RenamingAction.h" namespace clang { namespace tooling { -// Forward declare the individual create*Action functions. -#define REFACTORING_ACTION(Name) \ - std::unique_ptr create##Name##Action(); -#include "clang/Tooling/Refactoring/RefactoringActionRegistry.def" +namespace { + +class DeclNameOption final : public OptionalRefactoringOption { +public: + StringRef getName() const { return "name"; } + StringRef getDescription() const { + return "Name of the extracted declaration"; + } +}; + +// FIXME: Rewrite the Actions to avoid duplication of descriptions/names with +// rules. +class ExtractRefactoring final : public RefactoringAction { +public: + StringRef getCommand() const override { return "extract"; } + + StringRef getDescription() const override { + return "(WIP action; use with caution!) Extracts code into a new function"; + } + + /// Returns a set of refactoring actions rules that are defined by this + /// action. + RefactoringActionRules createActionRules() const override { + RefactoringActionRules Rules; + Rules.push_back(createRefactoringActionRule( + CodeRangeASTSelectionRequirement(), + OptionRequirement())); + return Rules; + } +}; + +class NewNameOption : public RequiredRefactoringOption { +public: + StringRef getName() const override { return "new-name"; } + StringRef getDescription() const override { + return "The new name to change the symbol to"; + } +}; + +// FIXME: Rewrite the Actions to avoid duplication of descriptions/names with +// rules. +class LocalRename final : public RefactoringAction { +public: + StringRef getCommand() const override { return "local-rename"; } + + StringRef getDescription() const override { + return "Finds and renames symbols in code with no indexer support"; + } + + /// Returns a set of refactoring actions rules that are defined by this + /// action. + RefactoringActionRules createActionRules() const override { + RefactoringActionRules Rules; + Rules.push_back(createRefactoringActionRule( + SourceRangeSelectionRequirement(), OptionRequirement())); + return Rules; + } +}; + +} // end anonymous namespace std::vector> createRefactoringActions() { std::vector> Actions; -#define REFACTORING_ACTION(Name) Actions.push_back(create##Name##Action()); -#include "clang/Tooling/Refactoring/RefactoringActionRegistry.def" + Actions.push_back(llvm::make_unique()); + Actions.push_back(llvm::make_unique()); return Actions; } diff --git a/clang/lib/Tooling/Refactoring/Rename/RenamingAction.cpp b/clang/lib/Tooling/Refactoring/Rename/RenamingAction.cpp index 28912c3e139fe..210b45b79ef22 100644 --- a/clang/lib/Tooling/Refactoring/Rename/RenamingAction.cpp +++ b/clang/lib/Tooling/Refactoring/Rename/RenamingAction.cpp @@ -41,22 +41,6 @@ namespace tooling { namespace { -class SymbolSelectionRequirement : public SourceRangeSelectionRequirement { -public: - Expected evaluate(RefactoringRuleContext &Context) const { - Expected Selection = - SourceRangeSelectionRequirement::evaluate(Context); - if (!Selection) - return Selection.takeError(); - const NamedDecl *ND = - getNamedDeclAt(Context.getASTContext(), Selection->getBegin()); - if (!ND) - return Context.createDiagnosticError( - Selection->getBegin(), diag::err_refactor_selection_no_symbol); - return getCanonicalSymbolDeclaration(ND); - } -}; - class OccurrenceFinder final : public FindSymbolOccurrencesRefactoringRule { public: OccurrenceFinder(const NamedDecl *ND) : ND(ND) {} @@ -74,50 +58,38 @@ class OccurrenceFinder final : public FindSymbolOccurrencesRefactoringRule { const NamedDecl *ND; }; -class RenameOccurrences final : public SourceChangeRefactoringRule { -public: - RenameOccurrences(const NamedDecl *ND, std::string NewName) - : Finder(ND), NewName(std::move(NewName)) {} - - Expected - createSourceReplacements(RefactoringRuleContext &Context) override { - Expected Occurrences = - Finder.findSymbolOccurrences(Context); - if (!Occurrences) - return Occurrences.takeError(); - // FIXME: Verify that the new name is valid. - SymbolName Name(NewName); - return createRenameReplacements( - *Occurrences, Context.getASTContext().getSourceManager(), Name); - } - -private: - OccurrenceFinder Finder; - std::string NewName; -}; - -class LocalRename final : public RefactoringAction { -public: - StringRef getCommand() const override { return "local-rename"; } - - StringRef getDescription() const override { - return "Finds and renames symbols in code with no indexer support"; - } +} // end anonymous namespace - /// Returns a set of refactoring actions rules that are defined by this - /// action. - RefactoringActionRules createActionRules() const override { - RefactoringActionRules Rules; - Rules.push_back(createRefactoringActionRule( - SymbolSelectionRequirement(), OptionRequirement())); - return Rules; - } -}; +const RefactoringDescriptor &RenameOccurrences::describe() { + static const RefactoringDescriptor Descriptor = { + "local-rename", + "Rename", + "Finds and renames symbols in code with no indexer support", + }; + return Descriptor; +} -} // end anonymous namespace +Expected +RenameOccurrences::initiate(RefactoringRuleContext &Context, + SourceRange SelectionRange, std::string NewName) { + const NamedDecl *ND = + getNamedDeclAt(Context.getASTContext(), SelectionRange.getBegin()); + if (!ND) + return Context.createDiagnosticError( + SelectionRange.getBegin(), diag::err_refactor_selection_no_symbol); + return RenameOccurrences(getCanonicalSymbolDeclaration(ND), NewName); +} -std::unique_ptr createLocalRenameAction() { - return llvm::make_unique(); +Expected +RenameOccurrences::createSourceReplacements(RefactoringRuleContext &Context) { + Expected Occurrences = + OccurrenceFinder(ND).findSymbolOccurrences(Context); + if (!Occurrences) + return Occurrences.takeError(); + // FIXME: Verify that the new name is valid. + SymbolName Name(NewName); + return createRenameReplacements( + *Occurrences, Context.getASTContext().getSourceManager(), Name); } Expected> diff --git a/clang/unittests/Tooling/RefactoringActionRulesTest.cpp b/clang/unittests/Tooling/RefactoringActionRulesTest.cpp index 132a3a4496351..f0b6466fec468 100644 --- a/clang/unittests/Tooling/RefactoringActionRulesTest.cpp +++ b/clang/unittests/Tooling/RefactoringActionRulesTest.cpp @@ -10,7 +10,8 @@ #include "ReplacementTest.h" #include "RewriterTestContext.h" #include "clang/Tooling/Refactoring.h" -#include "clang/Tooling/Refactoring/RefactoringActionRules.h" +#include "clang/Tooling/Refactoring/Extract/Extract.h" +#include "clang/Tooling/Refactoring/RefactoringAction.h" #include "clang/Tooling/Refactoring/RefactoringDiagnostic.h" #include "clang/Tooling/Refactoring/Rename/SymbolName.h" #include "clang/Tooling/Tooling.h" @@ -63,6 +64,12 @@ TEST_F(RefactoringActionRulesTest, MyFirstRefactoringRule) { ReplaceAWithB(std::pair Selection) : Selection(Selection) {} + static Expected + initiate(RefactoringRuleContext &Cotnext, + std::pair Selection) { + return ReplaceAWithB(Selection); + } + Expected createSourceReplacements(RefactoringRuleContext &Context) { const SourceManager &SM = Context.getSources(); @@ -141,6 +148,11 @@ TEST_F(RefactoringActionRulesTest, MyFirstRefactoringRule) { TEST_F(RefactoringActionRulesTest, ReturnError) { class ErrorRule : public SourceChangeRefactoringRule { public: + static Expected initiate(RefactoringRuleContext &, + SourceRange R) { + return ErrorRule(R); + } + ErrorRule(SourceRange R) {} Expected createSourceReplacements(RefactoringRuleContext &) { return llvm::make_error( @@ -191,6 +203,11 @@ TEST_F(RefactoringActionRulesTest, ReturnSymbolOccurrences) { public: FindOccurrences(SourceRange Selection) : Selection(Selection) {} + static Expected initiate(RefactoringRuleContext &, + SourceRange Selection) { + return FindOccurrences(Selection); + } + Expected findSymbolOccurrences(RefactoringRuleContext &) override { SymbolOccurrences Occurrences; @@ -219,4 +236,13 @@ TEST_F(RefactoringActionRulesTest, ReturnSymbolOccurrences) { SourceRange(Cursor, Cursor.getLocWithOffset(strlen("test")))); } +TEST_F(RefactoringActionRulesTest, EditorCommandBinding) { + const RefactoringDescriptor &Descriptor = ExtractFunction::describe(); + EXPECT_EQ(Descriptor.Name, "extract-function"); + EXPECT_EQ( + Descriptor.Description, + "(WIP action; use with caution!) Extracts code into a new function"); + EXPECT_EQ(Descriptor.Title, "Extract Function"); +} + } // end anonymous namespace