Skip to content

Commit

Permalink
[refactor] Describe refactorings in the operation classes
Browse files Browse the repository at this point in the history
This commit changes the way that the refactoring operation classes are
structured:
- Users have to call `initiate` instead of constructing an instance of the
  class. The `initiate` is now supposed to have custom initiation logic, and
  you don't need to subclass the builtin requirements.
- A new `describe` function returns a structure with the id, title and the
  description of the refactoring operation.

The refactoring action classes are now placed into one common place in
RefactoringActions.cpp instead of being separate.

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

llvm-svn: 316780
  • Loading branch information
hyp committed Oct 27, 2017
1 parent 1bfaa45 commit 0beca4d
Show file tree
Hide file tree
Showing 10 changed files with 242 additions and 164 deletions.
53 changes: 53 additions & 0 deletions clang/include/clang/Tooling/Refactoring/Extract/Extract.h
@@ -0,0 +1,53 @@
//===--- Extract.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_EXTRACT_EXTRACT_H
#define LLVM_CLANG_TOOLING_REFACTOR_EXTRACT_EXTRACT_H

#include "clang/Tooling/Refactoring/ASTSelection.h"
#include "clang/Tooling/Refactoring/RefactoringActionRules.h"

namespace clang {
namespace tooling {

/// An "Extract Function" refactoring moves code into a new function that's
/// then called from the place where the original code was.
class ExtractFunction final : public SourceChangeRefactoringRule {
public:
/// Initiates the extract function refactoring operation.
///
/// \param Code The selected set of statements.
/// \param DeclName The name name of the extract function. If None,
/// "extracted" is used.
static Expected<ExtractFunction> initiate(RefactoringRuleContext &Context,
CodeRangeASTSelection Code,
Optional<std::string> DeclName);

static const RefactoringDescriptor &describe();

private:
ExtractFunction(CodeRangeASTSelection Code, Optional<std::string> DeclName)
: Code(std::move(Code)),
DeclName(DeclName ? std::move(*DeclName) : "extracted") {}

Expected<AtomicChanges>
createSourceReplacements(RefactoringRuleContext &Context) override;

CodeRangeASTSelection Code;

// FIXME: Account for naming collisions:
// - error when name is specified by user.
// - rename to "extractedN" when name is implicit.
std::string DeclName;
};

} // end namespace tooling
} // end namespace clang

#endif // LLVM_CLANG_TOOLING_REFACTOR_EXTRACT_EXTRACT_H

This file was deleted.

14 changes: 14 additions & 0 deletions clang/include/clang/Tooling/Refactoring/RefactoringActionRule.h
Expand Up @@ -11,6 +11,8 @@
#define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_H

#include "clang/Basic/LLVM.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringRef.h"
#include <vector>

namespace clang {
Expand All @@ -20,6 +22,15 @@ class RefactoringOptionVisitor;
class RefactoringResultConsumer;
class RefactoringRuleContext;

struct RefactoringDescriptor {
/// A unique identifier for the specific refactoring.
StringRef Name;
/// A human readable title for the refactoring.
StringRef Title;
/// A human readable description of what the refactoring does.
StringRef Description;
};

/// A common refactoring action rule interface that defines the 'invoke'
/// function that performs the refactoring operation (either fully or
/// partially).
Expand All @@ -33,6 +44,9 @@ class RefactoringActionRuleBase {
/// consumer to propagate the result of the refactoring action.
virtual void invoke(RefactoringResultConsumer &Consumer,
RefactoringRuleContext &Context) = 0;

/// Returns the structure that describes the refactoring.
// static const RefactoringDescriptor &describe() = 0;
};

/// A refactoring action rule is a wrapper class around a specific refactoring
Expand Down
Expand Up @@ -57,7 +57,11 @@ void invokeRuleAfterValidatingRequirements(
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::move((*std::get<Is>(Values)))...).invoke(Consumer, Context);
auto Rule =
RuleType::initiate(Context, std::move((*std::get<Is>(Values)))...);
if (!Rule)
return Consumer.handleError(Rule.takeError());
Rule->invoke(Consumer, Context);
}

inline void visitRefactoringOptionsImpl(RefactoringOptionVisitor &) {}
Expand Down Expand Up @@ -141,7 +145,6 @@ createRefactoringActionRule(const RequirementTypes &... Requirements) {
Visitor, Requirements,
llvm::index_sequence_for<RequirementTypes...>());
}

private:
std::tuple<RequirementTypes...> Requirements;
};
Expand Down
22 changes: 17 additions & 5 deletions clang/include/clang/Tooling/Refactoring/Rename/RenamingAction.h
Expand Up @@ -17,6 +17,7 @@

