Clang: Report only diagnostics that can be shown in the editor

Change-Id: I9c258159d240c6ba7eeff34702d8512d9220b3af
Reviewed-by: Marco Bubke <marco.bubke@theqtcompany.com>

src/libs/clangbackendipc/sourcelocationcontainer.cpp

@@ -82,7 +82,13 @@ QDataStream &operator>>(QDataStream &in, SourceLocationContainer &container)
 
 bool operator==(const SourceLocationContainer &first, const SourceLocationContainer &second)
 {
-    return first.offset_ == second.offset_ && first.filePath_ == second.filePath_;
+    return !(first != second);
+}
+
+bool operator!=(const SourceLocationContainer &first, const SourceLocationContainer &second)
+{
+    return first.offset_ != second.offset_
+        || first.filePath_ != second.filePath_;
 }
 
 bool operator<(const SourceLocationContainer &first, const SourceLocationContainer &second)

src/libs/clangbackendipc/sourcelocationcontainer.h

@@ -44,6 +44,7 @@ class CMBIPC_EXPORT SourceLocationContainer
     friend CMBIPC_EXPORT QDataStream &operator<<(QDataStream &out, const SourceLocationContainer &container);
     friend CMBIPC_EXPORT QDataStream &operator>>(QDataStream &in, SourceLocationContainer &container);
     friend CMBIPC_EXPORT bool operator==(const SourceLocationContainer &first, const SourceLocationContainer &second);
+    friend CMBIPC_EXPORT bool operator!=(const SourceLocationContainer &first, const SourceLocationContainer &second);
     friend CMBIPC_EXPORT bool operator<(const SourceLocationContainer &first, const SourceLocationContainer &second);
 
 public:
@@ -65,6 +66,7 @@ private:
 CMBIPC_EXPORT QDataStream &operator<<(QDataStream &out, const SourceLocationContainer &container);
 CMBIPC_EXPORT QDataStream &operator>>(QDataStream &in, SourceLocationContainer &container);
 CMBIPC_EXPORT bool operator==(const SourceLocationContainer &first, const SourceLocationContainer &second);
+CMBIPC_EXPORT bool operator!=(const SourceLocationContainer &first, const SourceLocationContainer &second);
 CMBIPC_EXPORT bool operator<(const SourceLocationContainer &first, const SourceLocationContainer &second);
 
 CMBIPC_EXPORT QDebug operator<<(QDebug debug, const SourceLocationContainer &container);

src/tools/clangbackend/ipcsource/clangipcserver.cpp

@@ -233,7 +233,7 @@ void ClangIpcServer::requestDiagnostics(const RequestDiagnosticsMessage &message
                                                                 message.file().projectPartId());
 
         client()->diagnosticsChanged(DiagnosticsChangedMessage(translationUnit.fileContainer(),
-                                                               translationUnit.diagnostics().toDiagnosticContainers()));
+                                                               translationUnit.mainFileDiagnostics()));
     } catch (const TranslationUnitDoesNotExistException &exception) {
         client()->translationUnitDoesNotExist(TranslationUnitDoesNotExistMessage(exception.fileContainer()));
     } catch (const ProjectPartDoNotExistException &exception) {

src/tools/clangbackend/ipcsource/diagnostic.cpp

@@ -138,7 +138,7 @@ DiagnosticSet Diagnostic::childDiagnostics() const
     return DiagnosticSet(clang_getChildDiagnostics(cxDiagnostic));
 }
 
-DiagnosticContainer Diagnostic::toDiagnosticContainer() const
+DiagnosticContainer Diagnostic::toDiagnosticContainer(const IsAcceptedDiagnostic &isAcceptedChildDiagnostic) const
 {
     return DiagnosticContainer(text(),
                                category(),
@@ -147,7 +147,14 @@ DiagnosticContainer Diagnostic::toDiagnosticContainer() const
                                location().toSourceLocationContainer(),
                                getSourceRangeContainers(),
                                getFixItContainers(),
-                               childDiagnostics().toDiagnosticContainers());
+                               childDiagnostics().toDiagnosticContainers(isAcceptedChildDiagnostic));
+}
+
+DiagnosticContainer Diagnostic::toDiagnosticContainer() const
+{
+    const auto acceptAllDiagnostics = [](const Diagnostic &) { return true; };
+
+    return toDiagnosticContainer(acceptAllDiagnostics);
 }
 
 QVector<SourceRangeContainer> Diagnostic::getSourceRangeContainers() const

