Recommit r315087 "[refactor] add support for refactoring options"

The recommit fixes a UB bug that occurred only on a small number of bots.

Original message:

This commit adds initial support for refactoring options. One can now use
optional and required std::string options.

This commit also adds a NewNameOption for the local-rename refactoring action to
allow rename to work with custom names.

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

llvm-svn: 315661

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

@@ -16,6 +16,7 @@
 namespace clang {
 namespace tooling {
 
+class RefactoringOptionVisitor;
 class RefactoringResultConsumer;
 class RefactoringRuleContext;
 
@@ -43,6 +44,14 @@ 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;
+
+  /// Traverses each refactoring option used by the rule and invokes the
+  /// \c visit callback in the consumer for each option.
+  ///
+  /// Options are visited in the order of use, e.g. if a rule has two
+  /// requirements that use options, the options from the first requirement
+  /// are visited before the options in the second requirement.
+  virtual void visitRefactoringOptions(RefactoringOptionVisitor &Visitor) = 0;
 };
 
 } // end namespace tooling

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

@@ -11,6 +11,7 @@
 #define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_REQUIREMENTS_H
 
 #include "clang/Basic/LLVM.h"
+#include "clang/Tooling/Refactoring/RefactoringOption.h"
 #include "clang/Tooling/Refactoring/RefactoringRuleContext.h"
 #include "llvm/Support/Error.h"
 #include <type_traits>
@@ -53,6 +54,45 @@ public:
   }
 };
 
+/// A base class for any requirement that requires some refactoring options.
+class RefactoringOptionsRequirement : public RefactoringActionRuleRequirement {
+public:
+  virtual ~RefactoringOptionsRequirement() {}
+
+  /// Returns the set of refactoring options that are used when evaluating this
+  /// requirement.
+  virtual ArrayRef<std::shared_ptr<RefactoringOption>>
+  getRefactoringOptions() const = 0;
+};
+
+/// A requirement that evaluates to the value of the given \c OptionType when
+/// the \c OptionType is a required option. When the \c OptionType is an
+/// optional option, the requirement will evaluate to \c None if the option is
+/// not specified or to an appropriate value otherwise.
+template <typename OptionType>
+class OptionRequirement : public RefactoringOptionsRequirement {
+public:
+  OptionRequirement() : Opt(createRefactoringOption<OptionType>()) {}
+
+  ArrayRef<std::shared_ptr<RefactoringOption>>
+  getRefactoringOptions() const final override {
+    return Opt;
+  }
+
+  Expected<typename OptionType::ValueType>
+  evaluate(RefactoringRuleContext &) const {
+    return static_cast<OptionType *>(Opt.get())->getValue();
+  }
+
+private:
+  /// The partially-owned option.
+  ///
+  /// The ownership of the option is shared among the different requirements
+  /// because the same option can be used by multiple rules in one refactoring
+  /// action.
+  std::shared_ptr<RefactoringOption> Opt;
+};
+
 } // end namespace tooling
 } // end namespace clang
 

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

@@ -24,12 +24,23 @@ namespace internal {
 
 inline llvm::Error findError() { return llvm::Error::success(); }
 
+inline void ignoreError() {}
+
+template <typename FirstT, typename... RestT>
+void ignoreError(Expected<FirstT> &First, Expected<RestT> &... Rest) {
+  if (!First)
+    llvm::consumeError(First.takeError());
+  ignoreError(Rest...);
+}
+
 /// 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)
+  if (!First) {
+    ignoreError(Rest...);
     return First.takeError();
+  }
   return findError(Rest...);
 }
 
@@ -49,6 +60,34 @@ void invokeRuleAfterValidatingRequirements(
   RuleType((*std::get<Is>(Values))...).invoke(Consumer, Context);
 }
 
