From d1bd01c3183f537cbf63a71b8303a656b4c6cb65 Mon Sep 17 00:00:00 2001 From: Jonas Toth Date: Tue, 25 Sep 2018 18:12:28 +0000 Subject: [PATCH] [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 llvm-svn: 343000 --- .../clang-tidy/modernize/CMakeLists.txt | 1 + .../modernize/ConcatNestedNamespacesCheck.cpp | 113 +++++++++++++++ .../modernize/ConcatNestedNamespacesCheck.h | 41 ++++++ .../clang-tidy/modernize/ModernizeTidyModule.cpp | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 11 +- clang-tools-extra/docs/clang-tidy/checks/list.rst | 1 + .../checks/modernize-concat-nested-namespaces.rst | 49 +++++++ .../modernize-concat-nested-namespaces.cpp | 161 +++++++++++++++++++++ 8 files changed, 378 insertions(+), 2 deletions(-) create mode 100644 clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst create mode 100644 clang-tools-extra/test/clang-tidy/modernize-concat-nested-namespaces.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 907eb6c..06ea18d 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyModernizeModule AvoidBindCheck.cpp + ConcatNestedNamespacesCheck.cpp DeprecatedHeadersCheck.cpp LoopConvertCheck.cpp LoopConvertUtils.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp new file mode 100644 index 0000000..bef85f7 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp @@ -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 +#include + +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(*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("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 diff --git a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h new file mode 100644 index 0000000..547690d --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h @@ -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; + 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 diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 63b7d02..fa0a490 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/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 @@ class ModernizeModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck("modernize-avoid-bind"); + CheckFactories.registerCheck( + "modernize-concat-nested-namespaces"); CheckFactories.registerCheck( "modernize-deprecated-headers"); CheckFactories.registerCheck("modernize-loop-convert"); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index b18ee76..386c74e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -72,7 +72,7 @@ Improvements to clang-tidy - New :doc:`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 ` 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 + ` 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 ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 600969f..4180385f 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -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 diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst new file mode 100644 index 0000000..a1fdc7f --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst @@ -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(); + } + } + diff --git a/clang-tools-extra/test/clang-tidy/modernize-concat-nested-namespaces.cpp b/clang-tools-extra/test/clang-tidy/modernize-concat-nested-namespaces.cpp new file mode 100644 index 0000000..22a92f6 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/modernize-concat-nested-namespaces.cpp @@ -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; +} -- 2.7.4