src/tools/clangbackend/ipcsource/diagnostic.h

@@ -35,6 +35,7 @@
 
 #include <clang-c/Index.h>
 
+#include <functional>
 #include <vector>
 
 class Utf8String;
@@ -75,6 +76,9 @@ public:
     std::vector<FixIt> fixIts() const;
     DiagnosticSet childDiagnostics() const;
 
+    using IsAcceptedDiagnostic = std::function<bool (const Diagnostic &)>;
+    DiagnosticContainer toDiagnosticContainer(
+            const IsAcceptedDiagnostic &isAcceptedChildDiagnostic) const;
     DiagnosticContainer toDiagnosticContainer() const;
 
 private:

src/tools/clangbackend/ipcsource/diagnosticset.cpp

@@ -86,12 +86,22 @@ DiagnosticSet::ConstIterator DiagnosticSet::end() const
 }
 
 QVector<DiagnosticContainer> DiagnosticSet::toDiagnosticContainers() const
+{
+    const auto isAcceptedDiagnostic = [](const Diagnostic &) { return true; };
+
+    return toDiagnosticContainers(isAcceptedDiagnostic);
+}
+
+QVector<DiagnosticContainer> DiagnosticSet::toDiagnosticContainers(
+        const Diagnostic::IsAcceptedDiagnostic &isAcceptedDiagnostic) const
 {
     QVector<DiagnosticContainer> diagnosticContainers;
     diagnosticContainers.reserve(size());
 
-    for (const Diagnostic &diagnostic : *this)
+    for (const Diagnostic &diagnostic : *this) {
-        diagnosticContainers.push_back(diagnostic.toDiagnosticContainer());
+        if (isAcceptedDiagnostic(diagnostic))
+            diagnosticContainers.push_back(diagnostic.toDiagnosticContainer(isAcceptedDiagnostic));
+    }
 
     return diagnosticContainers;
 }

src/tools/clangbackend/ipcsource/diagnosticset.h

@@ -38,6 +38,8 @@
 
 #include <QVector>
 
+#include <functional>
+
 namespace ClangBackEnd {
 
 class DiagnosticSetIterator;
@@ -71,6 +73,8 @@ public:
     ConstIterator end() const;
 
     QVector<DiagnosticContainer> toDiagnosticContainers() const;
+    QVector<DiagnosticContainer> toDiagnosticContainers(
+            const Diagnostic::IsAcceptedDiagnostic &isAcceptedDiagnostic) const;
 
 private:
     DiagnosticSet(CXDiagnosticSet cxDiagnosticSet);

src/tools/clangbackend/ipcsource/translationunit.cpp

@@ -33,8 +33,10 @@
 #include "clangstring.h"
 #include "codecompleter.h"
 #include "commandlinearguments.h"
+#include "diagnosticcontainer.h"
 #include "diagnosticset.h"
 #include "projectpart.h"
+#include "sourcelocation.h"
 #include "translationunitfilenotexitexception.h"
 #include "translationunitisnullexception.h"
 #include "translationunitparseerrorexception.h"
@@ -218,6 +220,16 @@ DiagnosticSet TranslationUnit::diagnostics() const
     return DiagnosticSet(clang_getDiagnosticSetFromTU(cxTranslationUnit()));
 }
 
