Add used-qunused check for identifying unneeded/wrong Q_UNUSED/void castsupstream/work/alex/qunused

15 files changed, 187 insertions, 4 deletions

diff --git a/Changelog b/Changelog
index 146c97ed..586b5038 100644
--- a/Changelog
+++ b/Changelog
@@ -175,3 +175,5 @@
 
 * v0.0.1 (June 10th, 2015)
   - (...)
+
+  - <dont forget changelog entry for used-qunused-variable>
diff --git a/CheckSources.generated.cmake b/CheckSources.generated.cmake
index 5cff22bf..fc99719c 100644
--- a/CheckSources.generated.cmake
+++ b/CheckSources.generated.cmake
@@ -97,5 +97,6 @@ set(CLAZY_CHECKS_SRCS ${CLAZY_CHECKS_SRCS}
   ${CMAKE_CURRENT_LIST_DIR}/src/checks/level2/returning-void-expression.cpp
   ${CMAKE_CURRENT_LIST_DIR}/src/checks/level2/rule-of-three.cpp
   ${CMAKE_CURRENT_LIST_DIR}/src/checks/level2/static-pmf.cpp
+  ${CMAKE_CURRENT_LIST_DIR}/src/checks/level2/used-qunused-variable.cpp
   ${CMAKE_CURRENT_LIST_DIR}/src/checks/level2/virtual-call-ctor.cpp
 )
diff --git a/ClazyTests.generated.cmake b/ClazyTests.generated.cmake
index eac7e335..b31bc17f 100644
--- a/ClazyTests.generated.cmake
+++ b/ClazyTests.generated.cmake
@@ -103,6 +103,7 @@ add_clazy_test(qstring-allocations)
 add_clazy_test(returning-void-expression)
 add_clazy_test(rule-of-three)
 add_clazy_test(static-pmf)
+add_clazy_test(used-qunused-variable)
 add_clazy_test(virtual-call-ctor)
 add_clazy_test(clazy-standalone)
 add_clazy_test(clazy)
diff --git a/README.md b/README.md
index 820b088f..137c1cfd 100644
--- a/README.md
+++ b/README.md
@@ -318,6 +318,7 @@ clazy runs all checks from level1 by default.
     - [returning-void-expression](docs/checks/README-returning-void-expression.md)
     - [rule-of-three](docs/checks/README-rule-of-three.md)
     - [static-pmf](docs/checks/README-static-pmf.md)
+    - [used-qunused-variable](docs/checks/README-used-qunused-variable.md)
     - [virtual-call-ctor](docs/checks/README-virtual-call-ctor.md)
 
 # Selecting which checks to enable
diff --git a/checks.json b/checks.json
index f0704c23..3d1df292 100644
--- a/checks.json
+++ b/checks.json
@@ -755,6 +755,14 @@
             "categories" : ["bug"],
             "visits_stmts" : true,
             "ifndef" : "CLAZY_DISABLE_AST_MATCHERS"
+        },
+        {
+            "name"  : "used-qunused-variable",
+            "class_name" : "UsedQUnusedVariable",
+            "level" : 2,
+            "categories" : ["readability"],
+            "visits_decls" : true
         }
+
     ]
 }
diff --git a/docs/checks/README-unused-result-check.md b/docs/checks/README-unused-result-check.md
index 7ea1dc09..2a8c750c 100644
--- a/docs/checks/README-unused-result-check.md
+++ b/docs/checks/README-unused-result-check.md
@@ -1,7 +1,6 @@
 # unused-result-check
 