+inline void visitRefactoringOptionsImpl(RefactoringOptionVisitor &) {}
+
+/// Scans the list of requirements in a rule and visits all the refactoring
+/// options that are used by all the requirements.
+template <typename FirstT, typename... RestT>
+void visitRefactoringOptionsImpl(RefactoringOptionVisitor &Visitor,
+                                 const FirstT &First, const RestT &... Rest) {
+  struct OptionGatherer {
+    RefactoringOptionVisitor &Visitor;
+
+    void operator()(const RefactoringOptionsRequirement &Requirement) {
+      for (const auto &Option : Requirement.getRefactoringOptions())
+        Option->passToVisitor(Visitor);
+    }
+    void operator()(const RefactoringActionRuleRequirement &) {}
+  };
+  (OptionGatherer{Visitor})(First);
+  return visitRefactoringOptionsImpl(Visitor, Rest...);
+}
+
+template <typename... RequirementTypes, size_t... Is>
+void visitRefactoringOptions(
+    RefactoringOptionVisitor &Visitor,
+    const std::tuple<RequirementTypes...> &Requirements,
+    llvm::index_sequence<Is...>) {
+  visitRefactoringOptionsImpl(Visitor, std::get<Is>(Requirements)...);
+}
+
 /// 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>
@@ -97,6 +136,12 @@ createRefactoringActionRule(const RequirementTypes &... Requirements) {
                                  RequirementTypes...>::value;
     }
 
+    void visitRefactoringOptions(RefactoringOptionVisitor &Visitor) override {
+      internal::visitRefactoringOptions(
+          Visitor, Requirements,
+          llvm::index_sequence_for<RequirementTypes...>());
+    }
+
   private:
     std::tuple<RequirementTypes...> Requirements;
   };

clang/include/clang/Tooling/Refactoring/RefactoringOption.h

@@ -0,0 +1,64 @@
+//===--- RefactoringOption.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_OPTION_H
+#define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_OPTION_H
+
+#include "clang/Basic/LLVM.h"
+#include <memory>
+#include <type_traits>
+
+namespace clang {
+namespace tooling {
+
+class RefactoringOptionVisitor;
+
+/// A refactoring option is an interface that describes a value that
+/// has an impact on the outcome of a refactoring.
+///
+/// Refactoring options can be specified using command-line arguments when
+/// the clang-refactor tool is used.
+class RefactoringOption {
+public:
+  virtual ~RefactoringOption() {}
+
+  /// Returns the name of the refactoring option.
+  ///
+  /// Each refactoring option must have a unique name.
+  virtual StringRef getName() const = 0;
+
+  virtual StringRef getDescription() const = 0;
+
+  /// True when this option must be specified before invoking the refactoring
+  /// action.
+  virtual bool isRequired() const = 0;
+
+  /// Invokes the \c visit method in the option consumer that's appropriate
+  /// for the option's value type.
+  ///
+  /// For example, if the option stores a string value, this method will
+  /// invoke the \c visit method with a reference to an std::string value.
+  virtual void passToVisitor(RefactoringOptionVisitor &Visitor) = 0;
+};
+
+/// Constructs a refactoring option of the given type.
+///
+/// The ownership of options is shared among requirements that use it because
+/// one option can be used by multiple rules in a refactoring action.
+template <typename OptionType>
+std::shared_ptr<OptionType> createRefactoringOption() {
+  static_assert(std::is_base_of<RefactoringOption, OptionType>::value,
+                "invalid option type");
+  return std::make_shared<OptionType>();
+}
+
+} // end namespace tooling
+} // end namespace clang
+
+#endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_OPTION_H

clang/include/clang/Tooling/Refactoring/RefactoringOptionVisitor.h

