Add NamespaceCommentCheck to the Google module.

[alexfh]

Summary

This uses a bit hacky way to set the defaults for the spaces before
comments, but it's also one of the simplest ways. Fixed a bug with how the
SpacesBeforeComments option was used.

[djasper]

Hm.. Should we just move this to misc-? Doesn't seem to be specific to either Google or LLVM style..

clang-tidy/llvm/NamespaceCommentCheck.cpp

0

Maybe this should contain a string NameSpaceCommentPrefix so that we can configure more and e.g. also support:

namespace abc {
} // abc

[alexfh]

Moved NamespaceCommentCheck.* to clang-tidy/readability.

[alexfh]

clang-tidy/llvm/NamespaceCommentCheck.cpp

0

It could make sense once we have a style that can't be supported using existing approach.

[djasper]

Looks good, I think, although I find the display of added/deleted files a bit confusing.

clang-tidy/CMakeLists.txt

add_clang_library(clangTidy
   clangStaticAnalyzerFrontend
   clangTooling
   )
 
 add_subdirectory(tool)
 add_subdirectory(llvm)
 add_subdirectory(google)
 add_subdirectory(misc)
+add_subdirectory(readability)
 add_subdirectory(utils)

clang-tidy/Makefile

 ##===- tools/extra/clang-tidy/Makefile ---------------------*- Makefile -*-===##
 #
 #                     The LLVM Compiler Infrastructure
 #
 # This file is distributed under the University of Illinois Open Source
 # License. See LICENSE.TXT for details.
 #
 ##===----------------------------------------------------------------------===##
 
 CLANG_LEVEL := ../../..
 LIBRARYNAME := clangTidy
 include $(CLANG_LEVEL)/../../Makefile.config
 
-DIRS = utils llvm google misc tool
+DIRS = utils readability llvm google misc tool
 
 include $(CLANG_LEVEL)/Makefile

clang-tidy/google/GoogleTidyModule.cpp

 #include "IntegerTypesCheck.h"
 #include "MemsetZeroLengthCheck.h"
 #include "NamedParameterCheck.h"
 #include "OverloadedUnaryAndCheck.h"
 #include "StringReferenceMemberCheck.h"
 #include "TodoCommentCheck.h"
 #include "UnnamedNamespaceInHeaderCheck.h"
 #include "UsingNamespaceDirectiveCheck.h"
+#include "../readability/NamespaceCommentCheck.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 
 class GoogleModule : public ClangTidyModule {
 public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<runtime::MemsetZeroLengthCheck>(
         "google-runtime-memset");
     CheckFactories.registerCheck<readability::AvoidCStyleCastsCheck>(
         "google-readability-casting");
     CheckFactories.registerCheck<readability::NamedParameterCheck>(
         "google-readability-function");
     CheckFactories.registerCheck<readability::TodoCommentCheck>(
         "google-readability-todo");
+    CheckFactories.registerCheck<readability::NamespaceCommentCheck>(
+        "google-readability-namespace-comments");
   }
 };
 
 // Register the GoogleTidyModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<GoogleModule> X("google-module",
                                                     "Adds Google lint checks.");
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the GoogleModule.
 volatile int GoogleModuleAnchorSource = 0;
 
 } // namespace tidy
 } // namespace clang

clang-tidy/llvm/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyLLVMModule
   HeaderGuardCheck.cpp
   IncludeOrderCheck.cpp
   LLVMTidyModule.cpp
-  NamespaceCommentCheck.cpp
   TwineLocalCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
+  clangTidyReadability
   clangTidyUtils
   clangTooling
   )

clang-tidy/llvm/LLVMTidyModule.cpp

 //===--- LLVMTidyModule.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 "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "HeaderGuardCheck.h"
 #include "IncludeOrderCheck.h"
-#include "NamespaceCommentCheck.h"
+#include "../readability/NamespaceCommentCheck.h"
 #include "TwineLocalCheck.h"
 
 namespace clang {
 namespace tidy {
 
 class LLVMModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<LLVMHeaderGuardCheck>("llvm-header-guard");
     CheckFactories.registerCheck<IncludeOrderCheck>("llvm-include-order");
-    CheckFactories.registerCheck<NamespaceCommentCheck>(
+    CheckFactories.registerCheck<readability::NamespaceCommentCheck>(
         "llvm-namespace-comment");
     CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local");
   }
 };
 
 // Register the LLVMTidyModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<LLVMModule> X("llvm-module",
                                                   "Adds LLVM lint checks.");
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the LLVMModule.
 volatile int LLVMModuleAnchorSource = 0;
 
 } // namespace tidy
 } // namespace clang

