[clang-tools-extra] r343000 - [clang-tidy] Add modernize-concat-nested-namespaces check

Jonas Toth via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 25 11:12:28 PDT 2018


Author: jonastoth
Date: Tue Sep 25 11:12:28 2018
New Revision: 343000

URL: http://llvm.org/viewvc/llvm-project?rev=343000&view=rev
Log:
[clang-tidy] Add modernize-concat-nested-namespaces check

Summary:
Finds instances of namespaces concatenated using explicit syntax, such as `namespace a { namespace b { [...] }}` and offers fix to glue it to `namespace a::b { [...] }`.

Properly handles `inline` and unnamed namespaces. ~~Also, detects empty blocks in nested namespaces and offers to remove them.~~

Test with common use cases included.
I ran the check against entire llvm repository. Except for expected `nested namespace definitions only available with -std=c++17 or -std=gnu++17` warnings I noticed no issues when the check was performed.

Example:
```
namespace a { namespace b {
void test();
}}

```
can become
```
namespace a::b {
void test();
}
```

Patch by wgml!

Reviewers: alexfh, aaron.ballman, hokein

Reviewed By: aaron.ballman

Subscribers: JonasToth, Eugene.Zelenko, lebedev.ri, mgorny, xazax.hun, cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D52136

Added:
    clang-tools-extra/trunk/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
    clang-tools-extra/trunk/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
    clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
    clang-tools-extra/trunk/test/clang-tidy/modernize-concat-nested-namespaces.cpp
Modified:
    clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
    clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp
    clang-tools-extra/trunk/docs/ReleaseNotes.rst
    clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt?rev=343000&r1=342999&r2=343000&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt Tue Sep 25 11:12:28 2018
@@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyModernizeModule
   AvoidBindCheck.cpp
+  ConcatNestedNamespacesCheck.cpp
   DeprecatedHeadersCheck.cpp
   LoopConvertCheck.cpp
   LoopConvertUtils.cpp

Added: clang-tools-extra/trunk/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp?rev=343000&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp Tue Sep 25 11:12:28 2018
@@ -0,0 +1,113 @@
+//===--- ConcatNestedNamespacesCheck.cpp - clang-tidy----------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ConcatNestedNamespacesCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <algorithm>
+#include <iterator>
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+static bool locationsInSameFile(const SourceManager &Sources,
+                                SourceLocation Loc1, SourceLocation Loc2) {
+  return Loc1.isFileID() && Loc2.isFileID() &&
+         Sources.getFileID(Loc1) == Sources.getFileID(Loc2);
+}
+
+static bool anonymousOrInlineNamespace(const NamespaceDecl &ND) {
+  return ND.isAnonymousNamespace() || ND.isInlineNamespace();
+}
+
+static bool singleNamedNamespaceChild(const NamespaceDecl &ND) {
+  NamespaceDecl::decl_range Decls = ND.decls();
+  if (std::distance(Decls.begin(), Decls.end()) != 1)
+    return false;
+
+  const auto *ChildNamespace = dyn_cast<const NamespaceDecl>(*Decls.begin());
+  return ChildNamespace && !anonymousOrInlineNamespace(*ChildNamespace);
+}
+
+static bool alreadyConcatenated(std::size_t NumCandidates,
+                                const SourceRange &ReplacementRange,
+                                const SourceManager &Sources,
+                                const LangOptions &LangOpts) {
+  CharSourceRange TextRange =
+      Lexer::getAsCharRange(ReplacementRange, Sources, LangOpts);
+  StringRef CurrentNamespacesText =
+      Lexer::getSourceText(TextRange, Sources, LangOpts);
+  return CurrentNamespacesText.count(':') == (NumCandidates - 1) * 2;
+}
+
+ConcatNestedNamespacesCheck::NamespaceString
+ConcatNestedNamespacesCheck::concatNamespaces() {
+  NamespaceString Result("namespace ");
+  Result.append(Namespaces.front()->getName());
+
+  std::for_each(std::next(Namespaces.begin()), Namespaces.end(),
+                [&Result](const NamespaceDecl *ND) {
+                  Result.append("::");
+                  Result.append(ND->getName());
+                });
+
+  return Result;
+}
+
+void ConcatNestedNamespacesCheck::registerMatchers(
+    ast_matchers::MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus17)
+    return;
+
+  Finder->addMatcher(ast_matchers::namespaceDecl().bind("namespace"), this);
+}
+
+void ConcatNestedNamespacesCheck::reportDiagnostic(
+    const SourceRange &FrontReplacement, const SourceRange &BackReplacement) {
+  diag(Namespaces.front()->getBeginLoc(),
+       "nested namespaces can be concatenated", DiagnosticIDs::Warning)
+      << FixItHint::CreateReplacement(FrontReplacement, concatNamespaces())
+      << FixItHint::CreateReplacement(BackReplacement, "}");
+}
+
+void ConcatNestedNamespacesCheck::check(
+    const ast_matchers::MatchFinder::MatchResult &Result) {
+  const NamespaceDecl &ND = *Result.Nodes.getNodeAs<NamespaceDecl>("namespace");
+  const SourceManager &Sources = *Result.SourceManager;
+
+  if (!locationsInSameFile(Sources, ND.getBeginLoc(), ND.getRBraceLoc()))
+    return;
+
+  if (!Sources.isInMainFile(ND.getBeginLoc()))
+    return;
+
+  if (anonymousOrInlineNamespace(ND))
+    return;
+
+  Namespaces.push_back(&ND);
+
+  if (singleNamedNamespaceChild(ND))
+    return;
+
+  SourceRange FrontReplacement(Namespaces.front()->getBeginLoc(),
+                               Namespaces.back()->getLocation());
+  SourceRange BackReplacement(Namespaces.back()->getRBraceLoc(),
+                              Namespaces.front()->getRBraceLoc());
+
+  if (!alreadyConcatenated(Namespaces.size(), FrontReplacement, Sources,
+                           getLangOpts()))
+    reportDiagnostic(FrontReplacement, BackReplacement);
+
+  Namespaces.clear();
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang

Added: clang-tools-extra/trunk/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/ConcatNestedNamespacesCheck.h?rev=343000&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/ConcatNestedNamespacesCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/modernize/ConcatNestedNamespacesCheck.h Tue Sep 25 11:12:28 2018
@@ -0,0 +1,41 @@
+//===--- ConcatNestedNamespacesCheck.h - clang-tidy--------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_CONCATNESTEDNAMESPACESCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_CONCATNESTEDNAMESPACESCHECK_H
+
+#include "../ClangTidy.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/SmallVector.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+class ConcatNestedNamespacesCheck : public ClangTidyCheck {
+public:
+  ConcatNestedNamespacesCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  using NamespaceContextVec = llvm::SmallVector<const NamespaceDecl *, 6>;
+  using NamespaceString = llvm::SmallString<40>;
+
+  void reportDiagnostic(const SourceRange &FrontReplacement,
+                        const SourceRange &BackReplacement);
+  NamespaceString concatNamespaces();
+  NamespaceContextVec Namespaces;
+};
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_CONCATNESTEDNAMESPACESCHECK_H

Modified: clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp?rev=343000&r1=342999&r2=343000&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp Tue Sep 25 11:12:28 2018
@@ -11,6 +11,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "AvoidBindCheck.h"
+#include "ConcatNestedNamespacesCheck.h"
 #include "DeprecatedHeadersCheck.h"
 #include "LoopConvertCheck.h"
 #include "MakeSharedCheck.h"
@@ -46,6 +47,8 @@ class ModernizeModule : public ClangTidy
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<AvoidBindCheck>("modernize-avoid-bind");
+    CheckFactories.registerCheck<ConcatNestedNamespacesCheck>(
+        "modernize-concat-nested-namespaces");
     CheckFactories.registerCheck<DeprecatedHeadersCheck>(
         "modernize-deprecated-headers");
     CheckFactories.registerCheck<LoopConvertCheck>("modernize-loop-convert");

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=343000&r1=342999&r2=343000&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Tue Sep 25 11:12:28 2018
@@ -72,7 +72,7 @@ Improvements to clang-tidy
 
 - New :doc:`abseil-no-internal-dependencies
   <clang-tidy/checks/abseil-no-internal-dependencies>` check.
-  
+
   Gives a warning if code using Abseil depends on internal details.
 
 - New :doc:`abseil-no-namespace
@@ -90,9 +90,16 @@ Improvements to clang-tidy
 - New :doc:`abseil-str-cat-append
   <clang-tidy/checks/abseil-str-cat-append>` check.
 
-  Flags uses of ``absl::StrCat()`` to append to a ``std::string``. Suggests 
+  Flags uses of ``absl::StrCat()`` to append to a ``std::string``. Suggests
   ``absl::StrAppend()`` should be used instead.
 
+- New :doc:`modernize-concat-nested-namespaces
+  <clang-tidy/checks/modernize-concat-nested-namespaces>` check.
+
+  Checks for uses of nested namespaces in the form of
+  ``namespace a { namespace b { ... }}`` and offers change to
+  syntax introduced in C++17 standard: ``namespace a::b { ... }``.
+
 - New :doc:`readability-magic-numbers
   <clang-tidy/checks/readability-magic-numbers>` check.
 

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=343000&r1=342999&r2=343000&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Tue Sep 25 11:12:28 2018
@@ -172,6 +172,7 @@ Clang-Tidy Checks
    misc-unused-parameters
    misc-unused-using-decls
    modernize-avoid-bind
+   modernize-concat-nested-namespaces
    modernize-deprecated-headers
    modernize-loop-convert
    modernize-make-shared

Added: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst?rev=343000&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst Tue Sep 25 11:12:28 2018
@@ -0,0 +1,49 @@
+.. title:: clang-tidy - modernize-concat-nested-namespaces
+
+modernize-concat-nested-namespaces
+==================================
+
+Checks for use of nested namespaces such as ``namespace a { namespace b { ... } }``
+and suggests changing to the more concise syntax introduced in C++17: ``namespace a::b { ... }``.
+Inline namespaces are not modified.
+
+For example:
+
+.. code-block:: c++
+
+  namespace n1 {
+  namespace n2 {
+  void t();
+  }
+  }
+
+  namespace n3 {
+  namespace n4 {
+  namespace n5 {
+  void t();
+  }
+  }
+  namespace n6 {
+  namespace n7 {
+  void t();
+  }
+  }
+  }
+
+Will be modified to:
+
+.. code-block:: c++
+
+  namespace n1::n2 {
+  void t();
+  }
+
+  namespace n3 {
+  namespace n4::n5 {
+  void t();
+  }
+  namespace n6::n7 {
+  void t();
+  }
+  }
+

Added: clang-tools-extra/trunk/test/clang-tidy/modernize-concat-nested-namespaces.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-concat-nested-namespaces.cpp?rev=343000&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/modernize-concat-nested-namespaces.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-concat-nested-namespaces.cpp Tue Sep 25 11:12:28 2018
@@ -0,0 +1,161 @@
+// RUN: %check_clang_tidy %s modernize-concat-nested-namespaces %t -- -- -std=c++17
+
+namespace n1 {}
+
+namespace n2 {
+namespace n3 {
+void t();
+}
+namespace n4 {
+void t();
+}
+} // namespace n2
+
+namespace n5 {
+inline namespace n6 {
+void t();
+}
+} // namespace n5
+
+namespace n7 {
+void t();
+
+namespace n8 {
+void t();
+}
+} // namespace n7
+
+namespace n9 {
+namespace n10 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n9::n10
+void t();
+} // namespace n10
+} // namespace n9
+// CHECK-FIXES: }
+
+namespace n11 {
+namespace n12 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n11::n12
+namespace n13 {
+void t();
+}
+namespace n14 {
+void t();
+}
+} // namespace n12
+} // namespace n11
+// CHECK-FIXES: }
+
+namespace n15 {
+namespace n16 {
+void t();
+}
+
+inline namespace n17 {
+void t();
+}
+
+namespace n18 {
+namespace n19 {
+namespace n20 {
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n18::n19::n20
+void t();
+} // namespace n20
+} // namespace n19
+} // namespace n18
+// CHECK-FIXES: }
+
+namespace n21 {
+void t();
+}
+} // namespace n15
+
+namespace n22 {
+namespace {
+void t();
+}
+} // namespace n22
+
+namespace n23 {
+namespace {
+namespace n24 {
+namespace n25 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n24::n25
+void t();
+} // namespace n25
+} // namespace n24
+// CHECK-FIXES: }
+} // namespace
+} // namespace n23
+
+namespace n26::n27 {
+namespace n28 {
+namespace n29::n30 {
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n26::n27::n28::n29::n30
+void t() {}
+} // namespace n29::n30
+} // namespace n28
+} // namespace n26::n27
+// CHECK-FIXES: }
+
+namespace n31 {
+namespace n32 {}
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+} // namespace n31
+// CHECK-FIXES-EMPTY
+
+namespace n33 {
+namespace n34 {
+namespace n35 {}
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+} // namespace n34
+// CHECK-FIXES-EMPTY
+namespace n36 {
+void t();
+}
+} // namespace n33
+
+namespace n37::n38 {
+void t();
+}
+
+#define IEXIST
+namespace n39 {
+namespace n40 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n39::n40
+#ifdef IEXIST
+void t() {}
+#endif
+} // namespace n40
+} // namespace n39
+// CHECK-FIXES: }
+
+namespace n41 {
+namespace n42 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n41::n42
+#ifdef IDONTEXIST
+void t() {}
+#endif
+} // namespace n42
+} // namespace n41
+// CHECK-FIXES: }
+
+int main() {
+  n26::n27::n28::n29::n30::t();
+#ifdef IEXIST
+  n39::n40::t();
+#endif
+
+#ifdef IDONTEXIST
+  n41::n42::t();
+#endif
+
+  return 0;
+}




More information about the cfe-commits mailing list