This revision was automatically updated to reflect the committed changes.
Closed by commit rL312316: [refactor] Use a RefactoringResultConsumer instead
of tagged refactoring (authored by arphaman).
Changed prior to commit:
https://reviews.llvm.org/D37291?vs=113397&id=113520#toc
Repository:
rL LLVM
https://reviews.llvm.org/D37291
Files:
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRules.h
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp
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<Optional<AtomicChanges>>
+Expected<AtomicChanges>
createReplacements(const std::unique_ptr<RefactoringActionRule> &Rule,
RefactoringRuleContext &Context) {
- return cast<SourceChangeRefactoringRule>(*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<Expected<AtomicChanges>> 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<Optional<AtomicChanges>> ErrorOrResult =
+ Expected<AtomicChanges> 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<AtomicChange &>(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<Optional<AtomicChanges>> ErrorOrResult =
+ Expected<AtomicChanges> ErrorOrResult =
createReplacements(Rule, RefContext);
- ASSERT_FALSE(!ErrorOrResult);
- Optional<AtomicChanges> 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<Optional<AtomicChanges>> Result =
- createReplacements(Rule, RefContext);
+ Expected<AtomicChanges> 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<Optional<AtomicChanges>> Result =
- createReplacements(Rule, RefContext);
+ Expected<AtomicChanges> Result = createReplacements(Rule, RefContext);
ASSERT_TRUE(!Result);
std::string Message;
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<llvm::StringError>(
+ "unsupported refactoring result", llvm::inconvertibleErrorCode()));
+ }
+};
+
+namespace traits {
+namespace internal {
+
+template <typename T> struct HasHandle {
+private:
+ template <typename ClassT>
+ static auto check(ClassT *) -> typename std::is_same<
+ decltype(std::declval<ClassT>().handle(std::declval<T>())), void>::type;
+
+ template <typename> static std::false_type check(...);
+
+public:
+ using Type = decltype(check<RefactoringResultConsumer>(nullptr));
+};
+
+} // end namespace internal
+
+/// A type trait that returns true iff the given type is a valid refactoring
+/// result.
+template <typename T>
+struct IsValidRefactoringResult : internal::HasHandle<T>::Type {};
+
+} // end namespace traits
+} // end namespace tooling
+} // end namespace clang
+
+#endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RESULT_CONSUMER_H
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<ResultType> (*RefactoringFunction)(
typename RequirementTypes::OutputType...),
const RequirementTypes &... Requirements) {
- static_assert(
- std::is_base_of<
- RefactoringActionRule,
- internal::SpecificRefactoringRuleAdapter<ResultType>>::value,
- "invalid refactoring result type");
+ static_assert(tooling::traits::IsValidRefactoringResult<ResultType>::value,
+ "invalid refactoring result type");
static_assert(traits::IsRequirement<RequirementTypes...>::value,
"invalid refactoring action rule requirement");
return llvm::make_unique<internal::PlainFunctionRule<
- ResultType, decltype(RefactoringFunction), RequirementTypes...>>(
+ 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 <type_traits>
@@ -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 <typename T> struct SpecificRefactoringRuleAdapter {};
-
-template <>
-class SpecificRefactoringRuleAdapter<AtomicChanges>
- : public SourceChangeRefactoringRule {
-public:
- virtual Expected<Optional<AtomicChanges>>
- perform(RefactoringRuleContext &Context) = 0;
-
- Expected<Optional<AtomicChanges>>
- 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 <typename ResultType, typename FunctionType,
- typename... RequirementTypes>
-class PlainFunctionRule final
- : public SpecificRefactoringRuleAdapter<ResultType> {
+template <typename FunctionType, typename... RequirementTypes>
+class PlainFunctionRule final : public RefactoringActionRule {
public:
PlainFunctionRule(FunctionType Function,
std::tuple<RequirementTypes...> &&Requirements)
: Function(Function), Requirements(std::move(Requirements)) {}
- Expected<Optional<ResultType>>
- perform(RefactoringRuleContext &Context) override {
- return performImpl(Context,
- llvm::index_sequence_for<RequirementTypes...>());
+ void invoke(RefactoringResultConsumer &Consumer,
+ RefactoringRuleContext &Context) override {
+ return invokeImpl(Consumer, Context,
+ llvm::index_sequence_for<RequirementTypes...>());
}
private:
@@ -95,22 +77,30 @@
}
template <size_t... Is>
- Expected<Optional<ResultType>> performImpl(RefactoringRuleContext &Context,
- llvm::index_sequence<Is...>) {
+ void invokeImpl(RefactoringResultConsumer &Consumer,
+ RefactoringRuleContext &Context,
+ llvm::index_sequence<Is...>) {
// Initiate the operation.
auto Values =
std::make_tuple(std::get<Is>(Requirements).evaluate(Context)...);
Optional<llvm::Error> InitiationFailure =
findErrorNone(std::get<Is>(Values)...);
if (InitiationFailure) {
llvm::Error Error = std::move(*InitiationFailure);
if (!Error)
- return None;
- return std::move(Error);
+ // FIXME: Use a diagnostic.
+ return Consumer.handleError(llvm::make_error<llvm::StringError>(
+ "refactoring action can't be initiated with the specified "
+ "selection range",
+ llvm::inconvertibleErrorCode()));
+ return Consumer.handleError(std::move(Error));
}
// Perform the operation.
- return Function(
- unwrapRequirementResult(std::move(std::get<Is>(Values)))...);
+ auto Result =
+ Function(unwrapRequirementResult(std::move(std::get<Is>(Values)))...);
+ if (!Result)
+ return Consumer.handleError(Result.takeError());
+ Consumer.handle(std::move(*Result));
}
FunctionType Function;
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 <vector>
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<Optional<AtomicChanges>>
- 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