clang-tidy/llvm/NamespaceCommentCheck.cpp

-//===--- NamespaceCommentCheck.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 "NamespaceCommentCheck.h"
-#include "clang/AST/ASTContext.h"
-#include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/Lex/Lexer.h"
-#include "llvm/ADT/StringExtras.h"
-
-using namespace clang::ast_matchers;
-
-namespace clang {
-namespace tidy {
-
-NamespaceCommentCheck::NamespaceCommentCheck(StringRef Name,
-                                             ClangTidyContext *Context)
-    : ClangTidyCheck(Name, Context),
-      NamespaceCommentPattern("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
-                              "namespace( +([a-zA-Z0-9_]+))? *(\\*/)?$",
-                              llvm::Regex::IgnoreCase),
-      ShortNamespaceLines(Options.get("ShortNamespaceLines", 1u)),
-      SpacesBeforeComments(Options.get("SpacesBeforeComments", 1u)) {}
-
-void NamespaceCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
-  Options.store(Opts, "ShortNamespaceLines", ShortNamespaceLines);
-  Options.store(Opts, "SpacesBeforeComments", SpacesBeforeComments);
-}
-
-void NamespaceCommentCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(namespaceDecl().bind("namespace"), this);
-}
-
-bool locationsInSameFile(const SourceManager &Sources, SourceLocation Loc1,
-                         SourceLocation Loc2) {
-  return Loc1.isFileID() && Loc2.isFileID() &&
-         Sources.getFileID(Loc1) == Sources.getFileID(Loc2);
-}
-
-std::string getNamespaceComment(const NamespaceDecl *ND, bool InsertLineBreak,
-                                unsigned SpacesBeforeComments) {
-  std::string Fix = "// namespace";
-  if (!ND->isAnonymousNamespace())
-    Fix.append(std::string(SpacesBeforeComments, ' '))
-        .append(ND->getNameAsString());
-  if (InsertLineBreak)
-    Fix.append("\n");
-  return Fix;
-}
-
-void NamespaceCommentCheck::check(const MatchFinder::MatchResult &Result) {
-  const NamespaceDecl *ND = Result.Nodes.getNodeAs<NamespaceDecl>("namespace");
-  const SourceManager &Sources = *Result.SourceManager;
-
-  if (!locationsInSameFile(Sources, ND->getLocStart(), ND->getRBraceLoc()))
-    return;
-
-  // Don't require closing comments for namespaces spanning less than certain
-  // number of lines.
-  unsigned StartLine = Sources.getSpellingLineNumber(ND->getLocStart());
-  unsigned EndLine = Sources.getSpellingLineNumber(ND->getRBraceLoc());
-  if (EndLine - StartLine + 1 <= ShortNamespaceLines)
-    return;
-
-  // Find next token after the namespace closing brace.
-  SourceLocation AfterRBrace = ND->getRBraceLoc().getLocWithOffset(1);
-  SourceLocation Loc = AfterRBrace;
-  Token Tok;
-  // Skip whitespace until we find the next token.
-  while (Lexer::getRawToken(Loc, Tok, Sources, Result.Context->getLangOpts())) {
-    Loc = Loc.getLocWithOffset(1);
-  }
-  if (!locationsInSameFile(Sources, ND->getRBraceLoc(), Loc))
-    return;
-
-  bool NextTokenIsOnSameLine = Sources.getSpellingLineNumber(Loc) == EndLine;
-  // If we insert a line comment before the token in the same line, we need
-  // to insert a line break.
-  bool NeedLineBreak = NextTokenIsOnSameLine && Tok.isNot(tok::eof);
-
-  // Try to find existing namespace closing comment on the same line.
-  if (Tok.is(tok::comment) && NextTokenIsOnSameLine) {
-    StringRef Comment(Sources.getCharacterData(Loc), Tok.getLength());
-    SmallVector<StringRef, 6> Groups;
-    if (NamespaceCommentPattern.match(Comment, &Groups)) {
-      StringRef NamespaceNameInComment = Groups.size() >= 6 ? Groups[5] : "";
-
-      // Check if the namespace in the comment is the same.
-      if ((ND->isAnonymousNamespace() && NamespaceNameInComment.empty()) ||
-          ND->getNameAsString() == NamespaceNameInComment) {
-        // FIXME: Maybe we need a strict mode, where we always fix namespace
-        // comments with different format.
-        return;
-      }
-
-      // Otherwise we need to fix the comment.
-      NeedLineBreak = Comment.startswith("/*");
-      CharSourceRange OldCommentRange = CharSourceRange::getCharRange(
-          SourceRange(Loc, Loc.getLocWithOffset(Tok.getLength())));
-      diag(Loc, "namespace closing comment refers to a wrong namespace '%0'")
-          << NamespaceNameInComment
-          << FixItHint::CreateReplacement(
-                 OldCommentRange,
-                 getNamespaceComment(ND, NeedLineBreak, SpacesBeforeComments));
-      return;
-    }
-
-    // This is not a recognized form of a namespace closing comment.
-    // Leave line comment on the same line. Move block comment to the next line,
-    // as it can be multi-line or there may be other tokens behind it.
-    if (Comment.startswith("//"))
-      NeedLineBreak = false;
-  }
-
-  diag(ND->getLocation(), "namespace not terminated with a closing comment")
-      << FixItHint::CreateInsertion(
-          AfterRBrace,
-          " " + getNamespaceComment(ND, NeedLineBreak, SpacesBeforeComments));
-}
-
-} // namespace tidy
-} // namespace clang

