[llvm-branch-commits] [clang-tidy] Add FixIts for libc namespace macros (PR #99681)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Paul Kirth (ilovepi)

<details>
<summary>Changes</summary>

This adds a few FixIts that update the usage of namespaces other than
LIBC_NAMESPACE_DECL, and add the required header when its missing.


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


4 Files Affected:

- (modified) clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp (+33-4) 
- (modified) clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.h (+11-1) 
- (modified) clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h (+2) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp (+5-1) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
index bb436e4d12a30..e6e4900283c79 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
@@ -8,7 +8,6 @@
 
 #include "ImplementationInNamespaceCheck.h"
 #include "NamespaceConstants.h"
-#include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
 using namespace clang::ast_matchers;
@@ -25,6 +24,16 @@ void ImplementationInNamespaceCheck::registerMatchers(MatchFinder *Finder) {
       this);
 }
 
+void ImplementationInNamespaceCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+}
+
+void ImplementationInNamespaceCheck::registerPPCallbacks(
+    const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  IncludeInserter.registerPreprocessor(PP);
+}
+
 void ImplementationInNamespaceCheck::check(
     const MatchFinder::MatchResult &Result) {
   const auto *MatchedDecl =
@@ -41,8 +50,26 @@ void ImplementationInNamespaceCheck::check(
 
   // Enforce that the namespace is the result of macro expansion
   if (Result.SourceManager->isMacroBodyExpansion(NS->getLocation()) == false) {
-    diag(NS->getLocation(), "the outermost namespace should be the '%0' macro")
-        << RequiredNamespaceDeclMacroName;
+    auto DB = diag(NS->getLocation(),
+                   "the outermost namespace should be the '%0' macro")
+              << RequiredNamespaceDeclMacroName;
+
+    // TODO: Determine how to split inline namespaces correctly in the FixItHint
+    //
+    // We can't easily replace LIBC_NAMEPACE::inner::namespace { with
+    //
+    // namespace LIBC_NAMEPACE_DECL {
+    //   namespace inner::namespace {
+    //
+    // For now, just update the simple case w/ LIBC_NAMEPACE_DECL
+    if (!NS->isInlineNamespace())
+      DB << FixItHint::CreateReplacement(NS->getLocation(),
+                                         RequiredNamespaceDeclMacroName);
+
+    DB << IncludeInserter.createIncludeInsertion(
+        Result.SourceManager->getFileID(NS->getBeginLoc()),
+        NamespaceMacroHeader);
+
     return;
   }
 
@@ -51,7 +78,9 @@ void ImplementationInNamespaceCheck::check(
   // instead.
   if (NS->getVisibility() != Visibility::HiddenVisibility) {
     diag(NS->getLocation(), "the '%0' macro should start with '%1'")
-        << RequiredNamespaceDeclMacroName << RequiredNamespaceDeclStart;
+        << RequiredNamespaceDeclMacroName << RequiredNamespaceDeclStart
+        << FixItHint::CreateReplacement(NS->getLocation(),
+                                        RequiredNamespaceDeclMacroName);
     return;
   }
 
diff --git a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.h b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.h
index 42da38f728bb8..f7fc19fda180a 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.h
+++ b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVMLIBC_IMPLEMENTATIONINNAMESPACECHECK_H
 
 #include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
 
 namespace clang::tidy::llvm_libc {
 
@@ -20,13 +21,22 @@ namespace clang::tidy::llvm_libc {
 class ImplementationInNamespaceCheck : public ClangTidyCheck {
 public:
   ImplementationInNamespaceCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+      : ClangTidyCheck(Name, Context),
+        IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+                                                 utils::IncludeSorter::IS_LLVM),
+                        areDiagsSelfContained()) {}
 
   bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
     return LangOpts.CPlusPlus;
   }
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+                           Preprocessor *ModuleExpanderPP) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  utils::IncludeInserter IncludeInserter;
 };
 
 } // namespace clang::tidy::llvm_libc
diff --git a/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h b/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h
index 83908a7875d03..db85c7b8b93f0 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h
+++ b/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h
@@ -16,5 +16,7 @@ const static llvm::StringRef RequiredNamespaceDeclStart =
     "[[gnu::visibility(\"hidden\")]] __llvm_libc";
 const static llvm::StringRef RequiredNamespaceDeclMacroName =
     "LIBC_NAMESPACE_DECL";
+const static llvm::StringRef
+    NamespaceMacroHeader("src/__support/macros/config.h");
 
 } // namespace clang::tidy::llvm_libc
diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp
index 4a5576f2c5d62..ad60c04265533 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s llvmlibc-implementation-in-namespace %t
+// RUN: %check_clang_tidy %s llvmlibc-implementation-in-namespace %t -fix
+
+// CHECK-FIXES: #include "src/__support/macros/config.h"
 
 #define MACRO_A "defining macros outside namespace is valid"
 
@@ -15,6 +17,7 @@ void funcF() {}
 
 namespace outer_most {
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should be the 'LIBC_NAMESPACE_DECL' macro
+// CHECK-FIXES: namespace LIBC_NAMESPACE_DECL {
  class A {};
 }
 
@@ -26,6 +29,7 @@ namespace {
 
 namespace namespaceG {
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should be the 'LIBC_NAMESPACE_DECL' macro
+// CHECK-FIXES: namespace LIBC_NAMESPACE_DECL {
 namespace __llvm_libc {
 namespace namespaceH {
 class ClassB;

``````````

</details>


https://github.com/llvm/llvm-project/pull/99681