+QVector<ClangBackEnd::DiagnosticContainer> TranslationUnit::mainFileDiagnostics() const
+{
+    const auto mainFilePath = filePath();
+    const auto isMainFileDiagnostic = [mainFilePath](const Diagnostic &diagnostic) {
+        return diagnostic.location().filePath() == mainFilePath;
+    };
+
+    return diagnostics().toDiagnosticContainers(isMainFileDiagnostic);
+}
+
 const QSet<Utf8String> &TranslationUnit::dependedFilePaths() const
 {
     createTranslationUnitIfNeeded();

src/tools/clangbackend/ipcsource/translationunit.h

@@ -49,6 +49,7 @@ class TranslationUnitData;
 class CodeCompleter;
 class UnsavedFiles;
 class ProjectPart;
+class DiagnosticContainer;
 class DiagnosticSet;
 class FileContainer;
 class TranslationUnits;
@@ -102,6 +103,7 @@ public:
     bool hasNewDiagnostics() const;
 
     DiagnosticSet diagnostics() const;
+    QVector<DiagnosticContainer> mainFileDiagnostics() const;
 
     const QSet<Utf8String> &dependedFilePaths() const;
 

src/tools/clangbackend/ipcsource/translationunits.cpp

@@ -291,7 +291,7 @@ void TranslationUnits::sendDiagnosticChangedMessage(const TranslationUnit &trans
 {
     if (sendDiagnosticsChangedCallback) {
         DiagnosticsChangedMessage message(translationUnit.fileContainer(),
-                                          translationUnit.diagnostics().toDiagnosticContainers());
+                                          translationUnit.mainFileDiagnostics());
 
         sendDiagnosticsChangedCallback(std::move(message));
     }

tests/unit/unittest/data/diagnostic_diagnosticset_header.cpp

@@ -0,0 +1,2 @@
+void f() {}
+

tests/unit/unittest/data/diagnostic_diagnosticset_mainfile.cpp

@@ -0,0 +1,4 @@
+#include "diagnostic_diagnosticset_header.cpp"
+
+void f() {}
+

tests/unit/unittest/diagnosticsettest.cpp

@@ -28,9 +28,15 @@
 **
 ****************************************************************************/
 
+#include <clangbackendipc_global.h>
+#include <diagnosticcontainer.h>
 #include <diagnosticset.h>
+#include <fixitcontainer.h>
 #include <projectpart.h>
 #include <projects.h>
+#include <sourcelocation.h>
+#include <sourcelocationcontainer.h>
+#include <sourcerangecontainer.h>
 #include <translationunit.h>
 #include <translationunits.h>
 #include <unsavedfiles.h>
@@ -40,15 +46,27 @@
 #include <gmock/gmock.h>
 #include <gmock/gmock-matchers.h>
 #include <gtest/gtest.h>
+#include "matcher-diagnosticcontainer.h"
 #include "gtest-qt-printing.h"
 
+using ::testing::Contains;
+using ::testing::Not;
+using ::testing::PrintToString;
+
+using ::ClangBackEnd::Diagnostic;
 using ::ClangBackEnd::DiagnosticSet;
-using ::ClangBackEnd::TranslationUnit;
+using ::ClangBackEnd::DiagnosticContainer;
+using ::ClangBackEnd::FixItContainer;
 using ::ClangBackEnd::ProjectPart;
+using ::ClangBackEnd::SourceLocation;
+using ::ClangBackEnd::SourceLocationContainer;
+using ::ClangBackEnd::TranslationUnit;
 using ::ClangBackEnd::UnsavedFiles;
 
 namespace {
 
+const Utf8String headerFilePath = Utf8StringLiteral(TESTDATA_DIR"/diagnostic_diagnosticset_header.cpp");
+
 class DiagnosticSet : public ::testing::Test
 {
 protected:
@@ -60,6 +78,15 @@ protected:
                                     projectPart,
                                     Utf8StringVector(),
                                     translationUnits};
+    TranslationUnit translationUnitMainFile{Utf8StringLiteral(TESTDATA_DIR"/diagnostic_diagnosticset_mainfile.cpp"),
+                                            projectPart,
+                                            Utf8StringVector(),
+                                            translationUnits};
+    ::DiagnosticSet diagnosticSetWithChildren{translationUnitMainFile.diagnostics()};
+
+protected:
+    enum ChildMode { WithChild, WithoutChild };
+    DiagnosticContainer expectedDiagnostic(ChildMode childMode) const;
 };
 
 TEST_F(DiagnosticSet, SetHasContent)
@@ -120,4 +147,63 @@ TEST_F(DiagnosticSet, BeginPlusOneIsEqualEnd)
     ASSERT_TRUE(++set.begin() == set.end());
 }
 
+TEST_F(DiagnosticSet, ToDiagnosticContainersLetThroughByDefault)
+{
+    const auto diagnosticContainerWithoutChild = expectedDiagnostic(WithChild);
+
+    const auto diagnostics = translationUnitMainFile.diagnostics().toDiagnosticContainers();
+
+    ASSERT_THAT(diagnostics, Contains(IsDiagnosticContainer(diagnosticContainerWithoutChild)));
+}
+
+TEST_F(DiagnosticSet, ToDiagnosticContainersFiltersOutTopLevelItem)
+{
+    const auto acceptNoDiagnostics = [](const Diagnostic &) { return false; };
+
+    const auto diagnostics = diagnosticSetWithChildren.toDiagnosticContainers(acceptNoDiagnostics);
+
+    ASSERT_TRUE(diagnostics.isEmpty());
+}
+
+TEST_F(DiagnosticSet, ToDiagnosticContainersFiltersOutChildren)
+{
+    const auto diagnosticContainerWithoutChild = expectedDiagnostic(WithoutChild);
+    const auto acceptMainFileDiagnostics = [this](const Diagnostic &diagnostic) {
+        return diagnostic.location().filePath() == translationUnitMainFile.filePath();
+    };
+
+    const auto diagnostics = diagnosticSetWithChildren.toDiagnosticContainers(acceptMainFileDiagnostics);
+
+    ASSERT_THAT(diagnostics, Contains(IsDiagnosticContainer(diagnosticContainerWithoutChild)));
+}
+
+DiagnosticContainer DiagnosticSet::expectedDiagnostic(DiagnosticSet::ChildMode childMode) const
+{
+    QVector<DiagnosticContainer> children;
+    if (childMode == WithChild) {
+        const auto child = DiagnosticContainer(
+            Utf8StringLiteral("note: previous definition is here"),
+            Utf8StringLiteral("Semantic Issue"),
+            {Utf8String(), Utf8String()},
+            ClangBackEnd::DiagnosticSeverity::Note,
+            SourceLocationContainer(headerFilePath, 1, 5),
+            {},
+            {},
+            {}
+        );
+        children.append(child);
+    }
+
+    return DiagnosticContainer(
+        Utf8StringLiteral("error: redefinition of 'f'"),
+        Utf8StringLiteral("Semantic Issue"),
+        {Utf8String(), Utf8String()},
+        ClangBackEnd::DiagnosticSeverity::Error,
+        SourceLocationContainer(translationUnitMainFile.filePath(), 3, 53),
+        {},
+        {},
+        children
+    );
+}
+
 }