clang-tidy/readability/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyReadability
+  NamespaceCommentCheck.cpp
+
+  LINK_LIBS
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangTidy
+  clangTooling
+  )
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyReadability
+  NamespaceCommentCheck.cpp
+
+  LINK_LIBS
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangTidy
+  clangTooling
+  )

clang-tidy/readability/Makefile

+##===- clang-tidy/llvm/Makefile ----------------------------*- Makefile -*-===##
+#
+#                     The LLVM Compiler Infrastructure
+#
+# This file is distributed under the University of Illinois Open Source
+# License. See LICENSE.TXT for details.
+#
+##===----------------------------------------------------------------------===##
+CLANG_LEVEL := ../../../..
+LIBRARYNAME := clangTidyReadability
+
+include $(CLANG_LEVEL)/Makefile
+##===- clang-tidy/llvm/Makefile ----------------------------*- Makefile -*-===##
+#
+#                     The LLVM Compiler Infrastructure
+#
+# This file is distributed under the University of Illinois Open Source
+# License. See LICENSE.TXT for details.
+#
+##===----------------------------------------------------------------------===##
+CLANG_LEVEL := ../../../..
+LIBRARYNAME := clangTidyReadability
+
+include $(CLANG_LEVEL)/Makefile

clang-tidy/readability/NamespaceCommentCheck.h

 //===--- NamespaceCommentCheck.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_LLVM_NAMESPACE_COMMENT_CHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_NAMESPACE_COMMENT_CHECK_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NAMESPACECOMMENTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NAMESPACECOMMENTCHECK_H
 
 #include "../ClangTidy.h"
 #include "llvm/Support/Regex.h"
 
 namespace clang {
 namespace tidy {
+namespace readability {
 
 /// \brief Checks that long namespaces have a closing comment.
 ///
-/// see: http://llvm.org/docs/CodingStandards.html#namespace-indentation
+/// http://llvm.org/docs/CodingStandards.html#namespace-indentation
+/// http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Namespaces
 class NamespaceCommentCheck : public ClangTidyCheck {
 public:
   NamespaceCommentCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
   void storeOptions(ClangTidyOptions::OptionMap &Options) override;
 
   llvm::Regex NamespaceCommentPattern;
   const unsigned ShortNamespaceLines;
   const unsigned SpacesBeforeComments;
 };
 
+} // namespace readability
 } // namespace tidy
 } // namespace clang
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_NAMESPACE_COMMENT_CHECK_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NAMESPACECOMMENTCHECK_H
 //===--- NamespaceCommentCheck.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_LLVM_NAMESPACE_COMMENT_CHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_NAMESPACE_COMMENT_CHECK_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NAMESPACECOMMENTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NAMESPACECOMMENTCHECK_H
 
 #include "../ClangTidy.h"
 #include "llvm/Support/Regex.h"
 
 namespace clang {
 namespace tidy {
+namespace readability {
 
 /// \brief Checks that long namespaces have a closing comment.
 ///
-/// see: http://llvm.org/docs/CodingStandards.html#namespace-indentation
+/// http://llvm.org/docs/CodingStandards.html#namespace-indentation
+/// http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Namespaces
 class NamespaceCommentCheck : public ClangTidyCheck {
 public:
   NamespaceCommentCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
   void storeOptions(ClangTidyOptions::OptionMap &Options) override;
 
   llvm::Regex NamespaceCommentPattern;
   const unsigned ShortNamespaceLines;
   const unsigned SpacesBeforeComments;
 };
 
+} // namespace readability
 } // namespace tidy
 } // namespace clang
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_NAMESPACE_COMMENT_CHECK_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NAMESPACECOMMENTCHECK_H

clang-tidy/readability/NamespaceCommentCheck.cpp

 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringExtras.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
+namespace readability {
 
 NamespaceCommentCheck::NamespaceCommentCheck(StringRef Name,
                                              ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       NamespaceCommentPattern("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
                               "namespace( +([a-zA-Z0-9_]+))? *(\\*/)?$",
                               llvm::Regex::IgnoreCase),
       ShortNamespaceLines(Options.get("ShortNamespaceLines", 1u)),
-      SpacesBeforeComments(Options.get("SpacesBeforeComments", 1u)) {}
+      SpacesBeforeComments(Options.get("SpacesBeforeComments",
+                                       Name.startswith("google") ? 2u : 1u)) {}
 
 void NamespaceCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "ShortNamespaceLines", ShortNamespaceLines);
   Options.store(Opts, "SpacesBeforeComments", SpacesBeforeComments);
 }
 
 void NamespaceCommentCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(namespaceDecl().bind("namespace"), this);
 }
 
 bool locationsInSameFile(const SourceManager &Sources, SourceLocation Loc1,
                          SourceLocation Loc2) {
   return Loc1.isFileID() && Loc2.isFileID() &&
          Sources.getFileID(Loc1) == Sources.getFileID(Loc2);
 }
 
-std::string getNamespaceComment(const NamespaceDecl *ND, bool InsertLineBreak,
-                                unsigned SpacesBeforeComments) {
+std::string getNamespaceComment(const NamespaceDecl *ND, bool InsertLineBreak) {
   std::string Fix = "// namespace";
   if (!ND->isAnonymousNamespace())
-    Fix.append(std::string(SpacesBeforeComments, ' '))
-        .append(ND->getNameAsString());
+    Fix.append(" ").append(ND->getNameAsString());
   if (InsertLineBreak)
     Fix.append("\n");
   return Fix;
 }
 
 void NamespaceCommentCheck::check(const MatchFinder::MatchResult &Result) {
   const NamespaceDecl *ND = Result.Nodes.getNodeAs<NamespaceDecl>("namespace");
   const SourceManager &Sources = *Result.SourceManager;
if (NamespaceCommentPattern.match(Comment, &Groups)) {
 
       // Otherwise we need to fix the comment.
       NeedLineBreak = Comment.startswith("/*");
       CharSourceRange OldCommentRange = CharSourceRange::getCharRange(
           SourceRange(Loc, Loc.getLocWithOffset(Tok.getLength())));
       diag(Loc, "namespace closing comment refers to a wrong namespace '%0'")
           << NamespaceNameInComment
           << FixItHint::CreateReplacement(
-                 OldCommentRange,
-                 getNamespaceComment(ND, NeedLineBreak, SpacesBeforeComments));
+                 OldCommentRange, getNamespaceComment(ND, NeedLineBreak));
       return;
     }
 
     // This is not a recognized form of a namespace closing comment.
     // Leave line comment on the same line. Move block comment to the next line,
     // as it can be multi-line or there may be other tokens behind it.
     if (Comment.startswith("//"))
       NeedLineBreak = false;
   }
 
   diag(ND->getLocation(), "namespace not terminated with a closing comment")
-      << FixItHint::CreateInsertion(
-          AfterRBrace,
-          " " + getNamespaceComment(ND, NeedLineBreak, SpacesBeforeComments));
+      << FixItHint::CreateInsertion(AfterRBrace,
+                                    std::string(SpacesBeforeComments, ' ') +
+                                        getNamespaceComment(ND, NeedLineBreak));
 }
 
+} // namespace readability
 } // namespace tidy
 } // namespace clang
 //===--- NamespaceCommentCheck.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 "NamespaceCommentCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringExtras.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
+namespace readability {
 
 NamespaceCommentCheck::NamespaceCommentCheck(StringRef Name,
                                              ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       NamespaceCommentPattern("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
                               "namespace( +([a-zA-Z0-9_]+))? *(\\*/)?$",
                               llvm::Regex::IgnoreCase),
       ShortNamespaceLines(Options.get("ShortNamespaceLines", 1u)),
-      SpacesBeforeComments(Options.get("SpacesBeforeComments", 1u)) {}
+      SpacesBeforeComments(Options.get("SpacesBeforeComments",
+                                       Name.startswith("google") ? 2u : 1u)) {}
 
 void NamespaceCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "ShortNamespaceLines", ShortNamespaceLines);
   Options.store(Opts, "SpacesBeforeComments", SpacesBeforeComments);
 }
 
 void NamespaceCommentCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(namespaceDecl().bind("namespace"), this);
 }
 
 bool locationsInSameFile(const SourceManager &Sources, SourceLocation Loc1,
                          SourceLocation Loc2) {
   return Loc1.isFileID() && Loc2.isFileID() &&
          Sources.getFileID(Loc1) == Sources.getFileID(Loc2);
 }
 
-std::string getNamespaceComment(const NamespaceDecl *ND, bool InsertLineBreak,
-                                unsigned SpacesBeforeComments) {
+std::string getNamespaceComment(const NamespaceDecl *ND, bool InsertLineBreak) {
   std::string Fix = "// namespace";
   if (!ND->isAnonymousNamespace())
-    Fix.append(std::string(SpacesBeforeComments, ' '))
-        .append(ND->getNameAsString());
+    Fix.append(" ").append(ND->getNameAsString());
   if (InsertLineBreak)
     Fix.append("\n");
   return Fix;
 }
 
 void NamespaceCommentCheck::check(const MatchFinder::MatchResult &Result) {
   const NamespaceDecl *ND = Result.Nodes.getNodeAs<NamespaceDecl>("namespace");
   const SourceManager &Sources = *Result.SourceManager;
if (NamespaceCommentPattern.match(Comment, &Groups)) {
 
       // Otherwise we need to fix the comment.
       NeedLineBreak = Comment.startswith("/*");
       CharSourceRange OldCommentRange = CharSourceRange::getCharRange(
           SourceRange(Loc, Loc.getLocWithOffset(Tok.getLength())));
       diag(Loc, "namespace closing comment refers to a wrong namespace '%0'")
           << NamespaceNameInComment
           << FixItHint::CreateReplacement(
-                 OldCommentRange,
-                 getNamespaceComment(ND, NeedLineBreak, SpacesBeforeComments));
+                 OldCommentRange, getNamespaceComment(ND, NeedLineBreak));
       return;
     }
 
     // This is not a recognized form of a namespace closing comment.
     // Leave line comment on the same line. Move block comment to the next line,
     // as it can be multi-line or there may be other tokens behind it.
     if (Comment.startswith("//"))
       NeedLineBreak = false;
   }
 
   diag(ND->getLocation(), "namespace not terminated with a closing comment")
-      << FixItHint::CreateInsertion(
-          AfterRBrace,
-          " " + getNamespaceComment(ND, NeedLineBreak, SpacesBeforeComments));
+      << FixItHint::CreateInsertion(AfterRBrace,
+                                    std::string(SpacesBeforeComments, ' ') +
+                                        getNamespaceComment(ND, NeedLineBreak));
 }
 
+} // namespace readability
 } // namespace tidy
 } // namespace clang

test/clang-tidy/google-readability-namespace-comments.cpp

+// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s google-readability-namespace-comments %t
+// REQUIRES: shell
+
+// CHECK-MESSAGES: :[[@LINE+2]]:11: warning: namespace not terminated with a closing comment [google-readability-namespace-comments]
+// CHECK-MESSAGES: :[[@LINE+2]]:11: warning: namespace not terminated with a closing comment [google-readability-namespace-comments]
+namespace n1 {
+namespace n2 {
+
+}
+}
+// CHECK-FIXES: }  // namespace n2
+// CHECK-FIXES: }  // namespace n1
+
+
+namespace short1 { namespace short2 { } }

unittests/clang-tidy/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   support
   )
 
 get_filename_component(CLANG_LINT_SOURCE_DIR
   ${CMAKE_CURRENT_SOURCE_DIR}/../../clang-tidy REALPATH)
 include_directories(${CLANG_LINT_SOURCE_DIR})
 
 add_extra_unittest(ClangTidyTests
   ClangTidyDiagnosticConsumerTest.cpp
   ClangTidyOptionsTest.cpp
   LLVMModuleTest.cpp
+  ReadabilityChecksTest.cpp
   GoogleModuleTest.cpp
   MiscModuleTest.cpp)
 
 target_link_libraries(ClangTidyTests
   clangAST
   clangASTMatchers
   clangBasic
   clangFrontend
   clangTidy
   clangTidyGoogleModule
   clangTidyLLVMModule
   clangTidyMiscModule
+  clangTidyReadability
   clangTooling
   )

unittests/clang-tidy/LLVMModuleTest.cpp

 #include "ClangTidyTest.h"
 #include "llvm/HeaderGuardCheck.h"
 #include "llvm/IncludeOrderCheck.h"