-Warns about the unused return value of const member functions.
-for e.g. 
+Warns about the unused return value of const member functions. For example:  
 ```cpp
     class A : public QObject
 {
diff --git a/docs/checks/README-used-qunused-variable.md b/docs/checks/README-used-qunused-variable.md
new file mode 100644
index 00000000..95419591
--- /dev/null
+++ b/docs/checks/README-used-qunused-variable.md
@@ -0,0 +1,14 @@
+# used-qunused-variable
+
+Finds places where you use `Q_UNUSED` or `void` casts even though the variable is still used.
+This currently applies to function parameters.
+
+Example:
+
+```cpp
+void test(int a)
+{
+    Q_UNUSED(a)
+    int b = a + 5;
+}
+```
diff --git a/readmes.cmake b/readmes.cmake
index 0612058f..a3da8025 100644
--- a/readmes.cmake
+++ b/readmes.cmake
@@ -104,6 +104,7 @@ SET(README_LEVEL2_FILES
     ${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-returning-void-expression.md
     ${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-rule-of-three.md
     ${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-static-pmf.md
+    ${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-used-qunused-variable.md
     ${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-virtual-call-ctor.md
 )
 
diff --git a/src/Checks.h b/src/Checks.h
index fc01c365..27a61846 100644
--- a/src/Checks.h
+++ b/src/Checks.h
@@ -77,6 +77,7 @@
 #include "checks/level2/returning-void-expression.h"
 #include "checks/level2/rule-of-three.h"
 #include "checks/level2/static-pmf.h"
+#include "checks/level2/used-qunused-variable.h"
 #include "checks/level2/virtual-call-ctor.h"
 #include "checks/manuallevel/assert-with-side-effects.h"
 #include "checks/manuallevel/container-inside-loop.h"
@@ -246,5 +247,6 @@ void CheckManager::registerChecks()
     registerCheck(check<ReturningVoidExpression>("returning-void-expression", CheckLevel2, RegisteredCheck::Option_VisitsStmts));
     registerCheck(check<RuleOfThree>("rule-of-three", CheckLevel2, RegisteredCheck::Option_VisitsDecls));
     registerCheck(check<StaticPmf>("static-pmf", CheckLevel2, RegisteredCheck::Option_VisitsDecls));
+    registerCheck(check<UsedQUnusedVariable>("used-qunused-variable", CheckLevel2, RegisteredCheck::Option_VisitsDecls));
     registerCheck(check<VirtualCallCtor>("virtual-call-ctor", CheckLevel2, RegisteredCheck::Option_VisitsDecls));
 }
diff --git a/src/checks/level2/used-qunused-variable.cpp b/src/checks/level2/used-qunused-variable.cpp
new file mode 100644
index 00000000..1c9ff4e5
--- /dev/null
+++ b/src/checks/level2/used-qunused-variable.cpp
@@ -0,0 +1,101 @@
+/*
+    SPDX-FileCopyrightText: 2024 Alexander Lohnau <alexander.lohnau@gmx.de>
+    SPDX-License-Identifier: LGPL-2.0-or-later
+*/
+
+#include "used-qunused-variable.h"
+#include "ClazyContext.h"
+#include "MacroUtils.h"
+#include "PreProcessorVisitor.h"
+#include "TypeUtils.h"
+#include "Utils.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+
+#include <clang/AST/AST.h>
+
+using namespace clang;
+
+class ParameterUsageVisitor : public RecursiveASTVisitor<ParameterUsageVisitor>
+{
+public:
+    explicit ParameterUsageVisitor(const ParmVarDecl *param)
+        : param(param)
+    {
+    }
+
+    bool VisitStmt(Stmt *stmt)
+    {
+        if (checkUsage(stmt)) {
+            paramUsages.push_back(stmt);
+        }
+        return true;
+    }
+
+    std::vector<Stmt *> paramUsages;
+    Stmt *qunusedParamUsage = nullptr;
+
+private:
+    const ParmVarDecl *param;
+
+    bool checkUsage(Stmt *S, Stmt *parentStmt = nullptr)
+    {
+        if (!S)
+            return false;
+        if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(S)) {
+            if (DRE->getDecl() == param) {
+                return true;
+            }
+        }
+
+        if (CompoundStmt *cs = dyn_cast<CompoundStmt>(S)) {
+            for (auto *child : cs->children()) {
+                if (auto *castExpr = dyn_cast<CastExpr>(child); castExpr && castExpr->getType().getAsString() == "void") {
+                    for (auto *possibleDeclRef : castExpr->children()) {
+                        if (DeclRefExpr *declRef = dyn_cast<DeclRefExpr>(possibleDeclRef)) {
+                            if (declRef->getDecl() == param) {
+                                // We found a void cast
+                                qunusedParamUsage = possibleDeclRef;
+                            }
+                        }
+                    }
+                }
+                if (checkUsage(child, cs)) {
+                    return true;
+                }
+            }
+        }
+
+        return false;
+    }
+};
+
+UsedQUnusedVariable::UsedQUnusedVariable(const std::string &name, ClazyContext *context)
+    : CheckBase(name, context)
+{
+}
+
+void UsedQUnusedVariable::VisitDecl(clang::Decl *decl)
+{
+    const auto *fncDecl = dyn_cast<FunctionDecl>(decl);
+    if (!fncDecl) {
+        return;
+    }
+    for (auto *param : fncDecl->parameters()) {
+        if (param->isUsed()) {
+            ParameterUsageVisitor visitor(param);
+
+            // Visit the compound statement (the function body) to find usages
+            visitor.TraverseStmt(fncDecl->getBody());
+
+            if (visitor.paramUsages.size() > 1 && visitor.qunusedParamUsage) {
+                if (auto beginLoc = visitor.qunusedParamUsage->getBeginLoc();
+                    beginLoc.isMacroID() && Lexer::getImmediateMacroName(beginLoc, sm(), lo()) == "Q_UNUSED") {
+                    emitWarning(visitor.qunusedParamUsage, "Q_UNUSED used even though variable has usages");
+                } else {
+                    emitWarning(visitor.qunusedParamUsage, "void cast used even though variable has usages");
+                }
+            }
+        }
+    }
+}
diff --git a/src/checks/level2/used-qunused-variable.h b/src/checks/level2/used-qunused-variable.h
new file mode 100644
index 00000000..48ba6bdd
--- /dev/null
+++ b/src/checks/level2/used-qunused-variable.h
@@ -0,0 +1,18 @@
+/*
+    SPDX-FileCopyrightText: 2024 Alexander Lohnau <alexander.lohnau@gmx.de>
+    SPDX-License-Identifier: LGPL-2.0-or-later
+*/
+
+#pragma once
+
+#include "checkbase.h"
+
+/**
+ * See README-used-qunused-variable.md for more info.
+ */
+class UsedQUnusedVariable : public CheckBase
+{
+public:
+    explicit UsedQUnusedVariable(const std::string &name, ClazyContext *context);
+    void VisitDecl(clang::Decl *) override;
+};
diff --git a/src/checks/manuallevel/qt-keyword-emit.cpp b/src/checks/manuallevel/qt-keyword-emit.cpp
index adadb65c..a8b47b44 100644
--- a/src/checks/manuallevel/qt-keyword-emit.cpp
+++ b/src/checks/manuallevel/qt-keyword-emit.cpp
@@ -18,8 +18,6 @@
 #include <clang/Lex/Token.h>
 #include <llvm/ADT/StringRef.h>
 
-#include <algorithm>
-#include <ctype.h>
 #include <string_view>
 #include <vector>
 
diff --git a/tests/used-qunused-variable/config.json b/tests/used-qunused-variable/config.json
new file mode 100644
index 00000000..e7e6e0cb
--- /dev/null
+++ b/tests/used-qunused-variable/config.json
@@ -0,0 +1,7 @@
+{
+    "tests" : [
+        {
+            "filename" : "main.cpp"
+        }
+    ]
+}
diff --git a/tests/used-qunused-variable/main.cpp b/tests/used-qunused-variable/main.cpp
new file mode 100644
index 00000000..6161d5ad
--- /dev/null
+++ b/tests/used-qunused-variable/main.cpp
@@ -0,0 +1,28 @@
+#include <QtCore/QObject>
+#include <QtCore/QString>
+
+
+
+
+void testUsedVar(QString bla)
+{
+    Q_UNUSED(bla) // WARN
+    Q_UNUSED(0) // OK, just some other variable
+    bla.at(0);
+}
+
+void testUnusedVar(QString bla)
+{
+    Q_UNUSED(bla) // OK, it is really unused
+    int i = 1;
+}
+
+void testVoidStmt(const QString &bla)
+{
+  (void) bla; // WARN
+  bla.at(0);
+}
+void testCastStmt(int bla)
+{
+  auto casted = (float) bla; // OK - diferent from void cast
+}
diff --git a/tests/used-qunused-variable/main.cpp.expected b/tests/used-qunused-variable/main.cpp.expected
new file mode 100644
index 00000000..9d0e70ef
--- /dev/null
+++ b/tests/used-qunused-variable/main.cpp.expected
@@ -0,0 +1,2 @@
+used-qunused-variable/main.cpp:9:14: warning: Q_UNUSED used even though variable has usages [-Wclazy-used-qunused-variable]
+used-qunused-variable/main.cpp:22:10: warning: void cast used even though variable has usages [-Wclazy-used-qunused-variable]