CppEditor: Respect whitespace in operator names for more quick fixes

* Affected quick fixes: InsertDefFromDecl, MoveFuncDefOutside
* Fix also reformating pointer declaration of operator functions for
  qualified name ids

Change-Id: I6a7578f496221557d103f5fdbb5dacc9540ee779
Reviewed-by: Erik Verbruggen <erik.verbruggen@digia.com>

src/libs/cplusplus/NamePrettyPrinter.cpp

@@ -105,7 +105,9 @@ void NamePrettyPrinter::visit(const DestructorNameId *name)
 
 void NamePrettyPrinter::visit(const OperatorNameId *name)
 {
-    _name += QLatin1String("operator ");
+    _name += QLatin1String("operator");
+    if (_overview->includeWhiteSpaceInOperatorName)
+        _name += QLatin1Char(' ');
     switch (name->kind()) { // ### i should probably do this in OperatorNameId
     case OperatorNameId::InvalidOp:
         _name += QLatin1String("<invalid>");

src/libs/cplusplus/Overview.cpp

@@ -45,6 +45,7 @@ Overview::Overview()
       showFunctionSignatures(true),
       showDefaultArguments(true),
       showTemplateParameters(false),
+      includeWhiteSpaceInOperatorName(true),
       markedArgument(0),
       markedArgumentBegin(0),
       markedArgumentEnd(0)

src/libs/cplusplus/Overview.h

@@ -108,6 +108,7 @@ public:
     bool showFunctionSignatures: 1;
     bool showDefaultArguments: 1;
     bool showTemplateParameters: 1;
+    bool includeWhiteSpaceInOperatorName: 1; /// "operator =()" vs "operator=()"
 
     /*!
         You can get the start and end position of a function argument

src/plugins/cppeditor/cppeditorplugin.h

@@ -163,6 +163,8 @@ private slots:
     void test_quickfix_InsertDefFromDecl_notTriggeringWhenDefinitionExists();
     void test_quickfix_InsertDefFromDecl_notTriggeringStatement();
     void test_quickfix_InsertDefFromDecl_findRightImplementationFile();
+    void test_quickfix_InsertDefFromDecl_respectWsInOperatorNames1();
+    void test_quickfix_InsertDefFromDecl_respectWsInOperatorNames2();
 
     void test_quickfix_InsertDeclFromDef();
 
@@ -202,6 +204,8 @@ private slots:
     void test_quickfix_MoveFuncDefOutside_CtorWithInitialization1();
     void test_quickfix_MoveFuncDefOutside_CtorWithInitialization2();
     void test_quickfix_MoveFuncDefOutside_afterClass();
+    void test_quickfix_MoveFuncDefOutside_respectWsInOperatorNames1();
+    void test_quickfix_MoveFuncDefOutside_respectWsInOperatorNames2();
 
     void test_quickfix_MoveFuncDefToDecl_MemberFunc();
     void test_quickfix_MoveFuncDefToDecl_MemberFuncOutside();

src/plugins/cppeditor/cppquickfix_test.cpp

@@ -1178,6 +1178,58 @@ void CppEditorPlugin::test_quickfix_InsertDefFromDecl_findRightImplementationFil
     data.run(&factory);
 }
 
+/// Check if whitespace is respected for operator functions
+void CppEditorPlugin::test_quickfix_InsertDefFromDecl_respectWsInOperatorNames1()
+{
+    QByteArray original =
+        "class Foo\n"
+        "{\n"
+        "    Foo &opera@tor =();\n"
+        "};\n";
+    QByteArray expected =
+        "class Foo\n"
+        "{\n"
+        "    Foo &operator =();\n"
+        "};\n"
+        "\n"
+        "\n"
+        "Foo &Foo::operator =()\n"
+        "{\n"
+        "\n"
+        "}\n"
+        "\n";
+
+    InsertDefFromDecl factory;
+    TestCase data(original, expected);
+    data.run(&factory);
+}
+
+/// Check if whitespace is respected for operator functions
+void CppEditorPlugin::test_quickfix_InsertDefFromDecl_respectWsInOperatorNames2()
+{
+    QByteArray original =
+        "class Foo\n"
+        "{\n"
+        "    Foo &opera@tor=();\n"
+        "};\n";
+    QByteArray expected =
+        "class Foo\n"
+        "{\n"
+        "    Foo &operator=();\n"
+        "};\n"
+        "\n"
+        "\n"
+        "Foo &Foo::operator=()\n"
+        "{\n"
+        "\n"
+        "}\n"
+        "\n";
+
+    InsertDefFromDecl factory;
+    TestCase data(original, expected);
+    data.run(&factory);
+}
+
 // Function for one of InsertDeclDef section cases
 void insertToSectionDeclFromDef(const QByteArray &section, int sectionIndex)
 {
@@ -2458,6 +2510,52 @@ void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_afterClass()
     data.run(&factory, 1);
 }
 
+/// Check if whitespace is respected for operator functions
+void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_respectWsInOperatorNames1()
+{
+    QByteArray original =
+        "class Foo\n"
+        "{\n"
+        "    Foo &opera@tor =() {}\n"
+        "};\n";
+    QByteArray expected =
+        "class Foo\n"
+        "{\n"
+        "    Foo &operator =();\n"
+        "};\n"
+        "\n"
+        "\n"
+        "Foo &Foo::operator =() {}\n"
+        "\n";
+
+    MoveFuncDefOutside factory;
+    TestCase data(original, expected);
+    data.run(&factory);
+}
+
+/// Check if whitespace is respected for operator functions
+void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_respectWsInOperatorNames2()
+{
+    QByteArray original =
+        "class Foo\n"
+        "{\n"
+        "    Foo &opera@tor=() {}\n"
+        "};\n";
+    QByteArray expected =
+        "class Foo\n"
+        "{\n"
+        "    Foo &operator=();\n"
+        "};\n"
+        "\n"
+        "\n"
+        "Foo &Foo::operator=() {}\n"
+        "\n";
+
+    MoveFuncDefOutside factory;
+    TestCase data(original, expected);
+    data.run(&factory);
+}
+
 /// Check: revert test_quickfix_MoveFuncDefOutside_MemberFuncToCpp()
 void CppEditorPlugin::test_quickfix_MoveFuncDefToDecl_MemberFunc()
 {

src/plugins/cppeditor/cppquickfixes.cpp

@@ -258,6 +258,12 @@ void insertNewIncludeDirective(const QString &include, CppRefactoringFilePtr fil
     file->apply();
 }
 
+bool nameIncludesOperatorName(const Name *name)
+{
+    return name->isOperatorNameId()
+        || (name->isQualifiedNameId() && name->asQualifiedNameId()->name()->isOperatorNameId());
+}
+
 } // anonymous namespace
 
 namespace {
@@ -2482,10 +2488,12 @@ class InsertDefOperation: public CppQuickFixOperation
 public:
     // Make sure that either loc is valid or targetFileName is not empty.
     InsertDefOperation(const QSharedPointer<const CppQuickFixAssistInterface> &interface,
-                       Declaration *decl, const InsertionLocation &loc, const DefPos defpos,
+                       Declaration *decl, DeclaratorAST *declAST, const InsertionLocation &loc,
-                       const QString &targetFileName = QString(), bool freeFunction = false)
+                       const DefPos defpos, const QString &targetFileName = QString(),
+                       bool freeFunction = false)
         : CppQuickFixOperation(interface, 0)
         , m_decl(decl)
+        , m_declAST(declAST)
         , m_loc(loc)
         , m_defpos(defpos)
         , m_targetFileName(targetFileName)
@@ -2558,6 +2566,11 @@ public:
             const FullySpecifiedType tn = rewriteType(m_decl->type(), &env, control);
 
             // rewrite the function name
+            if (nameIncludesOperatorName(m_decl->name())) {
+                CppRefactoringFilePtr file = refactoring.file(fileName());
+                const QString operatorNameText = file->textOf(m_declAST->core_declarator);
+                oo.includeWhiteSpaceInOperatorName = operatorNameText.contains(QLatin1Char(' '));
+            }
             const QString name = oo.prettyName(LookupContext::minimalName(m_decl, targetCoN,
                                                                           control));
 
@@ -2590,6 +2603,7 @@ public:
 
 private:
     Declaration *m_decl;
+    DeclaratorAST *m_declAST;
     InsertionLocation m_loc;
     const DefPos m_defpos;
     const QString m_targetFileName;
@@ -2622,6 +2636,7 @@ void InsertDefFromDecl::match(const CppQuickFixInterface &interface, QuickFixOpe
                             }
 
                             // Insert Position: Implementation File
+                            DeclaratorAST *declAST = simpleDecl->declarator_list->value;
                             InsertDefOperation *op = 0;
                             ProjectFile::Kind kind = ProjectFile::classify(interface->fileName());
                             const bool isHeaderFile = ProjectFile::isHeader(kind);
@@ -2634,7 +2649,7 @@ void InsertDefFromDecl::match(const CppQuickFixInterface &interface, QuickFixOpe
                                 foreach (const InsertionLocation &location,
                                          locator.methodDefinition(decl, false, QString())) {
                                     if (location.isValid()) {
-                                        op = new InsertDefOperation(interface, decl,
+                                        op = new InsertDefOperation(interface, decl, declAST,
                                                                     InsertionLocation(),
                                                                     DefPosImplementationFile,
                                                                     location.fileName());
@@ -2649,8 +2664,8 @@ void InsertDefFromDecl::match(const CppQuickFixInterface &interface, QuickFixOpe
 
                             // Insert Position: Outside Class
                             if (!isFreeFunction) {
-                                op = new InsertDefOperation(interface, decl, InsertionLocation(),
+                                op = new InsertDefOperation(interface, decl, declAST,
-                                                            DefPosOutsideClass,
+                                                            InsertionLocation(), DefPosOutsideClass,
                                                             interface->fileName());
                                 result.append(CppQuickFixOperation::Ptr(op));
                             }
@@ -2663,8 +2678,9 @@ void InsertDefFromDecl::match(const CppQuickFixInterface &interface, QuickFixOpe
                             const InsertionLocation loc
                                     = InsertionLocation(interface->fileName(), QString(), QString(),
                                                         line, column);
-                            op = new InsertDefOperation(interface, decl, loc, DefPosInsideClass,
+                            op = new InsertDefOperation(interface, decl, declAST, loc,
-                                                        QString() , isFreeFunction);
+                                                        DefPosInsideClass, QString(),
+                                                        isFreeFunction);
                             result.append(CppQuickFixOperation::Ptr(op));
 
                             return;
@@ -3719,14 +3735,19 @@ void ApplyDeclDefLinkChanges::match(const CppQuickFixInterface &interface,
 
 namespace {
 
-QString getDefinitionSignature(const CppQuickFixAssistInterface *assist, Function *func,
+QString definitionSignature(const CppQuickFixAssistInterface *assist,
-                               CppRefactoringFilePtr &file, Scope *scope)
+                            FunctionDefinitionAST *functionDefinitionAST,
+                            CppRefactoringFilePtr &baseFile,
+                            CppRefactoringFilePtr &targetFile,
+                            Scope *scope)
 {
     QTC_ASSERT(assist, return QString());
-    QTC_ASSERT(func, return QString());
+    QTC_ASSERT(functionDefinitionAST, return QString());
     QTC_ASSERT(scope, return QString());
+    Function *func = functionDefinitionAST->symbol;
+    QTC_ASSERT(func, return QString());
 
-    LookupContext cppContext(file->cppDocument(), assist->snapshot());
+    LookupContext cppContext(targetFile->cppDocument(), assist->snapshot());
     ClassOrNamespace *cppCoN = cppContext.lookupType(scope);
     if (!cppCoN)
         cppCoN = cppContext.globalNamespace();
@@ -3740,10 +3761,16 @@ QString getDefinitionSignature(const CppQuickFixAssistInterface *assist, Functio
     oo.showFunctionSignatures = true;
     oo.showReturnTypes = true;
     oo.showArgumentNames = true;
+    const Name *name = func->name();
+    if (nameIncludesOperatorName(name)) {
+        CoreDeclaratorAST *coreDeclarator = functionDefinitionAST->declarator->core_declarator;
+        const QString operatorNameText = baseFile->textOf(coreDeclarator);
+        oo.includeWhiteSpaceInOperatorName = operatorNameText.contains(QLatin1Char(' '));
+    }
+    const QString nameText = oo.prettyName(LookupContext::minimalName(func, cppCoN, control));
     const FullySpecifiedType tn = rewriteType(func->type(), &env, control);
-    const QString name = oo.prettyName(LookupContext::minimalName(func, cppCoN, control));
 
-    return oo.prettyType(tn, name);
+    return oo.prettyType(tn, nameText);
 }
 
 class MoveFuncDefOutsideOp : public CppQuickFixOperation
@@ -3791,8 +3818,9 @@ public:
         Scope *scopeAtInsertPos = toFile->cppDocument()->scopeAt(l.line(), l.column());
 
         // construct definition
-        const QString funcDec = getDefinitionSignature(assistInterface(), m_func, toFile,
+        const QString funcDec = definitionSignature(assistInterface(), m_funcDef,
-                                                       scopeAtInsertPos);
+                                                    fromFile, toFile,
+                                                    scopeAtInsertPos);
         QString funcDef = prefix + funcDec;
         const int startPosition = fromFile->endOf(m_funcDef->declarator);
         const int endPosition = fromFile->endOf(m_funcDef->function_body);

src/plugins/cpptools/cpppointerdeclarationformatter.cpp

@@ -115,7 +115,7 @@ static unsigned firstTypeSpecifierWithoutFollowingAttribute(
 
 PointerDeclarationFormatter::PointerDeclarationFormatter(
         const CppRefactoringFilePtr refactoringFile,
-        const Overview &overview,
+        Overview &overview,
         CursorHandling cursorHandling)
     : ASTVisitor(refactoringFile->cppDocument()->translationUnit())
     , m_cppRefactoringFile(refactoringFile)
@@ -413,21 +413,14 @@ void PointerDeclarationFormatter::checkAndRewrite(DeclaratorAST *declarator,
     QString rewrittenDeclaration;
     const Name *name = symbol->name();
     if (name) {
-        if (name->isOperatorNameId()) {
+        if (name->isOperatorNameId()
-            // Take the operator name from the file instead from the AST, so the white
+                || (name->isQualifiedNameId()
-            // spaces within the operator names can be respected, e.g. in "operator =".
+                    && name->asQualifiedNameId()->name()->isOperatorNameId())) {
-            const QByteArray operatorText
+            const QString operatorText = m_cppRefactoringFile->textOf(declarator->core_declarator);
-                    = m_cppRefactoringFile->textOf(declarator->core_declarator).toLatin1();
+            m_overview.includeWhiteSpaceInOperatorName = operatorText.contains(QLatin1Char(' '));
-            Identifier operatorName(operatorText.constData(), operatorText.size());
-            rewrittenDeclaration = rewriteDeclaration(type, &operatorName);
-        } else {
-            rewrittenDeclaration = rewriteDeclaration(type, name);
         }
-    } else {
-        // The declaration will be correctly rewritten for name == 0 (e.g. "int *").
-        rewrittenDeclaration = rewriteDeclaration(type, name);
     }
-
+    rewrittenDeclaration = m_overview.prettyType(type, name);
     rewrittenDeclaration.remove(0, charactersToRemove);
 
     CHECK_R(originalDeclaration != rewrittenDeclaration, "Rewritten is same as original");
@@ -460,21 +453,6 @@ void PointerDeclarationFormatter::checkAndRewrite(DeclaratorAST *declarator,
         qDebug() << "Replacement operation failed";
 }
 
-/*! Rewrite/format the given type and name. */
-QString PointerDeclarationFormatter::rewriteDeclaration(FullySpecifiedType type, const Name *name)
-    const
-{
-    CHECK_RV(type.isValid(), "Invalid type", QString());
-
-    const char *identifier = 0;
-    if (const Name *declarationName = name) {
-        if (const Identifier *id = declarationName->identifier())
-            identifier = id->chars();
-    }
-
-    return m_overview.prettyType(type, QLatin1String(identifier));
-}
-
 void PointerDeclarationFormatter::printCandidate(AST *ast)
 {
 #if DEBUG_OUTPUT

src/plugins/cpptools/cpppointerdeclarationformatter.h

@@ -78,7 +78,7 @@ public:
     enum CursorHandling { RespectCursor, IgnoreCursor };
 
     explicit PointerDeclarationFormatter(const CppRefactoringFilePtr refactoringFile,
-                                         const Overview &overview,
+                                         Overview &overview,
                                          CursorHandling cursorHandling = IgnoreCursor);
 
     /*!
@@ -113,11 +113,10 @@ private:
     void processIfWhileForStatement(ExpressionAST *expression, Symbol *symbol);
     void checkAndRewrite(DeclaratorAST *declarator, Symbol *symbol, TokenRange range,
                          unsigned charactersToRemove = 0);
-    QString rewriteDeclaration(FullySpecifiedType type, const Name *name) const;
     void printCandidate(AST *ast);
 
     const CppRefactoringFilePtr m_cppRefactoringFile;
-    const Overview &m_overview;
+    Overview &m_overview;
     const CursorHandling m_cursorHandling;
 
     ChangeSet m_changeSet;

src/plugins/cpptools/cpppointerdeclarationformatter_test.cpp

@@ -349,9 +349,13 @@ void CppToolsPlugin::test_format_pointerdeclaration_in_simpledeclarations_data()
         << source << stripCursor(source);
 
     // Respect white space within operator names
-    QTest::newRow("operators")
+    QTest::newRow("operators1")
         << "class C { C@&operator = (const C &); };"
         << "class C { C & operator = (const C &); };";
+
+    QTest::newRow("operators2")
+        << "C &C::operator = (const C &) {}"
+        << "C & C::operator = (const C &) {}";
 }
 
 void CppToolsPlugin::test_format_pointerdeclaration_in_controlflowstatements()