CppHighlighter: highlight all functions/methods.

- Highlight all function/methods (not just virtual methods).
- Highlight as a function even if number of arguments does not match. In
  that case, add a diagnostic message to indicate there are too many/too
  few arguments.
- Fix highlighting of parameters in function declarations.
  These used to be handled indiferently, and they could be mistaken for
  type or field references.
- Properly highlight template method calls.

Change-Id: I6e61c9ee47763db95c62314f9cc1c4d398df38b3
Reviewed-by: Leandro Melo <leandro.melo@nokia.com>

10 files changed, 104 insertions, 49 deletions

diff --git a/src/libs/cplusplus/CppDocument.h b/src/libs/cplusplus/CppDocument.h
index 4147a95c34..2922798547 100644
--- a/src/libs/cplusplus/CppDocument.h
+++ b/src/libs/cplusplus/CppDocument.h
@@ -192,6 +192,9 @@ public:
     void addDiagnosticMessage(const DiagnosticMessage &d)
     { _diagnosticMessages.append(d); }
 
+    void clearDiagnosticMessages()
+    { _diagnosticMessages.clear(); }
+
     QList<DiagnosticMessage> diagnosticMessages() const
     { return _diagnosticMessages; }
 
diff --git a/src/plugins/cppeditor/cppeditor.cpp b/src/plugins/cppeditor/cppeditor.cpp
index 6b625dc2dd..668886aafc 100644
--- a/src/plugins/cppeditor/cppeditor.cpp
+++ b/src/plugins/cppeditor/cppeditor.cpp
@@ -1047,6 +1047,11 @@ void CPPEditorWidget::finishHighlightSymbolUsages()
 
     TextEditor::SemanticHighlighter::clearExtraAdditionalFormatsUntilEnd(
                 highlighter, m_highlighter);
+
+    if (m_modelManager)
+        m_modelManager->setExtraDiagnostics(m_lastSemanticInfo.doc->fileName(),
+                                            CPlusPlus::CppModelManagerInterface::CppSemanticsDiagnostic,
+                                            m_lastSemanticInfo.doc->diagnosticMessages());
 }
 
 
@@ -1744,6 +1749,8 @@ void CPPEditorWidget::setFontSettings(const TextEditor::FontSettings &fs)
             fs.toTextCharFormat(TextEditor::C_LABEL);
     m_semanticHighlightFormatMap[SemanticInfo::MacroUse] =
             fs.toTextCharFormat(TextEditor::C_PREPROCESSOR);
+    m_semanticHighlightFormatMap[SemanticInfo::FunctionUse] =
+            fs.toTextCharFormat(TextEditor::C_FUNCTION);
     m_keywordFormat = fs.toTextCharFormat(TextEditor::C_KEYWORD);
 
     // only set the background, we do not want to modify foreground properties set by the syntax highlighter or the link
diff --git a/src/plugins/cpptools/ModelManagerInterface.h b/src/plugins/cpptools/ModelManagerInterface.h
index b873e880fe..8bab7086e8 100644
--- a/src/plugins/cpptools/ModelManagerInterface.h
+++ b/src/plugins/cpptools/ModelManagerInterface.h
@@ -179,7 +179,8 @@ public:
     enum ExtraDiagnosticKind
     {
         AllExtraDiagnostics = -1,
-        ExportedQmlTypesDiagnostic
+        ExportedQmlTypesDiagnostic,
+        CppSemanticsDiagnostic
     };
 
 public:
diff --git a/src/plugins/cpptools/cppchecksymbols.cpp b/src/plugins/cpptools/cppchecksymbols.cpp
index f7bbb98885..050de221c1 100644
--- a/src/plugins/cpptools/cppchecksymbols.cpp
+++ b/src/plugins/cpptools/cppchecksymbols.cpp
@@ -68,7 +68,7 @@ class CollectSymbols: protected SymbolVisitor
     Snapshot _snapshot;
     QSet<QByteArray> _types;
     QSet<QByteArray> _members;
-    QSet<QByteArray> _virtualMethods;
+    QSet<QByteArray> _functions;
     QSet<QByteArray> _statics;
     bool _mainDocument;
 
@@ -90,9 +90,9 @@ public:
         return _members;
     }
 
-    const QSet<QByteArray> &virtualMethods() const
+    const QSet<QByteArray> &functions() const
     {
-        return _virtualMethods;
+        return _functions;
     }
 
     const QSet<QByteArray> &statics() const
@@ -149,15 +149,14 @@ protected:
         }
     }
 
-    void addVirtualMethod(const Name *name)
+    void addFunction(const Name *name)
     {
         if (! name) {
             return;
 
         } else if (name->isNameId()) {
             const Identifier *id = name->identifier();
-            _virtualMethods.insert(QByteArray::fromRawData(id->chars(), id->size()));
-
+            _functions.insert(QByteArray::fromRawData(id->chars(), id->size()));
         }
     }
 
@@ -181,9 +180,7 @@ protected:
 
     virtual bool visit(Function *symbol)
     {
-        if (symbol->isVirtual())
-            addVirtualMethod(symbol->name());
-
+        addFunction(symbol->name());
         return true;
     }
 
@@ -203,10 +200,8 @@ protected:
         if (symbol->enclosingEnum() != 0)
             addStatic(symbol->name());
 
-        if (Function *funTy = symbol->type()->asFunctionType()) {
-            if (funTy->isVirtual())
-                addVirtualMethod(symbol->name());
-        }
+        if (symbol->type()->isFunctionType())
+            addFunction(symbol->name());
 
         if (symbol->isTypedef())
             addType(symbol->name());
@@ -305,7 +300,7 @@ CheckSymbols::CheckSymbols(Document::Ptr doc, const LookupContext &context, cons
     _fileName = doc->fileName();
     _potentialTypes = collectTypes.types();
     _potentialMembers = collectTypes.members();
-    _potentialVirtualMethods = collectTypes.virtualMethods();
+    _potentialFunctions = collectTypes.functions();
     _potentialStatics = collectTypes.statics();
 
     typeOfExpression.init(_doc, _context.snapshot(), _context.bindings());
@@ -317,7 +312,7 @@ CheckSymbols::~CheckSymbols()
 void CheckSymbols::run()
 {
     qSort(_macroUses.begin(), _macroUses.end(), sortByLinePredicate);
-    _diagnosticMessages.clear();
+    _doc->clearDiagnosticMessages();
 
     if (! isCanceled()) {
         if (_doc->translationUnit()) {
@@ -333,7 +328,7 @@ void CheckSymbols::run()
 bool CheckSymbols::warning(unsigned line, unsigned column, const QString &text, unsigned length)
 {
     Document::DiagnosticMessage m(Document::DiagnosticMessage::Warning, _fileName, line, column, text, length);
-    _diagnosticMessages.append(m);
+    _doc->addDiagnosticMessage(m);
     return false;
 }
 
@@ -482,8 +477,8 @@ bool CheckSymbols::visit(SimpleDeclarationAST *ast)
                 if (Function *funTy = decl->type()->asFunctionType()) {
                     if (funTy->isVirtual()) {
                         addUse(declId, SemanticInfo::VirtualMethodUse);
-                    } else if (maybeVirtualMethod(decl->name())) {
-                        addVirtualMethod(_context.lookup(decl->name(), decl->enclosingScope()), declId, funTy->argumentCount());
+                    } else {
+                        addFunction(_context.lookup(decl->name(), decl->enclosingScope()), declId, funTy->argumentCount());
                     }
                 }
             }
@@ -542,7 +537,7 @@ bool CheckSymbols::visit(CallAST *ast)
 
         if (MemberAccessAST *access = ast->base_expression->asMemberAccess()) {
             if (access->member_name && access->member_name->name) {
-                if (maybeVirtualMethod(access->member_name->name)) {
+                if (maybeFunction(access->member_name->name)) {
                     const QByteArray expression = textOf(access);
 
                     const QList<LookupItem> candidates =
@@ -553,12 +548,12 @@ bool CheckSymbols::visit(CallAST *ast)
                     if (QualifiedNameAST *q = memberName->asQualifiedName())
                         memberName = q->unqualified_name;
 
-                    addVirtualMethod(candidates, memberName, argumentCount);
+                    addFunction(candidates, memberName, argumentCount);
                 }
             }
         } else if (IdExpressionAST *idExpr = ast->base_expression->asIdExpression()) {
             if (const Name *name = idExpr->name->name) {
-                if (maybeVirtualMethod(name)) {
+                if (maybeFunction(name)) {
                     NameAST *exprName = idExpr->name;
                     if (QualifiedNameAST *q = exprName->asQualifiedName())
                         exprName = q->unqualified_name;
@@ -566,7 +561,7 @@ bool CheckSymbols::visit(CallAST *ast)
                     const QList<LookupItem> candidates =
                         typeOfExpression(textOf(idExpr), enclosingScope(),
                                          TypeOfExpression::Preprocess);
-                    addVirtualMethod(candidates, exprName, argumentCount);
+                    addFunction(candidates, exprName, argumentCount);
                 }
             }
         }
@@ -658,6 +653,8 @@ void CheckSymbols::checkName(NameAST *ast, Scope *scope)
             Class *klass = scope->asClass();
             if (hasVirtualDestructor(_context.lookupType(klass)))
                 addUse(ast, SemanticInfo::VirtualMethodUse);
+            else
+                addUse(ast, SemanticInfo::FunctionUse);
         } else if (maybeType(ast->name) || maybeStatic(ast->name)) {
             const QList<LookupItem> candidates = _context.lookup(ast->name, scope);
             addTypeOrStatic(candidates, ast);
@@ -686,6 +683,14 @@ bool CheckSymbols::visit(DestructorNameAST *ast)
     return true;
 }
 
+bool CheckSymbols::visit(ParameterDeclarationAST *ast)
+{
+    accept(ast->type_specifier_list);
+    // Skip parameter name, it does not need to be colored
+    accept(ast->expression);
+    return false;
+}
+
 bool CheckSymbols::visit(QualifiedNameAST *ast)
 {
     if (ast->name) {
@@ -729,6 +734,8 @@ bool CheckSymbols::visit(QualifiedNameAST *ast)
             if (ast->unqualified_name->asDestructorName() != 0) {
                 if (hasVirtualDestructor(binding))
                     addUse(ast->unqualified_name, SemanticInfo::VirtualMethodUse);
+                else
+                    addUse(ast->unqualified_name, SemanticInfo::FunctionUse);
             } else {
                 addTypeOrStatic(binding->find(ast->unqualified_name->name), ast->unqualified_name);
             }
@@ -807,8 +814,8 @@ bool CheckSymbols::visit(FunctionDefinitionAST *ast)
 
             if (fun->isVirtual()) {
                 addUse(declId, SemanticInfo::VirtualMethodUse);
-            } else if (maybeVirtualMethod(fun->name())) {
-                addVirtualMethod(_context.lookup(fun->name(), fun->enclosingScope()), declId, fun->argumentCount());
+            } else {
+                addFunction(_context.lookup(fun->name(), fun->enclosingScope()), declId, fun->argumentCount());
             }
         }
     }
@@ -1022,7 +1029,7 @@ void CheckSymbols::addStatic(const QList<LookupItem> &candidates, NameAST *ast)
     }
 }
 
-void CheckSymbols::addVirtualMethod(const QList<LookupItem> &candidates, NameAST *ast, unsigned argumentCount)
+void CheckSymbols::addFunction(const QList<LookupItem> &candidates, NameAST *ast, unsigned argumentCount)
 {
     unsigned startToken = ast->firstToken();
     if (DestructorNameAST *dtor = ast->asDestructorName())
@@ -1033,30 +1040,57 @@ void CheckSymbols::addVirtualMethod(const QList<LookupItem> &candidates, NameAST
     if (tok.generated())
         return;
 
+    enum { Match_None, Match_TooManyArgs, Match_TooFewArgs, Match_Ok } matchType = Match_None;
+    SemanticInfo::UseKind kind = SemanticInfo::FunctionUse;
     foreach (const LookupItem &r, candidates) {
         Symbol *c = r.declaration();
         if (! c)
             continue;
 
-        Function *funTy = r.type()->asFunctionType();
+        Function *funTy = c->type()->asFunctionType();
+        if (! funTy) {
+            //Try to find a template function
+            if (Template * t = r.type()->asTemplateType())
+                if ((c = t->declaration()))
+                    funTy = c->type()->asFunctionType();
+        }
         if (! funTy)
-            continue;
-        if (! funTy->isVirtual())
-            continue;
-        else if (argumentCount < funTy->minimumArgumentCount())
-            continue;
-        else if (argumentCount > funTy->argumentCount()) {
-            if (! funTy->isVariadic())
-                continue;
+            continue; // TODO: add diagnostic messages and color call-operators calls too?
+
+        if (argumentCount < funTy->minimumArgumentCount()) {
+            if (matchType != Match_Ok) {
+                kind = funTy->isVirtual() ? SemanticInfo::VirtualMethodUse : SemanticInfo::FunctionUse;
+                matchType = Match_TooFewArgs;
+            }
+        } else if (argumentCount > funTy->argumentCount() && ! funTy->isVariadic()) {
+            if (matchType != Match_Ok) {
+                matchType = Match_TooManyArgs;
+                kind = funTy->isVirtual() ? SemanticInfo::VirtualMethodUse : SemanticInfo::FunctionUse;
+            }
+        } else if (!funTy->isVirtual()) {
+            matchType = Match_Ok;
+            kind = SemanticInfo::FunctionUse;
+            //continue, to check if there is a matching candidate which is virtual
+        } else {
+            matchType = Match_Ok;
+            kind = SemanticInfo::VirtualMethodUse;
+            break;
         }
+    }
 
+    if (matchType != Match_None) {
         unsigned line, column;
         getTokenStartPosition(startToken, &line, &column);
         const unsigned length = tok.length();
 
-        const Use use(line, column, length, SemanticInfo::VirtualMethodUse);
+        // Add a diagnostic message if argument count does not match
+        if (matchType == Match_TooFewArgs)
+            warning(line, column, "Too few arguments", length);
+        else if (matchType == Match_TooManyArgs)
+            warning(line, column, "Too many arguments", length);
+
+        const Use use(line, column, length, kind);
         addUse(use);
-        break;
     }
 }
 
@@ -1112,12 +1146,12 @@ bool CheckSymbols::maybeStatic(const Name *name) const
     return false;
 }
 
-bool CheckSymbols::maybeVirtualMethod(const Name *name) const
+bool CheckSymbols::maybeFunction(const Name *name) const
 {
     if (name) {
         if (const Identifier *ident = name->identifier()) {
             const QByteArray id = QByteArray::fromRawData(ident->chars(), ident->size());
-            if (_potentialVirtualMethods.contains(id))
+            if (_potentialFunctions.contains(id))
                 return true;
         }
     }
diff --git a/src/plugins/cpptools/cppchecksymbols.h b/src/plugins/cpptools/cppchecksymbols.h
index 5e134e6c73..2e4789433a 100644
--- a/src/plugins/cpptools/cppchecksymbols.h
+++ b/src/plugins/cpptools/cppchecksymbols.h
@@ -107,7 +107,7 @@ protected:
     bool maybeType(const Name *name) const;
     bool maybeMember(const Name *name) const;
     bool maybeStatic(const Name *name) const;
-    bool maybeVirtualMethod(const Name *name) const;
+    bool maybeFunction(const Name *name) const;
 
     void checkName(NameAST *ast, Scope *scope = 0);
     void checkNamespace(NameAST *name);
@@ -121,7 +121,7 @@ protected:
     void addTypeOrStatic(const QList<LookupItem> &candidates, NameAST *ast);
     void addStatic(const QList<LookupItem> &candidates, NameAST *ast);
     void addClassMember(const QList<LookupItem> &candidates, NameAST *ast);
-    void addVirtualMethod(const QList<LookupItem> &candidates, NameAST *ast, unsigned argumentCount);
+    void addFunction(const QList<LookupItem> &candidates, NameAST *ast, unsigned argumentCount);
 
     bool isTemplateClass(Symbol *s) const;
 
@@ -142,6 +142,7 @@ protected:
 
     virtual bool visit(SimpleNameAST *ast);
     virtual bool visit(DestructorNameAST *ast);
+    virtual bool visit(ParameterDeclarationAST *ast);
     virtual bool visit(QualifiedNameAST *ast);
     virtual bool visit(TemplateIdAST *ast);
 
@@ -166,10 +167,9 @@ private:
     LookupContext _context;
     TypeOfExpression typeOfExpression;
     QString _fileName;
-    QList<Document::DiagnosticMessage> _diagnosticMessages;
     QSet<QByteArray> _potentialTypes;
     QSet<QByteArray> _potentialMembers;
-    QSet<QByteArray> _potentialVirtualMethods;
+    QSet<QByteArray> _potentialFunctions;
     QSet<QByteArray> _potentialStatics;
     QList<AST *> _astStack;
     QVector<Use> _usages;
diff --git a/src/plugins/cpptools/cppmodelmanager.cpp b/src/plugins/cpptools/cppmodelmanager.cpp
index 0fae9871b4..79f91cbea8 100644
--- a/src/plugins/cpptools/cppmodelmanager.cpp
+++ b/src/plugins/cpptools/cppmodelmanager.cpp
@@ -1154,13 +1154,19 @@ void CppModelManager::updateEditor(Document::Ptr doc)
 
                     QTextCursor c(ed->document()->findBlockByNumber(m.line() - 1));
                     const QString text = c.block().text();
-                    for (int i = 0; i < text.size(); ++i) {
-                        if (! text.at(i).isSpace()) {
-                            c.setPosition(c.position() + i);
-                            break;
+                    if (m.length() > 0 && m.column() + m.length() < (unsigned)text.size()) {
+                        int column = m.column() > 0 ? m.column() - 1 : 0;
+                        c.setPosition(c.position() + column);
+                        c.movePosition(QTextCursor::NextCharacter, QTextCursor::KeepAnchor, m.length());
+                    } else {
+                        for (int i = 0; i < text.size(); ++i) {
+                            if (! text.at(i).isSpace()) {
+                                c.setPosition(c.position() + i);
+                                break;
+                            }
                         }
+                        c.movePosition(QTextCursor::EndOfBlock, QTextCursor::KeepAnchor);
                     }
-                    c.movePosition(QTextCursor::EndOfBlock, QTextCursor::KeepAnchor);
                     sel.cursor = c;
                     sel.format.setToolTip(m.text());
                     e.selections.append(sel);
diff --git a/src/plugins/cpptools/cppsemanticinfo.h b/src/plugins/cpptools/cppsemanticinfo.h
index f7adb15d18..612d380462 100644
--- a/src/plugins/cpptools/cppsemanticinfo.h
+++ b/src/plugins/cpptools/cppsemanticinfo.h
@@ -53,7 +53,8 @@ public:
         StaticUse,
         VirtualMethodUse,
         LabelUse,
-        MacroUse
+        MacroUse,
+        FunctionUse
     };
     typedef TextEditor::SemanticHighlighter::Result Use;
 
diff --git a/src/plugins/texteditor/texteditorconstants.cpp b/src/plugins/texteditor/texteditorconstants.cpp
index a0831fcbd0..066dd88174 100644
--- a/src/plugins/texteditor/texteditorconstants.cpp
+++ b/src/plugins/texteditor/texteditorconstants.cpp
@@ -61,6 +61,7 @@ const char *nameForStyle(TextStyle style)
     case C_FIELD:               return "Field";
     case C_STATIC:              return "Static";
     case C_VIRTUAL_METHOD:      return "VirtualMethod";
+    case C_FUNCTION:            return "Function";
     case C_KEYWORD:             return "Keyword";
     case C_OPERATOR:            return "Operator";
     case C_PREPROCESSOR:        return "Preprocessor";
diff --git a/src/plugins/texteditor/texteditorconstants.h b/src/plugins/texteditor/texteditorconstants.h
index d15cff50e6..7d23fb3d73 100644
--- a/src/plugins/texteditor/texteditorconstants.h
+++ b/src/plugins/texteditor/texteditorconstants.h
@@ -60,6 +60,7 @@ enum TextStyle {
     C_FIELD,
     C_STATIC,
     C_VIRTUAL_METHOD,
+    C_FUNCTION,
     C_KEYWORD,
     C_OPERATOR,
     C_PREPROCESSOR,
diff --git a/src/plugins/texteditor/texteditorsettings.cpp b/src/plugins/texteditor/texteditorsettings.cpp
index 3620dea191..118bb80d40 100644
--- a/src/plugins/texteditor/texteditorsettings.cpp
+++ b/src/plugins/texteditor/texteditorsettings.cpp
@@ -145,6 +145,7 @@ TextEditorSettings::TextEditorSettings(QObject *parent)
     formatDescriptions.append(FormatDescription(C_FIELD, tr("Field"), Qt::darkRed));
     formatDescriptions.append(FormatDescription(C_STATIC, tr("Static"), Qt::darkMagenta));
 
+    formatDescriptions.append(FormatDescription(C_FUNCTION, tr("Function")));
     FormatDescription virtualMethodFormatDescriptor(C_VIRTUAL_METHOD, tr("Virtual Method"));
     virtualMethodFormatDescriptor.format().setItalic(true);
     formatDescriptions.append(virtualMethodFormatDescriptor);