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.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/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/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/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("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/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/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("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 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/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 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("namespace i {\n}")); + EXPECT_EQ("namespace {\n} // namespace", + runCheckOnCode("namespace {\n}")); + EXPECT_EQ( + "namespace i { namespace j {\n} // namespace j\n } // namespace i", + runCheckOnCode("namespace i { namespace j {\n} }")); +} + +TEST(NamespaceCommentCheckTest, SingleLineNamespaces) { + EXPECT_EQ( + "namespace i { namespace j { } }", + runCheckOnCode("namespace i { namespace j { } }")); +} + +TEST(NamespaceCommentCheckTest, CheckExistingComments) { + EXPECT_EQ("namespace i { namespace j {\n" + "} /* namespace j */ } // namespace i\n" + " /* random comment */", + runCheckOnCode( + "namespace i { namespace j {\n" + "} /* namespace j */ } /* random comment */")); + EXPECT_EQ("namespace {\n" + "} // namespace", + runCheckOnCode("namespace {\n" + "} // namespace")); + EXPECT_EQ("namespace {\n" + "} //namespace", + runCheckOnCode("namespace {\n" + "} //namespace")); + EXPECT_EQ("namespace {\n" + "} // anonymous namespace", + runCheckOnCode("namespace {\n" + "} // anonymous namespace")); + EXPECT_EQ( + "namespace My_NameSpace123 {\n" + "} // namespace My_NameSpace123", + runCheckOnCode("namespace My_NameSpace123 {\n" + "} // namespace My_NameSpace123")); + EXPECT_EQ( + "namespace My_NameSpace123 {\n" + "} //namespace My_NameSpace123", + runCheckOnCode("namespace My_NameSpace123 {\n" + "} //namespace My_NameSpace123")); + EXPECT_EQ("namespace My_NameSpace123 {\n" + "} // end namespace My_NameSpace123", + runCheckOnCode( + "namespace My_NameSpace123 {\n" + "} // end namespace My_NameSpace123")); + // Understand comments only on the same line. + EXPECT_EQ("namespace {\n" + "} // namespace\n" + "// namespace", + runCheckOnCode("namespace {\n" + "}\n" + "// namespace")); + // Leave unknown comments. + EXPECT_EQ("namespace {\n" + "} // namespace // random text", + runCheckOnCode("namespace {\n" + "} // random text")); +} + +TEST(NamespaceCommentCheckTest, FixWrongComments) { + EXPECT_EQ("namespace i { namespace jJ0_ {\n" + "} // namespace jJ0_\n" + " } // namespace i\n" + " /* random comment */", + runCheckOnCode( + "namespace i { namespace jJ0_ {\n" + "} /* namespace qqq */ } /* random comment */")); + EXPECT_EQ("namespace {\n" + "} // namespace", + runCheckOnCode("namespace {\n" + "} // namespace asdf")); } } // namespace test