]> granicus.if.org Git - clang/commitdiff
[refactor] Describe refactorings in the operation classes
authorAlex Lorenz <arphaman@gmail.com>
Fri, 27 Oct 2017 18:19:11 +0000 (18:19 +0000)
committerAlex Lorenz <arphaman@gmail.com>
Fri, 27 Oct 2017 18:19:11 +0000 (18:19 +0000)
This commit changes the way that the refactoring operation classes are
structured:
- Users have to call `initiate` instead of constructing an instance of the
  class. The `initiate` is now supposed to have custom initiation logic, and
  you don't need to subclass the builtin requirements.
- A new `describe` function returns a structure with the id, title and the
  description of the refactoring operation.

The refactoring action classes are now placed into one common place in
RefactoringActions.cpp instead of being separate.

Differential Revision: https://reviews.llvm.org/D38985

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@316780 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Tooling/Refactoring/Extract/Extract.h [new file with mode: 0644]
include/clang/Tooling/Refactoring/RefactoringActionRegistry.def [deleted file]
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

diff --git a/include/clang/Tooling/Refactoring/Extract/Extract.h b/include/clang/Tooling/Refactoring/Extract/Extract.h
new file mode 100644 (file)
index 0000000..2fd76d2
--- /dev/null
@@ -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
diff --git a/include/clang/Tooling/Refactoring/RefactoringActionRegistry.def b/include/clang/Tooling/Refactoring/RefactoringActionRegistry.def
deleted file mode 100644 (file)
index 9aeead4..0000000
+++ /dev/null
@@ -1,8 +0,0 @@
-#ifndef REFACTORING_ACTION
-#define REFACTORING_ACTION(Name)
-#endif
-
-REFACTORING_ACTION(LocalRename)
-REFACTORING_ACTION(Extract)
-
-#undef REFACTORING_ACTION
index 294ccc381b5cb41ba62df23c544f46345f801b05..4130e29d8dea5d1c60b9e4dd41feb805080d7676 100644 (file)
@@ -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 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 @@ public:
   /// 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
index 443c7f86df812f62e975b029e3b16d9b6e96ea81..75b6c8f70d1733701a932e226963ce4485c35f30 100644 (file)
@@ -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<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 @@ createRefactoringActionRule(const RequirementTypes &... Requirements) {
           Visitor, Requirements,
           llvm::index_sequence_for<RequirementTypes...>());
     }
-
   private:
     std::tuple<RequirementTypes...> Requirements;
   };
index f2d9a7bb4d9a8309d282b4d69ecc16307d5b826a..d9ed7d3a1f3cb6889ef918d59888a7605433b729 100644 (file)
@@ -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 @@ private:
   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 1e3262188f99679aebbebad2d08fc6bf7282c8a5..41cf73d910d89acb37a36636cfa14a69a5d39862 100644 (file)
@@ -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 {
index 616900c1819eba7bec225e1ca0798bc4073bf0c7..b1000b60ee008c95956348584723e7ced45ff192 100644 (file)
 ///
 //===----------------------------------------------------------------------===//
 
+#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<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",
+      "Extract Function",
+      "(WIP action; use with caution!) Extracts code into a new 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 +177,5 @@ ExtractFunction::createSourceReplacements(RefactoringRuleContext &Context) {
   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 25f055b7270b870d6b389a10d00216bfc301748e..73a3118396208e9efd4a047751aeb7cdf2ba6238 100644 (file)
@@ -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<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: 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<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: 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<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 28912c3e139fe181d5fd99bf108f225e0ea465d9..210b45b79ef22ed1a5d67a54e4372eb12428b3e3 100644 (file)
@@ -41,22 +41,6 @@ namespace tooling {
 
 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,38 @@ private:
   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",
+      "Rename",
+      "Finds and renames symbols in code with no indexer support",
+  };
+  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 132a3a449635115559f2c530c2b9fe7de1ac0bdf..f0b6466fec468f10cf490f3dd7de953185fe9ad7 100644 (file)
@@ -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<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, MyFirstRefactoringRule) {
 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 @@ TEST_F(RefactoringActionRulesTest, ReturnSymbolOccurrences) {
   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 @@ 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