[clang-tidy] insert static keyword in correct position for misc-use-internal-linkage

[HerrCai0907]

Fixes: #108760

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Congcong Cai (HerrCai0907)

Changes

Fixes: #108760

---

Full diff: https://github.com/llvm/llvm-project/pull/108792.diff

5 Files Affected:

• (modified) clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp (+32-2)
• (modified) clang-tools-extra/clang-tidy/utils/LexerUtils.cpp (+3-1)
• (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
• (modified) clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp (+21)
• (modified) clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp (+12)

diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
index c086e7721e02bd..a92448c15ef3a9 100644
--- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
@@ -8,12 +8,15 @@
 
 #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"
 
 using namespace clang::ast_matchers;
@@ -110,10 +113,36 @@ static constexpr StringRef Message =
     "%0 %1 can be made static or moved into an anonymous namespace "
     "to enforce internal linkage";
 
+static SourceLocation getQualifiedTypeStartLoc(SourceLocation L,
+                                               const SourceManager &SM,
+                                               const ASTContext &Context) {
+  const SourceLocation StartOfFile = SM.getLocForStartOfFile(SM.getFileID(L));
+  if (L.isInvalid() || L.isMacroID())
+    return L;
+  bool HasChanged = true;
+  while (HasChanged) {
+    if (L == StartOfFile)
+      return L;
+    auto [Tok, Loc] =
+        utils::lexer::getPreviousTokenAndStart(L, SM, Context.getLangOpts());
+    if (Tok.is(tok::raw_identifier)) {
+      IdentifierInfo &Info = Context.Idents.get(
+          StringRef(SM.getCharacterData(Tok.getLocation()), Tok.getLength()));
+      Tok.setIdentifierInfo(&Info);
+      Tok.setKind(Info.getTokenID());
+    }
+    HasChanged = Tok.isOneOf(tok::kw_const, tok::kw_volatile);
+    if (HasChanged)
+      L = Loc;
+  }
+  return L;
+}
+
 void UseInternalLinkageCheck::check(const MatchFinder::MatchResult &Result) {
   if (const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("fn")) {
     DiagnosticBuilder DB = diag(FD->getLocation(), Message) << "function" << FD;
-    SourceLocation FixLoc = FD->getTypeSpecStartLoc();
+    const SourceLocation FixLoc = getQualifiedTypeStartLoc(
+        FD->getTypeSpecStartLoc(), *Result.SourceManager, *Result.Context);
     if (FixLoc.isInvalid() || FixLoc.isMacroID())
       return;
     if (FixMode == FixModeKind::UseStatic)
@@ -128,7 +157,8 @@ void UseInternalLinkageCheck::check(const MatchFinder::MatchResult &Result) {
       return;
 
     DiagnosticBuilder DB = diag(VD->getLocation(), Message) << "variable" << VD;
-    SourceLocation FixLoc = VD->getTypeSpecStartLoc();
+    const SourceLocation FixLoc = getQualifiedTypeStartLoc(
+        VD->getTypeSpecStartLoc(), *Result.SourceManager, *Result.Context);
     if (FixLoc.isInvalid() || FixLoc.isMacroID())
       return;
     if (FixMode == FixModeKind::UseStatic)
diff --git a/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
index df2b0bef576ca3..92c3e0ed7894e1 100644
--- a/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
@@ -24,13 +24,15 @@ getPreviousTokenAndStart(SourceLocation Location, const SourceManager &SM,
   if (Location.isInvalid())
     return {Token, Location};
 
-  auto StartOfFile = SM.getLocForStartOfFile(SM.getFileID(Location));
+  const auto StartOfFile = SM.getLocForStartOfFile(SM.getFileID(Location));
   while (Location != StartOfFile) {
     Location = Lexer::GetBeginningOfToken(Location, SM, LangOpts);
     if (!Lexer::getRawToken(Location, Token, SM, LangOpts) &&
         (!SkipComments || !Token.is(tok::comment))) {
       break;
     }
+    if (Location == StartOfFile)
+      return {Token, Location};
     Location = Location.getLocWithOffset(-1);
   }
   return {Token, Location};
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 8d0c093b312dd5..7cbcc23f28efaf 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -128,6 +128,10 @@ Changes in existing checks
   <clang-tidy/checks/misc/unconventional-assign-operator>` check to avoid
   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 qualifier such as ``const``, ``volatile``.
+
 - Improved :doc:`modernize-use-std-print
   <clang-tidy/checks/modernize/use-std-print>` check to support replacing
   member function calls too.
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 9c91389542b03d..ebb810d14f1633 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
@@ -17,6 +17,27 @@ void func_cpp_inc();
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_cpp_inc'
 // CHECK-FIXES: static void func_cpp_inc();
 
+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();
+
+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();
+
+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();
+
+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();
+
+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();
+
+
 #include "func_cpp.inc"
 
 void func_h_inc();
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp
index 6777ce4bb0265e..901272e40b8f24 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp
@@ -13,6 +13,18 @@ T global_template;
 // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: variable 'global_template'
 // CHECK-FIXES: static T global_template;
 
+int const* ptr_const_star;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'ptr_const_star'
+// CHECK-FIXES: static int const* ptr_const_star;
+
+const int* const_ptr_star;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'const_ptr_star'
+// CHECK-FIXES: static const int* const_ptr_star;
+
+const volatile int* const_volatile_ptr_star;
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: variable 'const_volatile_ptr_star'
+// CHECK-FIXES: static const volatile int* const_volatile_ptr_star;
+
 int gloabl_header;
 
 extern int global_extern;

[HerrCai0907]

friendly ping @5chmidti and other person who has time to review PR.