Added a comment.
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,115 @@
+//===--- 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);
+}
+
+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) {
+ std::string Fix = "// namespace";
+ if (!ND->isAnonymousNamespace())
+ 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 (!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;
+ 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));
+ 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));
+}
+
+} // 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 unsigned 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
