diff --git a/clang-tools-extra/clang-tidy/CMakeLists.txt b/clang-tools-extra/clang-tidy/CMakeLists.txt index 92ed4e3db1dd..6fcfb839e9e1 100644 --- a/clang-tools-extra/clang-tidy/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/CMakeLists.txt @@ -27,4 +27,5 @@ add_subdirectory(tool) add_subdirectory(llvm) add_subdirectory(google) add_subdirectory(misc) +add_subdirectory(readability) add_subdirectory(utils) diff --git a/clang-tools-extra/clang-tidy/Makefile b/clang-tools-extra/clang-tidy/Makefile index 0278294cfc4e..74937341d254 100644 --- a/clang-tools-extra/clang-tidy/Makefile +++ b/clang-tools-extra/clang-tidy/Makefile @@ -11,6 +11,6 @@ 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 diff --git a/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp b/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp index 11fa14faeeb5..b839fd0e55e3 100644 --- a/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp @@ -21,6 +21,7 @@ #include "TodoCommentCheck.h" #include "UnnamedNamespaceInHeaderCheck.h" #include "UsingNamespaceDirectiveCheck.h" +#include "../readability/NamespaceCommentCheck.h" using namespace clang::ast_matchers; @@ -52,6 +53,8 @@ public: "google-readability-function"); CheckFactories.registerCheck( "google-readability-todo"); + CheckFactories.registerCheck( + "google-readability-namespace-comments"); } }; diff --git a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt index 420745d4cb5c..360e9308e90b 100644 --- a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt @@ -4,7 +4,6 @@ add_clang_library(clangTidyLLVMModule HeaderGuardCheck.cpp IncludeOrderCheck.cpp LLVMTidyModule.cpp - NamespaceCommentCheck.cpp TwineLocalCheck.cpp LINK_LIBS @@ -13,6 +12,7 @@ add_clang_library(clangTidyLLVMModule clangBasic clangLex clangTidy + clangTidyReadability clangTidyUtils clangTooling ) diff --git a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp index 1eb82c58f598..73add5e8f5a1 100644 --- a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp @@ -12,7 +12,7 @@ #include "../ClangTidyModuleRegistry.h" #include "HeaderGuardCheck.h" #include "IncludeOrderCheck.h" -#include "NamespaceCommentCheck.h" +#include "../readability/NamespaceCommentCheck.h" #include "TwineLocalCheck.h" namespace clang { @@ -23,7 +23,7 @@ public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck("llvm-header-guard"); CheckFactories.registerCheck("llvm-include-order"); - CheckFactories.registerCheck( + CheckFactories.registerCheck( "llvm-namespace-comment"); CheckFactories.registerCheck("llvm-twine-local"); } diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt new file mode 100644 index 000000000000..dc805cb219cf --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -0,0 +1,13 @@ +set(LLVM_LINK_COMPONENTS support) + +add_clang_library(clangTidyReadability + NamespaceCommentCheck.cpp + + LINK_LIBS + clangAST + clangASTMatchers + clangBasic + clangLex + clangTidy + clangTooling + ) diff --git a/clang-tools-extra/clang-tidy/readability/Makefile b/clang-tools-extra/clang-tidy/readability/Makefile new file mode 100644 index 000000000000..7d9e2acec7e1 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/Makefile @@ -0,0 +1,12 @@ +##===- clang-tidy/readability/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 diff --git a/clang-tools-extra/clang-tidy/llvm/NamespaceCommentCheck.cpp b/clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp similarity index 89% rename from clang-tools-extra/clang-tidy/llvm/NamespaceCommentCheck.cpp rename to clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp index 897c60a9203d..da29c59f5b8b 100644 --- a/clang-tools-extra/clang-tidy/llvm/NamespaceCommentCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp @@ -17,6 +17,7 @@ using namespace clang::ast_matchers; namespace clang { namespace tidy { +namespace readability { NamespaceCommentCheck::NamespaceCommentCheck(StringRef Name, ClangTidyContext *Context) @@ -25,7 +26,8 @@ NamespaceCommentCheck::NamespaceCommentCheck(StringRef Name, "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); @@ -42,12 +44,10 @@ bool locationsInSameFile(const SourceManager &Sources, SourceLocation Loc1, 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; @@ -105,8 +105,7 @@ void NamespaceCommentCheck::check(const MatchFinder::MatchResult &Result) { diag(Loc, "namespace closing comment refers to a wrong namespace '%0'") << NamespaceNameInComment << FixItHint::CreateReplacement( - OldCommentRange, - getNamespaceComment(ND, NeedLineBreak, SpacesBeforeComments)); + OldCommentRange, getNamespaceComment(ND, NeedLineBreak)); return; } @@ -118,10 +117,11 @@ void NamespaceCommentCheck::check(const MatchFinder::MatchResult &Result) { } 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 diff --git a/clang-tools-extra/clang-tidy/llvm/NamespaceCommentCheck.h b/clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.h similarity index 70% rename from clang-tools-extra/clang-tidy/llvm/NamespaceCommentCheck.h rename to clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.h index f1b92e7b63bc..b72eb65150d9 100644 --- a/clang-tools-extra/clang-tidy/llvm/NamespaceCommentCheck.h +++ b/clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.h @@ -7,18 +7,20 @@ // //===----------------------------------------------------------------------===// -#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); @@ -33,7 +35,8 @@ private: 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 diff --git a/clang-tools-extra/test/clang-tidy/google-readability-namespace-comments.cpp b/clang-tools-extra/test/clang-tidy/google-readability-namespace-comments.cpp new file mode 100644 index 000000000000..5f55a360f5a9 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/google-readability-namespace-comments.cpp @@ -0,0 +1,15 @@ +// 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 { } } diff --git a/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt b/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt index 346f63a32a53..3c9706d8b1b5 100644 --- a/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt +++ b/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt @@ -10,6 +10,7 @@ add_extra_unittest(ClangTidyTests ClangTidyDiagnosticConsumerTest.cpp ClangTidyOptionsTest.cpp LLVMModuleTest.cpp + ReadabilityChecksTest.cpp GoogleModuleTest.cpp MiscModuleTest.cpp) @@ -22,5 +23,6 @@ target_link_libraries(ClangTidyTests clangTidyGoogleModule clangTidyLLVMModule clangTidyMiscModule + clangTidyReadability clangTooling ) diff --git a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp index be615080216d..a9afb820271e 100644 --- a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp @@ -1,91 +1,12 @@ #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("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")); -} - // FIXME: It seems this might be incompatible to dos path. Investigating. #if !defined(_WIN32) static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename, diff --git a/clang-tools-extra/unittests/clang-tidy/Makefile b/clang-tools-extra/unittests/clang-tidy/Makefile index 4e5395022668..627d813a5476 100644 --- a/clang-tools-extra/unittests/clang-tidy/Makefile +++ b/clang-tools-extra/unittests/clang-tidy/Makefile @@ -14,7 +14,8 @@ 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 \ diff --git a/clang-tools-extra/unittests/clang-tidy/ReadabilityChecksTest.cpp b/clang-tools-extra/unittests/clang-tidy/ReadabilityChecksTest.cpp new file mode 100644 index 000000000000..847c2baf5216 --- /dev/null +++ b/clang-tools-extra/unittests/clang-tidy/ReadabilityChecksTest.cpp @@ -0,0 +1,91 @@ +#include "ClangTidyTest.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("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 +} // namespace tidy +} // namespace clang