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; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits