[refactor] add support for refactoring options

[arphaman]

Summary

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

The options are implemented in the following manner:

• A base interface RefactoringOption declares methods that describe the option's name, description and whether or not it's required. It also declares a method that takes in a RefactoringOptionConsumer that will visit the option and its typed value.
• A visitor interface RefactoringOptionConsumer declares the handle methods that provide an overload for each valid option type (std::string, etc).
• Classes OptionalRequiredOption and RequiredRefactoringOption partially implement the RefactoringOption interface, store the value and provide a getter for the value. The individual options then just have to implement getName and getDescription.
• The RefactoringOptionsRequirement is a base requirement class the declares a method that can returns all of the options used in a single requirement.
• The OptionRequirement class is a helper class that implements a refactoring action rule requirement for a particular option.

clang-refactor now creates command-line arguments for each unique option used by all rules in an action.

This patch also adds a NewNameOption for the local-rename refactoring action to ensure that options work.

[ioeric]

Sorry for the delay (most of us are OOO this week).

include/clang/Tooling/Refactoring/RefactoringActionRule.h

48 ↗

(On Diff #115248)

Please document the behavior here. What options are visited? How is the consumer used?

include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h

62 ↗

(On Diff #115248)

that a used?

78 ↗

(On Diff #115248)

Why return a vector instead of a single value if there is only one element?

83 ↗

(On Diff #115248)

This could use some comment, e.g. what is getValue expected to do?

88 ↗

(On Diff #115248)

Please explain why shared_ptr is needed here.

include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h

56 ↗

(On Diff #115248)

nit: no braces around one liner in LLVM style.

68 ↗

(On Diff #115248)

Please elaborate on what's valid or invalid.

74 ↗

(On Diff #115248)

Why shared_ptr? Would unique_ptr work?

include/clang/Tooling/Refactoring/RefactoringOption.h

42 ↗

(On Diff #115248)

This could also use some comment. E.g. what is passed/cosumed?

47 ↗

(On Diff #115248)

Again, why shared_ptr instead of unique_ptr?

include/clang/Tooling/Refactoring/RefactoringOptionConsumer.h

26 ↗

(On Diff #115248)

IIUC, you are using visitor pattern to "handle" options. If so, consider s/consumer/visitor and s/handle/visit, just to be clearer.

tools/clang-refactor/ClangRefactor.cpp

372 ↗

(On Diff #115248)

Wondering if it is sensible to make selection also a general option so that we don't need to special-case selections.

[arphaman]

Address review comments

include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h

78 ↗

(On Diff #115248)

To support more complex requirements that need multiple options.

83 ↗

(On Diff #115248)

I've simplified this by using a typealias in the option itself.

88 ↗

(On Diff #115248)

The same option can be used by multiple rules, hence options can be owned by multiple requirements in different rules at once. I documented this.

include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h

68 ↗

(On Diff #115248)

Oops, copy-pasted comment. I fixed it up.

74 ↗

(On Diff #115248)

I refactored this to change the redundant ownership here.

tools/clang-refactor/ClangRefactor.cpp

372 ↗

(On Diff #115248)

I think right now it's tricky because of the testing support. However, I think that it would be better to use a similar mechanism for selection option as well. I'll think about how we can reconcile the two approaches and will hopefully come up with a patch for it.

[ioeric]

Looks good with some nits.

include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h

73 ↗

(On Diff #117124)

nit: OptionRequirement sounds more general than the base class RefactoringOptionsRequirement.

include/clang/Tooling/Refactoring/RefactoringOption.h

47 ↗

(On Diff #117124)

nit: s/Consumer/Visitor/

tools/clang-refactor/ClangRefactor.cpp

129 ↗

(On Diff #117124)

We use templated method for adding options above but type-specific interfaces (e.g. getStringOption) for getting options. I think we should make either both templated or separate interfaces. I am a bit inclined to separate interfaces for each type for clarity.

149 ↗

(On Diff #117124)

Visitor?

[arphaman]

include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h

73 ↗

(On Diff #117124)

I couldn't really think of a better name, sorry.

[hokein]

Sorry for the delay. I saw you have reverted this commit somehow. A post commit.

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

113

Thought it a bit more: it requires all of the requirements are satisfied, I think we need to support "one-of" option. For example, we have two option "-a" and "-b", only one of them is allowed to be present at the same time.

[arphaman]

In D37856#894638, @hokein wrote:

Sorry for the delay. I saw you have reverted this commit somehow. A post commit.

I had some issues with ppc64/s390x bots for some reason, so I had to revert. I'm still trying to investigate what went wrong there, it looks like ASAN/UBSAN are not catching anything though.

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

113

That should be straightforward enough to implement, either with a custom class or with a built-in requirement class. I'll probably add a builtin one when the need comes up.

[hokein]

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

113

Thanks. I'm asking it because I plan to add the -qualified-name option to the clang-refactor rename subtool. And obviously "-qualified-name" can not be present with "-selection".

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

 #define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_H
 
 #include "clang/Basic/LLVM.h"
 #include <vector>
 
 namespace clang {
 namespace tooling {
 
+class RefactoringOptionVisitor;
 class RefactoringResultConsumer;
 class RefactoringRuleContext;
 
 /// A common refactoring action rule interface that defines the 'invoke'
 /// function that performs the refactoring operation (either fully or
 /// partially).
 class RefactoringActionRuleBase {
 public:

 /// 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;
+
+  /// 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
 } // 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/Basic/LLVM.h"
+#include "clang/Tooling/Refactoring/RefactoringOption.h"
 #include "clang/Tooling/Refactoring/RefactoringRuleContext.h"
 #include "llvm/Support/Error.h"
 #include <type_traits>
 
 namespace clang {
 namespace tooling {
 
 /// A refactoring action rule requirement determines when a refactoring action
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());
   }
 };
 
+/// 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 static_cast<const std::shared_ptr<RefactoringOption> &>(Opt);
+  }
+
+  Expected<typename OptionType::ValueType>
+  evaluate(RefactoringRuleContext &) const {
+    return Opt->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<OptionType> Opt;
+};
+
 } // end namespace tooling
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_REQUIREMENTS_H

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

 #include <type_traits>
 
 namespace clang {
 namespace tooling {
 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...);
 }
 
 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);
 }
 
+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>
 struct HasBaseOf : std::conditional<HasBaseOf<Base, First>::value ||
                                         HasBaseOf<Base, Rest...>::value,
                                     std::true_type, std::false_type>::type {};
 
 template <typename Base, typename T>
void invoke(RefactoringResultConsumer &Consumer,
           llvm::index_sequence_for<RequirementTypes...>());
     }
 
     bool hasSelectionRequirement() override {
       return internal::HasBaseOf<SourceSelectionRequirement,
                                  RequirementTypes...>::value;
     }
 
+    void visitRefactoringOptions(RefactoringOptionVisitor &Visitor) override {
+      internal::visitRefactoringOptions(
+          Visitor, Requirements,
+          llvm::index_sequence_for<RequirementTypes...>());
+    }
+
   private:
     std::tuple<RequirementTypes...> Requirements;
   };
 
   return llvm::make_unique<Rule>(std::make_tuple(Requirements...));
 }
 
 } // end namespace tooling
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULES_INTERNAL_H

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

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

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

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

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

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

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

 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLING_REFACTOR_RENAME_RENAMING_ACTION_H
 #define LLVM_CLANG_TOOLING_REFACTOR_RENAME_RENAMING_ACTION_H
 
 #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"
 
 namespace clang {
 class ASTConsumer;
 class CompilerInstance;
 
 namespace tooling {

 
 private:
   const std::vector<std::string> &NewNames, &PrevNames;
   const std::vector<std::vector<std::string>> &USRList;
   std::map<std::string, tooling::Replacements> &FileToReplaces;
   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 {
 public:
   QualifiedRenamingAction(
       const std::vector<std::string> &NewNames,
       const std::vector<std::vector<std::string>> &USRList,
       std::map<std::string, tooling::Replacements> &FileToReplaces)

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/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"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/STLExtras.h"
 #include <string>
 #include <vector>
 
public:
   }
 
 private:
   const NamedDecl *ND;
 };
 
 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) {
     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";
+    // 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 {
 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()));
+        SymbolSelectionRequirement(), OptionRequirement<NewNameOption>()));
     return Rules;
   }
 };
 
 } // end anonymous namespace
 
 std::unique_ptr<RefactoringAction> createLocalRenameAction() {
   return llvm::make_unique<LocalRename>();
 }
 
 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);
     }
     Changes.push_back(std::move(Change));
   }
   return std::move(Changes);
 }
 
if (PrintLocations) {
                               SourceMgr);
         errs() << "clang-rename: renamed at: " << SourceMgr.getFilename(FullLoc)
                << ":" << FullLoc.getSpellingLineNumber() << ":"
                << FullLoc.getSpellingColumnNumber() << "\n";
       }
     }
     // 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) {
       llvm::errs() << "Failed to create renaming replacements for '" << PrevName
                    << "'! " << llvm::toString(Change.takeError()) << "\n";
       return;
     }
     convertChangesToFileReplacements(*Change, &FileToReplaces);

cfe/trunk/test/Refactor/LocalRename/Field.cpp

-// 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;
 public:
   Baz();
 };
 
 Baz::Baz() : /*range=*/Foo(0) {}  // CHECK: Baz::Baz() : /*range=*/Bar(0) {};

cfe/trunk/test/Refactor/tool-test-support.c

-// 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;
 
 /*range named=*/int test2;
 
 /*range= +1*/int test3;
 
 /* range = +100 */int test4;
 
 /*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:
 
 // CHECK: invoking action 'local-rename':
 // CHECK-NEXT: -selection={{.*}}tool-test-support.c:3:11
 
 // CHECK: invoking action 'local-rename':
 // CHECK-NEXT: -selection={{.*}}tool-test-support.c:7:15

cfe/trunk/tools/clang-refactor/ClangRefactor.cpp

 ///
 //===----------------------------------------------------------------------===//
 
 #include "TestSupport.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #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"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include <string>
 
 using namespace clang;
 using namespace tooling;
 using namespace refactor;
 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
 
 namespace {
 
 /// Stores the parsed `-selection` argument.
 class SourceSelectionArgument {
 public:
SourceSelectionArgument::fromString(StringRef Value) {
   }
   // FIXME: Support true selection ranges.
   llvm::errs() << "error: '-selection' option must be specified using "
                   "<file>:<line>:<column> or "
                   "<file>:<line>:<column>-<line>:<column> format";
   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:
   RefactoringActionSubcommand(std::unique_ptr<RefactoringAction> Action,
                               RefactoringActionRules ActionRules,
                               cl::OptionCategory &Category)
       : SubCommand(Action->getCommand(), Action->getDescription()),
         Action(std::move(Action)), ActionRules(std::move(ActionRules)) {
     // Check if the selection option is supported.
     bool HasSelection = false;
     for (const auto &Rule : this->ActionRules) {
       if ((HasSelection = Rule->hasSelectionRequirement()))
         break;
     }
     if (HasSelection) {
       Selection = llvm::make_unique<cl::opt<std::string>>(
           "selection",
           cl::desc("The selected source range in which the refactoring should "
                    "be initiated (<file>:<line>:<column>-<line>:<column> or "
                    "<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(); }
 
   const RefactoringActionRules &getActionRules() const { return ActionRules; }
 
   /// Parses the command-line arguments that are specific to this rule.
   ///
   /// \returns true on error, false otherwise.
   bool parseArguments() {
     if (Selection) {
       ParsedSelection = SourceSelectionArgument::fromString(*Selection);
       if (!ParsedSelection)
         return true;
     }
     return false;
   }
 
   SourceSelectionArgument *getSelection() const {
     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 {
 public:
   void handleError(llvm::Error Err) {
     llvm::errs() << llvm::toString(std::move(Err)) << "\n";
   }
 
bool invokeAction(RefactoringActionSubcommand &Subcommand,
                     const CompilationDatabase &DB,
                     ArrayRef<std::string> Sources) {
     // Find a set of matching rules.
     SmallVector<RefactoringActionRule *, 4> MatchingRules;
     llvm::StringSet<> MissingOptions;
 
     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;
+        }
+      }
+      CommandLineRefactoringOptionVisitor Visitor(Subcommand.getOptions());
+      Rule->visitRefactoringOptions(Visitor);
+      if (SelectionMatches && Visitor.getMissingRequiredOptions().empty()) {
+        MatchingRules.push_back(Rule.get());
+        continue;
       }
-      // FIXME (Alex L): Support custom options.
+      for (const RefactoringOption *Opt : Visitor.getMissingRequiredOptions())
+        MissingOptions.insert(Opt->getName());
     }
     if (MatchingRules.empty()) {
       llvm::errs() << "error: '" << Subcommand.getName()
                    << "' can't be invoked with the given arguments:\n";
       for (const auto &Opt : MissingOptions)
         llvm::errs() << "  missing '-" << Opt.getKey() << "' option\n";
       return true;
     }

 };
 
 } // end anonymous namespace
 
 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
   // the action using a command-line subcommand, e.g. the invocation
   // `clang-refactor local-rename` corresponds to the `LocalRename` refactoring
   // action. All subcommands must have a unique names. This allows us to figure
   // out which refactoring action should be invoked by looking at the first
   // subcommand that's enabled by LLVM's command-line parser.