From 809611f3461bfa9513d2620f62427cf175dea1bb Mon Sep 17 00:00:00 2001 From: Francois Ferrand <thetypz@gmail.com> Date: Mon, 18 Oct 2010 17:45:49 +0200 Subject: [PATCH] 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> --- share/qtcreator/styles/darkvim.xml | 1 + share/qtcreator/styles/grayscale.xml | 1 + share/qtcreator/styles/inkpot.xml | 1 + share/qtcreator/styles/intellij.xml | 1 + src/libs/cplusplus/CppDocument.h | 3 + src/plugins/cppeditor/cppeditor.cpp | 7 ++ src/plugins/cpptools/ModelManagerInterface.h | 3 +- src/plugins/cpptools/cppchecksymbols.cpp | 110 ++++++++++++------ src/plugins/cpptools/cppchecksymbols.h | 8 +- src/plugins/cpptools/cppmodelmanager.cpp | 16 ++- src/plugins/cpptools/cppsemanticinfo.h | 3 +- .../texteditor/texteditorconstants.cpp | 1 + src/plugins/texteditor/texteditorconstants.h | 1 + src/plugins/texteditor/texteditorsettings.cpp | 1 + 14 files changed, 108 insertions(+), 49 deletions(-) diff --git a/share/qtcreator/styles/darkvim.xml b/share/qtcreator/styles/darkvim.xml index afe1d939424..a7f0f924ab8 100644 --- a/share/qtcreator/styles/darkvim.xml +++ b/share/qtcreator/styles/darkvim.xml @@ -14,6 +14,7 @@ <style name="Doxygen.Comment" foreground="#55ffff"/> <style name="Doxygen.Tag" foreground="#00a0a0"/> <style name="Field"/> + <style name="Function"/> <style name="Keyword" foreground="#ffff55"/> <style name="Label" foreground="#ffff55"/> <style name="LineNumber" foreground="#888888" background="#232323"/> diff --git a/share/qtcreator/styles/grayscale.xml b/share/qtcreator/styles/grayscale.xml index b31ecd7815a..c1a29c20545 100644 --- a/share/qtcreator/styles/grayscale.xml +++ b/share/qtcreator/styles/grayscale.xml @@ -8,6 +8,7 @@ <style name="Doxygen.Comment" foreground="#808080"/> <style name="Doxygen.Tag" foreground="#808080" italic="true"/> <style name="Field"/> + <style name="Function"/> <style name="Keyword" bold="true"/> <style name="Label"/> <style name="LineNumber" foreground="#c7c4c1" background="#efebe7"/> diff --git a/share/qtcreator/styles/inkpot.xml b/share/qtcreator/styles/inkpot.xml index 7b24f8ada8a..730c170f56f 100644 --- a/share/qtcreator/styles/inkpot.xml +++ b/share/qtcreator/styles/inkpot.xml @@ -19,6 +19,7 @@ <style name="Doxygen.Comment" foreground="#737dd5"/> <style name="Doxygen.Tag" foreground="#4e5ab3"/> <style name="Field" bold="true"/> + <style name="Function"/> <style name="Keyword" foreground="#808bed"/> <style name="Label" foreground="#e76000"/> <style name="LineNumber" foreground="#8b8bcd" background="#2e2e2e"/> diff --git a/share/qtcreator/styles/intellij.xml b/share/qtcreator/styles/intellij.xml index b98500b620c..dfe5f7afd59 100644 --- a/share/qtcreator/styles/intellij.xml +++ b/share/qtcreator/styles/intellij.xml @@ -8,6 +8,7 @@ <style name="Doxygen.Comment" foreground="#808080" italic="true"/> <style name="Doxygen.Tag" foreground="#808080" bold="true" italic="true"/> <style name="Field" foreground="#660e7a" bold="true"/> + <style name="Function" foreground="#000000"/> <style name="Keyword" foreground="#000080" bold="true"/> <style name="Label" foreground="#800000" bold="true"/> <style name="Local" foreground="#000000"/> diff --git a/src/libs/cplusplus/CppDocument.h b/src/libs/cplusplus/CppDocument.h index 4147a95c34c..2922798547f 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 6b625dc2ddb..668886aafc1 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 b873e880fe9..8bab7086e83 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 f7bbb988855..050de221c1c 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 5e134e6c73f..2e4789433a5 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 0fae9871b46..79f91cbea8b 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 f7adb15d18b..612d3804626 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 a0831fcbd0f..066dd881749 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 d15cff50e68..7d23fb3d733 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 3620dea1910..118bb80d409 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); -- GitLab