@@ -0,0 +1,62 @@
+//===--- RefactoringOptionVisitor.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_OPTION_VISITOR_H
+#define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_OPTION_VISITOR_H
+
+#include "clang/Basic/LLVM.h"
+#include <type_traits>
+
+namespace clang {
+namespace tooling {
+
+class RefactoringOption;
+
+/// An interface that declares functions that handle different refactoring
+/// option types.
+///
+/// A valid refactoring option type must have a corresponding \c visit
+/// declaration in this interface.
+class RefactoringOptionVisitor {
+public:
+  virtual ~RefactoringOptionVisitor() {}
+
+  virtual void visit(const RefactoringOption &Opt,
+                     Optional<std::string> &Value) = 0;
+};
+
+namespace traits {
+namespace internal {
+
+template <typename T> struct HasHandle {
+private:
+  template <typename ClassT>
+  static auto check(ClassT *) -> typename std::is_same<
+      decltype(std::declval<RefactoringOptionVisitor>().visit(
+          std::declval<RefactoringOption>(), *std::declval<Optional<T> *>())),
+      void>::type;
+
+  template <typename> static std::false_type check(...);
+
+public:
+  using Type = decltype(check<RefactoringOptionVisitor>(nullptr));
+};
+
+} // end namespace internal
+
+/// A type trait that returns true iff the given type is a type that can be
+/// stored in a refactoring option.
+template <typename T>
+struct IsValidOptionType : internal::HasHandle<T>::Type {};
+
+} // end namespace traits
+} // end namespace tooling
+} // end namespace clang
+
+#endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_OPTION_VISITOR_H

clang/include/clang/Tooling/Refactoring/RefactoringOptions.h

@@ -0,0 +1,58 @@
+//===--- RefactoringOptions.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_OPTIONS_H
+#define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_OPTIONS_H
+
+#include "clang/Basic/LLVM.h"
+#include "clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h"
+#include "clang/Tooling/Refactoring/RefactoringOption.h"
+#include "clang/Tooling/Refactoring/RefactoringOptionVisitor.h"
+#include "llvm/Support/Error.h"
+#include <type_traits>
+
+namespace clang {
+namespace tooling {
+
+/// A refactoring option that stores a value of type \c T.
+template <typename T, typename = typename std::enable_if<
+                          traits::IsValidOptionType<T>::value>::type>
+class OptionalRefactoringOption : public RefactoringOption {
+public:
+  void passToVisitor(RefactoringOptionVisitor &Visitor) final override {
+    Visitor.visit(*this, Value);
+  }
+
+  bool isRequired() const override { return false; }
+
+  using ValueType = Optional<T>;
+
+  const ValueType &getValue() const { return Value; }
+
+protected:
+  Optional<T> Value;
+};
+
+/// A required refactoring option that stores a value of type \c T.
+template <typename T, typename = typename std::enable_if<
+                          traits::IsValidOptionType<T>::value>::type>
+class RequiredRefactoringOption : public OptionalRefactoringOption<T> {
+public:
+  using ValueType = T;
+
+  const ValueType &getValue() const {
+    return *OptionalRefactoringOption<T>::Value;
+  }
+  bool isRequired() const final override { return true; }
+};
+
+} // end namespace tooling
+} // end namespace clang
+
+#endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_OPTIONS_H

clang/include/clang/Tooling/Refactoring/Rename/RenamingAction.h

@@ -17,6 +17,7 @@
 
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Refactoring/AtomicChange.h"
+#include "clang/Tooling/Refactoring/RefactoringOptions.h"
 #include "clang/Tooling/Refactoring/Rename/SymbolOccurrences.h"
 #include "llvm/Support/Error.h"
 
@@ -45,12 +46,19 @@ private:
   bool PrintLocations;
 };
 
