Commit c6a7fb8c authored by Nikolai Kosjar's avatar Nikolai Kosjar
Browse files

C++: Refactor quick fixes



 - Put declarations into quickfixes.h to simplify testing
 - Give the factories more meaningful names

Change-Id: If74c29a8c17819d5369ffa3df94d146b14e53af9
Reviewed-by: default avatarErik Verbruggen <erik.verbruggen@digia.com>
parent 986fc8bf
......@@ -22,6 +22,7 @@ HEADERS += cppplugin.h \
cppinsertqtpropertymembers.h \
cppquickfixassistant.h \
cppquickfix.h \
cppquickfixes.h \
cppfunctiondecldeflink.h
SOURCES += cppplugin.cpp \
......
......@@ -54,6 +54,7 @@ QtcPlugin {
"cppquickfixassistant.cpp",
"cppquickfixassistant.h",
"cppquickfixes.cpp",
"cppquickfixes.h",
"cppsnippetprovider.cpp",
"cppsnippetprovider.h",
"cpptypehierarchy.cpp",
......
......@@ -151,7 +151,7 @@ Class *isMemberFunction(const LookupContext &context, Function *function)
} // anonymous namespace
void DeclFromDef::match(const CppQuickFixInterface &interface, QuickFixOperations &result)
void InsertDeclFromDef::match(const CppQuickFixInterface &interface, QuickFixOperations &result)
{
const QList<AST *> &path = interface->path();
CppRefactoringFilePtr file = interface->currentFile();
......@@ -287,7 +287,7 @@ private:
} // anonymous namespace
void DefFromDecl::match(const CppQuickFixInterface &interface, QuickFixOperations &result)
void InsertDefFromDecl::match(const CppQuickFixInterface &interface, QuickFixOperations &result)
{
const QList<AST *> &path = interface->path();
......@@ -319,11 +319,11 @@ void DefFromDecl::match(const CppQuickFixInterface &interface, QuickFixOperation
namespace {
class GetterSetterOperation : public CppQuickFixOperation
class GenerateGetterSetterOperation : public CppQuickFixOperation
{
public:
GetterSetterOperation(const QSharedPointer<const CppQuickFixAssistInterface> &interface,
bool testMode = false)
GenerateGetterSetterOperation(const QSharedPointer<const CppQuickFixAssistInterface> &interface,
bool testMode = false)
: CppQuickFixOperation(interface)
, m_variableName(0)
, m_declaratorId(0)
......@@ -602,9 +602,9 @@ public:
} // namespace
void GetterSetter::match(const CppQuickFixInterface &interface, QuickFixOperations &result)
void GenerateGetterSetter::match(const CppQuickFixInterface &interface, QuickFixOperations &result)
{
GetterSetterOperation *op = new GetterSetterOperation(interface, m_testMode);
GenerateGetterSetterOperation *op = new GenerateGetterSetterOperation(interface, m_testMode);
if (op->isValid())
result.append(CppQuickFixOperation::Ptr(op));
else
......
......@@ -35,13 +35,13 @@
namespace CppEditor {
namespace Internal {
class DeclFromDef: public CppQuickFixFactory
class InsertDeclFromDef: public CppQuickFixFactory
{
public:
void match(const CppQuickFixInterface &interface, TextEditor::QuickFixOperations &result);
};
class DefFromDecl: public CppQuickFixFactory
class InsertDefFromDecl: public CppQuickFixFactory
{
public:
void match(const CppQuickFixInterface &interface, TextEditor::QuickFixOperations &result);
......@@ -53,10 +53,10 @@ public:
void match(const CppQuickFixInterface &interface, TextEditor::QuickFixOperations &result);
};
class GetterSetter : public CppQuickFixFactory
class GenerateGetterSetter : public CppQuickFixFactory
{
public:
GetterSetter(const bool testMode = false) : m_testMode(testMode) {}
GenerateGetterSetter(const bool testMode = false) : m_testMode(testMode) {}
void match(const CppQuickFixInterface &interface, TextEditor::QuickFixOperations &result);
private:
const bool m_testMode;
......
......@@ -38,6 +38,7 @@
#include "cpptypehierarchy.h"
#include "cppsnippetprovider.h"
#include "cppquickfixassistant.h"
#include "cppquickfixes.h"
#include <coreplugin/icore.h>
#include <coreplugin/coreconstants.h>
......@@ -187,7 +188,7 @@ bool CppPlugin::initialize(const QStringList & /*arguments*/, QString *errorMess
m_quickFixProvider = new CppQuickFixAssistProvider;
addAutoReleasedObject(m_quickFixProvider);
registerQuickFixes(this);
CppEditor::Internal::registerQuickFixes(this);
QObject *core = Core::ICore::instance();
CppFileWizard::BaseFileWizardParameters wizardParameters(Core::IWizard::FileWizard);
......
......@@ -90,18 +90,18 @@ private slots:
#ifdef WITH_TESTS
private slots: // quickfix tests
void test_quickfix_GetterSetter_basicGetterWithPrefix();
void test_quickfix_GetterSetter_basicGetterWithoutPrefix();
void test_quickfix_GetterSetter_customType();
void test_quickfix_GetterSetter_constMember();
void test_quickfix_GetterSetter_pointerToNonConst();
void test_quickfix_GetterSetter_pointerToConst();
void test_quickfix_GetterSetter_staticMember();
void test_quickfix_GetterSetter_secondDeclarator();
void test_quickfix_GetterSetter_triggeringRightAfterPointerSign();
void test_quickfix_GetterSetter_notTriggeringOnMemberFunction();
void test_quickfix_GetterSetter_notTriggeringOnMemberArray();
void test_quickfix_GetterSetter_notTriggeringWhenGetterOrSetterExist();
void test_quickfix_GenerateGetterSetter_basicGetterWithPrefix();
void test_quickfix_GenerateGetterSetter_basicGetterWithoutPrefix();
void test_quickfix_GenerateGetterSetter_customType();
void test_quickfix_GenerateGetterSetter_constMember();
void test_quickfix_GenerateGetterSetter_pointerToNonConst();
void test_quickfix_GenerateGetterSetter_pointerToConst();
void test_quickfix_GenerateGetterSetter_staticMember();
void test_quickfix_GenerateGetterSetter_secondDeclarator();
void test_quickfix_GenerateGetterSetter_triggeringRightAfterPointerSign();
void test_quickfix_GenerateGetterSetter_notTriggeringOnMemberFunction();
void test_quickfix_GenerateGetterSetter_notTriggeringOnMemberArray();
void test_quickfix_GenerateGetterSetter_notTriggeringWhenGetterOrSetterExist();
#endif // WITH_TESTS
private:
......
......@@ -194,7 +194,7 @@ void TestCase::run(CppQuickFixFactory *factory, const QByteArray &expected,
/// 1. If the name does not start with ("m_" or "_") and does not
/// end with "_", we are forced to prefix the getter with "get".
/// 2. Setter: Use pass by value on integer/float and pointer types.
void CppPlugin::test_quickfix_GetterSetter_basicGetterWithPrefix()
void CppPlugin::test_quickfix_GenerateGetterSetter_basicGetterWithPrefix()
{
TestCase data("\n"
"class Something\n"
......@@ -224,14 +224,14 @@ void CppPlugin::test_quickfix_GetterSetter_basicGetterWithPrefix()
"\n"
;
GetterSetter factory(/*testMode=*/ true);
GenerateGetterSetter factory(/*testMode=*/ true);
data.run(&factory, expected);
}
/// Checks:
/// 1. Getter: "get" prefix is not necessary.
/// 2. Setter: Parameter name is base name.
void CppPlugin::test_quickfix_GetterSetter_basicGetterWithoutPrefix()
void CppPlugin::test_quickfix_GenerateGetterSetter_basicGetterWithoutPrefix()
{
TestCase data("\n"
"class Something\n"
......@@ -261,13 +261,13 @@ void CppPlugin::test_quickfix_GetterSetter_basicGetterWithoutPrefix()
"\n"
;
GetterSetter factory(/*testMode=*/ true);
GenerateGetterSetter factory(/*testMode=*/ true);
data.run(&factory, expected);
}
/// Check: Setter: Use pass by reference for parameters which
/// are not integer, float or pointers.
void CppPlugin::test_quickfix_GetterSetter_customType()
void CppPlugin::test_quickfix_GenerateGetterSetter_customType()
{
TestCase data("\n"
"class Something\n"
......@@ -297,14 +297,14 @@ void CppPlugin::test_quickfix_GetterSetter_customType()
"\n"
;
GetterSetter factory(/*testMode=*/ true);
GenerateGetterSetter factory(/*testMode=*/ true);
data.run(&factory, expected);
}
/// Checks:
/// 1. Setter: No setter is generated for const members.
/// 2. Getter: Return a non-const type since it pass by value anyway.
void CppPlugin::test_quickfix_GetterSetter_constMember()
void CppPlugin::test_quickfix_GenerateGetterSetter_constMember()
{
TestCase data("\n"
"class Something\n"
......@@ -328,12 +328,12 @@ void CppPlugin::test_quickfix_GetterSetter_constMember()
"\n"
;
GetterSetter factory(/*testMode=*/ true);
GenerateGetterSetter factory(/*testMode=*/ true);
data.run(&factory, expected);
}
/// Checks: No special treatment for pointer to non const.
void CppPlugin::test_quickfix_GetterSetter_pointerToNonConst()
void CppPlugin::test_quickfix_GenerateGetterSetter_pointerToNonConst()
{
TestCase data("\n"
"class Something\n"
......@@ -363,12 +363,12 @@ void CppPlugin::test_quickfix_GetterSetter_pointerToNonConst()
"\n"
;
GetterSetter factory(/*testMode=*/ true);
GenerateGetterSetter factory(/*testMode=*/ true);
data.run(&factory, expected);
}
/// Checks: No special treatment for pointer to const.
void CppPlugin::test_quickfix_GetterSetter_pointerToConst()
void CppPlugin::test_quickfix_GenerateGetterSetter_pointerToConst()
{
TestCase data("\n"
"class Something\n"
......@@ -398,14 +398,14 @@ void CppPlugin::test_quickfix_GetterSetter_pointerToConst()
"\n"
;
GetterSetter factory(/*testMode=*/ true);
GenerateGetterSetter factory(/*testMode=*/ true);
data.run(&factory, expected);
}
/// Checks:
/// 1. Setter: Setter is a static function.
/// 2. Getter: Getter is a static, non const function.
void CppPlugin::test_quickfix_GetterSetter_staticMember()
void CppPlugin::test_quickfix_GenerateGetterSetter_staticMember()
{
TestCase data("\n"
"class Something\n"
......@@ -435,12 +435,12 @@ void CppPlugin::test_quickfix_GetterSetter_staticMember()
"\n"
;
GetterSetter factory(/*testMode=*/ true);
GenerateGetterSetter factory(/*testMode=*/ true);
data.run(&factory, expected);
}
/// Check: Check if it works on the second declarator
void CppPlugin::test_quickfix_GetterSetter_secondDeclarator()
void CppPlugin::test_quickfix_GenerateGetterSetter_secondDeclarator()
{
TestCase data("\n"
"class Something\n"
......@@ -470,12 +470,12 @@ void CppPlugin::test_quickfix_GetterSetter_secondDeclarator()
"\n"
;
GetterSetter factory(/*testMode=*/ true);
GenerateGetterSetter factory(/*testMode=*/ true);
data.run(&factory, expected);
}
/// Check: Quick fix is offered for "int *@it;" ('@' denotes the text cursor position)
void CppPlugin::test_quickfix_GetterSetter_triggeringRightAfterPointerSign()
void CppPlugin::test_quickfix_GenerateGetterSetter_triggeringRightAfterPointerSign()
{
TestCase data("\n"
"class Something\n"
......@@ -505,33 +505,33 @@ void CppPlugin::test_quickfix_GetterSetter_triggeringRightAfterPointerSign()
"\n"
;
GetterSetter factory(/*testMode=*/ true);
GenerateGetterSetter factory(/*testMode=*/ true);
data.run(&factory, expected);
}
/// Check: Quick fix is not triggered on a member function.
void CppPlugin::test_quickfix_GetterSetter_notTriggeringOnMemberFunction()
void CppPlugin::test_quickfix_GenerateGetterSetter_notTriggeringOnMemberFunction()
{
TestCase data("class Something { void @f(); };");
QByteArray expected = data.originalText;
GetterSetter factory(/*testMode=*/ true);
GenerateGetterSetter factory(/*testMode=*/ true);
data.run(&factory, expected, /*changesExpected=*/ false);
}
/// Check: Quick fix is not triggered on an member array;
void CppPlugin::test_quickfix_GetterSetter_notTriggeringOnMemberArray()
void CppPlugin::test_quickfix_GenerateGetterSetter_notTriggeringOnMemberArray()
{
TestCase data("class Something { void @a[10]; };");
QByteArray expected = data.originalText;
GetterSetter factory(/*testMode=*/ true);
GenerateGetterSetter factory(/*testMode=*/ true);
data.run(&factory, expected, /*changesExpected=*/ false);
}
/// Check: Do not offer the quick fix if there is already a member with the
/// getter or setter name we would generate.
void CppPlugin::test_quickfix_GetterSetter_notTriggeringWhenGetterOrSetterExist()
void CppPlugin::test_quickfix_GenerateGetterSetter_notTriggeringWhenGetterOrSetterExist()
{
TestCase data("\n"
"class Something {\n"
......@@ -540,6 +540,6 @@ void CppPlugin::test_quickfix_GetterSetter_notTriggeringWhenGetterOrSetterExist(
"};\n");
QByteArray expected = data.originalText;
GetterSetter factory(/*testMode=*/ true);
GenerateGetterSetter factory(/*testMode=*/ true);
data.run(&factory, expected, /*changesExpected=*/ false);
}
......@@ -27,20 +27,14 @@
**
****************************************************************************/
#include "cppcompleteswitch.h"
#include "cppquickfixes.h"
#include "cppcompleteswitch.h"
#include "cppeditor.h"
#include "cppfunctiondecldeflink.h"
#include "cppinsertdecldef.h"
#include "cppinsertqtpropertymembers.h"
#include "cppquickfixassistant.h"
#include "cppquickfix.h"
#include <AST.h>
#include <ASTMatcher.h>
#include <ASTPatternBuilder.h>
#include <ASTVisitor.h>
#include <CoreTypes.h>
#include <Literals.h>
#include <Name.h>
#include <Names.h>
......@@ -48,7 +42,6 @@
#include <Symbols.h>
#include <Token.h>
#include <TranslationUnit.h>
#include <Type.h>
#include <cplusplus/CppRewriter.h>
#include <cplusplus/DependencyTable.h>
......@@ -57,29 +50,58 @@
#include <cpptools/cppclassesfilter.h>
#include <cpptools/cppcodestylesettings.h>
#include <cpptools/cpppointerdeclarationformatter.h>
#include <cpptools/cpprefactoringchanges.h>
#include <cpptools/cpptoolsconstants.h>
#include <cpptools/cpptoolsreuse.h>
#include <cpptools/insertionpointlocator.h>
#include <cpptools/ModelManagerInterface.h>
#include <cpptools/searchsymbols.h>
#include <cpptools/symbolfinder.h>
#include <extensionsystem/iplugin.h>
#include <extensionsystem/pluginmanager.h>
#include <utils/changeset.h>
#include <utils/qtcassert.h>
#include <cctype>
#include <QApplication>
#include <QFileInfo>
#include <QSharedPointer>
#include <QTextBlock>
#include <QTextCursor>
using namespace CppEditor;
using namespace CppEditor::Internal;
using namespace CppTools;
using namespace CPlusPlus;
using namespace TextEditor;
using namespace Utils;
void CppEditor::Internal::registerQuickFixes(ExtensionSystem::IPlugin *plugIn)
{
plugIn->addAutoReleasedObject(new AddIncludeForUndefinedIdentifier);
plugIn->addAutoReleasedObject(new AddIncludeForForwardDeclaration);
plugIn->addAutoReleasedObject(new FlipLogicalOperands);
plugIn->addAutoReleasedObject(new InverseLogicalComparison);
plugIn->addAutoReleasedObject(new RewriteLogicalAnd);
plugIn->addAutoReleasedObject(new ConvertToCamelCase);
plugIn->addAutoReleasedObject(new ConvertCStringToNSString);
plugIn->addAutoReleasedObject(new ConvertNumericLiteral);
plugIn->addAutoReleasedObject(new TranslateStringLiteral);
plugIn->addAutoReleasedObject(new WrapStringLiteral);
plugIn->addAutoReleasedObject(new MoveDeclarationOutOfIf);
plugIn->addAutoReleasedObject(new MoveDeclarationOutOfWhile);
plugIn->addAutoReleasedObject(new SplitIfStatement);
plugIn->addAutoReleasedObject(new SplitSimpleDeclaration);
plugIn->addAutoReleasedObject(new AddLocalDeclaration);
plugIn->addAutoReleasedObject(new AddBracesToIf);
plugIn->addAutoReleasedObject(new RearrangeParamDeclarationList);
plugIn->addAutoReleasedObject(new ReformatPointerDeclaration);
plugIn->addAutoReleasedObject(new CompleteSwitchCaseStatement);
plugIn->addAutoReleasedObject(new InsertQtPropertyMembers);
plugIn->addAutoReleasedObject(new ApplyDeclDefLinkChanges);
plugIn->addAutoReleasedObject(new ExtractFunction);
plugIn->addAutoReleasedObject(new GenerateGetterSetter);
plugIn->addAutoReleasedObject(new InsertDeclFromDef);
plugIn->addAutoReleasedObject(new InsertDefFromDecl);
}
static inline bool isQtStringLiteral(const QByteArray &id)
{
......@@ -91,838 +113,702 @@ static inline bool isQtStringTranslation(const QByteArray &id)
return id == "tr" || id == "trUtf8" || id == "translate" || id == "QT_TRANSLATE_NOOP";
}
namespace {
/*
Rewrite
a op b -> !(a invop b)
(a op b) -> !(a invop b)
!(a op b) -> (a invob b)
Activates on: <= < > >= == !=
*/
class UseInverseOp: public CppQuickFixFactory
class InverseLogicalComparisonOp: public CppQuickFixOperation
{
public:
void match(const CppQuickFixInterface &interface, QuickFixOperations &result)
InverseLogicalComparisonOp(const CppQuickFixInterface &interface, int priority,
BinaryExpressionAST *binary, Kind invertToken)
: CppQuickFixOperation(interface, priority)
, binary(binary), nested(0), negation(0)
{
CppRefactoringFilePtr file = interface->currentFile();
Token tok;
tok.f.kind = invertToken;
replacement = QLatin1String(tok.spell());
const QList<AST *> &path = interface->path();
int index = path.size() - 1;
BinaryExpressionAST *binary = path.at(index)->asBinaryExpression();
if (! binary)
return;
if (! interface->isCursorOn(binary->binary_op_token))
return;
// check for enclosing nested expression
if (priority - 1 >= 0)
nested = interface->path()[priority - 1]->asNestedExpression();
Kind invertToken;
switch (file->tokenAt(binary->binary_op_token).kind()) {
case T_LESS_EQUAL:
invertToken = T_GREATER;
break;
case T_LESS:
invertToken = T_GREATER_EQUAL;
break;
case T_GREATER:
invertToken = T_LESS_EQUAL;
break;
case T_GREATER_EQUAL:
invertToken = T_LESS;
break;
case T_EQUAL_EQUAL:
invertToken = T_EXCLAIM_EQUAL;
break;
case T_EXCLAIM_EQUAL:
invertToken = T_EQUAL_EQUAL;
break;
default:
return;
// check for ! before parentheses
if (nested && priority - 2 >= 0) {
negation = interface->path()[priority - 2]->asUnaryExpression();
if (negation && ! interface->currentFile()->tokenAt(negation->unary_op_token).is(T_EXCLAIM))
negation = 0;
}
result.append(CppQuickFixOperation::Ptr(new Operation(interface, index, binary, invertToken)));
}
private:
class Operation: public CppQuickFixOperation
QString description() const
{
BinaryExpressionAST *binary;
NestedExpressionAST *nested;
UnaryExpressionAST *negation;
QString replacement;
return QApplication::translate("CppTools::QuickFix", "Rewrite Using %1").arg(replacement);
}
public:
Operation(const CppQuickFixInterface &interface,
int priority, BinaryExpressionAST *binary, Kind invertToken)
: CppQuickFixOperation(interface, priority)
, binary(binary), nested(0), negation(0)
{
Token tok;
tok.f.kind = invertToken;
replacement = QLatin1String(tok.spell());
// check for enclosing nested expression
if (priority - 1 >= 0)
nested = interface->path()[priority - 1]->asNestedExpression();
// check for ! before parentheses
if (nested && priority - 2 >= 0) {
negation = interface->path()[priority - 2]->asUnaryExpression();
if (negation && ! interface->currentFile()->tokenAt(negation->unary_op_token).is(T_EXCLAIM))
negation = 0;
}
void perform()
{
CppRefactoringChanges refactoring(snapshot());
CppRefactoringFilePtr currentFile = refactoring.file(fileName());
ChangeSet changes;
if (negation) {
// can't remove parentheses since that might break precedence
changes.remove(currentFile->range(negation->unary_op_token));
} else if (nested) {
changes.insert(currentFile->startOf(nested), QLatin1String("!"));
} else {
changes.insert(currentFile->startOf(binary), QLatin1String("!("));
changes.insert(currentFile->endOf(binary), QLatin1String(")"));
}
changes.replace(currentFile->range(binary->binary_op_token), replacement);
currentFile->setChangeSet(changes);
currentFile->apply();
}
virtual QString description() const
{
return QApplication::translate("CppTools::QuickFix", "Rewrite Using %1").arg(replacement);
}
private:
BinaryExpressionAST *binary;
NestedExpressionAST *nested;
UnaryExpressionAST *negation;
void perform()
{
CppRefactoringChanges refactoring(snapshot());
CppRefactoringFilePtr currentFile = refactoring.file(fileName());
ChangeSet changes;
if (negation) {
// can't remove parentheses since that might break precedence
changes.remove(currentFile->range(negation->unary_op_token));
} else if (nested) {
changes.insert(currentFile->startOf(nested), QLatin1String("!"));
} else {
changes.insert(currentFile->startOf(binary), QLatin1String("!("));
changes.insert(currentFile->endOf(binary), QLatin1String(")"));
}
changes.replace(currentFile->range(binary->binary_op_token), replacement);
currentFile->setChangeSet(changes);
currentFile->apply();
}
};
QString replacement;
};
/*
Rewrite
a op b
void InverseLogicalComparison::match(const CppQuickFixInterface &interface,
QuickFixOperations &result)
{
CppRefactoringFilePtr file = interface->currentFile();
const QList<AST *> &path = interface->path();
int index = path.size() - 1;
BinaryExpressionAST *binary = path.at(index)->asBinaryExpression();
if (! binary)
return;
if (! interface->isCursorOn(binary->binary_op_token))
return;
Kind invertToken;
switch (file->tokenAt(binary->binary_op_token).kind()) {
case T_LESS_EQUAL:
invertToken = T_GREATER;
break;
case T_LESS:
invertToken = T_GREATER_EQUAL;
break;
case T_GREATER:
invertToken = T_LESS_EQUAL;
break;
case T_GREATER_EQUAL:
invertToken = T_LESS;
break;
case T_EQUAL_EQUAL:
invertToken = T_EXCLAIM_EQUAL;
break;
case T_EXCLAIM_EQUAL: