[refactor] Simplify the interface and remove some template magic

[arphaman]

Summary

This patch tries to simplify the interface for the refactoring action rules and refactoring requirements. It merges the selection constraints and selection requirements into one class. The refactoring actions rules must now be implemented using subclassing instead of raw function / lambda pointers. This allowed me to get rid of a bunch of traits templates and other template checks.

[ioeric]

This is very nice! Thanks!

Looks good to me; I'll let Manuel stamp the patch for you.

include/clang/Tooling/Refactoring/RefactoringActionRules.h

56 ↗

(On Diff #114578)

Can this be private?

77 ↗

(On Diff #114578)

private?

[arphaman]

Make methods private

[ioeric]

@klimek Have you got a chance to take a look?

[arphaman]

I will commit this today. @klimek, let me know if there any issues in post-commit review.

cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h

 #include <vector>
 
 namespace clang {
 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.
   ///
   /// The specific rule will invoke an appropriate \c handle method on a
   /// 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
 
 #endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_H

cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h

 //===--- RefactoringActionRuleRequirements.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_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);
+/// 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());
   }
+};
 
-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");
-}
-
-} // end namespace refactoring_action_rules
 } // end namespace tooling
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_REQUIREMENTS_H

cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h

-//===--- 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

cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRules.h

 #define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULES_H
 
 #include "clang/Tooling/Refactoring/RefactoringActionRule.h"
 #include "clang/Tooling/Refactoring/RefactoringActionRulesInternal.h"
 
 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).
-///
-/// \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:
+/// Creates a new refactoring action rule that constructs and invokes the
+/// \c RuleType rule when all of the requirements are satisfied.
 ///
-///  - AtomicChanges: the refactoring function will be used to create source
-///                   replacements.
+/// 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.
 ///
-/// \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:
-///
-///  - requiredSelection: The refactoring function won't be invoked unless the
-///                       given selection requirement is satisfied.
-template <typename ResultType, typename... RequirementTypes>
+/// 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>
-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...));
+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.
+///
+/// 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));
   }
 
-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...);
+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.
+///
+/// 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));
   }
 
-} // end namespace refactoring_action_rules
+private:
+  virtual Expected<SymbolOccurrences>
+  findSymbolOccurrences(RefactoringRuleContext &Context) = 0;
+};
+
 } // end namespace tooling
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULES_H

cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h

 #include "clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h"
 #include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
 #include "clang/Tooling/Refactoring/RefactoringRuleContext.h"
 #include "llvm/Support/Error.h"
 #include <type_traits>
 
 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)) {}
+inline llvm::Error findError() { return llvm::Error::success(); }
 
-  void invoke(RefactoringResultConsumer &Consumer,
-              RefactoringRuleContext &Context) override {
-    return invokeImpl(Consumer, Context,
-                      llvm::index_sequence_for<RequirementTypes...>());
-  }
+/// 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...);
 
-  bool hasSelectionRequirement() override {
-    return traits::HasSelectionRequirement<RequirementTypes...>::value;
-  }
+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 {};
 
-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);
-  }
+template <typename Base, typename T>
+struct AreBaseOf<Base, T> : std::is_base_of<Base, T> {};
 
-  /// 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...);
-  }
+} // end namespace internal
 
-  static Optional<llvm::Error> findErrorNone() { return None; }
+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");
 
-  template <typename T> static Optional<llvm::Error> takeErrorNone(T &) {
-    return None;
-  }
+  class Rule final : public RefactoringActionRule {
+  public:
+    Rule(std::tuple<RequirementTypes...> Requirements)
+        : Requirements(Requirements) {}
 
-  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;
+    void invoke(RefactoringResultConsumer &Consumer,
+                RefactoringRuleContext &Context) override {
+      internal::invokeRuleAfterValidatingRequirements<RuleType>(
+          Consumer, Context, Requirements,
+          llvm::index_sequence_for<RequirementTypes...>());
     }
 
-  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));
+    bool hasSelectionRequirement() override {
+      return internal::HasBaseOf<SourceSelectionRequirement,
+                                 RequirementTypes...>::value;
     }
 
-  FunctionType Function;
+  private:
     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...);
-};
+  return llvm::make_unique<Rule>(std::make_tuple(Requirements...));
+}
 
-} // end namespace internal
-} // end namespace refactoring_action_rules
 } // end namespace tooling
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULES_INTERNAL_H

cfe/trunk/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h

 
 private:
   void defaultResultHandler() {
     handleError(llvm::make_error<llvm::StringError>(
         "unsupported refactoring result", llvm::inconvertibleErrorCode()));
   }
 };
 
-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
 
 #endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RESULT_CONSUMER_H

cfe/trunk/include/clang/Tooling/Refactoring/SourceSelectionConstraints.h

-//===--- 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

cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS Support)
 
 add_clang_library(clangToolingRefactor
   ASTSelection.cpp
   AtomicChange.cpp
   RefactoringActions.cpp
   Rename/RenamingAction.cpp
   Rename/SymbolOccurrences.cpp
   Rename/USRFinder.cpp
   Rename/USRFindingAction.cpp
   Rename/USRLocFinder.cpp
-  SourceSelectionConstraints.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangFormat
   clangIndex
   clangLex
   clangToolingCore
   )

cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp

 #include "clang/Basic/FileManager.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
 #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"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/STLExtras.h"
 #include <string>
 #include <vector>
 
 using namespace llvm;
 
 namespace clang {
 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());
     }
-
-  /// 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;
+    return getCanonicalSymbolDeclaration(ND);
   }
+};
 
-private:
-  static Expected<AtomicChanges>
-  renameOccurrences(const RefactoringRuleContext &Context,
-                    const NamedDecl *ND) {
+class OccurrenceFinder final : public FindSymbolOccurrencesRefactoringRule {
+public:
+  OccurrenceFinder(const NamedDecl *ND) : ND(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 {
+  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
 
 std::unique_ptr<RefactoringAction> createLocalRenameAction() {
   return llvm::make_unique<LocalRename>();
 }
 

cfe/trunk/lib/Tooling/Refactoring/SourceSelectionConstraints.cpp

-//===--- 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);
-}

cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp

 #include "clang/Tooling/Refactoring/RefactoringActionRules.h"
 #include "clang/Tooling/Refactoring/Rename/SymbolName.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/Errc.h"
 #include "gtest/gtest.h"
 
 using namespace clang;
 using namespace tooling;
-using namespace refactoring_action_rules;
 
 namespace {
 
 class RefactoringActionRulesTest : public ::testing::Test {
 protected:
   void SetUp() override {
     Context.Sources.setMainFileID(
         Context.createInMemoryFile("input.cpp", DefaultCode));
createReplacements(const std::unique_ptr<RefactoringActionRule> &Rule,
   };
 
   Consumer C;
   Rule->invoke(C, Context);
   return std::move(*C.Result);
 }
 
 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);
+  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 selection::Requirement {
+
+  class SelectionRequirement : public SourceRangeSelectionRequirement {
   public:
-    std::pair<selection::SourceSelectionRange, int>
-    evaluateSelection(const RefactoringRuleContext &,
-                      selection::SourceSelectionRange Selection) const {
-      return std::make_pair(Selection, 20);
+    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.
   {
     RefactoringRuleContext RefContext(Context.Sources);
     SourceLocation Cursor =
         Context.Sources.getLocForStartOfFile(Context.Sources.getMainFileID())
             .getLocWithOffset(10);
     RefContext.setSelectionRange({Cursor, Cursor});
// valid error.
     Expected<AtomicChanges> ErrorOrResult =
         createReplacements(Rule, RefContext);
 
     ASSERT_TRUE(!ErrorOrResult);
     std::string Message;
     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> {
+  class ErrorRule : public SourceChangeRefactoringRule {
+  public:
+    ErrorRule(SourceRange R) {}
+    Expected<AtomicChanges> createSourceReplacements(RefactoringRuleContext &) {
       return llvm::make_error<llvm::StringError>(
           "Error", llvm::make_error_code(llvm::errc::invalid_argument));
+    }
   };
-  auto Rule = createRefactoringRule(
-      Func, requiredSelection(
-                selection::identity<selection::SourceSelectionRange>()));
 
+  auto Rule =
+      createRefactoringActionRule<ErrorRule>(SourceRangeSelectionRequirement());
   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 {
-  public:
-    Expected<Optional<int>>
-    evaluateSelection(const RefactoringRuleContext &,
-                      selection::SourceSelectionRange Selection) const {
-      return llvm::make_error<llvm::StringError>(
-          "bad selection", 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()));
-
-  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, "bad selection");
-}
-
 Optional<SymbolOccurrences> findOccurrences(RefactoringActionRule &Rule,
                                             RefactoringRuleContext &Context) {
   class Consumer final : public RefactoringResultConsumer {
     void handleError(llvm::Error) override {}
     void handle(SymbolOccurrences Occurrences) override {
       Result = std::move(Occurrences);
     }
     void handle(AtomicChanges Changes) override {
       RefactoringResultConsumer::handle(std::move(Changes));
     }
 
   public:
     Optional<SymbolOccurrences> Result;
   };
 
   Consumer C;
   Rule.invoke(C, Context);
   return std::move(C.Result);
 }
 
 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 {
+      Occurrences.push_back(SymbolOccurrence(SymbolName("test"),
+                                             SymbolOccurrence::MatchingSymbol,
+                                             Selection.getBegin()));
+      return Occurrences;
+    }
+  };
+
+  auto Rule = createRefactoringActionRule<FindOccurrences>(
+      SourceRangeSelectionRequirement());
 
   RefactoringRuleContext RefContext(Context.Sources);
   SourceLocation Cursor =
       Context.Sources.getLocForStartOfFile(Context.Sources.getMainFileID());
   RefContext.setSelectionRange({Cursor, Cursor});
   Optional<SymbolOccurrences> Result = findOccurrences(*Rule, RefContext);
 
   ASSERT_FALSE(!Result);