Hi klimek,
Handle various forms of existing namespace closing comments, fix
existing comments with wrong namespace name, ignore short namespaces.
The state of this check now seems to be enough to enable it by default to gather
user feedback ;)
http://reviews.llvm.org/D3825
Files:
clang-tidy/llvm/CMakeLists.txt
clang-tidy/llvm/IncludeOrderCheck.cpp
clang-tidy/llvm/IncludeOrderCheck.h
clang-tidy/llvm/LLVMTidyModule.cpp
clang-tidy/llvm/LLVMTidyModule.h
clang-tidy/llvm/NamespaceCommentCheck.cpp
clang-tidy/llvm/NamespaceCommentCheck.h
clang-tidy/tool/ClangTidyMain.cpp
unittests/clang-tidy/LLVMModuleTest.cpp
Index: clang-tidy/llvm/CMakeLists.txt
===================================================================
--- clang-tidy/llvm/CMakeLists.txt
+++ clang-tidy/llvm/CMakeLists.txt
@@ -1,7 +1,9 @@
set(LLVM_LINK_COMPONENTS support)
add_clang_library(clangTidyLLVMModule
+ IncludeOrderCheck.cpp
LLVMTidyModule.cpp
+ NamespaceCommentCheck.cpp
LINK_LIBS
clangAST
Index: clang-tidy/llvm/IncludeOrderCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/llvm/IncludeOrderCheck.cpp
@@ -0,0 +1,44 @@
+//===--- IncludeOrderCheck.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 "IncludeOrderCheck.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+
+namespace clang {
+namespace tidy {
+
+namespace {
+class IncludeOrderPPCallbacks : public PPCallbacks {
+public:
+ explicit IncludeOrderPPCallbacks(IncludeOrderCheck &Check) : Check(Check) {}
+
+ void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
+ StringRef FileName, bool IsAngled,
+ CharSourceRange FilenameRange, const FileEntry *File,
+ StringRef SearchPath, StringRef RelativePath,
+ const Module *Imported) override {
+ // FIXME: This is a dummy implementation to show how to get at preprocessor
+ // information. Implement a real include order check.
+ Check.diag(HashLoc, "This is an include");
+ }
+
+private:
+ IncludeOrderCheck &Check;
+};
+} // namespace
+
+void IncludeOrderCheck::registerPPCallbacks(CompilerInstance &Compiler) {
+ Compiler.getPreprocessor()
+ .addPPCallbacks(new IncludeOrderPPCallbacks(*this));
+}
+
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/llvm/IncludeOrderCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/llvm/IncludeOrderCheck.h
@@ -0,0 +1,29 @@
+//===--- IncludeOrderCheck.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_INCLUDE_ORDER_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_INCLUDE_ORDER_CHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// \brief Checks the correct order of \c #includes.
+///
+/// see: http://llvm.org/docs/CodingStandards.html#include-style
+class IncludeOrderCheck : public ClangTidyCheck {
+public:
+ void registerPPCallbacks(CompilerInstance &Compiler) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_INCLUDE_ORDER_CHECK_H
Index: clang-tidy/llvm/LLVMTidyModule.cpp
===================================================================
--- clang-tidy/llvm/LLVMTidyModule.cpp
+++ clang-tidy/llvm/LLVMTidyModule.cpp
@@ -7,75 +7,15 @@
//
//===----------------------------------------------------------------------===//
-#include "LLVMTidyModule.h"
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
-#include "clang/AST/ASTContext.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/Frontend/CompilerInstance.h"
-#include "clang/Lex/PPCallbacks.h"
-#include "clang/Lex/Preprocessor.h"
-#include "llvm/Support/raw_ostream.h"
-
-using namespace clang::ast_matchers;
+#include "IncludeOrderCheck.h"
+#include "NamespaceCommentCheck.h"
namespace clang {
namespace tidy {
-void NamespaceCommentCheck::registerMatchers(MatchFinder *Finder) {
- Finder->addMatcher(namespaceDecl().bind("namespace"), this);
-}
-
-void NamespaceCommentCheck::check(const MatchFinder::MatchResult &Result) {
- const NamespaceDecl *ND = Result.Nodes.getNodeAs<NamespaceDecl>("namespace");
- Token Tok;
- SourceLocation Loc = ND->getRBraceLoc().getLocWithOffset(1);
- while (Lexer::getRawToken(Loc, Tok, *Result.SourceManager,
- Result.Context->getLangOpts())) {
- Loc = Loc.getLocWithOffset(1);
- }
- // FIXME: Check that this namespace is "long".
- if (Tok.is(tok::comment)) {
- // FIXME: Check comment content.
- // FIXME: Check comment placement on the same line.
- return;
- }
- std::string Fix = " // namespace";
- if (!ND->isAnonymousNamespace())
- Fix = Fix.append(" ").append(ND->getNameAsString());
-
- diag(ND->getLocation(), "namespace not terminated with a closing comment")
- << FixItHint::CreateInsertion(ND->getRBraceLoc().getLocWithOffset(1),
- Fix);
-}
-
-namespace {
-class IncludeOrderPPCallbacks : public PPCallbacks {
-public:
- explicit IncludeOrderPPCallbacks(IncludeOrderCheck &Check) : Check(Check) {}
-
- void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
- StringRef FileName, bool IsAngled,
- CharSourceRange FilenameRange, const FileEntry *File,
- StringRef SearchPath, StringRef RelativePath,
- const Module *Imported) override {
- // FIXME: This is a dummy implementation to show how to get at preprocessor
- // information. Implement a real include order check.
- Check.diag(HashLoc, "This is an include");
- }
-
-private:
- IncludeOrderCheck &Check;
-};
-} // namespace
-
-void IncludeOrderCheck::registerPPCallbacks(CompilerInstance &Compiler) {
- Compiler.getPreprocessor()
- .addPPCallbacks(new IncludeOrderPPCallbacks(*this));
-}
-
class LLVMModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
Index: clang-tidy/llvm/LLVMTidyModule.h
===================================================================
--- clang-tidy/llvm/LLVMTidyModule.h
+++ /dev/null
@@ -1,38 +0,0 @@
-//===--- LLVMTidyModule.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_LLVM_TIDY_MODULE_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_LLVM_TIDY_MODULE_H
-
-#include "../ClangTidy.h"
-
-namespace clang {
-namespace tidy {
-
-/// \brief Checks the correct order of \c #includes.
-///
-/// see: http://llvm.org/docs/CodingStandards.html#include-style
-class IncludeOrderCheck : public ClangTidyCheck {
-public:
- void registerPPCallbacks(CompilerInstance &Compiler) override;
-};
-
-/// \brief Checks that long namespaces have a closing comment.
-///
-/// see: http://llvm.org/docs/CodingStandards.html#namespace-indentation
-class NamespaceCommentCheck : public ClangTidyCheck {
-public:
- void registerMatchers(ast_matchers::MatchFinder *Finder) override;
- void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-};
-
-} // namespace tidy
-} // namespace clang
-
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_TIDY_LLVM_MODULE_H
Index: clang-tidy/llvm/NamespaceCommentCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/llvm/NamespaceCommentCheck.cpp
@@ -0,0 +1,87 @@
+//===--- 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/Support/raw_ostream.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+NamespaceCommentCheck::NamespaceCommentCheck()
+ : NamespaceCommentPattern("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
+ "namespace( +([a-zA-Z0-9_]+))? *(\\*/)?$",
+ llvm::Regex::IgnoreCase),
+ ShortNamespaceLines(1) {}
+
+void NamespaceCommentCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(namespaceDecl().bind("namespace"), this);
+}
+
+void NamespaceCommentCheck::check(const MatchFinder::MatchResult &Result) {
+ const NamespaceDecl *ND = Result.Nodes.getNodeAs<NamespaceDecl>("namespace");
+ const SourceManager &Sources = *Result.SourceManager;
+ SourceLocation AfterRBrace = ND->getRBraceLoc().getLocWithOffset(1);
+ unsigned EndLine = Sources.getSpellingLineNumber(AfterRBrace);
+ int Lines = EndLine - Sources.getSpellingLineNumber(ND->getLocStart()) + 1;
+ // FIXME: Don't just ignore short namespaces.
+ if (Lines <= ShortNamespaceLines)
+ return;
+ SourceLocation Loc = AfterRBrace;
+ Token Tok;
+ while (Lexer::getRawToken(Loc, Tok, Sources, Result.Context->getLangOpts())) {
+ Loc = Loc.getLocWithOffset(1);
+ }
+ // Macros are hard. Let's go shopping.
+ if (!AfterRBrace.isFileID() || !Loc.isFileID() ||
+ Sources.getFileID(AfterRBrace) != Sources.getFileID(Loc))
+ return;
+
+ std::string Fix = "// namespace";
+ if (!ND->isAnonymousNamespace())
+ Fix.append(" ").append(ND->getNameAsString());
+ bool NextTokenIsOnSameLine = Sources.getSpellingLineNumber(Loc) == EndLine;
+ bool NeedLineBreak = NextTokenIsOnSameLine && Tok.isNot(tok::eof);
+ if (Tok.is(tok::comment) && NextTokenIsOnSameLine) {
+ SmallVector<StringRef, 6> Groups;
+ StringRef Comment(Sources.getCharacterData(Tok.getLocation()),
+ Tok.getLength());
+ if (NamespaceCommentPattern.match(Comment, &Groups)) {
+ StringRef NamespaceNameInComment = Groups.size() >= 6 ? Groups[5] : "";
+ if (ND->isAnonymousNamespace() && NamespaceNameInComment.empty())
+ return;
+ if (ND->getNameAsString() == NamespaceNameInComment)
+ return;
+ if (Comment.startswith("/*"))
+ Fix.append("\n");
+ 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, Fix);
+ return;
+ }
+ if (Comment.startswith("//"))
+ NeedLineBreak = false;
+ }
+ if (NeedLineBreak)
+ Fix.append("\n");
+ Fix = " " + Fix;
+ diag(ND->getLocation(), "namespace not terminated with a closing comment")
+ << FixItHint::CreateInsertion(AfterRBrace, Fix);
+}
+
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/llvm/NamespaceCommentCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/llvm/NamespaceCommentCheck.h
@@ -0,0 +1,36 @@
+//===--- 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
+
+#include "../ClangTidy.h"
+#include "llvm/Support/Regex.h"
+
+namespace clang {
+namespace tidy {
+
+/// \brief Checks that long namespaces have a closing comment.
+///
+/// see: http://llvm.org/docs/CodingStandards.html#namespace-indentation
+class NamespaceCommentCheck : public ClangTidyCheck {
+public:
+ NamespaceCommentCheck();
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ llvm::Regex NamespaceCommentPattern;
+ const int ShortNamespaceLines;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_NAMESPACE_COMMENT_CHECK_H
Index: clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -31,7 +31,6 @@
"*," // Enable all checks, except these:
"-clang-analyzer-alpha*," // Too many false positives.
"-llvm-include-order," // Not implemented yet.
- "-llvm-namespace-comment," // Not complete.
"-google-*,"; // Doesn't apply to LLVM.
static cl::opt<std::string>
Checks("checks", cl::desc("Comma-separated list of globs with optional '-'\n"
Index: unittests/clang-tidy/LLVMModuleTest.cpp
===================================================================
--- unittests/clang-tidy/LLVMModuleTest.cpp
+++ unittests/clang-tidy/LLVMModuleTest.cpp
@@ -1,5 +1,6 @@
#include "ClangTidyTest.h"
-#include "llvm/LLVMTidyModule.h"
+#include "llvm/IncludeOrderCheck.h"
+#include "llvm/NamespaceCommentCheck.h"
#include "gtest/gtest.h"
namespace clang {
@@ -9,6 +10,79 @@
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"));
}
} // namespace test
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