#include "clang/Tooling/Refactoring.h"
#include "clang/Tooling/Refactoring/AtomicChange.h"
#include "clang/Tooling/Refactoring/RefactoringActionRules.h"
#include "clang/Tooling/Refactoring/RefactoringOptions.h"
#include "clang/Tooling/Refactoring/Rename/SymbolOccurrences.h"
#include "llvm/Support/Error.h"
Expand Down Expand Up @@ -46,12 +47,23 @@ class RenamingAction {
bool PrintLocations;
};

class NewNameOption : public RequiredRefactoringOption<std::string> {
class RenameOccurrences final : public SourceChangeRefactoringRule {
public:
StringRef getName() const override { return "new-name"; }
StringRef getDescription() const override {
return "The new name to change the symbol to";
}
static Expected<RenameOccurrences> initiate(RefactoringRuleContext &Context,
SourceRange SelectionRange,
std::string NewName);

static const RefactoringDescriptor &describe();

private:
RenameOccurrences(const NamedDecl *ND, std::string NewName)
: ND(ND), NewName(std::move(NewName)) {}

Expected<AtomicChanges>
createSourceReplacements(RefactoringRuleContext &Context) override;

const NamedDecl *ND;
std::string NewName;
};

/// Returns source replacements that correspond to the rename of the given
Expand Down
2 changes: 0 additions & 2 deletions clang/include/clang/module.modulemap
Expand Up @@ -146,8 +146,6 @@ module Clang_Tooling {
// importing the AST matchers library gives a link dependency on the AST
// matchers (and thus the AST), which clang-format should not have.
exclude header "Tooling/RefactoringCallbacks.h"

textual header "Tooling/Refactoring/RefactoringActionRegistry.def"
}

module Clang_ToolingCore {
Expand Down
115 changes: 32 additions & 83 deletions clang/lib/Tooling/Refactoring/Extract.cpp
Expand Up @@ -13,12 +13,10 @@
///
//===----------------------------------------------------------------------===//

#include "clang/Tooling/Refactoring/Extract/Extract.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Expr.h"
#include "clang/Rewrite/Core/Rewriter.h"
#include "clang/Tooling/Refactoring/RefactoringAction.h"
#include "clang/Tooling/Refactoring/RefactoringActionRules.h"
#include "clang/Tooling/Refactoring/RefactoringOptions.h"

namespace clang {
namespace tooling {
Expand All @@ -44,58 +42,43 @@ bool isSimpleExpression(const Expr *E) {
}
}

class ExtractableCodeSelectionRequirement final
: public CodeRangeASTSelectionRequirement {
public:
Expected<CodeRangeASTSelection>
evaluate(RefactoringRuleContext &Context) const {
Expected<CodeRangeASTSelection> Selection =
CodeRangeASTSelectionRequirement::evaluate(Context);
if (!Selection)
return Selection.takeError();
CodeRangeASTSelection &Code = *Selection;

// We would like to extract code out of functions/methods/blocks.
// Prohibit extraction from things like global variable / field
// initializers and other top-level expressions.
if (!Code.isInFunctionLikeBodyOfCode())
return Context.createDiagnosticError(
diag::err_refactor_code_outside_of_function);

// Avoid extraction of simple literals and references.
if (Code.size() == 1 && isSimpleExpression(dyn_cast<Expr>(Code[0])))
return Context.createDiagnosticError(
diag::err_refactor_extract_simple_expression);

// FIXME (Alex L): Prohibit extraction of Objective-C property setters.
return Selection;
}
};

class ExtractFunction final : public SourceChangeRefactoringRule {
public:
ExtractFunction(CodeRangeASTSelection Code, Optional<std::string> DeclName)
: Code(std::move(Code)),
DeclName(DeclName ? std::move(*DeclName) : "extracted") {}

Expected<AtomicChanges>
createSourceReplacements(RefactoringRuleContext &Context) override;

private:
CodeRangeASTSelection Code;

// FIXME: Account for naming collisions:
// - error when name is specified by user.
// - rename to "extractedN" when name is implicit.
std::string DeclName;
};

SourceLocation computeFunctionExtractionLocation(const Decl *D) {
// FIXME (Alex L): Method -> function extraction should place function before
// C++ record if the method is defined inside the record.
return D->getLocStart();
}

} // end anonymous namespace

const RefactoringDescriptor &ExtractFunction::describe() {
static const RefactoringDescriptor Descriptor = {
"extract-function",
"Extract Function",
"(WIP action; use with caution!) Extracts code into a new function",
};
return Descriptor;
}

Expected<ExtractFunction>
ExtractFunction::initiate(RefactoringRuleContext &Context,
CodeRangeASTSelection Code,
Optional<std::string> DeclName) {
// We would like to extract code out of functions/methods/blocks.
// Prohibit extraction from things like global variable / field
// initializers and other top-level expressions.
if (!Code.isInFunctionLikeBodyOfCode())
return Context.createDiagnosticError(
diag::err_refactor_code_outside_of_function);

// Avoid extraction of simple literals and references.
if (Code.size() == 1 && isSimpleExpression(dyn_cast<Expr>(Code[0])))
return Context.createDiagnosticError(
diag::err_refactor_extract_simple_expression);

// FIXME (Alex L): Prohibit extraction of Objective-C property setters.
return ExtractFunction(std::move(Code), DeclName);
}

// FIXME: Support C++ method extraction.
// FIXME: Support Objective-C method extraction.
Expected<AtomicChanges>
Expand Down Expand Up @@ -194,39 +177,5 @@ ExtractFunction::createSourceReplacements(RefactoringRuleContext &Context) {
return AtomicChanges{std::move(Change)};
}

class DeclNameOption final : public OptionalRefactoringOption<std::string> {
public:
StringRef getName() const { return "name"; }
StringRef getDescription() const {
return "Name of the extracted declaration";
}
};

class ExtractRefactoring final : public RefactoringAction {
public:
StringRef getCommand() const override { return "extract"; }

StringRef getDescription() const override {
return "(WIP action; use with caution!) Extracts code into a new function "
"/ method / variable";
}

/// Returns a set of refactoring actions rules that are defined by this
/// action.
RefactoringActionRules createActionRules() const override {
RefactoringActionRules Rules;
Rules.push_back(createRefactoringActionRule<ExtractFunction>(
ExtractableCodeSelectionRequirement(),
OptionRequirement<DeclNameOption>()));
return Rules;
}
};

} // end anonymous namespace

std::unique_ptr<RefactoringAction> createExtractAction() {
return llvm::make_unique<ExtractRefactoring>();
}

} // end namespace tooling
} // end namespace clang
71 changes: 65 additions & 6 deletions clang/lib/Tooling/Refactoring/RefactoringActions.cpp
Expand Up @@ -7,21 +7,80 @@
//
//===----------------------------------------------------------------------===//

