From 7b93fc9054c185272b5e4b3c4bc9cd8f512fd9f4 Mon Sep 17 00:00:00 2001 From: Alex Lorenz Date: Mon, 2 Oct 2017 18:42:43 +0000 Subject: [PATCH] [refactor] Simplify the refactoring interface This commit simplifies the interface for the refactoring action rules and the refactoring requirements. It merges the selection constraints and the selection requirements into one class. The refactoring actions rules must now be implemented using subclassing instead of raw function / lambda pointers. This change also removes a bunch of template-based traits and other template definitions that are now redundant. Differential Revision: https://reviews.llvm.org/D37681 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@314704 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Refactoring/RefactoringActionRule.h | 20 +- .../RefactoringActionRuleRequirements.h | 71 +++---- ...efactoringActionRuleRequirementsInternal.h | 104 ---------- .../Refactoring/RefactoringActionRules.h | 114 ++++++----- .../RefactoringActionRulesInternal.h | 178 ++++++++---------- .../Refactoring/RefactoringResultConsumer.h | 23 --- .../Refactoring/SourceSelectionConstraints.h | 114 ----------- lib/Tooling/Refactoring/CMakeLists.txt | 1 - .../Refactoring/Rename/RenamingAction.cpp | 90 +++++---- .../SourceSelectionConstraints.cpp | 23 --- .../Tooling/RefactoringActionRulesTest.cpp | 129 ++++++------- 11 files changed, 308 insertions(+), 559 deletions(-) delete mode 100644 include/clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h delete mode 100644 include/clang/Tooling/Refactoring/SourceSelectionConstraints.h delete mode 100644 lib/Tooling/Refactoring/SourceSelectionConstraints.cpp diff --git a/include/clang/Tooling/Refactoring/RefactoringActionRule.h b/include/clang/Tooling/Refactoring/RefactoringActionRule.h index 1eb6342dfb..c72b37c91d 100644 --- a/include/clang/Tooling/Refactoring/RefactoringActionRule.h +++ b/include/clang/Tooling/Refactoring/RefactoringActionRule.h @@ -19,10 +19,12 @@ namespace tooling { class RefactoringResultConsumer; class RefactoringRuleContext; -/// A common refactoring action rule interface. -class RefactoringActionRule { +/// A common refactoring action rule interface that defines the 'invoke' +/// function that performs the refactoring operation (either fully or +/// partially). +class RefactoringActionRuleBase { public: - virtual ~RefactoringActionRule() {} + virtual ~RefactoringActionRuleBase() {} /// Initiates and performs a specific refactoring action. /// @@ -30,17 +32,19 @@ public: /// consumer to propagate the result of the refactoring action. virtual void invoke(RefactoringResultConsumer &Consumer, RefactoringRuleContext &Context) = 0; +}; +/// A refactoring action rule is a wrapper class around a specific refactoring +/// action rule (SourceChangeRefactoringRule, etc) that, in addition to invoking +/// the action, describes the requirements that determine when the action can be +/// initiated. +class RefactoringActionRule : public RefactoringActionRuleBase { +public: /// Returns true when the rule has a source selection requirement that has /// to be fullfilled before refactoring can be performed. virtual bool hasSelectionRequirement() = 0; }; -/// A set of refactoring action rules that should have unique initiation -/// requirements. -using RefactoringActionRules = - std::vector>; - } // end namespace tooling } // end namespace clang diff --git a/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h b/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h index 4435a9f865..ebfeeda589 100644 --- a/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h +++ b/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h @@ -10,48 +10,49 @@ #ifndef LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_REQUIREMENTS_H #define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_REQUIREMENTS_H -#include "clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h" +#include "clang/Basic/LLVM.h" +#include "clang/Tooling/Refactoring/RefactoringRuleContext.h" #include "llvm/Support/Error.h" #include 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 -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::value>::type - * = nullptr) { - return internal::SourceSelectionRequirement< - typename selection::internal::EvaluateSelectionChecker::ArgType, - typename selection::internal::EvaluateSelectionChecker::ReturnType, - T>(Requirement); -} - -template -void requiredSelection( - const T &, - typename std::enable_if< - !std::is_base_of::value>::type * = nullptr) { - static_assert( - sizeof(T) && false, - "selection requirement must be a class derived from Requirement"); -} - -} // end namespace refactoring_action_rules +/// Subclasses must implement the +/// 'Expected 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 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 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 evaluate(RefactoringRuleContext &Context) const { + if (Context.getSelectionRange().isValid()) + return Context.getSelectionRange(); + // FIXME: Use a diagnostic. + return llvm::make_error( + "refactoring action can't be initiated without a selection", + llvm::inconvertibleErrorCode()); + } +}; + } // end namespace tooling } // end namespace clang diff --git a/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h b/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h deleted file mode 100644 index bbad23f8fb..0000000000 --- a/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h +++ /dev/null @@ -1,104 +0,0 @@ -//===--- RefactoringActionRuleRequirementsInternal.h - --------------------===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// - -#ifndef LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_REQUIREMENTS_INTERNAL_H -#define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_REQUIREMENTS_INTERNAL_H - -#include "clang/Basic/LLVM.h" -#include "clang/Tooling/Refactoring/RefactoringRuleContext.h" -#include "clang/Tooling/Refactoring/SourceSelectionConstraints.h" -#include - -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>, or -/// \c T otherwise. -template struct DropExpectedOptional { using Type = T; }; - -template struct DropExpectedOptional>> { - using Type = T; -}; - -/// The \c requiredSelection refactoring action requirement is represented -/// using this type. -template -struct SourceSelectionRequirement - : std::enable_if::value && - selection::traits::IsRequirement::value, - RequirementBase>::type { - using OutputType = typename DropExpectedOptional::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> which is then unwrapped by the - /// refactoring action rule before passing the value to the refactoring - /// function. - Expected> evaluate(RefactoringRuleContext &Context) { - Optional 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 -struct IsRequirement : std::conditional::value && - IsRequirement::value, - std::true_type, std::false_type>::type { -}; - -template -struct IsRequirement - : std::conditional::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 -struct HasSelectionRequirement - : std::conditional::value || - HasSelectionRequirement::value, - std::true_type, std::false_type>::type {}; - -template -struct HasSelectionRequirement> - : std::true_type {}; - -template struct HasSelectionRequirement : 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 diff --git a/include/clang/Tooling/Refactoring/RefactoringActionRules.h b/include/clang/Tooling/Refactoring/RefactoringActionRules.h index 1ae9953485..33206d9a5d 100644 --- a/include/clang/Tooling/Refactoring/RefactoringActionRules.h +++ b/include/clang/Tooling/Refactoring/RefactoringActionRules.h @@ -16,60 +16,78 @@ namespace clang { namespace tooling { -class RefactoringRuleContext; - -namespace refactoring_action_rules { - -/// Creates a new refactoring action rule that invokes the given function once -/// all of the requirements are satisfied. The values produced during the -/// evaluation of requirements are passed to the given function (in the order of -/// requirements). +/// Creates a new refactoring action rule that constructs and invokes the +/// \c RuleType rule when all of the requirements are satisfied. /// -/// \param RefactoringFunction the function that will perform the refactoring -/// once the requirements are satisfied. The function must return a valid -/// refactoring result type wrapped in an \c Expected type. The following result -/// types are currently supported: +/// This function takes in a list of values whose type derives from +/// \c RefactoringActionRuleRequirement. These values describe the initiation +/// requirements that have to be satisfied by the refactoring engine before +/// the provided action rule can be constructed and invoked. The engine +/// verifies that the requirements are satisfied by evaluating them (using the +/// 'evaluate' member function) and checking that the results don't contain +/// any errors. Once all requirements are satisfied, the provided refactoring +/// rule is constructed by passing in the values returned by the requirements' +/// evaluate functions as arguments to the constructor. The rule is then invoked +/// immediately after construction. /// -/// - AtomicChanges: the refactoring function will be used to create source -/// replacements. +/// The separation of requirements, their evaluation and the invocation of the +/// refactoring action rule allows the refactoring clients to: +/// - Disable refactoring action rules whose requirements are not supported. +/// - Gather the set of options and define a command-line / visual interface +/// that allows users to input these options without ever invoking the +/// action. +template +std::unique_ptr +createRefactoringActionRule(const RequirementTypes &... Requirements); + +/// A set of refactoring action rules that should have unique initiation +/// requirements. +using RefactoringActionRules = + std::vector>; + +/// A type of refactoring action rule that produces source replacements in the +/// form of atomic changes. /// -/// \param Requirements a set of rule requirements that have to be satisfied. -/// Each requirement must be a valid requirement, i.e. the value of -/// \c traits::IsRequirement must be true. The following requirements are -/// currently supported: +/// This action rule is typically used for local refactorings that replace +/// source in a single AST unit. +class SourceChangeRefactoringRule : public RefactoringActionRuleBase { +public: + void invoke(RefactoringResultConsumer &Consumer, + RefactoringRuleContext &Context) final override { + Expected Changes = createSourceReplacements(Context); + if (!Changes) + Consumer.handleError(Changes.takeError()); + else + Consumer.handle(std::move(*Changes)); + } + +private: + virtual Expected + createSourceReplacements(RefactoringRuleContext &Context) = 0; +}; + +/// A type of refactoring action rule that finds a set of symbol occurrences +/// that reference a particular symbol. /// -/// - requiredSelection: The refactoring function won't be invoked unless the -/// given selection requirement is satisfied. -template -std::unique_ptr -createRefactoringRule(Expected (*RefactoringFunction)( - const RefactoringRuleContext &, - typename RequirementTypes::OutputType...), - const RequirementTypes &... Requirements) { - static_assert(tooling::traits::IsValidRefactoringResult::value, - "invalid refactoring result type"); - static_assert(traits::IsRequirement::value, - "invalid refactoring action rule requirement"); - return llvm::make_unique>( - RefactoringFunction, std::make_tuple(Requirements...)); -} +/// This action rule is typically used for an interactive rename that allows +/// users to specify the new name and the set of selected occurrences during +/// the refactoring. +class FindSymbolOccurrencesRefactoringRule : public RefactoringActionRuleBase { +public: + void invoke(RefactoringResultConsumer &Consumer, + RefactoringRuleContext &Context) final override { + Expected Occurrences = findSymbolOccurrences(Context); + if (!Occurrences) + Consumer.handleError(Occurrences.takeError()); + else + Consumer.handle(std::move(*Occurrences)); + } -template < - typename Callable, typename... RequirementTypes, - typename Fn = decltype(&Callable::operator()), - typename ResultType = typename internal::LambdaDeducer::ReturnType, - bool IsNonCapturingLambda = std::is_convertible< - Callable, typename internal::LambdaDeducer::FunctionType>::value, - typename = typename std::enable_if::type> -std::unique_ptr -createRefactoringRule(const Callable &C, - const RequirementTypes &... Requirements) { - typename internal::LambdaDeducer::FunctionType Func = C; - return createRefactoringRule(Func, Requirements...); -} +private: + virtual Expected + findSymbolOccurrences(RefactoringRuleContext &Context) = 0; +}; -} // end namespace refactoring_action_rules } // end namespace tooling } // end namespace clang diff --git a/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h b/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h index e26b92dd64..61db7400ac 100644 --- a/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h +++ b/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h @@ -20,108 +20,90 @@ namespace clang { namespace tooling { -namespace refactoring_action_rules { namespace internal { -/// A specialized refactoring action rule that calls the stored function once -/// all the of the requirements are fullfilled. The values produced during the -/// evaluation of requirements are passed to the stored function. -template -class PlainFunctionRule final : public RefactoringActionRule { -public: - PlainFunctionRule(FunctionType Function, - std::tuple &&Requirements) - : Function(Function), Requirements(std::move(Requirements)) {} - - void invoke(RefactoringResultConsumer &Consumer, - RefactoringRuleContext &Context) override { - return invokeImpl(Consumer, Context, - llvm::index_sequence_for()); - } - - bool hasSelectionRequirement() override { - return traits::HasSelectionRequirement::value; - } - -private: - /// Returns \c T when given \c Expected>, or \c T otherwise. - template - static T &&unwrapRequirementResult(Expected> &&X) { - assert(X && "unexpected diagnostic!"); - return std::move(**X); - } - template static T &&unwrapRequirementResult(T &&X) { - return std::move(X); - } - - /// Scans the tuple and returns a \c PartialDiagnosticAt - /// from the first invalid \c DiagnosticOr value. Returns \c None if all - /// values are valid. - template - static Optional findErrorNone(FirstT &First, RestT &... Rest) { - Optional Result = takeErrorNone(First); - if (Result) - return Result; - return findErrorNone(Rest...); - } - - static Optional findErrorNone() { return None; } - - template static Optional takeErrorNone(T &) { - return None; - } - - template - static Optional takeErrorNone(Expected> &Diag) { - if (!Diag) - return std::move(Diag.takeError()); - if (!*Diag) - return llvm::Error::success(); // Initiation failed without a diagnostic. - return None; - } - - template - void invokeImpl(RefactoringResultConsumer &Consumer, - RefactoringRuleContext &Context, - llvm::index_sequence Seq) { - // Initiate the operation. - auto Values = - std::make_tuple(std::get(Requirements).evaluate(Context)...); - Optional InitiationFailure = - findErrorNone(std::get(Values)...); - if (InitiationFailure) { - llvm::Error Error = std::move(*InitiationFailure); - if (!Error) - // FIXME: Use a diagnostic. - return Consumer.handleError(llvm::make_error( - "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(Values)))...); - if (!Result) - return Consumer.handleError(Result.takeError()); - Consumer.handle(std::move(*Result)); - } - - FunctionType Function; - std::tuple Requirements; -}; - -/// Used to deduce the refactoring result type for the lambda that passed into -/// createRefactoringRule. -template struct LambdaDeducer; -template -struct LambdaDeducer { - using ReturnType = R; - using FunctionType = R (*)(const RefactoringRuleContext &, Args...); -}; +inline llvm::Error findError() { return llvm::Error::success(); } + +/// Scans the tuple and returns a valid \c Error if any of the values are +/// invalid. +template +llvm::Error findError(Expected &First, Expected &... Rest) { + if (!First) + return First.takeError(); + return findError(Rest...); +} + +template +void invokeRuleAfterValidatingRequirements( + RefactoringResultConsumer &Consumer, RefactoringRuleContext &Context, + const std::tuple &Requirements, + llvm::index_sequence) { + // Check if the requirements we're interested in can be evaluated. + auto Values = + std::make_tuple(std::get(Requirements).evaluate(Context)...); + auto Err = findError(std::get(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(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 +struct HasBaseOf : std::conditional::value || + HasBaseOf::value, + std::true_type, std::false_type>::type {}; + +template +struct HasBaseOf : std::is_base_of {}; + +/// A type trait that returns true when the given type list contains types that +/// derive from Base. +template +struct AreBaseOf : std::conditional::value && + AreBaseOf::value, + std::true_type, std::false_type>::type {}; + +template +struct AreBaseOf : std::is_base_of {}; } // end namespace internal -} // end namespace refactoring_action_rules + +template +std::unique_ptr +createRefactoringActionRule(const RequirementTypes &... Requirements) { + static_assert(std::is_base_of::value, + "Expected a refactoring action rule type"); + static_assert(internal::AreBaseOf::value, + "Expected a list of refactoring action rules"); + + class Rule final : public RefactoringActionRule { + public: + Rule(std::tuple Requirements) + : Requirements(Requirements) {} + + void invoke(RefactoringResultConsumer &Consumer, + RefactoringRuleContext &Context) override { + internal::invokeRuleAfterValidatingRequirements( + Consumer, Context, Requirements, + llvm::index_sequence_for()); + } + + bool hasSelectionRequirement() override { + return internal::HasBaseOf::value; + } + + private: + std::tuple Requirements; + }; + + return llvm::make_unique(std::make_tuple(Requirements...)); +} + } // end namespace tooling } // end namespace clang diff --git a/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h b/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h index 391c4d8b6d..fe7738e734 100644 --- a/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h +++ b/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h @@ -46,29 +46,6 @@ private: } }; -namespace traits { -namespace internal { - -template struct HasHandle { -private: - template - static auto check(ClassT *) -> typename std::is_same< - decltype(std::declval().handle(std::declval())), void>::type; - - template static std::false_type check(...); - -public: - using Type = decltype(check(nullptr)); -}; - -} // end namespace internal - -/// A type trait that returns true iff the given type is a valid refactoring -/// result. -template -struct IsValidRefactoringResult : internal::HasHandle::Type {}; - -} // end namespace traits } // end namespace tooling } // end namespace clang diff --git a/include/clang/Tooling/Refactoring/SourceSelectionConstraints.h b/include/clang/Tooling/Refactoring/SourceSelectionConstraints.h deleted file mode 100644 index 7369ca0408..0000000000 --- a/include/clang/Tooling/Refactoring/SourceSelectionConstraints.h +++ /dev/null @@ -1,114 +0,0 @@ -//===--- SourceSelectionConstraints.h - Clang refactoring library ---------===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// - -#ifndef LLVM_CLANG_TOOLING_REFACTOR_SOURCE_SELECTION_CONSTRAINTS_H -#define LLVM_CLANG_TOOLING_REFACTOR_SOURCE_SELECTION_CONSTRAINTS_H - -#include "clang/Basic/LLVM.h" -#include "clang/Basic/SourceLocation.h" -#include "clang/Tooling/Refactoring/RefactoringRuleContext.h" -#include - -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 - 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 , 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 struct IsConstraint : public std::false_type {}; - -} // end namespace traits - -namespace internal { - -template struct EvaluateSelectionChecker : std::false_type {}; - -template -struct EvaluateSelectionChecker : std::true_type { - using ReturnType = R; - using ArgType = A; -}; - -template 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 internal::Identity identity() { - static_assert( - traits::IsConstraint::value, - "selection::identity can be used with selection constraints only"); - return internal::Identity(); -} - -namespace traits { - -template <> -struct IsConstraint : public std::true_type {}; - -/// A type trait that returns true iff \c T is a valid selection requirement. -template -struct IsRequirement - : std::conditional< - std::is_base_of::value && - internal::EvaluateSelectionChecker::value && - IsConstraint::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 diff --git a/lib/Tooling/Refactoring/CMakeLists.txt b/lib/Tooling/Refactoring/CMakeLists.txt index ff9cd1ff9e..43ea1d9c54 100644 --- a/lib/Tooling/Refactoring/CMakeLists.txt +++ b/lib/Tooling/Refactoring/CMakeLists.txt @@ -9,7 +9,6 @@ add_clang_library(clangToolingRefactor Rename/USRFinder.cpp Rename/USRFindingAction.cpp Rename/USRLocFinder.cpp - SourceSelectionConstraints.cpp LINK_LIBS clangAST diff --git a/lib/Tooling/Refactoring/Rename/RenamingAction.cpp b/lib/Tooling/Refactoring/Rename/RenamingAction.cpp index 384e466015..fe3067f825 100644 --- a/lib/Tooling/Refactoring/Rename/RenamingAction.cpp +++ b/lib/Tooling/Refactoring/Rename/RenamingAction.cpp @@ -23,7 +23,6 @@ #include "clang/Tooling/CommonOptionsParser.h" #include "clang/Tooling/Refactoring.h" #include "clang/Tooling/Refactoring/RefactoringAction.h" -#include "clang/Tooling/Refactoring/RefactoringActionRules.h" #include "clang/Tooling/Refactoring/Rename/USRFinder.h" #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h" #include "clang/Tooling/Refactoring/Rename/USRLocFinder.h" @@ -39,53 +38,78 @@ namespace tooling { namespace { -class LocalRename : public RefactoringAction { +class SymbolSelectionRequirement : public SourceRangeSelectionRequirement { public: - StringRef getCommand() const override { return "local-rename"; } - - StringRef getDescription() const override { - return "Finds and renames symbols in code with no indexer support"; + Expected evaluate(RefactoringRuleContext &Context) const { + Expected 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("no symbol selected", + llvm::inconvertibleErrorCode()); + } + return getCanonicalSymbolDeclaration(ND); } +}; - /// Returns a set of refactoring actions rules that are defined by this - /// action. - RefactoringActionRules createActionRules() const override { - using namespace refactoring_action_rules; - RefactoringActionRules Rules; - Rules.push_back(createRefactoringRule( - renameOccurrences, requiredSelection(SymbolSelectionRequirement()))); - return Rules; - } +class OccurrenceFinder final : public FindSymbolOccurrencesRefactoringRule { +public: + OccurrenceFinder(const NamedDecl *ND) : ND(ND) {} -private: - static Expected - renameOccurrences(const RefactoringRuleContext &Context, - const NamedDecl *ND) { + Expected + findSymbolOccurrences(RefactoringRuleContext &Context) override { std::vector 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 + createSourceReplacements(RefactoringRuleContext &Context) { + Expected 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> - evaluateSelection(const RefactoringRuleContext &Context, - selection::SourceSelectionRange Selection) const { - const NamedDecl *ND = getNamedDeclAt(Context.getASTContext(), - Selection.getRange().getBegin()); - if (!ND) - return None; - return getCanonicalSymbolDeclaration(ND); - } - }; +private: + OccurrenceFinder Finder; +}; + +class LocalRename final : public RefactoringAction { +public: + StringRef getCommand() const override { return "local-rename"; } + + StringRef getDescription() const override { + return "Finds and renames symbols in code with no indexer support"; + } + + /// Returns a set of refactoring actions rules that are defined by this + /// action. + RefactoringActionRules createActionRules() const override { + RefactoringActionRules Rules; + Rules.push_back(createRefactoringActionRule( + SymbolSelectionRequirement())); + return Rules; + } }; } // end anonymous namespace diff --git a/lib/Tooling/Refactoring/SourceSelectionConstraints.cpp b/lib/Tooling/Refactoring/SourceSelectionConstraints.cpp deleted file mode 100644 index 5fbe2946a9..0000000000 --- a/lib/Tooling/Refactoring/SourceSelectionConstraints.cpp +++ /dev/null @@ -1,23 +0,0 @@ -//===--- SourceSelectionConstraints.cpp - Evaluate selection constraints --===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// - -#include "clang/Tooling/Refactoring/SourceSelectionConstraints.h" -#include "clang/Tooling/Refactoring/RefactoringRuleContext.h" - -using namespace clang; -using namespace tooling; -using namespace selection; - -Optional -SourceSelectionRange::evaluate(RefactoringRuleContext &Context) { - SourceRange R = Context.getSelectionRange(); - if (R.isInvalid()) - return None; - return SourceSelectionRange(Context.getSources(), R); -} diff --git a/unittests/Tooling/RefactoringActionRulesTest.cpp b/unittests/Tooling/RefactoringActionRulesTest.cpp index cf91b5fc8f..afab34ef10 100644 --- a/unittests/Tooling/RefactoringActionRulesTest.cpp +++ b/unittests/Tooling/RefactoringActionRulesTest.cpp @@ -18,7 +18,6 @@ using namespace clang; using namespace tooling; -using namespace refactoring_action_rules; namespace { @@ -56,29 +55,39 @@ createReplacements(const std::unique_ptr &Rule, } TEST_F(RefactoringActionRulesTest, MyFirstRefactoringRule) { - auto ReplaceAWithB = - [](const RefactoringRuleContext &, - std::pair Selection) - -> Expected { - const SourceManager &SM = Selection.first.getSources(); - SourceLocation Loc = Selection.first.getRange().getBegin().getLocWithOffset( - Selection.second); - AtomicChange Change(SM, Loc); - llvm::Error E = Change.replace(SM, Loc, 1, "b"); - if (E) - return std::move(E); - return AtomicChanges{Change}; + class ReplaceAWithB : public SourceChangeRefactoringRule { + std::pair Selection; + + public: + ReplaceAWithB(std::pair Selection) + : Selection(Selection) {} + + Expected + 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 - evaluateSelection(const RefactoringRuleContext &, - selection::SourceSelectionRange Selection) const { - return std::make_pair(Selection, 20); + Expected> + evaluate(RefactoringRuleContext &Context) const { + Expected R = + SourceRangeSelectionRequirement::evaluate(Context); + if (!R) + return R.takeError(); + return std::make_pair(*R, 20); } }; - auto Rule = createRefactoringRule(ReplaceAWithB, - requiredSelection(SelectionRequirement())); + auto Rule = + createRefactoringActionRule(SelectionRequirement()); // When the requirements are satisifed, the rule's function must be invoked. { @@ -123,54 +132,24 @@ TEST_F(RefactoringActionRulesTest, MyFirstRefactoringRule) { 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 (*Func)(const RefactoringRuleContext &, - selection::SourceSelectionRange) = - [](const RefactoringRuleContext &, - selection::SourceSelectionRange) -> Expected { - return llvm::make_error( - "Error", llvm::make_error_code(llvm::errc::invalid_argument)); - }; - auto Rule = createRefactoringRule( - Func, requiredSelection( - selection::identity())); - - RefactoringRuleContext RefContext(Context.Sources); - SourceLocation Cursor = - Context.Sources.getLocForStartOfFile(Context.Sources.getMainFileID()); - RefContext.setSelectionRange({Cursor, Cursor}); - Expected Result = createReplacements(Rule, RefContext); - - ASSERT_TRUE(!Result); - std::string Message; - llvm::handleAllErrors(Result.takeError(), [&](llvm::StringError &Error) { - Message = Error.getMessage(); - }); - EXPECT_EQ(Message, "Error"); -} - -TEST_F(RefactoringActionRulesTest, ReturnInitiationDiagnostic) { - RefactoringRuleContext RefContext(Context.Sources); - class SelectionRequirement : public selection::Requirement { + class ErrorRule : public SourceChangeRefactoringRule { public: - Expected> - evaluateSelection(const RefactoringRuleContext &, - selection::SourceSelectionRange Selection) const { + ErrorRule(SourceRange R) {} + Expected createSourceReplacements(RefactoringRuleContext &) { return llvm::make_error( - "bad selection", llvm::make_error_code(llvm::errc::invalid_argument)); + "Error", llvm::make_error_code(llvm::errc::invalid_argument)); } }; - auto Rule = createRefactoringRule( - [](const RefactoringRuleContext &, int) -> Expected { - llvm::report_fatal_error("Should not run!"); - }, - requiredSelection(SelectionRequirement())); + auto Rule = + createRefactoringActionRule(SourceRangeSelectionRequirement()); + RefactoringRuleContext RefContext(Context.Sources); SourceLocation Cursor = Context.Sources.getLocForStartOfFile(Context.Sources.getMainFileID()); RefContext.setSelectionRange({Cursor, Cursor}); @@ -181,7 +160,7 @@ TEST_F(RefactoringActionRulesTest, ReturnInitiationDiagnostic) { llvm::handleAllErrors(Result.takeError(), [&](llvm::StringError &Error) { Message = Error.getMessage(); }); - EXPECT_EQ(Message, "bad selection"); + EXPECT_EQ(Message, "Error"); } Optional findOccurrences(RefactoringActionRule &Rule, @@ -205,18 +184,24 @@ Optional findOccurrences(RefactoringActionRule &Rule, } TEST_F(RefactoringActionRulesTest, ReturnSymbolOccurrences) { - auto Rule = createRefactoringRule( - [](const RefactoringRuleContext &, - selection::SourceSelectionRange Selection) - -> Expected { - SymbolOccurrences Occurrences; - Occurrences.push_back(SymbolOccurrence( - SymbolName("test"), SymbolOccurrence::MatchingSymbol, - Selection.getRange().getBegin())); - return std::move(Occurrences); - }, - requiredSelection( - selection::identity())); + class FindOccurrences : public FindSymbolOccurrencesRefactoringRule { + SourceRange Selection; + + public: + FindOccurrences(SourceRange Selection) : Selection(Selection) {} + + Expected + findSymbolOccurrences(RefactoringRuleContext &) override { + SymbolOccurrences Occurrences; + Occurrences.push_back(SymbolOccurrence(SymbolName("test"), + SymbolOccurrence::MatchingSymbol, + Selection.getBegin())); + return Occurrences; + } + }; + + auto Rule = createRefactoringActionRule( + SourceRangeSelectionRequirement()); RefactoringRuleContext RefContext(Context.Sources); SourceLocation Cursor = -- 2.40.0