-#include "llvm/NamespaceCommentCheck.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace tidy {
 namespace test {
 
-TEST(NamespaceCommentCheckTest, Basic) {
-  EXPECT_EQ("namespace i {\n} // namespace i",
-            runCheckOnCode<NamespaceCommentCheck>("namespace i {\n}"));
-  EXPECT_EQ("namespace {\n} // namespace",
-            runCheckOnCode<NamespaceCommentCheck>("namespace {\n}"));
-  EXPECT_EQ(
-      "namespace i { namespace j {\n} // namespace j\n } // namespace i",
-      runCheckOnCode<NamespaceCommentCheck>("namespace i { namespace j {\n} }"));
-}
-
-TEST(NamespaceCommentCheckTest, SingleLineNamespaces) {
-  EXPECT_EQ(
-      "namespace i { namespace j { } }",
-      runCheckOnCode<NamespaceCommentCheck>("namespace i { namespace j { } }"));
-}
-
-TEST(NamespaceCommentCheckTest, CheckExistingComments) {
-  EXPECT_EQ("namespace i { namespace j {\n"
-            "} /* namespace j */ } // namespace i\n"
-            " /* random comment */",
-            runCheckOnCode<NamespaceCommentCheck>(
-                "namespace i { namespace j {\n"
-                "} /* namespace j */ } /* random comment */"));
-  EXPECT_EQ("namespace {\n"
-            "} // namespace",
-            runCheckOnCode<NamespaceCommentCheck>("namespace {\n"
-                                                  "} // namespace"));
-  EXPECT_EQ("namespace {\n"
-            "} //namespace",
-            runCheckOnCode<NamespaceCommentCheck>("namespace {\n"
-                                                  "} //namespace"));
-  EXPECT_EQ("namespace {\n"
-            "} // anonymous namespace",
-            runCheckOnCode<NamespaceCommentCheck>("namespace {\n"
-                                                  "} // anonymous namespace"));
-  EXPECT_EQ(
-      "namespace My_NameSpace123 {\n"
-      "} // namespace My_NameSpace123",
-      runCheckOnCode<NamespaceCommentCheck>("namespace My_NameSpace123 {\n"
-                                            "} // namespace My_NameSpace123"));
-  EXPECT_EQ(
-      "namespace My_NameSpace123 {\n"
-      "} //namespace My_NameSpace123",
-      runCheckOnCode<NamespaceCommentCheck>("namespace My_NameSpace123 {\n"
-                                            "} //namespace My_NameSpace123"));
-  EXPECT_EQ("namespace My_NameSpace123 {\n"
-            "} //  end namespace   My_NameSpace123",
-            runCheckOnCode<NamespaceCommentCheck>(
-                "namespace My_NameSpace123 {\n"
-                "} //  end namespace   My_NameSpace123"));
-  // Understand comments only on the same line.
-  EXPECT_EQ("namespace {\n"
-            "} // namespace\n"
-            "// namespace",
-            runCheckOnCode<NamespaceCommentCheck>("namespace {\n"
-                                                  "}\n"
-                                                  "// namespace"));
-  // Leave unknown comments.
-  EXPECT_EQ("namespace {\n"
-            "} // namespace // random text",
-            runCheckOnCode<NamespaceCommentCheck>("namespace {\n"
-                                                  "} // random text"));
-}
-
-TEST(NamespaceCommentCheckTest, FixWrongComments) {
-  EXPECT_EQ("namespace i { namespace jJ0_ {\n"
-            "} // namespace jJ0_\n"
-            " } // namespace i\n"
-            " /* random comment */",
-            runCheckOnCode<NamespaceCommentCheck>(
-                "namespace i { namespace jJ0_ {\n"
-                "} /* namespace qqq */ } /* random comment */"));
-  EXPECT_EQ("namespace {\n"
-            "} // namespace",
-            runCheckOnCode<NamespaceCommentCheck>("namespace {\n"
-                                                  "} // namespace asdf"));
-}
-
 // FIXME: It seems this might be incompatible to dos path. Investigating.
 #if !defined(_WIN32)
 static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
                                        unsigned ExpectedWarnings) {
   std::vector<ClangTidyError> Errors;
   std::string Result = test::runCheckOnCode<LLVMHeaderGuardCheck>(
       Code, &Errors, Filename, std::string("-xc++-header"));
   return Errors.size() == ExpectedWarnings ? Result : "invalid error count";

unittests/clang-tidy/Makefile

 ##===- unittests/clang-tidy/Makefile -----------------------*- Makefile -*-===##
 #
 #                     The LLVM Compiler Infrastructure
 #
 # This file is distributed under the University of Illinois Open Source
 # License. See LICENSE.TXT for details.
 #
 ##===----------------------------------------------------------------------===##
 
 CLANG_LEVEL = ../../../..
 include $(CLANG_LEVEL)/../../Makefile.config
 
 TESTNAME = ClangTidy
 LINK_COMPONENTS := asmparser bitreader support MC MCParser option \
      TransformUtils
 USEDLIBS = clangTidy.a clangTidyLLVMModule.a clangTidyGoogleModule.a \
-     clangTidyMiscModule.a clangTidy.a clangTidyUtils.a \
+     clangTidyMiscModule.a clangTidyReadability.a clangTidy.a \
+     clangTidyUtils.a \
      clangStaticAnalyzerFrontend.a clangStaticAnalyzerCheckers.a \
      clangStaticAnalyzerCore.a \
      clangFormat.a clangTooling.a clangFrontend.a clangSerialization.a \
      clangDriver.a clangRewriteFrontend.a clangRewrite.a \
      clangParse.a clangSema.a clangAnalysis.a clangAST.a \
      clangASTMatchers.a clangEdit.a clangLex.a clangBasic.a
 
 include $(CLANG_LEVEL)/Makefile
 MAKEFILE_UNITTEST_NO_INCLUDE_COMMON := 1
 CPP.Flags += -I$(PROJ_SRC_DIR)/../../clang-tidy
 include $(LLVM_SRC_ROOT)/unittests/Makefile.unittest

unittests/clang-tidy/ReadabilityChecksTest.cpp

 #include "ClangTidyTest.h"
-#include "llvm/HeaderGuardCheck.h"
-#include "llvm/IncludeOrderCheck.h"
-#include "llvm/NamespaceCommentCheck.h"
+#include "readability/NamespaceCommentCheck.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace tidy {
 namespace test {
 
+using readability::NamespaceCommentCheck;
+
 TEST(NamespaceCommentCheckTest, Basic) {
   EXPECT_EQ("namespace i {\n} // namespace i",
             runCheckOnCode<NamespaceCommentCheck>("namespace i {\n}"));
   EXPECT_EQ("namespace {\n} // namespace",
             runCheckOnCode<NamespaceCommentCheck>("namespace {\n}"));
   EXPECT_EQ(
       "namespace i { namespace j {\n} // namespace j\n } // namespace i",
       runCheckOnCode<NamespaceCommentCheck>("namespace i { namespace j {\n} }"));
EXPECT_EQ("namespace i { namespace jJ0_ {\n"
                 "namespace i { namespace jJ0_ {\n"
                 "} /* namespace qqq */ } /* random comment */"));
   EXPECT_EQ("namespace {\n"
             "} // namespace",
             runCheckOnCode<NamespaceCommentCheck>("namespace {\n"
                                                   "} // namespace asdf"));
 }
 
-// FIXME: It seems this might be incompatible to dos path. Investigating.
-#if !defined(_WIN32)
-static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
-                                       unsigned ExpectedWarnings) {
-  std::vector<ClangTidyError> Errors;
-  std::string Result = test::runCheckOnCode<LLVMHeaderGuardCheck>(
-      Code, &Errors, Filename, std::string("-xc++-header"));
-  return Errors.size() == ExpectedWarnings ? Result : "invalid error count";
-}
-
-namespace {
-struct WithEndifComment : public LLVMHeaderGuardCheck {
-  WithEndifComment(StringRef Name, ClangTidyContext *Context)
-      : LLVMHeaderGuardCheck(Name, Context) {}
-  bool shouldSuggestEndifComment(StringRef Filename) override { return true; }
-};
-} // namespace
-
-static std::string runHeaderGuardCheckWithEndif(StringRef Code,
-                                                const Twine &Filename,
-                                                unsigned ExpectedWarnings) {
-  std::vector<ClangTidyError> Errors;
-  std::string Result = test::runCheckOnCode<WithEndifComment>(
-      Code, &Errors, Filename, std::string("-xc++-header"));
-  return Errors.size() == ExpectedWarnings ? Result : "invalid error count";
-}
-
-TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
-  EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif\n",
-            runHeaderGuardCheck("#ifndef FOO\n#define FOO\n#endif\n",
-                                "include/llvm/ADT/foo.h",
-                                /*ExpectedWarnings=*/1));
-
-  // Allow trailing underscores.
-  EXPECT_EQ("#ifndef LLVM_ADT_FOO_H_\n#define LLVM_ADT_FOO_H_\n#endif\n",
-            runHeaderGuardCheck(
-                "#ifndef LLVM_ADT_FOO_H_\n#define LLVM_ADT_FOO_H_\n#endif\n",
-                "include/llvm/ADT/foo.h", /*ExpectedWarnings=*/0));
-
-  EXPECT_EQ(
-      "#ifndef LLVM_CLANG_C_BAR_H\n#define LLVM_CLANG_C_BAR_H\n\n\n#endif\n",
-      runHeaderGuardCheck("", "./include/clang-c/bar.h",
-                          /*ExpectedWarnings=*/1));
-
-  EXPECT_EQ("#ifndef LLVM_CLANG_LIB_CODEGEN_C_H\n#define "
-            "LLVM_CLANG_LIB_CODEGEN_C_H\n\n\n#endif\n",
-            runHeaderGuardCheck("", "tools/clang/lib/CodeGen/c.h",
-                                /*ExpectedWarnings=*/1));
-
-  EXPECT_EQ("#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_X_H\n#define "
-            "LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_X_H\n\n\n#endif\n",
-            runHeaderGuardCheck("", "tools/clang/tools/extra/clang-tidy/x.h",
-                                /*ExpectedWarnings=*/1));
-
-  EXPECT_EQ(
-      "int foo;\n#ifndef LLVM_CLANG_BAR_H\n#define LLVM_CLANG_BAR_H\n#endif\n",
-      runHeaderGuardCheck("int foo;\n#ifndef LLVM_CLANG_BAR_H\n"
-                          "#define LLVM_CLANG_BAR_H\n#endif\n",
-                          "include/clang/bar.h", /*ExpectedWarnings=*/1));
-
-  EXPECT_EQ("#ifndef LLVM_CLANG_BAR_H\n#define LLVM_CLANG_BAR_H\n\n"
-            "int foo;\n#ifndef FOOLOLO\n#define FOOLOLO\n#endif\n\n#endif\n",
-            runHeaderGuardCheck(
-                "int foo;\n#ifndef FOOLOLO\n#define FOOLOLO\n#endif\n",
-                "include/clang/bar.h", /*ExpectedWarnings=*/1));
-
-  EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif "
-            " // LLVM_ADT_FOO_H\n",
-            runHeaderGuardCheckWithEndif("#ifndef FOO\n#define FOO\n#endif\n",
-                                         "include/llvm/ADT/foo.h",
-                                         /*ExpectedWarnings=*/1));
-
-  EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif "
-            " // LLVM_ADT_FOO_H\n",
-            runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H\n#define "
-                                         "LLVM_ADT_FOO_H\n#endif // LLVM_H\n",
-                                         "include/llvm/ADT/foo.h",
-                                         /*ExpectedWarnings=*/1));
-
-  EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif"
-            " /* LLVM_ADT_FOO_H */\n",
-            runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H\n#define "
-                                         "LLVM_ADT_FOO_H\n"
-                                         "#endif /* LLVM_ADT_FOO_H */\n",
-                                         "include/llvm/ADT/foo.h",
-                                         /*ExpectedWarnings=*/0));
-
-  EXPECT_EQ("#ifndef LLVM_ADT_FOO_H_\n#define LLVM_ADT_FOO_H_\n#endif "
-            "// LLVM_ADT_FOO_H_\n",
-            runHeaderGuardCheckWithEndif(
-                "#ifndef LLVM_ADT_FOO_H_\n#define "
-                "LLVM_ADT_FOO_H_\n#endif // LLVM_ADT_FOO_H_\n",
-                "include/llvm/ADT/foo.h", /*ExpectedWarnings=*/0));
-
-  EXPECT_EQ(
-      "#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif  // "
-      "LLVM_ADT_FOO_H\n",
-      runHeaderGuardCheckWithEndif(
-          "#ifndef LLVM_ADT_FOO_H_\n#define LLVM_ADT_FOO_H_\n#endif // LLVM\n",
-          "include/llvm/ADT/foo.h", /*ExpectedWarnings=*/1));
-
-  EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif \\ \n// "
-            "LLVM_ADT_FOO_H\n",
-            runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H\n#define "
-                                         "LLVM_ADT_FOO_H\n#endif \\ \n// "
-                                         "LLVM_ADT_FOO_H\n",
-                                         "include/llvm/ADT/foo.h",
-                                         /*ExpectedWarnings=*/0));
-
-  EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif  /* "
-            "LLVM_ADT_FOO_H\\ \n FOO */",
-            runHeaderGuardCheckWithEndif(
-                "#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif  /* "
-                "LLVM_ADT_FOO_H\\ \n FOO */",
-                "include/llvm/ADT/foo.h", /*ExpectedWarnings=*/0));
-}
-#endif
-
 } // namespace test
 } // namespace tidy
 } // namespace clang