[clang-tools-extra] [clang-tidy] fix false positive use-internal-linkage for func decl without body (PR #117490)

[Congcong Cai via cfe-commits]

https://github.com/HerrCai0907 created https://github.com/llvm/llvm-project/pull/117490

If in one TU, function only have declaration without body, this function should be external linkage.
Fixed #117488


>From a1f2a6523c69bf09f39e478a8108a621ba959c20 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Sun, 24 Nov 2024 21:55:15 +0800
Subject: [PATCH] [clang-tidy] fix false positive use-internal-linkage for func
 decl without body

If in one TU, function only have declaration without body, this function should be external linkage.
Fixed #117488
---
 .../misc/UseInternalLinkageCheck.cpp          |  6 +-
 clang-tools-extra/docs/ReleaseNotes.rst       |  5 +-
 .../checks/misc/use-internal-linkage.rst      |  5 +-
 .../misc/use-internal-linkage-func.cpp        | 56 +++++++++++--------
 4 files changed, 43 insertions(+), 29 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
index d900978f65a944..71eb2d94cd4f26 100644
--- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
@@ -8,14 +8,12 @@
 
 #include "UseInternalLinkageCheck.h"
 #include "../utils/FileExtensionsUtils.h"
-#include "../utils/LexerUtils.h"
 #include "clang/AST/Decl.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/ASTMatchers/ASTMatchersMacros.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
-#include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/STLExtras.h"
 
@@ -47,6 +45,8 @@ namespace {
 
 AST_MATCHER(Decl, isFirstDecl) { return Node.isFirstDecl(); }
 
+AST_MATCHER(FunctionDecl, hasBody) { return Node.hasBody(); }
+
 static bool isInMainFile(SourceLocation L, SourceManager &SM,
                          const FileExtensionsSet &HeaderFileExtensions) {
   for (;;) {
@@ -103,7 +103,7 @@ void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) {
                 // 4. friend
                 hasAncestor(friendDecl()))));
   Finder->addMatcher(
-      functionDecl(Common, unless(cxxMethodDecl()), unless(isMain()))
+      functionDecl(Common, hasBody(), unless(cxxMethodDecl()), unless(isMain()))
           .bind("fn"),
       this);
   Finder->addMatcher(varDecl(Common, hasGlobalStorage()).bind("var"), this);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 8c9fedf4b4406c..1f338f23f4e6ea 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -229,8 +229,9 @@ Changes in existing checks
   false positive for C++23 deducing this.
 
 - Improved :doc:`misc-use-internal-linkage
-  <clang-tidy/checks/misc/use-internal-linkage>` check to insert ``static`` keyword
-  before type qualifiers such as ``const`` and ``volatile``.
+  <clang-tidy/checks/misc/use-internal-linkage>` check to insert ``static``
+  keyword before type qualifiers such as ``const`` and ``volatile`` and fix
+  false positives for function declaration without body.
 
 - Improved :doc:`modernize-avoid-c-arrays
   <clang-tidy/checks/modernize/avoid-c-arrays>` check to suggest using 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
index 7147af9a7919bc..b8bbcc62706101 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
@@ -16,7 +16,7 @@ Example:
 
   int v1; // can be marked as static
 
-  void fn1(); // can be marked as static
+  void fn1() {} // can be marked as static
 
   namespace {
     // already in anonymous namespace
@@ -26,6 +26,9 @@ Example:
   // already declared as extern
   extern int v2;
 
+  void fn3(); // without function body in all declaration, maybe external linkage
+  void fn3();
+
 Options
 -------
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp
index 8dc739da3a2734..bf0d2c2513e562 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp
@@ -13,59 +13,59 @@ void func_template() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_template'
 // CHECK-FIXES: static void func_template() {}
 
-void func_cpp_inc();
+void func_cpp_inc() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_cpp_inc'
-// CHECK-FIXES: static void func_cpp_inc();
+// CHECK-FIXES: static void func_cpp_inc() {}
 
-int* func_cpp_inc_return_ptr();
+int* func_cpp_inc_return_ptr() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_cpp_inc_return_ptr'
-// CHECK-FIXES: static int* func_cpp_inc_return_ptr();
+// CHECK-FIXES: static int* func_cpp_inc_return_ptr() {}
 
-const int* func_cpp_inc_return_const_ptr();
+const int* func_cpp_inc_return_const_ptr() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: function 'func_cpp_inc_return_const_ptr'
-// CHECK-FIXES: static const int* func_cpp_inc_return_const_ptr();
+// CHECK-FIXES: static const int* func_cpp_inc_return_const_ptr() {}
 
-int const* func_cpp_inc_return_ptr_const();
+int const* func_cpp_inc_return_ptr_const() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: function 'func_cpp_inc_return_ptr_const'
-// CHECK-FIXES: static int const* func_cpp_inc_return_ptr_const();
+// CHECK-FIXES: static int const* func_cpp_inc_return_ptr_const() {}
 
-int * const func_cpp_inc_return_const();
+int * const func_cpp_inc_return_const() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function 'func_cpp_inc_return_const'
-// CHECK-FIXES: static int * const func_cpp_inc_return_const();
+// CHECK-FIXES: static int * const func_cpp_inc_return_const() {}
 
-volatile const int* func_cpp_inc_return_volatile_const_ptr();
+volatile const int* func_cpp_inc_return_volatile_const_ptr() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: function 'func_cpp_inc_return_volatile_const_ptr'
-// CHECK-FIXES: static volatile const int* func_cpp_inc_return_volatile_const_ptr();
+// CHECK-FIXES: static volatile const int* func_cpp_inc_return_volatile_const_ptr() {}
 
-[[nodiscard]] void func_nodiscard();
+[[nodiscard]] void func_nodiscard() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function 'func_nodiscard'
-// CHECK-FIXES: {{\[\[nodiscard\]\]}} static void func_nodiscard();
+// CHECK-FIXES: {{\[\[nodiscard\]\]}} static void func_nodiscard() {}
 
 #define NDS [[nodiscard]]
 #define NNDS
 
-NDS void func_nds();
+NDS void func_nds() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: function 'func_nds'
-// CHECK-FIXES: NDS static void func_nds();
+// CHECK-FIXES: NDS static void func_nds() {}
 
-NNDS void func_nnds();
+NNDS void func_nnds() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: function 'func_nnds'
-// CHECK-FIXES: NNDS static void func_nnds();
+// CHECK-FIXES: NNDS static void func_nnds() {}
 
 #include "func_cpp.inc"
 
-void func_h_inc();
+void func_h_inc() {}
 
 struct S {
   void method();
 };
 void S::method() {}
 
-void func_header();
-extern void func_extern();
-static void func_static();
+void func_header() {}
+extern void func_extern() {}
+static void func_static() {}
 namespace {
-void func_anonymous_ns();
+void func_anonymous_ns() {}
 } // namespace
 
 int main(int argc, const char*argv[]) {}
@@ -75,3 +75,13 @@ void func_extern_c_1() {}
 }
 
 extern "C" void func_extern_c_2() {}
+
+namespace gh117488 {
+void func_with_body();
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_with_body'
+// CHECK-FIXES: static void func_with_body();
+void func_with_body() {}
+
+void func_without_body();
+void func_without_body();
+}