[clang-tools-extra] [clang-tidy] add fixhint for misc-use-internal-linkage (PR #96203)

Congcong Cai via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 24 07:45:09 PDT 2024


https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/96203

>From 669205c0f659239c58a3bde3ddadabb0a8ecbad8 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Thu, 20 Jun 2024 15:05:57 +0000
Subject: [PATCH 1/3] [clang-tidy] add fixhint for misc-use-internal-linkage

---
 .../clang-tidy/misc/UseInternalLinkageCheck.cpp      | 12 ++++++++++--
 .../checkers/misc/use-internal-linkage-func.cpp      |  1 +
 .../checkers/misc/use-internal-linkage-var.cpp       |  1 +
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
index 70d0281df28fa..e36ccdba42ef1 100644
--- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
@@ -82,11 +82,19 @@ static constexpr StringRef Message =
 
 void UseInternalLinkageCheck::check(const MatchFinder::MatchResult &Result) {
   if (const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("fn")) {
-    diag(FD->getLocation(), Message) << "function" << FD;
+    DiagnosticBuilder DB = diag(FD->getLocation(), Message) << "function" << FD;
+    SourceLocation FixLoc = FD->getTypeSpecStartLoc();
+    if (FixLoc.isInvalid() || FixLoc.isMacroID())
+      return;
+    DB << FixItHint::CreateInsertion(FixLoc, "static ");
     return;
   }
   if (const auto *VD = Result.Nodes.getNodeAs<VarDecl>("var")) {
-    diag(VD->getLocation(), Message) << "variable" << VD;
+    DiagnosticBuilder DB = diag(VD->getLocation(), Message) << "variable" << VD;
+    SourceLocation FixLoc = VD->getTypeSpecStartLoc();
+    if (FixLoc.isInvalid() || FixLoc.isMacroID())
+      return;
+    DB << FixItHint::CreateInsertion(FixLoc, "static ");
     return;
   }
   llvm_unreachable("");
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 c6c513fe0b0c0..c244f32db8e96 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
@@ -4,6 +4,7 @@
 
 void func() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func'
+// CHECK-FIXES: static void func() {}
 
 template<class T>
 void func_template() {}
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 bd5ef5431de6c..b076e9125fc35 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
@@ -4,6 +4,7 @@
 
 int global;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'global'
+// CHECK-FIXES: static int global;
 
 template<class T>
 T global_template;

>From d5bb05e2af683a2dcb08c6e8cabc448001c07414 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcong.cai at bmw.com>
Date: Mon, 24 Jun 2024 21:58:56 +0800
Subject: [PATCH 2/3] add FixMode option

---
 .../misc/UseInternalLinkageCheck.cpp          | 36 +++++++++++++++++--
 .../clang-tidy/misc/UseInternalLinkageCheck.h | 11 ++++--
 .../checks/misc/use-internal-linkage.rst      | 13 +++++++
 .../use-internal-linkage-fix-mode-none.cpp    | 10 ++++++
 .../misc/use-internal-linkage-func.cpp        |  2 ++
 .../misc/use-internal-linkage-var.cpp         |  2 ++
 6 files changed, 69 insertions(+), 5 deletions(-)
 create mode 100644 clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-fix-mode-none.cpp

diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
index e36ccdba42ef1..44ccc2bc906a5 100644
--- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
@@ -18,6 +18,26 @@
 
 using namespace clang::ast_matchers;
 
+namespace clang::tidy {
+
+template <>
+struct OptionEnumMapping<misc::UseInternalLinkageCheck::FixModeKind> {
+  static llvm::ArrayRef<
+      std::pair<misc::UseInternalLinkageCheck::FixModeKind, StringRef>>
+  getEnumMapping() {
+    static constexpr std::pair<misc::UseInternalLinkageCheck::FixModeKind,
+                               StringRef>
+        Mapping[] = {
+            {misc::UseInternalLinkageCheck::FixModeKind::None, "None"},
+            {misc::UseInternalLinkageCheck::FixModeKind::UseStatic,
+             "UseStatic"},
+        };
+    return {Mapping};
+  }
+};
+
+} // namespace clang::tidy
+
 namespace clang::tidy::misc {
 
 namespace {
@@ -57,6 +77,16 @@ AST_POLYMORPHIC_MATCHER(isExternStorageClass,
 
 } // namespace
 
+UseInternalLinkageCheck::UseInternalLinkageCheck(StringRef Name,
+                                                 ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      HeaderFileExtensions(Context->getHeaderFileExtensions()),
+      FixMode(Options.get("FixMode", FixModeKind::UseStatic)) {}
+
+void UseInternalLinkageCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "FixMode", FixMode);
+}
+
 void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) {
   auto Common =
       allOf(isFirstDecl(), isAllRedeclsInMainFile(HeaderFileExtensions),
@@ -86,7 +116,8 @@ void UseInternalLinkageCheck::check(const MatchFinder::MatchResult &Result) {
     SourceLocation FixLoc = FD->getTypeSpecStartLoc();
     if (FixLoc.isInvalid() || FixLoc.isMacroID())
       return;
-    DB << FixItHint::CreateInsertion(FixLoc, "static ");
+    if (FixMode == FixModeKind::UseStatic)
+      DB << FixItHint::CreateInsertion(FixLoc, "static ");
     return;
   }
   if (const auto *VD = Result.Nodes.getNodeAs<VarDecl>("var")) {
@@ -94,7 +125,8 @@ void UseInternalLinkageCheck::check(const MatchFinder::MatchResult &Result) {
     SourceLocation FixLoc = VD->getTypeSpecStartLoc();
     if (FixLoc.isInvalid() || FixLoc.isMacroID())
       return;
-    DB << FixItHint::CreateInsertion(FixLoc, "static ");
+    if (FixMode == FixModeKind::UseStatic)
+      DB << FixItHint::CreateInsertion(FixLoc, "static ");
     return;
   }
   llvm_unreachable("");
diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h
index a3c1c33965903..0d6c3e43aa945 100644
--- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h
+++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h
@@ -20,17 +20,22 @@ namespace clang::tidy::misc {
 /// http://clang.llvm.org/extra/clang-tidy/checks/misc/use-internal-linkage.html
 class UseInternalLinkageCheck : public ClangTidyCheck {
 public:
-  UseInternalLinkageCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context),
-        HeaderFileExtensions(Context->getHeaderFileExtensions()) {}
+  UseInternalLinkageCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   std::optional<TraversalKind> getCheckTraversalKind() const override {
     return TK_IgnoreUnlessSpelledInSource;
   }
 
+  enum class FixModeKind {
+    None,
+    UseStatic,
+  };
+
 private:
   FileExtensionsSet HeaderFileExtensions;
+  FixModeKind FixMode;
 };
 
 } // namespace clang::tidy::misc
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 e8e43a1fb3d63..1fa019b6de0ea 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
@@ -25,3 +25,16 @@ Example:
   }
   // already declared as extern
   extern int v2;
+
+Options
+-------
+
+.. option:: FixMode
+
+  Selects the fix mode when fixing automatically.
+
+  ``None``
+    Don't fix automatically.
+
+  ``UseStatic``
+    Add ``static`` for internal linkage variable and function.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-fix-mode-none.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-fix-mode-none.cpp
new file mode 100644
index 0000000000000..3f2f5897bf718
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-fix-mode-none.cpp
@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
+// RUN:   -config="{CheckOptions: {misc-use-internal-linkage.FixMode: 'None'}}"  -- -I%S/Inputs/use-internal-linkage
+
+void func() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func'
+// CHECK-FIXES-NOT: static void func() {}
+
+int global;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'global'
+// CHECK-FIXES-NOT: static int global;
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 c244f32db8e96..1cdb655ee4454 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
@@ -1,4 +1,6 @@
 // RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage
+// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
+// RUN:   -config="{CheckOptions: {misc-use-internal-linkage.FixMode: 'UseStatic'}}"  -- -I%S/Inputs/use-internal-linkage
 
 #include "func.h"
 
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 b076e9125fc35..4a4d9c787d44b 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
@@ -1,4 +1,6 @@
 // RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage
+// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
+// RUN:   -config="{CheckOptions: {misc-use-internal-linkage.FixMode: 'UseStatic'}}"  -- -I%S/Inputs/use-internal-linkage
 
 #include "var.h"
 

>From 1dbad4a128660f85dfd1b3189a373a79c0087df4 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcong.cai at bmw.com>
Date: Mon, 24 Jun 2024 22:31:59 +0800
Subject: [PATCH 3/3] add more check

---
 .../test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp | 2 ++
 .../test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp  | 1 +
 2 files changed, 3 insertions(+)

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 1cdb655ee4454..9c91389542b03 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
@@ -11,9 +11,11 @@ void func() {}
 template<class T>
 void func_template() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_template'
+// CHECK-FIXES: static void func_template() {}
 
 void func_cpp_inc();
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_cpp_inc'
+// CHECK-FIXES: static void func_cpp_inc();
 
 #include "func_cpp.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 4a4d9c787d44b..01b8d28e61230 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
@@ -11,6 +11,7 @@ int global;
 template<class T>
 T global_template;
 // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: variable 'global_template'
+// CHECK-FIXES: static T global_template;
 
 int gloabl_header;
 



More information about the cfe-commits mailing list