r314704 - [refactor] Simplify the refactoring interface

Alex Lorenz via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 2 11:42:43 PDT 2017


Author: arphaman
Date: Mon Oct  2 11:42:43 2017
New Revision: 314704

URL: http://llvm.org/viewvc/llvm-project?rev=314704&view=rev
Log:
[refactor] Simplify the refactoring interface

This commit simplifies the interface for the refactoring action rules and the
refactoring requirements. It merges the selection constraints and the selection
requirements into one class. The refactoring actions rules must now be
implemented using subclassing instead of raw function / lambda pointers. This
change also removes a bunch of template-based traits and other
template definitions that are now redundant.

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

Removed:
    cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h
    cfe/trunk/include/clang/Tooling/Refactoring/SourceSelectionConstraints.h
    cfe/trunk/lib/Tooling/Refactoring/SourceSelectionConstraints.cpp
Modified:
    cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h
    cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h
    cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRules.h
    cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
    cfe/trunk/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
    cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
    cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
    cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp

Modified: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h?rev=314704&r1=314703&r2=314704&view=diff
==============================================================================
--- cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h (original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h Mon Oct  2 11:42:43 2017
@@ -19,10 +19,12 @@ namespace tooling {
 class RefactoringResultConsumer;
 class RefactoringRuleContext;
 
-/// A common refactoring action rule interface.
-class RefactoringActionRule {
+/// A common refactoring action rule interface that defines the 'invoke'
+/// function that performs the refactoring operation (either fully or
+/// partially).
+class RefactoringActionRuleBase {
 public:
-  virtual ~RefactoringActionRule() {}
+  virtual ~RefactoringActionRuleBase() {}
 
   /// Initiates and performs a specific refactoring action.
   ///
@@ -30,17 +32,19 @@ public:
   /// consumer to propagate the result of the refactoring action.
   virtual void invoke(RefactoringResultConsumer &Consumer,
                       RefactoringRuleContext &Context) = 0;
+};
 
+/// A refactoring action rule is a wrapper class around a specific refactoring
+/// action rule (SourceChangeRefactoringRule, etc) that, in addition to invoking
+/// the action, describes the requirements that determine when the action can be
+/// initiated.
+class RefactoringActionRule : public RefactoringActionRuleBase {
+public:
   /// Returns true when the rule has a source selection requirement that has
   /// to be fullfilled before refactoring can be performed.
   virtual bool hasSelectionRequirement() = 0;
 };
 
-/// A set of refactoring action rules that should have unique initiation
-/// requirements.
-using RefactoringActionRules =
-    std::vector<std::unique_ptr<RefactoringActionRule>>;
-
 } // end namespace tooling
 } // end namespace clang
 

Modified: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h?rev=314704&r1=314703&r2=314704&view=diff
==============================================================================
--- cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h (original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h Mon Oct  2 11:42:43 2017
@@ -10,48 +10,49 @@
 #ifndef LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_REQUIREMENTS_H
 #define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_REQUIREMENTS_H
 
-#include "clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Tooling/Refactoring/RefactoringRuleContext.h"
 #include "llvm/Support/Error.h"
 #include <type_traits>
 
 namespace clang {
 namespace tooling {
-namespace refactoring_action_rules {
 
-/// Creates a selection requirement from the given requirement.
+/// A refactoring action rule requirement determines when a refactoring action
+/// rule can be invoked. The rule can be invoked only when all of the
+/// requirements are satisfied.
 ///
-/// Requirements must subclass \c selection::Requirement and implement
-/// evaluateSelection member function.
-template <typename T>
-internal::SourceSelectionRequirement<
-    typename selection::internal::EvaluateSelectionChecker<
-        decltype(&T::evaluateSelection)>::ArgType,
-    typename selection::internal::EvaluateSelectionChecker<
-        decltype(&T::evaluateSelection)>::ReturnType,
-    T>
-requiredSelection(
-    const T &Requirement,
-    typename std::enable_if<selection::traits::IsRequirement<T>::value>::type
-        * = nullptr) {
-  return internal::SourceSelectionRequirement<
-      typename selection::internal::EvaluateSelectionChecker<decltype(
-          &T::evaluateSelection)>::ArgType,
-      typename selection::internal::EvaluateSelectionChecker<decltype(
-          &T::evaluateSelection)>::ReturnType,
-      T>(Requirement);
-}
-
-template <typename T>
-void requiredSelection(
-    const T &,
-    typename std::enable_if<
-        !std::is_base_of<selection::Requirement, T>::value>::type * = nullptr) {
-  static_assert(
-      sizeof(T) && false,
-      "selection requirement must be a class derived from Requirement");
-}
+/// Subclasses must implement the
+/// 'Expected<T> evaluate(RefactoringRuleContext &) const' member function.
+/// \c T is used to determine the return type that is passed to the
+/// refactoring rule's constructor.
+/// For example, the \c SourceRangeSelectionRequirement subclass defines
+/// 'Expected<SourceRange> evaluate(RefactoringRuleContext &Context) const'
+/// function. When this function returns a non-error value, the resulting
+/// source range is passed to the specific refactoring action rule
+/// constructor (provided all other requirements are satisfied).
+class RefactoringActionRuleRequirement {
+  // Expected<T> evaluate(RefactoringRuleContext &Context) const;
+};
+
+/// A base class for any requirement that expects some part of the source to be
+/// selected in an editor (or the refactoring tool with the -selection option).
+class SourceSelectionRequirement : public RefactoringActionRuleRequirement {};
+
+/// A selection requirement that is satisfied when any portion of the source
+/// text is selected.
+class SourceRangeSelectionRequirement : public SourceSelectionRequirement {
+public:
+  Expected<SourceRange> evaluate(RefactoringRuleContext &Context) const {
+    if (Context.getSelectionRange().isValid())
+      return Context.getSelectionRange();
+    // FIXME: Use a diagnostic.
+    return llvm::make_error<llvm::StringError>(
+        "refactoring action can't be initiated without a selection",
+        llvm::inconvertibleErrorCode());
+  }
+};
 
-} // end namespace refactoring_action_rules
 } // end namespace tooling
 } // end namespace clang
 

Removed: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h?rev=314703&view=auto
==============================================================================
--- cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h (original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h (removed)
@@ -1,104 +0,0 @@
-//===--- RefactoringActionRuleRequirementsInternal.h - --------------------===//
-//
-//                     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_REFACTORING_ACTION_REQUIREMENTS_INTERNAL_H
-#define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_REQUIREMENTS_INTERNAL_H
-
-#include "clang/Basic/LLVM.h"
-#include "clang/Tooling/Refactoring/RefactoringRuleContext.h"
-#include "clang/Tooling/Refactoring/SourceSelectionConstraints.h"
-#include <type_traits>
-
-namespace clang {
-namespace tooling {
-namespace refactoring_action_rules {
-namespace internal {
-
-/// A base class for any requirement. Used by the \c IsRequirement trait to
-/// determine if a class is a valid requirement.
-struct RequirementBase {};
-
-/// Defines a type alias of type \T when given \c Expected<Optional<T>>, or
-/// \c T otherwise.
-template <typename T> struct DropExpectedOptional { using Type = T; };
-
-template <typename T> struct DropExpectedOptional<Expected<Optional<T>>> {
-  using Type = T;
-};
-
-/// The \c requiredSelection refactoring action requirement is represented
-/// using this type.
-template <typename InputT, typename OutputT, typename RequirementT>
-struct SourceSelectionRequirement
-    : std::enable_if<selection::traits::IsConstraint<InputT>::value &&
-                         selection::traits::IsRequirement<RequirementT>::value,
-                     RequirementBase>::type {
-  using OutputType = typename DropExpectedOptional<OutputT>::Type;
-
-  SourceSelectionRequirement(const RequirementT &Requirement)
-      : Requirement(Requirement) {}
-
-  /// Evaluates the action rule requirement by ensuring that both the selection
-  /// constraint and the selection requirement can be evaluated with the given
-  /// context.
-  ///
-  /// \returns None if the selection constraint is not evaluated successfully,
-  /// Error if the selection requirement is not evaluated successfully or
-  /// an OutputT if the selection requirement was successfully. The OutpuT
-  /// value is wrapped in Expected<Optional<>> which is then unwrapped by the
-  /// refactoring action rule before passing the value to the refactoring
-  /// function.
-  Expected<Optional<OutputType>> evaluate(RefactoringRuleContext &Context) {
-    Optional<InputT> Value = InputT::evaluate(Context);
-    if (!Value)
-      return None;
-    return std::move(Requirement.evaluateSelection(Context, *Value));
-  }
-
-private:
-  const RequirementT Requirement;
-};
-
-} // end namespace internal
-
-namespace traits {
-
-/// A type trait that returns true iff the given type is a valid rule
-/// requirement.
-template <typename First, typename... Rest>
-struct IsRequirement : std::conditional<IsRequirement<First>::value &&
-                                            IsRequirement<Rest...>::value,
-                                        std::true_type, std::false_type>::type {
-};
-
-template <typename T>
-struct IsRequirement<T>
-    : std::conditional<std::is_base_of<internal::RequirementBase, T>::value,
-                       std::true_type, std::false_type>::type {};
-
-/// A type trait that returns true when the given type has at least one source
-/// selection requirement.
-template <typename First, typename... Rest>
-struct HasSelectionRequirement
-    : std::conditional<HasSelectionRequirement<First>::value ||
-                           HasSelectionRequirement<Rest...>::value,
-                       std::true_type, std::false_type>::type {};
-
-template <typename I, typename O, typename R>
-struct HasSelectionRequirement<internal::SourceSelectionRequirement<I, O, R>>
-    : std::true_type {};
-
-template <typename T> struct HasSelectionRequirement<T> : std::false_type {};
-
-} // end namespace traits
-} // end namespace refactoring_action_rules
-} // end namespace tooling
-} // end namespace clang
-
-#endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_REQUIREMENTS_INTERNAL_H

Modified: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRules.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRules.h?rev=314704&r1=314703&r2=314704&view=diff
==============================================================================
--- cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRules.h (original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRules.h Mon Oct  2 11:42:43 2017
@@ -16,60 +16,78 @@
 namespace clang {
 namespace tooling {
 
-class RefactoringRuleContext;
-
-namespace refactoring_action_rules {
-
-/// Creates a new refactoring action rule that invokes the given function once
-/// all of the requirements are satisfied. The values produced during the
-/// evaluation of requirements are passed to the given function (in the order of
-/// requirements).
+/// Creates a new refactoring action rule that constructs and invokes the
+/// \c RuleType rule when all of the requirements are satisfied.
 ///
-/// \param RefactoringFunction the function that will perform the refactoring
-/// once the requirements are satisfied. The function must return a valid
-/// refactoring result type wrapped in an \c Expected type. The following result
-/// types are currently supported:
+/// This function takes in a list of values whose type derives from
+/// \c RefactoringActionRuleRequirement. These values describe the initiation
+/// requirements that have to be satisfied by the refactoring engine before
+/// the provided action rule can be constructed and invoked. The engine
+/// verifies that the requirements are satisfied by evaluating them (using the
+/// 'evaluate' member function) and checking that the results don't contain
+/// any errors. Once all requirements are satisfied, the provided refactoring
+/// rule is constructed by passing in the values returned by the requirements'
+/// evaluate functions as arguments to the constructor. The rule is then invoked
+/// immediately after construction.
 ///
-///  - AtomicChanges: the refactoring function will be used to create source
-///                   replacements.
+/// The separation of requirements, their evaluation and the invocation of the
+/// refactoring action rule allows the refactoring clients to:
+///   - Disable refactoring action rules whose requirements are not supported.
+///   - Gather the set of options and define a command-line / visual interface
+///     that allows users to input these options without ever invoking the
+///     action.
+template <typename RuleType, typename... RequirementTypes>
+std::unique_ptr<RefactoringActionRule>
+createRefactoringActionRule(const RequirementTypes &... Requirements);
+
+/// A set of refactoring action rules that should have unique initiation
+/// requirements.
+using RefactoringActionRules =
+    std::vector<std::unique_ptr<RefactoringActionRule>>;
+
+/// A type of refactoring action rule that produces source replacements in the
+/// form of atomic changes.
 ///
-/// \param Requirements a set of rule requirements that have to be satisfied.
-/// Each requirement must be a valid requirement, i.e. the value of
-/// \c traits::IsRequirement<T> must be true. The following requirements are
-/// currently supported:
+/// This action rule is typically used for local refactorings that replace
+/// source in a single AST unit.
+class SourceChangeRefactoringRule : public RefactoringActionRuleBase {
+public:
+  void invoke(RefactoringResultConsumer &Consumer,
+              RefactoringRuleContext &Context) final override {
+    Expected<AtomicChanges> Changes = createSourceReplacements(Context);
+    if (!Changes)
+      Consumer.handleError(Changes.takeError());
+    else
+      Consumer.handle(std::move(*Changes));
+  }
+
+private:
+  virtual Expected<AtomicChanges>
+  createSourceReplacements(RefactoringRuleContext &Context) = 0;
+};
+
+/// A type of refactoring action rule that finds a set of symbol occurrences
+/// that reference a particular symbol.
 ///
-///  - requiredSelection: The refactoring function won't be invoked unless the
-///                       given selection requirement is satisfied.
-template <typename ResultType, typename... RequirementTypes>
-std::unique_ptr<RefactoringActionRule>
-createRefactoringRule(Expected<ResultType> (*RefactoringFunction)(
-                          const RefactoringRuleContext &,
-                          typename RequirementTypes::OutputType...),
-                      const RequirementTypes &... Requirements) {
-  static_assert(tooling::traits::IsValidRefactoringResult<ResultType>::value,
-                "invalid refactoring result type");
-  static_assert(traits::IsRequirement<RequirementTypes...>::value,
-                "invalid refactoring action rule requirement");
-  return llvm::make_unique<internal::PlainFunctionRule<
-      decltype(RefactoringFunction), RequirementTypes...>>(
-      RefactoringFunction, std::make_tuple(Requirements...));
-}
-
-template <
-    typename Callable, typename... RequirementTypes,
-    typename Fn = decltype(&Callable::operator()),
-    typename ResultType = typename internal::LambdaDeducer<Fn>::ReturnType,
-    bool IsNonCapturingLambda = std::is_convertible<
-        Callable, typename internal::LambdaDeducer<Fn>::FunctionType>::value,
-    typename = typename std::enable_if<IsNonCapturingLambda>::type>
-std::unique_ptr<RefactoringActionRule>
-createRefactoringRule(const Callable &C,
-                      const RequirementTypes &... Requirements) {
-  typename internal::LambdaDeducer<Fn>::FunctionType Func = C;
-  return createRefactoringRule(Func, Requirements...);
-}
+/// This action rule is typically used for an interactive rename that allows
+/// users to specify the new name and the set of selected occurrences during
+/// the refactoring.
+class FindSymbolOccurrencesRefactoringRule : public RefactoringActionRuleBase {
+public:
+  void invoke(RefactoringResultConsumer &Consumer,
+              RefactoringRuleContext &Context) final override {
+    Expected<SymbolOccurrences> Occurrences = findSymbolOccurrences(Context);
+    if (!Occurrences)
+      Consumer.handleError(Occurrences.takeError());
+    else
+      Consumer.handle(std::move(*Occurrences));
+  }
+
+private:
+  virtual Expected<SymbolOccurrences>
+  findSymbolOccurrences(RefactoringRuleContext &Context) = 0;
+};
 
-} // end namespace refactoring_action_rules
 } // end namespace tooling
 } // end namespace clang
 

Modified: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h?rev=314704&r1=314703&r2=314704&view=diff
==============================================================================
--- cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h (original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h Mon Oct  2 11:42:43 2017
@@ -20,108 +20,90 @@
 
 namespace clang {
 namespace tooling {
-namespace refactoring_action_rules {
 namespace internal {
 
-/// A specialized refactoring action rule that calls the stored function once
-/// all the of the requirements are fullfilled. The values produced during the
-/// evaluation of requirements are passed to the stored function.
-template <typename FunctionType, typename... RequirementTypes>
-class PlainFunctionRule final : public RefactoringActionRule {
-public:
-  PlainFunctionRule(FunctionType Function,
-                    std::tuple<RequirementTypes...> &&Requirements)
-      : Function(Function), Requirements(std::move(Requirements)) {}
-
-  void invoke(RefactoringResultConsumer &Consumer,
-              RefactoringRuleContext &Context) override {
-    return invokeImpl(Consumer, Context,
-                      llvm::index_sequence_for<RequirementTypes...>());
-  }
-
-  bool hasSelectionRequirement() override {
-    return traits::HasSelectionRequirement<RequirementTypes...>::value;
-  }
-
-private:
-  /// Returns \c T when given \c Expected<Optional<T>>, or \c T otherwise.
-  template <typename T>
-  static T &&unwrapRequirementResult(Expected<Optional<T>> &&X) {
-    assert(X && "unexpected diagnostic!");
-    return std::move(**X);
-  }
-  template <typename T> static T &&unwrapRequirementResult(T &&X) {
-    return std::move(X);
-  }
-
-  /// Scans the tuple and returns a \c PartialDiagnosticAt
-  /// from the first invalid \c DiagnosticOr value. Returns \c None if all
-  /// values are valid.
-  template <typename FirstT, typename... RestT>
-  static Optional<llvm::Error> findErrorNone(FirstT &First, RestT &... Rest) {
-    Optional<llvm::Error> Result = takeErrorNone(First);
-    if (Result)
-      return Result;
-    return findErrorNone(Rest...);
-  }
-
-  static Optional<llvm::Error> findErrorNone() { return None; }
-
-  template <typename T> static Optional<llvm::Error> takeErrorNone(T &) {
-    return None;
-  }
-
-  template <typename T>
-  static Optional<llvm::Error> takeErrorNone(Expected<Optional<T>> &Diag) {
-    if (!Diag)
-      return std::move(Diag.takeError());
-    if (!*Diag)
-      return llvm::Error::success(); // Initiation failed without a diagnostic.
-    return None;
-  }
-
-  template <size_t... Is>
-  void invokeImpl(RefactoringResultConsumer &Consumer,
-                  RefactoringRuleContext &Context,
-                  llvm::index_sequence<Is...> Seq) {
-    // Initiate the operation.
-    auto Values =
-        std::make_tuple(std::get<Is>(Requirements).evaluate(Context)...);
-    Optional<llvm::Error> InitiationFailure =
-        findErrorNone(std::get<Is>(Values)...);
-    if (InitiationFailure) {
-      llvm::Error Error = std::move(*InitiationFailure);
-      if (!Error)
-        // FIXME: Use a diagnostic.
-        return Consumer.handleError(llvm::make_error<llvm::StringError>(
-            "refactoring action can't be initiated with the specified "
-            "selection range",
-            llvm::inconvertibleErrorCode()));
-      return Consumer.handleError(std::move(Error));
-    }
-    // Perform the operation.
-    auto Result = Function(
-        Context, unwrapRequirementResult(std::move(std::get<Is>(Values)))...);
-    if (!Result)
-      return Consumer.handleError(Result.takeError());
-    Consumer.handle(std::move(*Result));
-  }
-
-  FunctionType Function;
-  std::tuple<RequirementTypes...> Requirements;
-};
-
-/// Used to deduce the refactoring result type for the lambda that passed into
-/// createRefactoringRule.
-template <typename T> struct LambdaDeducer;
-template <typename T, typename R, typename... Args>
-struct LambdaDeducer<R (T::*)(const RefactoringRuleContext &, Args...) const> {
-  using ReturnType = R;
-  using FunctionType = R (*)(const RefactoringRuleContext &, Args...);
-};
+inline llvm::Error findError() { return llvm::Error::success(); }
+
+/// Scans the tuple and returns a valid \c Error if any of the values are
+/// invalid.
+template <typename FirstT, typename... RestT>
+llvm::Error findError(Expected<FirstT> &First, Expected<RestT> &... Rest) {
+  if (!First)
+    return First.takeError();
+  return findError(Rest...);
+}
+
+template <typename RuleType, typename... RequirementTypes, size_t... Is>
+void invokeRuleAfterValidatingRequirements(
+    RefactoringResultConsumer &Consumer, RefactoringRuleContext &Context,
+    const std::tuple<RequirementTypes...> &Requirements,
+    llvm::index_sequence<Is...>) {
+  // Check if the requirements we're interested in can be evaluated.
+  auto Values =
+      std::make_tuple(std::get<Is>(Requirements).evaluate(Context)...);
+  auto Err = findError(std::get<Is>(Values)...);
+  if (Err)
+    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::get<Is>(Values))...).invoke(Consumer, Context);
+}
+
+/// A type trait that returns true when the given type list has at least one
+/// type whose base is the given base type.
+template <typename Base, typename First, typename... Rest>
+struct HasBaseOf : std::conditional<HasBaseOf<Base, First>::value ||
+                                        HasBaseOf<Base, Rest...>::value,
+                                    std::true_type, std::false_type>::type {};
+
+template <typename Base, typename T>
+struct HasBaseOf<Base, T> : std::is_base_of<Base, T> {};
+
+/// A type trait that returns true when the given type list contains types that
+/// derive from Base.
+template <typename Base, typename First, typename... Rest>
+struct AreBaseOf : std::conditional<AreBaseOf<Base, First>::value &&
+                                        AreBaseOf<Base, Rest...>::value,
+                                    std::true_type, std::false_type>::type {};
+
+template <typename Base, typename T>
+struct AreBaseOf<Base, T> : std::is_base_of<Base, T> {};
 
 } // end namespace internal
-} // end namespace refactoring_action_rules
+
+template <typename RuleType, typename... RequirementTypes>
+std::unique_ptr<RefactoringActionRule>
+createRefactoringActionRule(const RequirementTypes &... Requirements) {
+  static_assert(std::is_base_of<RefactoringActionRuleBase, RuleType>::value,
+                "Expected a refactoring action rule type");
+  static_assert(internal::AreBaseOf<RefactoringActionRuleRequirement,
+                                    RequirementTypes...>::value,
+                "Expected a list of refactoring action rules");
+
+  class Rule final : public RefactoringActionRule {
+  public:
+    Rule(std::tuple<RequirementTypes...> Requirements)
+        : Requirements(Requirements) {}
+
+    void invoke(RefactoringResultConsumer &Consumer,
+                RefactoringRuleContext &Context) override {
+      internal::invokeRuleAfterValidatingRequirements<RuleType>(
+          Consumer, Context, Requirements,
+          llvm::index_sequence_for<RequirementTypes...>());
+    }
+
+    bool hasSelectionRequirement() override {
+      return internal::HasBaseOf<SourceSelectionRequirement,
+                                 RequirementTypes...>::value;
+    }
+
+  private:
+    std::tuple<RequirementTypes...> Requirements;
+  };
+
+  return llvm::make_unique<Rule>(std::make_tuple(Requirements...));
+}
+
 } // end namespace tooling
 } // end namespace clang
 

Modified: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h?rev=314704&r1=314703&r2=314704&view=diff
==============================================================================
--- cfe/trunk/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h (original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h Mon Oct  2 11:42:43 2017
@@ -46,29 +46,6 @@ private:
   }
 };
 
-namespace traits {
-namespace internal {
-
-template <typename T> struct HasHandle {
-private:
-  template <typename ClassT>
-  static auto check(ClassT *) -> typename std::is_same<
-      decltype(std::declval<ClassT>().handle(std::declval<T>())), void>::type;
-
-  template <typename> static std::false_type check(...);
-
-public:
-  using Type = decltype(check<RefactoringResultConsumer>(nullptr));
-};
-
-} // end namespace internal
-
-/// A type trait that returns true iff the given type is a valid refactoring
-/// result.
-template <typename T>
-struct IsValidRefactoringResult : internal::HasHandle<T>::Type {};
-
-} // end namespace traits
 } // end namespace tooling
 } // end namespace clang
 