+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";
+  }
+};
+
 /// Returns source replacements that correspond to the rename of the given
 /// symbol occurrences.
 llvm::Expected<std::vector<AtomicChange>>
 createRenameReplacements(const SymbolOccurrences &Occurrences,
-                         const SourceManager &SM,
-                         ArrayRef<StringRef> NewNameStrings);
+                         const SourceManager &SM, const SymbolName &NewName);
 
 /// Rename all symbols identified by the given USRs.
 class QualifiedRenamingAction {

clang/lib/Tooling/Refactoring/Rename/RenamingAction.cpp

@@ -23,6 +23,8 @@
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Refactoring/RefactoringAction.h"
+#include "clang/Tooling/Refactoring/RefactoringOptions.h"
+#include "clang/Tooling/Refactoring/Rename/SymbolName.h"
 #include "clang/Tooling/Refactoring/Rename/USRFinder.h"
 #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
 #include "clang/Tooling/Refactoring/Rename/USRLocFinder.h"
@@ -75,7 +77,8 @@ private:
 
 class RenameOccurrences final : public SourceChangeRefactoringRule {
 public:
-  RenameOccurrences(const NamedDecl *ND) : Finder(ND) {}
+  RenameOccurrences(const NamedDecl *ND, std::string NewName)
+      : Finder(ND), NewName(NewName) {}
 
   Expected<AtomicChanges>
   createSourceReplacements(RefactoringRuleContext &Context) {
@@ -83,15 +86,15 @@ public:
         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";
+    // FIXME: Verify that the new name is valid.
+    SymbolName Name(NewName);
     return createRenameReplacements(
-        *Occurrences, Context.getASTContext().getSourceManager(), NewName);
+        *Occurrences, Context.getASTContext().getSourceManager(), Name);
   }
 
 private:
   OccurrenceFinder Finder;
+  std::string NewName;
 };
 
 class LocalRename final : public RefactoringAction {
@@ -107,7 +110,7 @@ public:
   RefactoringActionRules createActionRules() const override {
     RefactoringActionRules Rules;
     Rules.push_back(createRefactoringActionRule<RenameOccurrences>(
-        SymbolSelectionRequirement()));
+        SymbolSelectionRequirement(), OptionRequirement<NewNameOption>()));
     return Rules;
   }
 };