tests/unit/unittest/diagnostictest.cpp

@@ -29,7 +29,9 @@
 ****************************************************************************/
 
 #include <diagnostic.h>
+#include <diagnosticcontainer.h>
 #include <diagnosticset.h>
+#include <fixitcontainer.h>
 #include <projectpart.h>
 #include <translationunit.h>
 #include <translationunits.h>
@@ -43,9 +45,16 @@
 #include <gmock/gmock.h>
 #include <gmock/gmock-matchers.h>
 #include <gtest/gtest.h>
+
 #include "gtest-qt-printing.h"
+#include "matcher-diagnosticcontainer.h"
+
+using ::testing::Contains;
+using ::testing::Not;
+using ::testing::PrintToString;
 
 using ClangBackEnd::DiagnosticSet;
+using ClangBackEnd::DiagnosticContainer;
 using ClangBackEnd::TranslationUnit;
 using ClangBackEnd::ProjectPart;
 using ClangBackEnd::UnsavedFiles;
@@ -53,7 +62,8 @@ using ClangBackEnd::Diagnostic;
 using ClangBackEnd::SourceLocation;
 using ClangBackEnd::DiagnosticSeverity;
 using ClangBackEnd::TranslationUnits;
-using testing::PrintToString;
+using ClangBackEnd::FixItContainer;
+using ClangBackEnd::SourceLocationContainer;
 
 namespace {
 
@@ -87,6 +97,10 @@ protected:
                                     translationUnits};
     DiagnosticSet diagnosticSet{translationUnit.diagnostics()};
     ::Diagnostic diagnostic{diagnosticSet.back()};
+
+protected:
+    enum ChildMode { WithChild, WithoutChild };
+    DiagnosticContainer expectedDiagnostic(ChildMode childMode) const;
 };
 
 TEST_F(Diagnostic, MoveContructor)
@@ -152,4 +166,55 @@ TEST_F(Diagnostic, ChildDiagnosticsText)
     ASSERT_THAT(childDiagnostic.text(), Utf8StringLiteral("note: previous declaration is here"));
 }
 
