Skip to content

Commit

Permalink
[clang-tidy] Add modernize-concat-nested-namespaces check
Browse files Browse the repository at this point in the history
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
  • Loading branch information
JonasToth committed Sep 25, 2018
1 parent 02d600d commit d1bd01c
Show file tree
Hide file tree
Showing 8 changed files with 378 additions and 2 deletions.
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
Expand Up @@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support)

add_clang_library(clangTidyModernizeModule
AvoidBindCheck.cpp
ConcatNestedNamespacesCheck.cpp
DeprecatedHeadersCheck.cpp
LoopConvertCheck.cpp
LoopConvertUtils.cpp
Expand Down
113 changes: 113 additions & 0 deletions 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 <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
@@ -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
Expand Up @@ -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"
Expand Down Expand Up @@ -46,6 +47,8 @@ class ModernizeModule : public ClangTidyModule {
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");
Expand Down
11 changes: 9 additions & 2 deletions clang-tools-extra/docs/ReleaseNotes.rst
Expand Up @@ -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
Expand All @@ -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.

Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/docs/clang-tidy/checks/list.rst
Expand Up @@ -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
Expand Down
@@ -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();
}
}

0 comments on commit d1bd01c

Please sign in to comment.