Add the qdeleteall checker

Warns on calls to qDeleteAll(QSet|QHash|QMap::values())

CMakeLists.txt

@@ -112,6 +112,7 @@ add_clang_plugin(ClangLazy
                  checks/onlywritten.cpp
                  checks/qlistint.cpp
                  checks/functionargsbyref.cpp
+                 checks/qdeleteall.cpp
                  checks/qmapkey.cpp
                  checks/qstringarg.cpp
                  checks/qstringref.cpp

checks/README-qdeleteall

@@ -0,0 +1,9 @@
+qdeleteall
+
+Finds places where a call to qDeleteAll has a redundant values() call, for example:
+    QSet<QString *> s;
+    qDeleteAll(s.values());
+instead of
+    QSet<QString *> s;
+    qDeleteAll(s);
+

checks/qdeleteall.cpp

@@ -0,0 +1,66 @@
+/*
+   This file is part of the clang-lazy static checker.
+
+  Copyright (C) 2015 Albert Astals Cid <[email protected]>
+
+  This program is free software; you can redistribute it and/or modify
+  it under the terms of the GNU General Public License as published by
+  the Free Software Foundation; either version 2 of the License, or
+  (at your option) any later version.
+
+  This program is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+  GNU General Public License for more details.
+
+  You should have received a copy of the GNU General Public License along
+  with this program; if not, write to the Free Software Foundation, Inc.,
+  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+
+  As a special exception, permission is given to link this program
+  with any edition of Qt, and distribute the resulting executable,
+  without including the source code for Qt in the source distribution.
+*/
+
+#include "qdeleteall.h"
+#include "Utils.h"
+#include "checkmanager.h"
+
+#include <clang/AST/AST.h>
+
+using namespace clang;
+
+QDeleteAll::QDeleteAll(const std::string &name)
+    : CheckBase(name)
+{
+}
+
+void QDeleteAll::VisitStmt(clang::Stmt *stmt)
+{
+    // Find a call to QMap/QSet/QHash::values
+    CXXMemberCallExpr *valuesCall = dyn_cast<CXXMemberCallExpr>(stmt);
+    if (valuesCall &&
+        valuesCall->getDirectCallee() &&
+        valuesCall->getDirectCallee()->getNameAsString() == "values")
+    {
+        const std::string valuesClassName = valuesCall->getMethodDecl()->getParent()->getNameAsString();
+        if (valuesClassName == "QMap" || valuesClassName == "QSet" || valuesClassName == "QHash") {
+            // Once found see if the first parent call is qDeleteAll
+            int i = 1;
+            Stmt *p = Utils::parent(m_parentMap, stmt, i);
+            while (p) {
+                CallExpr *pc = dyn_cast<CallExpr>(p);
+                if (pc) {
+                    if (pc->getDirectCallee() && pc->getDirectCallee()->getNameAsString() == "qDeleteAll") {
+                        emitWarning(p->getLocStart(), "Calling qDeleteAll with " + valuesClassName + "::values, call qDeleteAll on the container itself");
+                    }
+                    break;
+                }
+                ++i;
+                p = Utils::parent(m_parentMap, stmt, i);
+            }
+        }
+    }
+}
+
+REGISTER_CHECK("qdeleteall", QDeleteAll)

checks/qdeleteall.h

@@ -0,0 +1,41 @@
+/*
+   This file is part of the clang-lazy static checker.
+
+  Copyright (C) 2015 Albert Astals Cid <[email protected]>
+
+  This program is free software; you can redistribute it and/or modify
+  it under the terms of the GNU General Public License as published by
+  the Free Software Foundation; either version 2 of the License, or
+  (at your option) any later version.
+
+  This program is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+  GNU General Public License for more details.
+
+  You should have received a copy of the GNU General Public License along
+  with this program; if not, write to the Free Software Foundation, Inc.,
+  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+
+  As a special exception, permission is given to link this program
+  with any edition of Qt, and distribute the resulting executable,
+  without including the source code for Qt in the source distribution.
+*/
+
+#ifndef QDELETEALL_H
+#define QDELETEALL_H
+
+#include "checkbase.h"
+
+/**
+ * - QDeleteAll:
+ *   - Finds places where you call qDeleteAll(set/map/hash.values())
+ */
+class QDeleteAll : public CheckBase
+{
+public:
+    QDeleteAll(const std::string &name);
+    void VisitStmt(clang::Stmt *stmt) override;
+};
+
+#endif

tests/qdeleteall/main.cpp

@@ -0,0 +1,60 @@
+// clang+++ test.cpp -I /usr/include/qt/ -fPIC -lQt5Core -c
+
+#include <QtCore/QHash>
+#include <QtCore/QMap>
+#include <QtCore/QSet>
+#include <QtCore/QString>
+
+QSet<QString *> values()
+{
+    QSet<QString *> s;
+    return s;
+}
+
+struct Foo {
+    QSet<QString *> values()
+    {
+        QSet<QString *> s;
+        return s;
+    }
+
+    QSet<QString *> doSomethingWithValues(const QList<QString*> &)
+    {
+        QSet<QString *> s;
+        return s;
+    }
+};
+
+int main()
+{
+    QSet<QString *> s;
+    qDeleteAll(s);
+    qDeleteAll(s.begin(), s.end());
+    qDeleteAll(s.values()); // warning
+
+    QHash<int, QString *> h;
+    qDeleteAll(h);
+    qDeleteAll(h.begin(), h.end());
+    qDeleteAll(h.values()); // warning
+
+    QMap<int, QString *> m;
+    qDeleteAll(m);
+    qDeleteAll(m.begin(), m.end());
+    qDeleteAll(m.values()); // warning
+
+    QMultiHash<int, QString *> mh;
+    qDeleteAll(mh);
+    qDeleteAll(mh.begin(), mh.end());
+    qDeleteAll(mh.values()); // warning
+
+    QMultiMap<int, QString *> mm;
+    qDeleteAll(mm);
+    qDeleteAll(mm.begin(), mm.end());
+    qDeleteAll(mm.values()); // warning
+
+    qDeleteAll(values());  // ok
+
+    Foo foo;
+    qDeleteAll(foo.values());  // ok
+    qDeleteAll(foo.doSomethingWithValues(h.values()));  // ok
+}

tests/qdeleteall/test.expected

@@ -0,0 +1,5 @@
+main.cpp:33:5: warning: Calling qDeleteAll with QSet::values, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
+main.cpp:38:5: warning: Calling qDeleteAll with QHash::values, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
+main.cpp:43:5: warning: Calling qDeleteAll with QMap::values, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
+main.cpp:48:5: warning: Calling qDeleteAll with QHash::values, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
+main.cpp:53:5: warning: Calling qDeleteAll with QMap::values, call qDeleteAll on the container itself [-Wclazy-qdeleteall]