Removed: cfe/trunk/include/clang/Tooling/Refactoring/SourceSelectionConstraints.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/SourceSelectionConstraints.h?rev=314703&view=auto
==============================================================================
--- cfe/trunk/include/clang/Tooling/Refactoring/SourceSelectionConstraints.h (original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/SourceSelectionConstraints.h (removed)
@@ -1,114 +0,0 @@
-//===--- SourceSelectionConstraints.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_SOURCE_SELECTION_CONSTRAINTS_H
-#define LLVM_CLANG_TOOLING_REFACTOR_SOURCE_SELECTION_CONSTRAINTS_H
-
-#include "clang/Basic/LLVM.h"
-#include "clang/Basic/SourceLocation.h"
-#include "clang/Tooling/Refactoring/RefactoringRuleContext.h"
-#include <type_traits>
-
-namespace clang {
-namespace tooling {
-
-class RefactoringRuleContext;
-
-namespace selection {
-
-/// This constraint is satisfied when any portion of the source text is
-/// selected. It can be used to obtain the raw source selection range.
-struct SourceSelectionRange {
-  SourceSelectionRange(const SourceManager &SM, SourceRange Range)
-      : SM(SM), Range(Range) {}
-
-  const SourceManager &getSources() const { return SM; }
-  SourceRange getRange() const { return Range; }
-
-  static Optional<SourceSelectionRange>
-  evaluate(RefactoringRuleContext &Context);
-
-private:
-  const SourceManager &SM;
-  SourceRange Range;
-};
-
-/// A custom selection requirement.
-class Requirement {
-  /// Subclasses must implement
-  /// 'T evaluateSelection(const RefactoringRuleContext &,
-  /// SelectionConstraint) const' member function. \c T is used to determine
-  /// the return type that is passed to the refactoring rule's function.
-  /// If T is \c DiagnosticOr<S> , then \c S is passed to the rule's function
-  /// using move semantics.
-  /// Otherwise, T is passed to the function directly using move semantics.
-  ///
-  /// The different return type rules allow refactoring actions to fail
-  /// initiation when the relevant portions of AST aren't selected.
-};
-
-namespace traits {
-
-/// A type trait that returns true iff the given type is a valid selection
-/// constraint.
-template <typename T> struct IsConstraint : public std::false_type {};
-
-} // end namespace traits
-
-namespace internal {
-
-template <typename T> struct EvaluateSelectionChecker : std::false_type {};
-
-template <typename T, typename R, typename A>
-struct EvaluateSelectionChecker<R (T::*)(const RefactoringRuleContext &, A)
-                                    const> : std::true_type {
-  using ReturnType = R;
-  using ArgType = A;
-};
-
-template <typename T> class Identity : public Requirement {
-public:
-  T evaluateSelection(const RefactoringRuleContext &, T Value) const {
-    return std::move(Value);
-  }
-};
-
-} // end namespace internal
-
-/// A identity function that returns the given selection constraint is provided
-/// for convenience, as it can be passed to \c requiredSelection directly.
-template <typename T> internal::Identity<T> identity() {
-  static_assert(
-      traits::IsConstraint<T>::value,
-      "selection::identity can be used with selection constraints only");
-  return internal::Identity<T>();
-}
-
-namespace traits {
-
-template <>
-struct IsConstraint<SourceSelectionRange> : public std::true_type {};
-
-/// A type trait that returns true iff \c T is a valid selection requirement.
-template <typename T>
-struct IsRequirement
-    : std::conditional<
-          std::is_base_of<Requirement, T>::value &&
-              internal::EvaluateSelectionChecker<decltype(
-                  &T::evaluateSelection)>::value &&
-              IsConstraint<typename internal::EvaluateSelectionChecker<decltype(
-                  &T::evaluateSelection)>::ArgType>::value,
-          std::true_type, std::false_type>::type {};
-
-} // end namespace traits
-} // end namespace selection
-} // end namespace tooling
-} // end namespace clang
-
-#endif // LLVM_CLANG_TOOLING_REFACTOR_SOURCE_SELECTION_CONSTRAINTS_H

Modified: cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt?rev=314704&r1=314703&r2=314704&view=diff
==============================================================================
--- cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt (original)
+++ cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt Mon Oct  2 11:42:43 2017
@@ -9,7 +9,6 @@ add_clang_library(clangToolingRefactor
   Rename/USRFinder.cpp
   Rename/USRFindingAction.cpp
   Rename/USRLocFinder.cpp
-  SourceSelectionConstraints.cpp
 
   LINK_LIBS
   clangAST

Modified: cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp?rev=314704&r1=314703&r2=314704&view=diff
==============================================================================
--- cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp Mon Oct  2 11:42:43 2017
@@ -23,7 +23,6 @@
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Refactoring/RefactoringAction.h"
-#include "clang/Tooling/Refactoring/RefactoringActionRules.h"
 #include "clang/Tooling/Refactoring/Rename/USRFinder.h"
 #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
 #include "clang/Tooling/Refactoring/Rename/USRLocFinder.h"
@@ -39,53 +38,78 @@ namespace tooling {
 
 namespace {
 
-class LocalRename : public RefactoringAction {
+class SymbolSelectionRequirement : public SourceRangeSelectionRequirement {
 public:
-  StringRef getCommand() const override { return "local-rename"; }
-
-  StringRef getDescription() const override {
-    return "Finds and renames symbols in code with no indexer support";
+  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) {
+      // FIXME: Use a diagnostic.
+      return llvm::make_error<StringError>("no symbol selected",
+                                           llvm::inconvertibleErrorCode());
+    }
+    return getCanonicalSymbolDeclaration(ND);
   }
+};
 
-  /// Returns a set of refactoring actions rules that are defined by this
-  /// action.
-  RefactoringActionRules createActionRules() const override {
-    using namespace refactoring_action_rules;
-    RefactoringActionRules Rules;
-    Rules.push_back(createRefactoringRule(
-        renameOccurrences, requiredSelection(SymbolSelectionRequirement())));
-    return Rules;
-  }
+class OccurrenceFinder final : public FindSymbolOccurrencesRefactoringRule {
+public:
+  OccurrenceFinder(const NamedDecl *ND) : ND(ND) {}
 
-private:
-  static Expected<AtomicChanges>
-  renameOccurrences(const RefactoringRuleContext &Context,
-                    const NamedDecl *ND) {
+  Expected<SymbolOccurrences>
+  findSymbolOccurrences(RefactoringRuleContext &Context) override {
     std::vector<std::string> USRs =
         getUSRsForDeclaration(ND, Context.getASTContext());
     std::string PrevName = ND->getNameAsString();
-    auto Occurrences = getOccurrencesOfUSRs(
+    return getOccurrencesOfUSRs(
         USRs, PrevName, Context.getASTContext().getTranslationUnitDecl());
+  }
 
+private:
+  const NamedDecl *ND;
+};
+
+class RenameOccurrences final : public SourceChangeRefactoringRule {
+public:
+  RenameOccurrences(const NamedDecl *ND) : Finder(ND) {}
+
+  Expected<AtomicChanges>
+  createSourceReplacements(RefactoringRuleContext &Context) {
+    Expected<SymbolOccurrences> Occurrences =
+        Finder.findSymbolOccurrences(Context);
+    if (!Occurrences)
+      return Occurrences.takeError();
     // FIXME: This is a temporary workaround that's needed until the refactoring
     // options are implemented.
     StringRef NewName = "Bar";
     return createRenameReplacements(
-        Occurrences, Context.getASTContext().getSourceManager(), NewName);
+        *Occurrences, Context.getASTContext().getSourceManager(), NewName);
   }
 
-  class SymbolSelectionRequirement : public selection::Requirement {
-  public:
-    Expected<Optional<const NamedDecl *>>
-    evaluateSelection(const RefactoringRuleContext &Context,
-                      selection::SourceSelectionRange Selection) const {
-      const NamedDecl *ND = getNamedDeclAt(Context.getASTContext(),
-                                           Selection.getRange().getBegin());
-      if (!ND)
-        return None;
-      return getCanonicalSymbolDeclaration(ND);
-    }
-  };
+private:
+  OccurrenceFinder Finder;
+};
+
+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>(
+        SymbolSelectionRequirement()));
+    return Rules;
+  }
 };
 
 } // end anonymous namespace

Removed: cfe/trunk/lib/Tooling/Refactoring/SourceSelectionConstraints.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/SourceSelectionConstraints.cpp?rev=314703&view=auto
==============================================================================
--- cfe/trunk/lib/Tooling/Refactoring/SourceSelectionConstraints.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/SourceSelectionConstraints.cpp (removed)
@@ -1,23 +0,0 @@
-//===--- SourceSelectionConstraints.cpp - Evaluate selection constraints --===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#include "clang/Tooling/Refactoring/SourceSelectionConstraints.h"
-#include "clang/Tooling/Refactoring/RefactoringRuleContext.h"
-
-using namespace clang;
-using namespace tooling;
-using namespace selection;
-
-Optional<SourceSelectionRange>
-SourceSelectionRange::evaluate(RefactoringRuleContext &Context) {
-  SourceRange R = Context.getSelectionRange();
-  if (R.isInvalid())
-    return None;
-  return SourceSelectionRange(Context.getSources(), R);
-}

Modified: cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp?rev=314704&r1=314703&r2=314704&view=diff
==============================================================================
--- cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp Mon Oct  2 11:42:43 2017
@@ -18,7 +18,6 @@
 
 using namespace clang;
 using namespace tooling;
-using namespace refactoring_action_rules;
 
 namespace {
 
@@ -56,29 +55,39 @@ createReplacements(const std::unique_ptr
 }
 
 TEST_F(RefactoringActionRulesTest, MyFirstRefactoringRule) {
-  auto ReplaceAWithB =
-      [](const RefactoringRuleContext &,
-         std::pair<selection::SourceSelectionRange, int> Selection)
-      -> Expected<AtomicChanges> {
-    const SourceManager &SM = Selection.first.getSources();
-    SourceLocation Loc = Selection.first.getRange().getBegin().getLocWithOffset(
-        Selection.second);
-    AtomicChange Change(SM, Loc);
-    llvm::Error E = Change.replace(SM, Loc, 1, "b");
-    if (E)
-      return std::move(E);
-    return AtomicChanges{Change};
-  };
-  class SelectionRequirement : public selection::Requirement {
-  public:
-    std::pair<selection::SourceSelectionRange, int>
-    evaluateSelection(const RefactoringRuleContext &,
-                      selection::SourceSelectionRange Selection) const {
-      return std::make_pair(Selection, 20);
+  class ReplaceAWithB : public SourceChangeRefactoringRule {
+    std::pair<SourceRange, int> Selection;
+
+  public:
+    ReplaceAWithB(std::pair<SourceRange, int> Selection)
+        : Selection(Selection) {}
+
+    Expected<AtomicChanges>
+    createSourceReplacements(RefactoringRuleContext &Context) {
+      const SourceManager &SM = Context.getSources();
+      SourceLocation Loc =
+          Selection.first.getBegin().getLocWithOffset(Selection.second);
+      AtomicChange Change(SM, Loc);
+      llvm::Error E = Change.replace(SM, Loc, 1, "b");
+      if (E)
+        return std::move(E);
+      return AtomicChanges{Change};
+    }
+  };
+
+  class SelectionRequirement : public SourceRangeSelectionRequirement {
+  public:
+    Expected<std::pair<SourceRange, int>>
+    evaluate(RefactoringRuleContext &Context) const {
+      Expected<SourceRange> R =
+          SourceRangeSelectionRequirement::evaluate(Context);
+      if (!R)
+        return R.takeError();
+      return std::make_pair(*R, 20);
     }
   };
-  auto Rule = createRefactoringRule(ReplaceAWithB,
-                                    requiredSelection(SelectionRequirement()));
+  auto Rule =
+      createRefactoringActionRule<ReplaceAWithB>(SelectionRequirement());
 
   // When the requirements are satisifed, the rule's function must be invoked.
   {
@@ -123,54 +132,24 @@ TEST_F(RefactoringActionRulesTest, MyFir
     llvm::handleAllErrors(
         ErrorOrResult.takeError(),
         [&](llvm::StringError &Error) { Message = Error.getMessage(); });
-    EXPECT_EQ(Message, "refactoring action can't be initiated with the "
-                       "specified selection range");
+    EXPECT_EQ(Message,
+              "refactoring action can't be initiated without a selection");
   }
 }
 
 TEST_F(RefactoringActionRulesTest, ReturnError) {
-  Expected<AtomicChanges> (*Func)(const RefactoringRuleContext &,
-                                  selection::SourceSelectionRange) =
-      [](const RefactoringRuleContext &,
-         selection::SourceSelectionRange) -> Expected<AtomicChanges> {
-    return llvm::make_error<llvm::StringError>(
-        "Error", llvm::make_error_code(llvm::errc::invalid_argument));
-  };
-  auto Rule = createRefactoringRule(
-      Func, requiredSelection(
-                selection::identity<selection::SourceSelectionRange>()));
-
-  RefactoringRuleContext RefContext(Context.Sources);
-  SourceLocation Cursor =
-      Context.Sources.getLocForStartOfFile(Context.Sources.getMainFileID());
-  RefContext.setSelectionRange({Cursor, Cursor});
-  Expected<AtomicChanges> Result = createReplacements(Rule, RefContext);
-
-  ASSERT_TRUE(!Result);
-  std::string Message;
-  llvm::handleAllErrors(Result.takeError(), [&](llvm::StringError &Error) {
-    Message = Error.getMessage();
-  });
-  EXPECT_EQ(Message, "Error");
-}
-
-TEST_F(RefactoringActionRulesTest, ReturnInitiationDiagnostic) {
-  RefactoringRuleContext RefContext(Context.Sources);
-  class SelectionRequirement : public selection::Requirement {
+  class ErrorRule : public SourceChangeRefactoringRule {
   public:
-    Expected<Optional<int>>
-    evaluateSelection(const RefactoringRuleContext &,
-                      selection::SourceSelectionRange Selection) const {
+    ErrorRule(SourceRange R) {}
+    Expected<AtomicChanges> createSourceReplacements(RefactoringRuleContext &) {
       return llvm::make_error<llvm::StringError>(
-          "bad selection", llvm::make_error_code(llvm::errc::invalid_argument));
+          "Error", llvm::make_error_code(llvm::errc::invalid_argument));
     }
   };
-  auto Rule = createRefactoringRule(
-      [](const RefactoringRuleContext &, int) -> Expected<AtomicChanges> {
-        llvm::report_fatal_error("Should not run!");
-      },
-      requiredSelection(SelectionRequirement()));
 
+  auto Rule =
+      createRefactoringActionRule<ErrorRule>(SourceRangeSelectionRequirement());
+  RefactoringRuleContext RefContext(Context.Sources);
   SourceLocation Cursor =
       Context.Sources.getLocForStartOfFile(Context.Sources.getMainFileID());
   RefContext.setSelectionRange({Cursor, Cursor});
@@ -181,7 +160,7 @@ TEST_F(RefactoringActionRulesTest, Retur
   llvm::handleAllErrors(Result.takeError(), [&](llvm::StringError &Error) {
     Message = Error.getMessage();
   });
-  EXPECT_EQ(Message, "bad selection");
+  EXPECT_EQ(Message, "Error");
 }
 
 Optional<SymbolOccurrences> findOccurrences(RefactoringActionRule &Rule,
@@ -205,18 +184,24 @@ Optional<SymbolOccurrences> findOccurren
 }
 
 TEST_F(RefactoringActionRulesTest, ReturnSymbolOccurrences) {
-  auto Rule = createRefactoringRule(
-      [](const RefactoringRuleContext &,
-         selection::SourceSelectionRange Selection)
-          -> Expected<SymbolOccurrences> {
-        SymbolOccurrences Occurrences;
-        Occurrences.push_back(SymbolOccurrence(
-            SymbolName("test"), SymbolOccurrence::MatchingSymbol,
-            Selection.getRange().getBegin()));
-        return std::move(Occurrences);
-      },
-      requiredSelection(
-          selection::identity<selection::SourceSelectionRange>()));
+  class FindOccurrences : public FindSymbolOccurrencesRefactoringRule {
+    SourceRange Selection;
+
+  public:
+    FindOccurrences(SourceRange Selection) : Selection(Selection) {}
+
+    Expected<SymbolOccurrences>
+    findSymbolOccurrences(RefactoringRuleContext &) override {
+      SymbolOccurrences Occurrences;
+      Occurrences.push_back(SymbolOccurrence(SymbolName("test"),
+                                             SymbolOccurrence::MatchingSymbol,
+                                             Selection.getBegin()));
+      return Occurrences;
+    }
+  };
+
+  auto Rule = createRefactoringActionRule<FindOccurrences>(
+      SourceRangeSelectionRequirement());
 
   RefactoringRuleContext RefContext(Context.Sources);
   SourceLocation Cursor =




More information about the cfe-commits mailing list