Index: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h =================================================================== --- cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h +++ cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h @@ -11,52 +11,25 @@ #define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_H #include "clang/Basic/LLVM.h" -#include "clang/Tooling/Refactoring/AtomicChange.h" -#include "llvm/Support/Error.h" #include namespace clang { namespace tooling { +class RefactoringResultConsumer; class RefactoringRuleContext; /// A common refactoring action rule interface. class RefactoringActionRule { public: - enum RuleKind { SourceChangeRefactoringRuleKind }; - - RuleKind getRuleKind() const { return Kind; } - virtual ~RefactoringActionRule() {} -protected: - RefactoringActionRule(RuleKind Kind) : Kind(Kind) {} - -private: - RuleKind Kind; -}; - -/// A type of refactoring action rule that produces source replacements in the -/// form of atomic changes. -/// -/// This action rule is typically used for local refactorings that replace -/// source in a single AST unit. -class SourceChangeRefactoringRule : public RefactoringActionRule { -public: - SourceChangeRefactoringRule() - : RefactoringActionRule(SourceChangeRefactoringRuleKind) {} - - /// Initiates and performs a refactoring action that modifies the sources. + /// Initiates and performs a specific refactoring action. /// - /// The specific rule must return an llvm::Error with a DiagnosticError - /// payload or None when the refactoring action couldn't be initiated/ - /// performed, or \c AtomicChanges when the action was performed successfully. - virtual Expected> - createSourceReplacements(RefactoringRuleContext &Context) = 0; - - static bool classof(const RefactoringActionRule *Rule) { - return Rule->getRuleKind() == SourceChangeRefactoringRuleKind; - } + /// The specific rule will invoke an appropriate \c handle method on a + /// consumer to propagate the result of the refactoring action. + virtual void invoke(RefactoringResultConsumer &Consumer, + RefactoringRuleContext &Context) = 0; }; /// A set of refactoring action rules that should have unique initiation Index: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRules.h =================================================================== --- cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRules.h +++ cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRules.h @@ -42,15 +42,12 @@ createRefactoringRule(Expected (*RefactoringFunction)( typename RequirementTypes::OutputType...), const RequirementTypes &... Requirements) { - static_assert( - std::is_base_of< - RefactoringActionRule, - internal::SpecificRefactoringRuleAdapter>::value, - "invalid refactoring result type"); + static_assert(tooling::traits::IsValidRefactoringResult::value, + "invalid refactoring result type"); static_assert(traits::IsRequirement::value, "invalid refactoring action rule requirement"); return llvm::make_unique>( + decltype(RefactoringFunction), RequirementTypes...>>( RefactoringFunction, std::make_tuple(Requirements...)); } Index: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h =================================================================== --- cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h +++ cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h @@ -13,6 +13,7 @@ #include "clang/Basic/LLVM.h" #include "clang/Tooling/Refactoring/RefactoringActionRule.h" #include "clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h" +#include "clang/Tooling/Refactoring/RefactoringResultConsumer.h" #include "clang/Tooling/Refactoring/RefactoringRuleContext.h" #include "llvm/Support/Error.h" #include @@ -22,39 +23,20 @@ namespace refactoring_action_rules { namespace internal { -/// A wrapper around a specific refactoring action rule that calls a generic -/// 'perform' method from the specific refactoring method. -template struct SpecificRefactoringRuleAdapter {}; - -template <> -class SpecificRefactoringRuleAdapter - : public SourceChangeRefactoringRule { -public: - virtual Expected> - perform(RefactoringRuleContext &Context) = 0; - - Expected> - createSourceReplacements(RefactoringRuleContext &Context) final override { - return perform(Context); - } -}; - /// 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 SpecificRefactoringRuleAdapter { +template +class PlainFunctionRule final : public RefactoringActionRule { public: PlainFunctionRule(FunctionType Function, std::tuple &&Requirements) : Function(Function), Requirements(std::move(Requirements)) {} - Expected> - perform(RefactoringRuleContext &Context) override { - return performImpl(Context, - llvm::index_sequence_for()); + void invoke(RefactoringResultConsumer &Consumer, + RefactoringRuleContext &Context) override { + return invokeImpl(Consumer, Context, + llvm::index_sequence_for()); } private: @@ -95,8 +77,9 @@ } template - Expected> performImpl(RefactoringRuleContext &Context, - llvm::index_sequence) { + void invokeImpl(RefactoringResultConsumer &Consumer, + RefactoringRuleContext &Context, + llvm::index_sequence) { // Initiate the operation. auto Values = std::make_tuple(std::get(Requirements).evaluate(Context)...); @@ -105,12 +88,19 @@ if (InitiationFailure) { llvm::Error Error = std::move(*InitiationFailure); if (!Error) - return None; - return std::move(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. - return Function( - unwrapRequirementResult(std::move(std::get(Values)))...); + auto Result = + Function(unwrapRequirementResult(std::move(std::get(Values)))...); + if (!Result) + return Consumer.handleError(Result.takeError()); + Consumer.handle(std::move(*Result)); } FunctionType Function; Index: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h =================================================================== --- cfe/trunk/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h +++ cfe/trunk/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h @@ -0,0 +1,70 @@ +//===--- RefactoringResultConsumer.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_RESULT_CONSUMER_H +#define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RESULT_CONSUMER_H + +#include "clang/Basic/LLVM.h" +#include "clang/Tooling/Refactoring/AtomicChange.h" +#include "llvm/Support/Error.h" + +namespace clang { +namespace tooling { + +/// An abstract interface that consumes the various refactoring results that can +/// be produced by refactoring actions. +/// +/// A valid refactoring result must be handled by a \c handle method. +class RefactoringResultConsumer { +public: + virtual ~RefactoringResultConsumer() {} + + /// Handles an initation or an invication error. An initiation error typically + /// has a \c DiagnosticError payload that describes why initation failed. + virtual void handleError(llvm::Error Err) = 0; + + /// Handles the source replacements that are produced by a refactoring action. + virtual void handle(AtomicChanges SourceReplacements) { + defaultResultHandler(); + } + +private: + void defaultResultHandler() { + handleError(llvm::make_error( + "unsupported refactoring result", llvm::inconvertibleErrorCode())); + } +}; + +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 + +#endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RESULT_CONSUMER_H Index: cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp =================================================================== --- cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp +++ cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp @@ -32,11 +32,23 @@ std::string DefaultCode = std::string(100, 'a'); }; -Expected> +Expected createReplacements(const std::unique_ptr &Rule, RefactoringRuleContext &Context) { - return cast(*Rule).createSourceReplacements( - Context); + class Consumer final : public RefactoringResultConsumer { + void handleError(llvm::Error Err) override { Result = std::move(Err); } + + void handle(AtomicChanges SourceReplacements) override { + Result = std::move(SourceReplacements); + } + + public: + Optional> Result; + }; + + Consumer C; + Rule->invoke(C, Context); + return std::move(*C.Result); } TEST_F(RefactoringActionRulesTest, MyFirstRefactoringRule) { @@ -70,11 +82,10 @@ .getLocWithOffset(10); RefContext.setSelectionRange({Cursor, Cursor}); - Expected> ErrorOrResult = + Expected ErrorOrResult = createReplacements(Rule, RefContext); ASSERT_FALSE(!ErrorOrResult); - ASSERT_FALSE(!*ErrorOrResult); - AtomicChanges Result = std::move(**ErrorOrResult); + AtomicChanges Result = std::move(*ErrorOrResult); ASSERT_EQ(Result.size(), 1u); std::string YAMLString = const_cast(Result[0]).toYAMLString(); @@ -94,16 +105,20 @@ YAMLString.c_str()); } - // When one of the requirements is not satisfied, perform should return either - // None or a valid diagnostic. + // When one of the requirements is not satisfied, invoke should return a + // valid error. { RefactoringRuleContext RefContext(Context.Sources); - Expected> ErrorOrResult = + Expected ErrorOrResult = createReplacements(Rule, RefContext); - ASSERT_FALSE(!ErrorOrResult); - Optional Value = std::move(*ErrorOrResult); - EXPECT_TRUE(!Value); + ASSERT_TRUE(!ErrorOrResult); + std::string Message; + llvm::handleAllErrors( + ErrorOrResult.takeError(), + [&](llvm::StringError &Error) { Message = Error.getMessage(); }); + EXPECT_EQ(Message, "refactoring action can't be initiated with the " + "specified selection range"); } } @@ -121,8 +136,7 @@ SourceLocation Cursor = Context.Sources.getLocForStartOfFile(Context.Sources.getMainFileID()); RefContext.setSelectionRange({Cursor, Cursor}); - Expected> Result = - createReplacements(Rule, RefContext); + Expected Result = createReplacements(Rule, RefContext); ASSERT_TRUE(!Result); std::string Message; @@ -151,8 +165,7 @@ SourceLocation Cursor = Context.Sources.getLocForStartOfFile(Context.Sources.getMainFileID()); RefContext.setSelectionRange({Cursor, Cursor}); - Expected> Result = - createReplacements(Rule, RefContext); + Expected Result = createReplacements(Rule, RefContext); ASSERT_TRUE(!Result); std::string Message;