[PATCH] D61989: [clang-tidy] enable modernize-concat-nested-namespaces on header files

[Xiao Shi via Phabricator via cfe-commits]

shixiao created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.

For header files that pass the -header-filter, the
modernize-concat-nested-namespaces check does not generate the expected
warning and fix for un-concatenated namespaces. This diff fixes it.

https://bugs.llvm.org/show_bug.cgi?id=41670

Test Plan:

a.h
---

namespace foo {
namespace bar {
namespace baz {
struct S {};
}
}

}
=

a.cpp
-----

namespace foo {
namespace bar {
namespace baz {
void foo(const S&) {}
}
}
}

int main () {

  return 0;

}
=

$ /clang-tidy -p sample/compile_commands.json 
-checks="modernize-concat-nested-namespaces" -header-filter="a.h" sample/a.cpp

  4 warnings generated.
  sample/a.h:5:1: warning: nested namespaces can be concatenated 
[modernize-concat-nested-namespaces]
  namespace foo {
  ^~~~~~~~~~~~~~~
  namespace foo::bar::baz
  sample/a.cpp:5:1: warning: nested namespaces can be concatenated 
[modernize-concat-nested-namespaces]
  namespace foo {
  ^~~~~~~~~~~~~~~
  namespace foo::bar::baz


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61989

Files:
  clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst


Index: clang-tools-extra/docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
@@ -47,3 +47,13 @@
   }
   }
 
+Options
+-------
+
+.. option:: HeaderFileExtensions
+
+   A comma-separated list of filename extensions of header files (the filename
+   extensions should not contain "." prefix). Default is "h,hh,hpp,hxx".
+   For header files without an extension, use an empty string (if there are no
+   other desired extensions) or leave an empty element in the list. e.g.,
+   "h,hh,hpp,hxx," (note the trailing comma).
Index: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
+++ clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_CONCATNESTEDNAMESPACESCHECK_H
 
 #include "../ClangTidyCheck.h"
+#include "../utils/HeaderFileExtensionsUtils.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 
@@ -17,10 +18,16 @@
 namespace tidy {
 namespace modernize {
 
+/// The check supports these options:
+///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
+///     header files (The filename extensions should not contain "." prefix).
+///     "h,hh,hpp,hxx" by default.
+///     For extension-less header files, using an empty string or leaving an
+///     empty string between "," if there are other filename extensions.
 class ConcatNestedNamespacesCheck : public ClangTidyCheck {
 public:
-  ConcatNestedNamespacesCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  ConcatNestedNamespacesCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
@@ -32,6 +39,8 @@
                         const SourceRange &BackReplacement);
   NamespaceString concatNamespaces();
   NamespaceContextVec Namespaces;
+  const std::string RawStringHeaderFileExtensions;
+  utils::HeaderFileExtensionsSet HeaderFileExtensions;
 };
 } // namespace modernize
 } // namespace tidy
Index: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
@@ -61,6 +61,23 @@
   return Result;
 }
 
+ConcatNestedNamespacesCheck::ConcatNestedNamespacesCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
+          "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
+  if (!utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+                                        HeaderFileExtensions, ',')) {
+    llvm::errs() << "Invalid header file extension: "
+                 << RawStringHeaderFileExtensions << "\n";
+  }
+}
+
+void ConcatNestedNamespacesCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions);
+}
+
 void ConcatNestedNamespacesCheck::registerMatchers(
     ast_matchers::MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus17)
@@ -85,8 +102,12 @@
   if (!locationsInSameFile(Sources, ND.getBeginLoc(), ND.getRBraceLoc()))
     return;
 
-  if (!Sources.isInMainFile(ND.getBeginLoc()))
-    return;
+  if (!Sources.isInMainFile(ND.getBeginLoc())) {
+    if (!utils::isSpellingLocInHeaderFile(
+            ND.getBeginLoc(), *Result.SourceManager, HeaderFileExtensions)) {
+      return;
+    }
+  }
 
   if (anonymousOrInlineNamespace(ND))
     return;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits