wgml created this revision.
wgml added reviewers: alexfh, aaron.ballman, hokein.
Herald added subscribers: xazax.hun, mgorny.
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();
}}
namespace c { namespace d { namespace e { }}}
can become
namespace a::b {
void test();
}
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52136
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
clang-tidy/modernize/ConcatNestedNamespacesCheck.h
clang-tidy/modernize/ModernizeTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
test/clang-tidy/modernize-concat-nested-namespaces.cpp
Index: test/clang-tidy/modernize-concat-nested-namespaces.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-concat-nested-namespaces.cpp
@@ -0,0 +1,124 @@
+// 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 are empty and can be removed [modernize-concat-nested-namespaces]
+} // namespace n31
+
+namespace n33 {
+namespace n34 {
+namespace n35 {}
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Nested namespaces are empty and can be removed [modernize-concat-nested-namespaces]
+} // namespace n34
+namespace n36 {
+void t();
+}
+} // namespace n33
+
+namespace n37::n38 {
+void t();
+}
+
+int main() {
+ n26::n27::n28::n29::n30::t();
+ return 0;
+}
Index: docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
@@ -0,0 +1,47 @@
+.. title:: clang-tidy - modernize-concat-nested-namespaces
+
+modernize-concat-nested-namespaces
+==================================
+
+Checks for use of nested namespaces in a form of ``namespace a { namespace b { ... } }``
+and offers change to syntax introduced in C++17: ``namespace a::b { ... }``.
+Inlined namespaces are not modified.
+
+For example:
+
+.. code-block:: c++
+ namspace 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++
+ namspace n1::n2 {
+ void t();
+ }
+
+ namespace n3 {
+ namespace n4::n5 {
+ void t();
+ }
+ namespace n6::n7 {
+ void t();
+ }
+ }
+
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -9,8 +9,8 @@
abseil-no-internal-dependencies
abseil-no-namespace
abseil-redundant-strcat-calls
- abseil-string-find-startswith
abseil-str-cat-append
+ abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
@@ -172,6 +172,7 @@
misc-unused-parameters
misc-unused-using-decls
modernize-avoid-bind
+ modernize-concat-nested-namespaces
modernize-deprecated-headers
modernize-loop-convert
modernize-make-shared
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -72,7 +72,7 @@
- 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 @@
- 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.
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
===================================================================
--- clang-tidy/modernize/ModernizeTidyModule.cpp
+++ clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -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 @@
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");
Index: clang-tidy/modernize/ConcatNestedNamespacesCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/modernize/ConcatNestedNamespacesCheck.h
@@ -0,0 +1,42 @@
+//===--- 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/SmallVector.h"
+#include <string>
+
+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:
+ struct NamespaceContext {
+ std::string Name;
+ SourceLocation Begin, NameBegin, RBrace;
+ };
+ using NamespaceContextVec = llvm::SmallVector<NamespaceContext, 6>;
+
+ static std::string concatNamespaces(NamespaceContextVec const &Namespaces);
+ NamespaceContextVec Namespaces;
+};
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_CONCATNESTEDNAMESPACESCHECK_H
Index: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
@@ -0,0 +1,129 @@
+//===--- 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 <iterator>
+#include <sstream>
+
+using namespace clang::ast_matchers;
+
+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(NamespaceDecl const &ND) {
+ return ND.isAnonymousNamespace() || ND.isInlineNamespace();
+}
+
+static std::size_t childrenCount(NamespaceDecl::decl_range const &Decls) {
+ return std::distance(Decls.begin(), Decls.end());
+}
+
+static bool singleNamedNamespaceChild(NamespaceDecl const &ND) {
+ auto const Decls = ND.decls();
+ if (childrenCount(Decls) != 1)
+ return false;
+
+ auto const Decl = *Decls.begin();
+ if (Decl->getKind() != Decl::Kind::Namespace)
+ return false;
+
+ auto const *NS = reinterpret_cast<NamespaceDecl const *>(Decl);
+ return !anonymousOrInlineNamespace(*NS);
+}
+
+static bool alreadyConcatenated(std::size_t NumCandidates,
+ SourceRange const &ReplacementRange,
+ SourceManager const &Sources,
+ LangOptions const &LangOpts) {
+ auto const TextRange =
+ Lexer::getAsCharRange(ReplacementRange, Sources, LangOpts);
+ auto const CurrentNamespacesText =
+ Lexer::getSourceText(TextRange, Sources, LangOpts);
+ return CurrentNamespacesText.count(':') == (NumCandidates - 1) * 2;
+}
+
+std::string ConcatNestedNamespacesCheck::concatNamespaces(
+ NamespaceContextVec const &Namespaces) {
+ std::ostringstream Result;
+ bool First = true;
+ for (auto const &NS : Namespaces) {
+ Result << (First ? "namespace " : "::") << NS.Name;
+ First = false;
+ }
+
+ return Result.str();
+}
+
+void ConcatNestedNamespacesCheck::registerMatchers(MatchFinder *Finder) {
+ if (getLangOpts().CPlusPlus)
+ Finder->addMatcher(namespaceDecl().bind("namespace"), this);
+}
+
+void ConcatNestedNamespacesCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ NamespaceDecl const &ND = *Result.Nodes.getNodeAs<NamespaceDecl>("namespace");
+ SourceManager const &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.getNameAsString(), ND.getBeginLoc(),
+ ND.getLocation(), ND.getRBraceLoc()});
+
+ if (singleNamedNamespaceChild(ND))
+ return;
+
+ if (Namespaces.size() == 1) {
+ Namespaces.clear();
+ return;
+ }
+
+ if (childrenCount(ND.decls()) == 0) {
+ SourceRange Removal(Namespaces.front().Begin, Namespaces.front().RBrace);
+
+ diag(Namespaces.front().Begin,
+ "Nested namespaces are empty and can be removed",
+ DiagnosticIDs::Warning)
+ << FixItHint::CreateRemoval(Removal);
+ } else {
+ SourceRange FrontReplacement(Namespaces.front().Begin,
+ Namespaces.back().NameBegin);
+ SourceRange BackReplacement(Namespaces.back().RBrace,
+ Namespaces.front().RBrace);
+
+ if (!alreadyConcatenated(Namespaces.size(), FrontReplacement, Sources,
+ getLangOpts())) {
+ diag(Namespaces.front().Begin, "Nested namespaces can be concatenated",
+ DiagnosticIDs::Warning)
+ << FixItHint::CreateReplacement(FrontReplacement,
+ concatNamespaces(Namespaces))
+ << FixItHint::CreateReplacement(BackReplacement, std::string("}"));
+ }
+ }
+ Namespaces.clear();
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/modernize/CMakeLists.txt
===================================================================
--- clang-tidy/modernize/CMakeLists.txt
+++ clang-tidy/modernize/CMakeLists.txt
@@ -2,6 +2,7 @@
add_clang_library(clangTidyModernizeModule
AvoidBindCheck.cpp
+ ConcatNestedNamespacesCheck.cpp
DeprecatedHeadersCheck.cpp
LoopConvertCheck.cpp
LoopConvertUtils.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits