diff --git a/src/libs/cplusplus/CMakeLists.txt b/src/libs/cplusplus/CMakeLists.txt index c99a66c1f89..2fe5c63fe17 100644 --- a/src/libs/cplusplus/CMakeLists.txt +++ b/src/libs/cplusplus/CMakeLists.txt @@ -32,6 +32,7 @@ add_qtc_library(CPlusPlus TypeOfExpression.cpp TypeOfExpression.h TypePrettyPrinter.cpp TypePrettyPrinter.h cppmodelmanagerbase.cpp cppmodelmanagerbase.h + declarationcomments.cpp declarationcomments.h findcdbbreakpoint.cpp findcdbbreakpoint.h pp-cctype.h pp-engine.cpp pp-engine.h pp-scanner.cpp diff --git a/src/libs/cplusplus/cplusplus.qbs b/src/libs/cplusplus/cplusplus.qbs index 0aed0ab438f..b774618f75c 100644 --- a/src/libs/cplusplus/cplusplus.qbs +++ b/src/libs/cplusplus/cplusplus.qbs @@ -96,6 +96,7 @@ Project { "CppDocument.cpp", "CppDocument.h", "CppRewriter.cpp", "CppRewriter.h", "cppmodelmanagerbase.cpp", "cppmodelmanagerbase.h", + "declarationcomments.cpp", "declarationcomments.h", "DependencyTable.cpp", "DependencyTable.h", "DeprecatedGenTemplateInstance.cpp", "DeprecatedGenTemplateInstance.h", "ExpressionUnderCursor.cpp", "ExpressionUnderCursor.h", diff --git a/src/libs/cplusplus/declarationcomments.cpp b/src/libs/cplusplus/declarationcomments.cpp new file mode 100644 index 00000000000..2283fa847ff --- /dev/null +++ b/src/libs/cplusplus/declarationcomments.cpp @@ -0,0 +1,145 @@ +// Copyright (C) 2023 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#include "declarationcomments.h" + +#include +#include +#include + +#include + +#include +#include +#include + +namespace CPlusPlus { + +QList commentsForDeclaration(const Symbol *symbol, const Snapshot &snapshot, + const QTextDocument &textDoc) +{ + // Set up cpp document. + const Document::Ptr cppDoc = snapshot.preprocessedDocument(textDoc.toPlainText().toUtf8(), + symbol->filePath()); + cppDoc->parse(); + TranslationUnit * const tu = cppDoc->translationUnit(); + if (!tu || !tu->isParsed()) + return {}; + + // Find the symbol declaration's AST node. + // We stop at the last declaration node that precedes the symbol, except: + // - For parameter declarations, we just continue, because we are interested in the function. + // - If the declaration node is preceded directly by another one, we choose that one instead, + // because with nested declarations we want the outer one (e.g. templates). + int line, column; + tu->getTokenPosition(symbol->sourceLocation(), &line, &column); + const QList astPath = ASTPath(cppDoc)(line, column); + if (astPath.isEmpty()) + return {}; + if (astPath.last()->firstToken() != symbol->sourceLocation()) + return {}; + const AST *declAst = nullptr; + bool needsSymbolReference = false; + bool isParameter = false; + for (auto it = std::next(std::rbegin(astPath)); it != std::rend(astPath); ++it) { + AST * const node = *it; + if (node->asParameterDeclaration()) { + needsSymbolReference = true; + isParameter = true; + continue; + } + if (node->asDeclaration()) { + declAst = node; + continue; + } + if (declAst) + break; + } + if (!declAst) + return {}; + + // Get the list of all tokens (including comments) and find the declaration start token there. + const Token &declToken = tu->tokenAt(declAst->firstToken()); + std::vector allTokens = tu->allTokens(); + QTC_ASSERT(!allTokens.empty(), return {}); + int tokenPos = -1; + for (int i = 0; i < int(allTokens.size()); ++i) { + if (allTokens.at(i).byteOffset == declToken.byteOffset) { + tokenPos = i; + break; + } + } + if (tokenPos == -1) + return {}; + + // Go backwards in the token list and collect all associated comments. + struct Comment { + Token token; + QTextBlock startBlock; + QTextBlock endBlock; + }; + QList comments; + Kind commentKind = T_EOF_SYMBOL; + const auto blockForTokenStart = [&](const Token &tok) { + return textDoc.findBlock(tu->getTokenPositionInDocument(tok, &textDoc)); + }; + const auto blockForTokenEnd = [&](const Token &tok) { + return textDoc.findBlock(tu->getTokenEndPositionInDocument(tok, &textDoc)); + }; + for (int i = tokenPos - 1; i >= 0; --i) { + const Token &tok = allTokens.at(i); + if (!tok.isComment()) + break; + const QTextBlock tokenEndBlock = blockForTokenEnd(tok); + if (commentKind == T_EOF_SYMBOL) { + if (tokenEndBlock.next() != blockForTokenStart(declToken)) + needsSymbolReference = true; + commentKind = tok.kind(); + } else { + // If it's not the same kind of comment, it's not part of our comment block. + if (tok.kind() != commentKind) + break; + + // If there are empty lines between the comments, we don't consider them as + // belonging together. + if (tokenEndBlock.next() != comments.first().startBlock) + break; + } + + comments.push_front({tok, blockForTokenStart(tok), tokenEndBlock}); + } + + if (comments.isEmpty()) + return {}; + + const auto tokenList = [&] { + return Utils::transform>(comments, &Comment::token); + }; + + // We consider the comment block as associated with the symbol if it + // a) precedes it directly, without any empty lines in between or + // b) the symbol name occurs in it. + // Obviously, this heuristic can yield false positives in the case of very short names, + // but if a symbol is important enough to get documented, it should also have a proper name. + // Note that for function parameters, we always require the name to occur in the comment. + + if (!needsSymbolReference) // a) + return tokenList(); + + // b) + const QString symbolName = Overview().prettyName(symbol->name()); + const Kind tokenKind = comments.first().token.kind(); + const bool isDoxygenComment = tokenKind == T_DOXY_COMMENT || tokenKind == T_CPP_DOXY_COMMENT; + const QRegularExpression symbolRegExp(QString("%1\\b%2\\b").arg( + isParameter && isDoxygenComment ? "[\\@]param\\s+" : QString(), symbolName)); + for (const Comment &c : std::as_const(comments)) { + for (QTextBlock b = c.startBlock; b.blockNumber() <= c.endBlock.blockNumber(); + b = b.next()) { + if (b.text().contains(symbolRegExp)) + return tokenList(); + } + } + return {}; +} + +} // namespace CPlusPlus diff --git a/src/libs/cplusplus/declarationcomments.h b/src/libs/cplusplus/declarationcomments.h new file mode 100644 index 00000000000..490290450ce --- /dev/null +++ b/src/libs/cplusplus/declarationcomments.h @@ -0,0 +1,21 @@ +// Copyright (C) 2023 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#pragma once + +#include + +#include + +QT_BEGIN_NAMESPACE +class QTextDocument; +QT_END_NAMESPACE + +namespace CPlusPlus { +class Snapshot; + +QList CPLUSPLUS_EXPORT commentsForDeclaration(const Symbol *symbol, + const Snapshot &snapshot, + const QTextDocument &textDoc); + +} // namespace CPlusPlus diff --git a/tests/auto/cplusplus/CMakeLists.txt b/tests/auto/cplusplus/CMakeLists.txt index 1c85a988cc9..ab83eb9b542 100644 --- a/tests/auto/cplusplus/CMakeLists.txt +++ b/tests/auto/cplusplus/CMakeLists.txt @@ -4,6 +4,7 @@ add_subdirectory(checksymbols) add_subdirectory(codeformatter) add_subdirectory(cppselectionchanger) add_subdirectory(cxx11) +add_subdirectory(declarationcomments) add_subdirectory(fileiterationorder) add_subdirectory(findusages) add_subdirectory(lexer) diff --git a/tests/auto/cplusplus/cplusplus.qbs b/tests/auto/cplusplus/cplusplus.qbs index e50b6881ef5..44b6bde7fae 100644 --- a/tests/auto/cplusplus/cplusplus.qbs +++ b/tests/auto/cplusplus/cplusplus.qbs @@ -9,6 +9,7 @@ Project { "codeformatter/codeformatter.qbs", "cppselectionchanger/cppselectionchanger.qbs", "cxx11/cxx11.qbs", + "declarationcomments/declarationcomments.qbs", "fileiterationorder/fileiterationorder.qbs", "findusages/findusages.qbs", "lexer/lexer.qbs", diff --git a/tests/auto/cplusplus/declarationcomments/CMakeLists.txt b/tests/auto/cplusplus/declarationcomments/CMakeLists.txt new file mode 100644 index 00000000000..bb36a8e306d --- /dev/null +++ b/tests/auto/cplusplus/declarationcomments/CMakeLists.txt @@ -0,0 +1,4 @@ +add_qtc_test(tst_cplusplus_declarationcomments + DEPENDS CppEditor Utils + SOURCES tst_declarationcomments.cpp +) diff --git a/tests/auto/cplusplus/declarationcomments/declarationcomments.qbs b/tests/auto/cplusplus/declarationcomments/declarationcomments.qbs new file mode 100644 index 00000000000..601eb30ec0b --- /dev/null +++ b/tests/auto/cplusplus/declarationcomments/declarationcomments.qbs @@ -0,0 +1,6 @@ +import "../cplusplusautotest.qbs" as CPlusPlusAutotest + +CPlusPlusAutotest { + name: "declaration comments autotest" + files: "tst_declarationcomments.cpp" +} diff --git a/tests/auto/cplusplus/declarationcomments/tst_declarationcomments.cpp b/tests/auto/cplusplus/declarationcomments/tst_declarationcomments.cpp new file mode 100644 index 00000000000..0a92cfbbd61 --- /dev/null +++ b/tests/auto/cplusplus/declarationcomments/tst_declarationcomments.cpp @@ -0,0 +1,225 @@ +// Copyright (C) 2023 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#include "cplusplus/Overview.h" +#include +#include +#include + +#include +#include + +using namespace CPlusPlus; +using namespace Utils; + +const char testSource[] = R"TheFile( +//! Unrelated comment, even though it mentions MyEnum. + +//! Related comment because of proximity. +//! Related comment that mentions MyEnum. +//! Related comment that mentions MyEnumA. + +enum MyEnum { MyEnumA, MyEnumB }; + +// Unrelated comment because of different comment type. +//! Related comment that mentions MyEnumClass::A. + +enum class MyEnumClass { A, B }; + +// Related comment because of proximity. +void myFunc(); + +/* + * Related comment because of proximity. + */ +template class MyClassTemplate {}; + +/* + * Related comment, because it mentions MyOtherClass. + */ + +class MyOtherClass { + /// Related comment for function and parameter a2, but not parameter a. + /// @param a2 + void memberFunc(int a, int a2); +}; + +//! Comment for member function at implementation site. +void MyOtherClass::memberFunc(int, int) {} + +//! An unrelated comment + +void funcWithoutComment(); + +)TheFile"; + +class SymbolFinder : public SymbolVisitor +{ +public: + SymbolFinder(const QString &name, Document *doc, int expectedOccurrence) + : m_name(name), m_doc(doc), m_expectedOccurrence(expectedOccurrence) {} + const Symbol *find(); + +private: + bool preVisit(Symbol *) override { return !m_symbol; } + bool visit(Namespace *ns) override { return visitScope(ns); } + bool visit(Enum *e) override { return visitScope(e); } + bool visit(Class *c) override { return visitScope(c); } + bool visit(Function *f) override { return visitScope(f); } + bool visit(Argument *a) override; + bool visit(Declaration *d) override; + + bool visitScope(Scope *scope); + + const QString &m_name; + Document * const m_doc; + const Symbol *m_symbol = nullptr; + const int m_expectedOccurrence; + int m_tokenLocation = -1; +}; + +class TestDeclarationComments : public QObject +{ + Q_OBJECT + +private Q_SLOTS: + void initTestCase(); + + void commentsForDecl_data(); + void commentsForDecl(); + +private: + Snapshot m_snapshot; + Document::Ptr m_cppDoc; + QTextDocument m_textDoc; +}; + +void TestDeclarationComments::initTestCase() +{ + m_cppDoc = m_snapshot.preprocessedDocument(testSource, "dummy.cpp"); + m_cppDoc->check(); + const bool hasErrors = !m_cppDoc->diagnosticMessages().isEmpty(); + if (hasErrors) { + for (const Document::DiagnosticMessage &msg : m_cppDoc->diagnosticMessages()) + qDebug() << '\t' << msg.text(); + } + QVERIFY(!hasErrors); + m_textDoc.setPlainText(testSource); +} + +void TestDeclarationComments::commentsForDecl_data() +{ + QTest::addColumn("symbolName"); + QTest::addColumn("expectedCommentPrefix"); + QTest::addColumn("occurrence"); + + QTest::newRow("enum type") << "MyEnum" << "//! Related comment because of proximity" << 1; + QTest::newRow("enum value (positive)") << "MyEnumA" + << "//! Related comment because of proximity" << 1; + QTest::newRow("enum value (negative") << "MyEnumB" << QString() << 1; + + QTest::newRow("enum class type") << "MyEnumClass" + << "//! Related comment that mentions MyEnumClass::A" << 1; + QTest::newRow("enum class value (positive)") + << "A" << "//! Related comment that mentions MyEnumClass::A" << 1; + QTest::newRow("enum class value (negative") << "B" << QString() << 1; + + QTest::newRow("function declaration with comment") + << "myFunc" << "// Related comment because of proximity" << 1; + + QTest::newRow("class template") + << "MyClassTemplate" << "/*\n * Related comment because of proximity." << 1; + + QTest::newRow("class") + << "MyOtherClass" << "/*\n * Related comment, because it mentions MyOtherClass." << 1; + QTest::newRow("member function declaration") + << "memberFunc" << "/// Related comment for function and parameter a2, but not parameter a." + << 1; + QTest::newRow("function parameter (negative)") << "a" << QString() << 1; + QTest::newRow("function parameter (positive)") << "a2" << "/// Related comment for function " + "and parameter a2, but not parameter a." << 1; + + QTest::newRow("member function definition") << "memberFunc" << + "//! Comment for member function at implementation site." << 2; + + QTest::newRow("function declaration without comment") << "funcWithoutComment" << QString() << 1; +} + +void TestDeclarationComments::commentsForDecl() +{ + QFETCH(QString, symbolName); + QFETCH(QString, expectedCommentPrefix); + QFETCH(int, occurrence); + + SymbolFinder finder(symbolName, m_cppDoc.get(), occurrence); + const Symbol * const symbol = finder.find(); + QVERIFY(symbol); + + const QList commentTokens = commentsForDeclaration(symbol, m_snapshot, m_textDoc); + if (expectedCommentPrefix.isEmpty()) { + QVERIFY(commentTokens.isEmpty()); + return; + } + QVERIFY(!commentTokens.isEmpty()); + + const int firstCommentPos = m_cppDoc->translationUnit()->getTokenPositionInDocument( + commentTokens.first(), &m_textDoc); + const QString actualCommentPrefix = m_textDoc.toPlainText().mid(firstCommentPos, + expectedCommentPrefix.length()); + QCOMPARE(actualCommentPrefix, expectedCommentPrefix); +} + + +const Symbol *SymbolFinder::find() +{ + for (int i = 0, occurrences = 0; i < m_doc->translationUnit()->tokenCount(); ++i) { + const Token &tok = m_doc->translationUnit()->tokenAt(i); + if (tok.kind() != T_IDENTIFIER) + continue; + if (tok.spell() != m_name) + continue; + if (++occurrences == m_expectedOccurrence) { + m_tokenLocation = i; + break; + } + } + if (m_tokenLocation == -1) + return nullptr; + + visit(m_doc->globalNamespace()); + return m_symbol; +} + +bool SymbolFinder::visit(Argument *a) +{ + if (a->sourceLocation() == m_tokenLocation) + m_symbol = a; + return false; +} + +bool SymbolFinder::visit(Declaration *d) +{ + if (d->sourceLocation() == m_tokenLocation) { + m_symbol = d; + return false; + } + if (const auto func = d->type().type()->asFunctionType()) + return visit(func); + return true; +} + +bool SymbolFinder::visitScope(Scope *scope) +{ + for (int i = 0; i < scope->memberCount(); ++i) { + Symbol * const s = scope->memberAt(i); + if (s->sourceLocation() == m_tokenLocation) { + m_symbol = s; + return false; + } + accept(s); + } + return false; +} + +QTEST_APPLESS_MAIN(TestDeclarationComments) +#include "tst_declarationcomments.moc"