@@ -120,19 +123,18 @@ std::unique_ptr<RefactoringAction> createLocalRenameAction() {
 
 Expected<std::vector<AtomicChange>>
 createRenameReplacements(const SymbolOccurrences &Occurrences,
-                         const SourceManager &SM,
-                         ArrayRef<StringRef> NewNameStrings) {
+                         const SourceManager &SM, const SymbolName &NewName) {
   // FIXME: A true local rename can use just one AtomicChange.
   std::vector<AtomicChange> Changes;
   for (const auto &Occurrence : Occurrences) {
     ArrayRef<SourceRange> Ranges = Occurrence.getNameRanges();
-    assert(NewNameStrings.size() == Ranges.size() &&
+    assert(NewName.getNamePieces().size() == Ranges.size() &&
            "Mismatching number of ranges and name pieces");
     AtomicChange Change(SM, Ranges[0].getBegin());
     for (const auto &Range : llvm::enumerate(Ranges)) {
       auto Error =
           Change.replace(SM, CharSourceRange::getCharRange(Range.value()),
-                         NewNameStrings[Range.index()]);
+                         NewName.getNamePieces()[Range.index()]);
       if (Error)
         return std::move(Error);
     }
@@ -196,7 +198,7 @@ public:
     }
     // FIXME: Support multi-piece names.
     // FIXME: better error handling (propagate error out).
-    StringRef NewNameRef = NewName;
+    SymbolName NewNameRef(NewName);
     Expected<std::vector<AtomicChange>> Change =
         createRenameReplacements(Occurrences, SourceMgr, NewNameRef);
     if (!Change) {

clang/test/Refactor/LocalRename/Field.cpp

@@ -1,4 +1,4 @@
-// RUN: clang-refactor local-rename -selection=test:%s %s -- | FileCheck %s
+// RUN: clang-refactor local-rename -selection=test:%s -new-name=Bar %s -- | FileCheck %s
 
 class Baz {
   int /*range=*/Foo; // CHECK: int /*range=*/Bar;

clang/test/Refactor/tool-test-support.c

@@ -1,4 +1,4 @@
-// RUN: clang-refactor local-rename -selection=test:%s -v %s -- | FileCheck %s
+// RUN: clang-refactor local-rename -selection=test:%s -new-name=test -v %s -- | FileCheck %s
 
 /*range=*/int test;
 
@@ -11,12 +11,12 @@
 /*range named =+0*/int test5;
 
 // CHECK: Test selection group '':
-// CHECK-NEXT:   90-90
-// CHECK-NEXT:   143-143
-// CHECK-NEXT:   182-182
+// CHECK-NEXT:   105-105
+// CHECK-NEXT:   158-158
+// CHECK-NEXT:   197-197
 // CHECK-NEXT: Test selection group 'named':
-// CHECK-NEXT:   117-117
-// CHECK-NEXT:   203-203
+// CHECK-NEXT:   132-132
+// CHECK-NEXT:   218-218
 
 // The following invocations are in the default group:
 

clang/tools/clang-refactor/ClangRefactor.cpp

@@ -18,6 +18,7 @@
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Refactoring/RefactoringAction.h"
+#include "clang/Tooling/Refactoring/RefactoringOptions.h"
 #include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
@@ -32,10 +33,10 @@ namespace cl = llvm::cl;
 
 namespace opts {
 
-static cl::OptionCategory CommonRefactorOptions("Common refactoring options");
+static cl::OptionCategory CommonRefactorOptions("Refactoring options");
 
 static cl::opt<bool> Verbose("v", cl::desc("Use verbose output"),
-                             cl::cat(CommonRefactorOptions),
+                             cl::cat(cl::GeneralCategory),
                              cl::sub(*cl::AllSubCommands));
 } // end namespace opts
 
@@ -116,6 +117,92 @@ SourceSelectionArgument::fromString(StringRef Value) {
   return nullptr;
 }
 
+/// A container that stores the command-line options used by a single
+/// refactoring option.
+class RefactoringActionCommandLineOptions {
+public:
+  void addStringOption(const RefactoringOption &Option,
+                       std::unique_ptr<cl::opt<std::string>> CLOption) {
+    StringOptions[&Option] = std::move(CLOption);
+  }
+
+  const cl::opt<std::string> &
+  getStringOption(const RefactoringOption &Opt) const {
+    auto It = StringOptions.find(&Opt);
+    return *It->second;
+  }
+
+private:
+  llvm::DenseMap<const RefactoringOption *,
+                 std::unique_ptr<cl::opt<std::string>>>
+      StringOptions;
+};
+
+/// Passes the command-line option values to the options used by a single
+/// refactoring action rule.
+class CommandLineRefactoringOptionVisitor final
+    : public RefactoringOptionVisitor {
+public:
+  CommandLineRefactoringOptionVisitor(
+      const RefactoringActionCommandLineOptions &Options)
+      : Options(Options) {}
+
+  void visit(const RefactoringOption &Opt,
+             Optional<std::string> &Value) override {
+    const cl::opt<std::string> &CLOpt = Options.getStringOption(Opt);
+    if (!CLOpt.getValue().empty()) {
+      Value = CLOpt.getValue();
+      return;
+    }
+    Value = None;
+    if (Opt.isRequired())
+      MissingRequiredOptions.push_back(&Opt);
+  }
+
+  ArrayRef<const RefactoringOption *> getMissingRequiredOptions() const {
+    return MissingRequiredOptions;
+  }
+
+private:
+  llvm::SmallVector<const RefactoringOption *, 4> MissingRequiredOptions;
+  const RefactoringActionCommandLineOptions &Options;
+};
+
+/// Creates the refactoring options used by all the rules in a single
+/// refactoring action.
+class CommandLineRefactoringOptionCreator final
+    : public RefactoringOptionVisitor {
+public:
+  CommandLineRefactoringOptionCreator(
+      cl::OptionCategory &Category, cl::SubCommand &Subcommand,
+      RefactoringActionCommandLineOptions &Options)
+      : Category(Category), Subcommand(Subcommand), Options(Options) {}
+
+  void visit(const RefactoringOption &Opt, Optional<std::string> &) override {
+    if (Visited.insert(&Opt).second)
+      Options.addStringOption(Opt, create<std::string>(Opt));
+  }
+
+private:
+  template <typename T>
+  std::unique_ptr<cl::opt<T>> create(const RefactoringOption &Opt) {
+    if (!OptionNames.insert(Opt.getName()).second)
+      llvm::report_fatal_error("Multiple identical refactoring options "
+                               "specified for one refactoring action");
+    // FIXME: cl::Required can be specified when this option is present
+    // in all rules in an action.
+    return llvm::make_unique<cl::opt<T>>(
+        Opt.getName(), cl::desc(Opt.getDescription()), cl::Optional,
+        cl::cat(Category), cl::sub(Subcommand));
+  }
+
+  llvm::SmallPtrSet<const RefactoringOption *, 8> Visited;
+  llvm::StringSet<> OptionNames;
+  cl::OptionCategory &Category;
+  cl::SubCommand &Subcommand;
+  RefactoringActionCommandLineOptions &Options;
+};
+
 /// A subcommand that corresponds to individual refactoring action.
 class RefactoringActionSubcommand : public cl::SubCommand {
 public:
@@ -138,6 +225,12 @@ public:
                    "<file>:<line>:<column>)"),
           cl::cat(Category), cl::sub(*this));
     }
+    // Create the refactoring options.
+    for (const auto &Rule : this->ActionRules) {
+      CommandLineRefactoringOptionCreator OptionCreator(Category, *this,
+                                                        Options);
+      Rule->visitRefactoringOptions(OptionCreator);
+    }
   }
 
   ~RefactoringActionSubcommand() { unregisterSubCommand(); }
@@ -160,11 +253,17 @@ public:
     assert(Selection && "selection not supported!");
     return ParsedSelection.get();
   }
