[clang-tools-extra] [clang-tidy] Add check readability-return-expression-in-void-function (PR #76249)
Danny Mösch via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 22 08:18:34 PST 2023
https://github.com/SimplyDanny created https://github.com/llvm/llvm-project/pull/76249
Closes #75788.
The idea is to check each return statements for an expression that has type `void` as this is only possible in a function with `void` return type.
The implementation seems too simple. I might have missed something.
>From 579cfd2eec9932aff10bf258811ec772a3503ce0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moesch at icloud.com>
Date: Fri, 22 Dec 2023 17:09:59 +0100
Subject: [PATCH] [clang-tidy] Add check
readability-return-expression-in-void-function
---
.../clang-tidy/readability/CMakeLists.txt | 1 +
.../readability/ReadabilityTidyModule.cpp | 3 ++
.../ReturnExpressionInVoidFunctionCheck.cpp | 27 +++++++++++++++++
.../ReturnExpressionInVoidFunctionCheck.h | 30 +++++++++++++++++++
clang-tools-extra/docs/ReleaseNotes.rst | 7 +++++
.../docs/clang-tidy/checks/list.rst | 1 +
.../return-expression-in-void-function.rst | 30 +++++++++++++++++++
.../return-expression-in-void-function.cpp | 23 ++++++++++++++
8 files changed, 122 insertions(+)
create mode 100644 clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.h
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/return-expression-in-void-function.rst
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 5452c2d48a4617..7d0399ed901267 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -42,6 +42,7 @@ add_clang_library(clangTidyReadabilityModule
RedundantStringCStrCheck.cpp
RedundantStringInitCheck.cpp
ReferenceToConstructedTemporaryCheck.cpp
+ ReturnExpressionInVoidFunctionCheck.cpp
SimplifyBooleanExprCheck.cpp
SimplifySubscriptExprCheck.cpp
StaticAccessedThroughInstanceCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index b8e6e641432060..2a9134071b9a72 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -45,6 +45,7 @@
#include "RedundantStringCStrCheck.h"
#include "RedundantStringInitCheck.h"
#include "ReferenceToConstructedTemporaryCheck.h"
+#include "ReturnExpressionInVoidFunctionCheck.h"
#include "SimplifyBooleanExprCheck.h"
#include "SimplifySubscriptExprCheck.h"
#include "StaticAccessedThroughInstanceCheck.h"
@@ -119,6 +120,8 @@ class ReadabilityModule : public ClangTidyModule {
"readability-redundant-preprocessor");
CheckFactories.registerCheck<ReferenceToConstructedTemporaryCheck>(
"readability-reference-to-constructed-temporary");
+ CheckFactories.registerCheck<ReturnExpressionInVoidFunctionCheck>(
+ "readability-return-expression-in-void-function");
CheckFactories.registerCheck<SimplifySubscriptExprCheck>(
"readability-simplify-subscript-expr");
CheckFactories.registerCheck<StaticAccessedThroughInstanceCheck>(
diff --git a/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp
new file mode 100644
index 00000000000000..fb6073b60b27f5
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp
@@ -0,0 +1,27 @@
+//===--- ReturnExpressionInVoidFunctionCheck.cpp - clang-tidy -------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ReturnExpressionInVoidFunctionCheck.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+void ReturnExpressionInVoidFunctionCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(returnStmt(hasReturnValue(hasType(voidType()))).bind("void_return"), this);
+}
+
+void ReturnExpressionInVoidFunctionCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *VoidReturn = Result.Nodes.getNodeAs<ReturnStmt>("void_return");
+ diag(VoidReturn->getBeginLoc(), "return statements should not return void");
+}
+
+} // namespace clang::tidy::readability
diff --git a/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.h b/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.h
new file mode 100644
index 00000000000000..e28f9cebd2620e
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.h
@@ -0,0 +1,30 @@
+//===--- ReturnExpressionInVoidFunctionCheck.h - clang-tidy -----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_RETURNEXPRESSIONINVOIDFUNCTIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_RETURNEXPRESSIONINVOIDFUNCTIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::readability {
+
+/// Looks for statements returning expressions of type `void`.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability/return-expression-in-void-function.html
+class ReturnExpressionInVoidFunctionCheck : public ClangTidyCheck {
+public:
+ ReturnExpressionInVoidFunctionCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace clang::tidy::readability
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_RETURNEXPRESSIONINVOIDFUNCTIONCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6d91748e4cef18..20896df0d340aa 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -218,6 +218,13 @@ New checks
Detects C++ code where a reference variable is used to extend the lifetime
of a temporary object that has just been constructed.
+- New :doc:`readability-return-expression-in-void-function
+ <clang-tidy/checks/readability/return-expression-in-void-function>` check.
+
+ Complains about statements returning expressions of type ``void``. It can be
+ confusing if a function returns an expression even though its return type is
+ ``void``.
+
New check aliases
^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 31f0e090db1d7d..f1282b4a6b766e 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -371,6 +371,7 @@ Clang-Tidy Checks
:doc:`readability-redundant-string-cstr <readability/redundant-string-cstr>`, "Yes"
:doc:`readability-redundant-string-init <readability/redundant-string-init>`, "Yes"
:doc:`readability-reference-to-constructed-temporary <readability/reference-to-constructed-temporary>`,
+ :doc:`readability-return-expression-in-void-function <readability/return-expression-in-void-function>`, "Yes"
:doc:`readability-simplify-boolean-expr <readability/simplify-boolean-expr>`, "Yes"
:doc:`readability-simplify-subscript-expr <readability/simplify-subscript-expr>`, "Yes"
:doc:`readability-static-accessed-through-instance <readability/static-accessed-through-instance>`, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/return-expression-in-void-function.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/return-expression-in-void-function.rst
new file mode 100644
index 00000000000000..dd07b95550e6f8
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/return-expression-in-void-function.rst
@@ -0,0 +1,30 @@
+.. title:: clang-tidy - readability-return-expression-in-void-function
+
+readability-return-expression-in-void-function
+==============================================
+
+Complains about statements returning expressions of type ``void``. It can be
+confusing if a function returns an expression even though its return type is
+``void``.
+
+Example:
+
+.. code-block::
+
+ void g();
+ void f() {
+ // ...
+ return g();
+ }
+
+In a long function body, the ``return`` statements suggests that the function
+returns a value. However, ``return g();`` is combination of two statements that
+should be written as
+
+.. code-block::
+
+ g();
+ return;
+
+to make clear that ``g()`` is called and immediately afterwards the function
+returns (nothing).
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp
new file mode 100644
index 00000000000000..05e2f454abcd35
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s readability-return-expression-in-void-function %t
+
+void f1();
+
+void f2() {
+ return f1();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statements should not return void [readability-return-expression-in-void-function]
+}
+
+void f3(bool b) {
+ if (b) return f1();
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: return statements should not return void [readability-return-expression-in-void-function]
+ return f2();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statements should not return void [readability-return-expression-in-void-function]
+}
+
+template<class T>
+T f4() {}
+
+void f5() {
+ return f4<void>();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statements should not return void [readability-return-expression-in-void-function]
+}
More information about the cfe-commits
mailing list