+TEST_F(Diagnostic, toDiagnosticContainerLetChildrenThroughByDefault)
+{
+    const auto diagnosticWithChild = expectedDiagnostic(WithChild);
+
+    const auto diagnostic = diagnosticSet.front().toDiagnosticContainer();
+
+    ASSERT_THAT(diagnostic, IsDiagnosticContainer(diagnosticWithChild));
+}
+
+TEST_F(Diagnostic, toDiagnosticContainerFiltersOutChildren)
+{
+    const auto diagnosticWithoutChild = expectedDiagnostic(WithoutChild);
+    const auto acceptDiagnosticWithSpecificText = [](const ::Diagnostic &diagnostic) {
+        return diagnostic.text() != Utf8StringLiteral("note: previous declaration is here");
+    };
+
+    const auto diagnostic = diagnosticSet.front().toDiagnosticContainer(acceptDiagnosticWithSpecificText);
+
+    ASSERT_THAT(diagnostic, IsDiagnosticContainer(diagnosticWithoutChild));
+}
+
+DiagnosticContainer Diagnostic::expectedDiagnostic(Diagnostic::ChildMode childMode) const
+{
+    QVector<DiagnosticContainer> children;
+    if (childMode == WithChild) {
+        const auto child = DiagnosticContainer(
+            Utf8StringLiteral("note: previous declaration is here"),
+            Utf8StringLiteral("Semantic Issue"),
+            {Utf8String(), Utf8String()},
+            ClangBackEnd::DiagnosticSeverity::Note,
+            SourceLocationContainer(translationUnit.filePath(), 2, 14),
+            {},
+            {},
+            {}
+        );
+        children.append(child);
+    }
+
+    return
+        DiagnosticContainer(
+            Utf8StringLiteral("warning: 'X' is missing exception specification 'noexcept'"),
+            Utf8StringLiteral("Semantic Issue"),
+            {Utf8String(), Utf8String()},
+            ClangBackEnd::DiagnosticSeverity::Warning,
+            SourceLocationContainer(translationUnit.filePath(), 5, 38),
+            {},
+            {},
+            children
+        );
+}
+
 }

tests/unit/unittest/matcher-diagnosticcontainer.h

@@ -0,0 +1,63 @@
+/****************************************************************************
+**
+** Copyright (C) 2015 The Qt Company Ltd.
+** Contact: http://www.qt.io/licensing
+**
+** This file is part of Qt Creator.
+**
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company.  For licensing terms and
+** conditions see http://www.qt.io/terms-conditions.  For further information
+** use the contact form at http://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 or version 3 as published by the Free
+** Software Foundation and appearing in the file LICENSE.LGPLv21 and
+** LICENSE.LGPLv3 included in the packaging of this file.  Please review the
+** following information to ensure the GNU Lesser General Public License
+** requirements will be met: https://www.gnu.org/licenses/lgpl.html and
+** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** In addition, as a special exception, The Qt Company gives you certain additional
+** rights.  These rights are described in The Qt Company LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+****************************************************************************/
+
+#include <gmock/gmock.h>
+#include <gmock/gmock-matchers.h>
+#include <gtest/gtest.h>
+#include "gtest-qt-printing.h"
+
+namespace {
+
+using ::testing::PrintToString;
+
+MATCHER_P(IsDiagnosticContainer, diagnosticContainer, "")
+{
+    if (arg.text() != diagnosticContainer.text()) {
+        *result_listener << "text is " + PrintToString(arg.text())
+                            + " and not " + PrintToString(diagnosticContainer.text());
+        return false;
+    }
+
+    if (arg.location() != diagnosticContainer.location()) {
+        *result_listener << "location is " + PrintToString(arg.location())
+                            + " and not " + PrintToString(diagnosticContainer.location());
+        return false;
+    }
+
+    if (arg.children() != diagnosticContainer.children()) {
+        *result_listener << "children are " + PrintToString(arg.children())
+                            + " and not " + PrintToString(diagnosticContainer.children());
+        return false;
+    }
+
+    return true;
+}
+
+} // anonymous

tests/unit/unittest/unittest.pro

@@ -59,6 +59,7 @@ HEADERS += \
     gtest-qt-printing.h \
     mockipclient.h \
     mockipcserver.h \
-    spydummy.h
+    spydummy.h \
+    matcher-diagnosticcontainer.h
 
 OTHER_FILES += $$files(data/*)