+
+  const RefactoringActionCommandLineOptions &getOptions() const {
+    return Options;
+  }
+
 private:
   std::unique_ptr<RefactoringAction> Action;
   RefactoringActionRules ActionRules;
   std::unique_ptr<cl::opt<std::string>> Selection;
   std::unique_ptr<SourceSelectionArgument> ParsedSelection;
+  RefactoringActionCommandLineOptions Options;
 };
 
 class ClangRefactorConsumer : public RefactoringResultConsumer {
@@ -262,14 +361,22 @@ public:
 
     bool HasSelection = false;
     for (const auto &Rule : Subcommand.getActionRules()) {
+      bool SelectionMatches = true;
       if (Rule->hasSelectionRequirement()) {
         HasSelection = true;
-        if (Subcommand.getSelection())
-          MatchingRules.push_back(Rule.get());
-        else
+        if (!Subcommand.getSelection()) {
           MissingOptions.insert("selection");
+          SelectionMatches = false;
+        }
       }
-      // FIXME (Alex L): Support custom options.
+      CommandLineRefactoringOptionVisitor Visitor(Subcommand.getOptions());
+      Rule->visitRefactoringOptions(Visitor);
+      if (SelectionMatches && Visitor.getMissingRequiredOptions().empty()) {
+        MatchingRules.push_back(Rule.get());
+        continue;
+      }
+      for (const RefactoringOption *Opt : Visitor.getMissingRequiredOptions())
+        MissingOptions.insert(Opt->getName());
     }
     if (MatchingRules.empty()) {
       llvm::errs() << "error: '" << Subcommand.getName()
@@ -326,7 +433,7 @@ int main(int argc, const char **argv) {
   ClangRefactorTool Tool;
 
   CommonOptionsParser Options(
-      argc, argv, opts::CommonRefactorOptions, cl::ZeroOrMore,
+      argc, argv, cl::GeneralCategory, cl::ZeroOrMore,
       "Clang-based refactoring tool for C, C++ and Objective-C");
 
   // Figure out which action is specified by the user. The user must specify