arphaman updated this revision to Diff 120466.
arphaman added a comment.
- Use a `RefactoringDescriptor` struct that's accessible from a static function
in an operation class.
- Make `createSourceReplacements` private.
Repository:
rL LLVM
https://reviews.llvm.org/D38985
Files:
include/clang/Tooling/Refactoring/Extract/Extract.h
include/clang/Tooling/Refactoring/RefactoringActionRegistry.def
include/clang/Tooling/Refactoring/RefactoringActionRule.h
include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
include/clang/Tooling/Refactoring/Rename/RenamingAction.h
include/clang/module.modulemap
lib/Tooling/Refactoring/Extract.cpp
lib/Tooling/Refactoring/RefactoringActions.cpp
lib/Tooling/Refactoring/Rename/RenamingAction.cpp
unittests/Tooling/RefactoringActionRulesTest.cpp
Index: unittests/Tooling/RefactoringActionRulesTest.cpp
===================================================================
--- unittests/Tooling/RefactoringActionRulesTest.cpp
+++ 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 @@
ReplaceAWithB(std::pair<SourceRange, int> Selection)
: Selection(Selection) {}
+ static Expected<ReplaceAWithB>
+ initiate(RefactoringRuleContext &Cotnext,
+ std::pair<SourceRange, int> Selection) {
+ return ReplaceAWithB(Selection);
+ }
+
Expected<AtomicChanges>
createSourceReplacements(RefactoringRuleContext &Context) {
const SourceManager &SM = Context.getSources();
@@ -141,6 +148,11 @@
TEST_F(RefactoringActionRulesTest, ReturnError) {
class ErrorRule : public SourceChangeRefactoringRule {
public:
+ static Expected<ErrorRule> initiate(RefactoringRuleContext &,
+ SourceRange R) {
+ return ErrorRule(R);
+ }
+
ErrorRule(SourceRange R) {}
Expected<AtomicChanges> createSourceReplacements(RefactoringRuleContext &) {
return llvm::make_error<llvm::StringError>(
@@ -191,6 +203,11 @@
public:
FindOccurrences(SourceRange Selection) : Selection(Selection) {}
+ static Expected<FindOccurrences> initiate(RefactoringRuleContext &,
+ SourceRange Selection) {
+ return FindOccurrences(Selection);
+ }
+
Expected<SymbolOccurrences>
findSymbolOccurrences(RefactoringRuleContext &) override {
SymbolOccurrences Occurrences;
@@ -219,4 +236,13 @@
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
Index: lib/Tooling/Refactoring/Rename/RenamingAction.cpp
===================================================================
--- lib/Tooling/Refactoring/Rename/RenamingAction.cpp
+++ lib/Tooling/Refactoring/Rename/RenamingAction.cpp
@@ -41,22 +41,6 @@
namespace {
-class SymbolSelectionRequirement : public SourceRangeSelectionRequirement {
-public:
- Expected<const NamedDecl *> evaluate(RefactoringRuleContext &Context) const {
- Expected<SourceRange> 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,36 @@
const NamedDecl *ND;
};
-class RenameOccurrences final : public SourceChangeRefactoringRule {
-public:
- RenameOccurrences(const NamedDecl *ND, std::string NewName)
- : Finder(ND), NewName(std::move(NewName)) {}
-
- Expected<AtomicChanges>
- createSourceReplacements(RefactoringRuleContext &Context) override {
- Expected<SymbolOccurrences> 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<RenameOccurrences>(
- SymbolSelectionRequirement(), OptionRequirement<NewNameOption>()));
- return Rules;
- }
-};
+const RefactoringDescriptor &RenameOccurrences::describe() {
+ static const RefactoringDescriptor Descriptor = {
+ "local-rename",
+ "Finds and renames symbols in code with no indexer support", "Rename"};
+ return Descriptor;
+}
-} // end anonymous namespace
+Expected<RenameOccurrences>
+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<RefactoringAction> createLocalRenameAction() {
- return llvm::make_unique<LocalRename>();
+Expected<AtomicChanges>
+RenameOccurrences::createSourceReplacements(RefactoringRuleContext &Context) {
+ Expected<SymbolOccurrences> 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<std::vector<AtomicChange>>
Index: lib/Tooling/Refactoring/RefactoringActions.cpp
===================================================================
--- lib/Tooling/Refactoring/RefactoringActions.cpp
+++ lib/Tooling/Refactoring/RefactoringActions.cpp
@@ -7,21 +7,78 @@
//
//===----------------------------------------------------------------------===//
+#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<RefactoringAction> create##Name##Action();
-#include "clang/Tooling/Refactoring/RefactoringActionRegistry.def"
+namespace {
+
+class DeclNameOption final : public OptionalRefactoringOption<std::string> {
+public:
+ StringRef getName() const { return "name"; }
+ StringRef getDescription() const {
+ return "Name of the extracted declaration";
+ }
+};
+
+// FIXME: Remove the Actions alltogether.
+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<ExtractFunction>(
+ CodeRangeASTSelectionRequirement(),
+ OptionRequirement<DeclNameOption>()));
+ return Rules;
+ }
+};
+
+class NewNameOption : public RequiredRefactoringOption<std::string> {
+public:
+ StringRef getName() const override { return "new-name"; }
+ StringRef getDescription() const override {
+ return "The new name to change the symbol to";
+ }
+};
+
+// FIXME: Remove the Actions alltogether.
+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<RenameOccurrences>(
+ SourceRangeSelectionRequirement(), OptionRequirement<NewNameOption>()));
+ return Rules;
+ }
+};
+
+} // end anonymous namespace
std::vector<std::unique_ptr<RefactoringAction>> createRefactoringActions() {
std::vector<std::unique_ptr<RefactoringAction>> Actions;
-#define REFACTORING_ACTION(Name) Actions.push_back(create##Name##Action());
-#include "clang/Tooling/Refactoring/RefactoringActionRegistry.def"
+ Actions.push_back(llvm::make_unique<LocalRename>());
+ Actions.push_back(llvm::make_unique<ExtractRefactoring>());
return Actions;
}
Index: lib/Tooling/Refactoring/Extract.cpp
===================================================================
--- lib/Tooling/Refactoring/Extract.cpp
+++ 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,42 @@
}
}
-class ExtractableCodeSelectionRequirement final
- : public CodeRangeASTSelectionRequirement {
-public:
- Expected<CodeRangeASTSelection>
- evaluate(RefactoringRuleContext &Context) const {
- Expected<CodeRangeASTSelection> 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<Expr>(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<std::string> DeclName)
- : Code(std::move(Code)),
- DeclName(DeclName ? std::move(*DeclName) : "extracted") {}
-
- Expected<AtomicChanges>
- 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",
+ "(WIP action; use with caution!) Extracts code into a new function",
+ "Extract Function"};
+ return Descriptor;
+}
+
+Expected<ExtractFunction>
+ExtractFunction::initiate(RefactoringRuleContext &Context,
+ CodeRangeASTSelection Code,
+ Optional<std::string> 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<Expr>(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<AtomicChanges>
@@ -194,39 +176,5 @@
return AtomicChanges{std::move(Change)};
}
-class DeclNameOption final : public OptionalRefactoringOption<std::string> {
-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<ExtractFunction>(
- ExtractableCodeSelectionRequirement(),
- OptionRequirement<DeclNameOption>()));
- return Rules;
- }
-};
-
-} // end anonymous namespace
-
-std::unique_ptr<RefactoringAction> createExtractAction() {
- return llvm::make_unique<ExtractRefactoring>();
-}
-
} // end namespace tooling
} // end namespace clang
Index: include/clang/module.modulemap
===================================================================
--- include/clang/module.modulemap
+++ include/clang/module.modulemap
@@ -146,8 +146,6 @@
// 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 {
Index: include/clang/Tooling/Refactoring/Rename/RenamingAction.h
===================================================================
--- include/clang/Tooling/Refactoring/Rename/RenamingAction.h
+++ 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 @@
bool PrintLocations;
};
-class NewNameOption : public RequiredRefactoringOption<std::string> {
+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<RenameOccurrences> 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<AtomicChanges>
+ createSourceReplacements(RefactoringRuleContext &Context) override;
+
+ const NamedDecl *ND;
+ std::string NewName;
};
/// Returns source replacements that correspond to the rename of the given
Index: include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
===================================================================
--- include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
+++ include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
@@ -57,7 +57,11 @@
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<Is>(Values)))...).invoke(Consumer, Context);
+ auto Rule =
+ RuleType::initiate(Context, std::move((*std::get<Is>(Values)))...);
+ if (!Rule)
+ return Consumer.handleError(Rule.takeError());
+ Rule->invoke(Consumer, Context);
}
inline void visitRefactoringOptionsImpl(RefactoringOptionVisitor &) {}
@@ -141,7 +145,6 @@
Visitor, Requirements,
llvm::index_sequence_for<RequirementTypes...>());
}
-
private:
std::tuple<RequirementTypes...> Requirements;
};
Index: include/clang/Tooling/Refactoring/RefactoringActionRule.h
===================================================================
--- include/clang/Tooling/Refactoring/RefactoringActionRule.h
+++ 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 <vector>
namespace clang {
@@ -20,6 +22,15 @@
class RefactoringResultConsumer;
class RefactoringRuleContext;
+struct RefactoringDescriptor {
+ /// A unique identifier for the specific refactoring.
+ StringRef Name;
+ /// A human readable description of what the refactoring does.
+ StringRef Description;
+ /// A human readable title for the refactoring.
+ StringRef Title;
+};
+
/// A common refactoring action rule interface that defines the 'invoke'
/// function that performs the refactoring operation (either fully or
/// partially).
@@ -33,6 +44,8 @@
/// consumer to propagate the result of the refactoring action.
virtual void invoke(RefactoringResultConsumer &Consumer,
RefactoringRuleContext &Context) = 0;
+
+ // static const RefactoringDescriptor &describe() = 0;
};
/// A refactoring action rule is a wrapper class around a specific refactoring
Index: include/clang/Tooling/Refactoring/RefactoringActionRegistry.def
===================================================================
--- 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
Index: include/clang/Tooling/Refactoring/Extract/Extract.h
===================================================================
--- /dev/null
+++ 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<ExtractFunction> initiate(RefactoringRuleContext &Context,
+ CodeRangeASTSelection Code,
+ Optional<std::string> DeclName);
+
+ static const RefactoringDescriptor &describe();
+
+private:
+ ExtractFunction(CodeRangeASTSelection Code, Optional<std::string> DeclName)
+ : Code(std::move(Code)),
+ DeclName(DeclName ? std::move(*DeclName) : "extracted") {}
+
+ Expected<AtomicChanges>
+ 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