#include "clang/Tooling/Refactoring/Extract/Extract.h"
#include "clang/Tooling/Refactoring/RefactoringAction.h"
#include "clang/Tooling/Refactoring/RefactoringOptions.h"
#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"

namespace clang {
namespace tooling {

// Forward declare the individual create*Action functions.
#define REFACTORING_ACTION(Name) \
std::unique_ptr<RefactoringAction> create##Name##Action();
#include "clang/Tooling/Refactoring/RefactoringActionRegistry.def"
namespace {

class DeclNameOption final : public OptionalRefactoringOption<std::string> {
public:
StringRef getName() const { return "name"; }
StringRef getDescription() const {
return "Name of the extracted declaration";
}
};

// FIXME: Rewrite the Actions to avoid duplication of descriptions/names with
// rules.
class ExtractRefactoring final : public RefactoringAction {
public:
StringRef getCommand() const override { return "extract"; }

StringRef getDescription() const override {
return "(WIP action; use with caution!) Extracts code into a new function";
}

/// Returns a set of refactoring actions rules that are defined by this
/// action.
RefactoringActionRules createActionRules() const override {
RefactoringActionRules Rules;
Rules.push_back(createRefactoringActionRule<ExtractFunction>(
CodeRangeASTSelectionRequirement(),
OptionRequirement<DeclNameOption>()));
return Rules;
}
};

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";
}
};

// FIXME: Rewrite the Actions to avoid duplication of descriptions/names with
// rules.
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>(
SourceRangeSelectionRequirement(), OptionRequirement<NewNameOption>()));
return Rules;
}
};

} // end anonymous namespace

std::vector<std::unique_ptr<RefactoringAction>> createRefactoringActions() {
std::vector<std::unique_ptr<RefactoringAction>> Actions;

#define REFACTORING_ACTION(Name) Actions.push_back(create##Name##Action());
#include "clang/Tooling/Refactoring/RefactoringActionRegistry.def"
Actions.push_back(llvm::make_unique<LocalRename>());
Actions.push_back(llvm::make_unique<ExtractRefactoring>());

return Actions;
}
Expand Down

0 comments on commit 0beca4d

Please sign in to comment.