[clang-tidy] Move some of the misc checks to readability/

[alexfh]

Summary

Some of the misc checks belong to readability/. I'm moving them there
without changing check names for now. As the next step, I want to register some
of these checks in the google and llvm modules with suitable settings (e.g.
BracesAroundStatementsCheck). I'm not sure if we want to create a "readability"
module, probably not.

[djasper]

clang-tidy/misc/CMakeLists.txt

18

Hm. I understand that you don't want to create a separate readability at this point. But leave a comment here, otherwise the layering seems quite confusing.

clang-tidy/readability/BracesAroundStatementsCheck.h

10–11

Not sure whether this is allowed/desired for LLVM style, but we should fix the comment on the #endif accordingly.

clang-tidy/readability/FunctionSize.h

48

Fix comment.

clang-tidy/readability/RedundantSmartptrGet.h

37

Same here.

[alexfh]

Fixed #endif comments, documented the clangTidyMisc -> clangTidyReadability
dependency.

clang-tidy/misc/CMakeLists.txt

18

Done.

clang-tidy/readability/BracesAroundStatementsCheck.h

10–11

Yeah, the llvm-header-guard check should at least fix existing comments if not add new ones. Its current behavior is slightly harmful.

clang-tidy/readability/FunctionSize.h

48

Fixed #endif comments.

[djasper]

Looks good.

clang-tidy/misc/BracesAroundStatementsCheck.h

-//===--- BracesAroundStatementsCheck.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_MISC_BRACES_AROUND_STATEMENTS_CHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BRACES_AROUND_STATEMENTS_CHECK_H
-
-#include "../ClangTidy.h"
-
-namespace clang {
-namespace tidy {
-
-/// \brief Checks that bodies of 'if' statements and loops ('for', 'range-for',
-/// 'do-while', and 'while') are inside braces
-///
-/// Before:
-/// if (condition)
-///   statement;
-///
-/// After:
-/// if (condition) {
-///   statement;
-/// }
-///
-/// Additionally, one can define an option `ShortStatementLines` defining the
-/// minimal number of lines that the statement should have in order to trigger
-/// this check.
-/// The number of lines is counted from the end of condition or initial keyword
-/// (do/else) until the last line of the inner statement.
-/// Default value 0 means that braces will be added to all statements (not
-/// having them already).
-class BracesAroundStatementsCheck : public ClangTidyCheck {
-public:
-  BracesAroundStatementsCheck(StringRef Name, ClangTidyContext *Context);
-  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
-  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
-  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-
-private:
-  void checkStmt(const ast_matchers::MatchFinder::MatchResult &Result,
-                 const Stmt *S, SourceLocation StartLoc,
-                 SourceLocation EndLocHint = SourceLocation());
-  template <typename IfOrWhileStmt>
-  SourceLocation findRParenLoc(const IfOrWhileStmt *S, const SourceManager &SM,
-                               const ASTContext *Context);
-
-private:
-  const unsigned ShortStatementLines;
-};
-
-} // namespace tidy
-} // namespace clang
-
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BRACES_AROUND_STATEMENTS_CHECK_H

clang-tidy/misc/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyMiscModule
   ArgumentCommentCheck.cpp
   BoolPointerImplicitConversion.cpp
-  BracesAroundStatementsCheck.cpp
-  FunctionSize.cpp
   MiscTidyModule.cpp
-  RedundantSmartptrGet.cpp
   SwappedArgumentsCheck.cpp
   UndelegatedConstructor.cpp
   UnusedRAII.cpp
   UseOverride.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
+  # Some readability checks are currently registered in the misc module.
+  clangTidyReadability
   )

clang-tidy/misc/FunctionSize.h

-//===--- FunctionSize.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_MISC_FUNCTIONSIZE_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_FUNCTIONSIZE_H
-
-#include "../ClangTidy.h"
-
-namespace clang {
-namespace tidy {
-
-/// \brief Checks for large functions based on various metrics.
-class FunctionSizeCheck : public ClangTidyCheck {
-public:
-  FunctionSizeCheck(StringRef Name, ClangTidyContext *Context);
-
-  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
-  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
-  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-  void onEndOfTranslationUnit() override;
-
-private:
-  struct FunctionInfo {
-    FunctionInfo() : Lines(0), Statements(0), Branches(0) {}
-    unsigned Lines;
-    unsigned Statements;
-    unsigned Branches;
-  };
-
-  const unsigned LineThreshold;
-  const unsigned StatementThreshold;
-  const unsigned BranchThreshold;
-
-  llvm::DenseMap<const FunctionDecl *, FunctionInfo> FunctionInfos;
-};
-
-} // namespace tidy
-} // namespace clang
-
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_FUNCTIONSIZE_H

clang-tidy/misc/MiscTidyModule.cpp

 //===--- MiscTidyModule.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"
+// FIXME: Figure out if we want to create a separate module for readability
+// checks instead of registering them here.
+#include "../readability/BracesAroundStatementsCheck.h"
+#include "../readability/FunctionSize.h"
+#include "../readability/RedundantSmartptrGet.h"
 #include "ArgumentCommentCheck.h"
 #include "BoolPointerImplicitConversion.h"
-#include "BracesAroundStatementsCheck.h"
-#include "FunctionSize.h"
-#include "RedundantSmartptrGet.h"
 #include "SwappedArgumentsCheck.h"
 #include "UndelegatedConstructor.h"
 #include "UnusedRAII.h"
 #include "UseOverride.h"
 
 namespace clang {
 namespace tidy {
 
 class MiscModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<ArgumentCommentCheck>("misc-argument-comment");
     CheckFactories.registerCheck<BoolPointerImplicitConversion>(
         "misc-bool-pointer-implicit-conversion");
-    CheckFactories.registerCheck<BracesAroundStatementsCheck>(
+    CheckFactories.registerCheck<readability::BracesAroundStatementsCheck>(
         "misc-braces-around-statements");
-    CheckFactories.registerCheck<FunctionSizeCheck>("misc-function-size");
-    CheckFactories.registerCheck<RedundantSmartptrGet>(
+    CheckFactories.registerCheck<readability::FunctionSizeCheck>(
+        "misc-function-size");
+    CheckFactories.registerCheck<readability::RedundantSmartptrGet>(
         "misc-redundant-smartptr-get");
     CheckFactories.registerCheck<SwappedArgumentsCheck>(
         "misc-swapped-arguments");
     CheckFactories.registerCheck<UndelegatedConstructorCheck>(
         "misc-undelegated-constructor");
     CheckFactories.registerCheck<UnusedRAIICheck>("misc-unused-raii");
     CheckFactories.registerCheck<UseOverride>("misc-use-override");
   }

clang-tidy/misc/RedundantSmartptrGet.h

-//===--- RedundantSmartptrGet.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_MISC_REDUNDANT_SMARTPTR_GET_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANT_SMARTPTR_GET_H
-
-#include "../ClangTidy.h"
-
-namespace clang {
-namespace tidy {
-
-/// \brief Find and remove redundant calls to smart pointer's .get() method.
-///
-/// Examples:
-///   ptr.get()->Foo()  ==>  ptr->Foo()
-///   *ptr.get()  ==>  *ptr
-///   *ptr->get()  ==>  **ptr
-class RedundantSmartptrGet : public ClangTidyCheck {
-public:
-  RedundantSmartptrGet(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
-  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_MISC_REDUNDANT_SMARTPTR_GET_H
-

clang-tidy/readability/BracesAroundStatementsCheck.h

 //===--- BracesAroundStatementsCheck.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_MISC_BRACES_AROUND_STATEMENTS_CHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BRACES_AROUND_STATEMENTS_CHECK_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_BRACESAROUNDSTATEMENTSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_BRACESAROUNDSTATEMENTSCHECK_H
 
 #include "../ClangTidy.h"
 
 namespace clang {
 namespace tidy {
+namespace readability {
 
 /// \brief Checks that bodies of 'if' statements and loops ('for', 'range-for',
 /// 'do-while', and 'while') are inside braces
 ///
 /// Before:
 /// if (condition)
 ///   statement;
 ///
private:
   template <typename IfOrWhileStmt>
   SourceLocation findRParenLoc(const IfOrWhileStmt *S, const SourceManager &SM,
                                const ASTContext *Context);
 
 private:
   const unsigned ShortStatementLines;
 };
 
+} // namespace readability
 } // namespace tidy
 } // namespace clang
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BRACES_AROUND_STATEMENTS_CHECK_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_BRACESAROUNDSTATEMENTSCHECK_H

clang-tidy/readability/BracesAroundStatementsCheck.cpp

 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
+namespace readability {
 namespace {
 
 tok::TokenKind getTokenKind(SourceLocation Loc, const SourceManager &SM,
                             const ASTContext *Context) {
   Token Tok;
   SourceLocation Beginning =
       Lexer::GetBeginningOfToken(Loc, SM, Context->getLangOpts());
   const bool Invalid =
if (EndLine - StartLine < ShortStatementLines)
       return;
   }
 
   auto Diag = diag(StartLoc, "statement should be inside braces");
   Diag << FixItHint::CreateInsertion(StartLoc, " {")
        << FixItHint::CreateInsertion(EndLoc, ClosingInsertion);
 }
 
+} // namespace readability
 } // namespace tidy
 } // namespace clang

clang-tidy/readability/CMakeLists.txt

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

clang-tidy/readability/FunctionSize.h

 //===--- FunctionSize.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_MISC_FUNCTIONSIZE_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_FUNCTIONSIZE_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_FUNCTIONSIZE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_FUNCTIONSIZE_H
 
 #include "../ClangTidy.h"
 
 namespace clang {
 namespace tidy {
+namespace readability {
 
 /// \brief Checks for large functions based on various metrics.
 class FunctionSizeCheck : public ClangTidyCheck {
 public:
   FunctionSizeCheck(StringRef Name, ClangTidyContext *Context);
 
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
private:
 
   const unsigned LineThreshold;
   const unsigned StatementThreshold;
   const unsigned BranchThreshold;
 
   llvm::DenseMap<const FunctionDecl *, FunctionInfo> FunctionInfos;
 };
 
+} // namespace readability
 } // namespace tidy
 } // namespace clang
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_FUNCTIONSIZE_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_FUNCTIONSIZE_H

clang-tidy/readability/FunctionSize.cpp

 //===--- FunctionSize.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 "FunctionSize.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
+namespace readability {
 
 FunctionSizeCheck::FunctionSizeCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       LineThreshold(Options.get("LineThreshold", -1U)),
       StatementThreshold(Options.get("StatementThreshold", 800U)),
       BranchThreshold(Options.get("BranchThreshold", -1U)) {}
 
 void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
if (FI.Branches > BranchThreshold) {
            DiagnosticIDs::Note)
           << FI.Branches << BranchThreshold;
     }
   }
 
   FunctionInfos.clear();
 }
 
+} // namespace readability
 } // namespace tidy
 } // namespace clang

clang-tidy/readability/RedundantSmartptrGet.h

 //===--- RedundantSmartptrGet.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_MISC_REDUNDANT_SMARTPTR_GET_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANT_SMARTPTR_GET_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTSMARTPTRGET_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTSMARTPTRGET_H
 
 #include "../ClangTidy.h"
 
 namespace clang {
 namespace tidy {
+namespace readability {
 
 /// \brief Find and remove redundant calls to smart pointer's .get() method.
 ///
 /// Examples:
 ///   ptr.get()->Foo()  ==>  ptr->Foo()
 ///   *ptr.get()  ==>  *ptr
 ///   *ptr->get()  ==>  **ptr
 class RedundantSmartptrGet : public ClangTidyCheck {
 public:
   RedundantSmartptrGet(StringRef Name, ClangTidyContext *Context)
       : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 };
 
+} // namespace readability
 } // namespace tidy
 } // namespace clang
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANT_SMARTPTR_GET_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTSMARTPTRGET_H
 

clang-tidy/readability/RedundantSmartptrGet.cpp

 #include "RedundantSmartptrGet.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
+namespace readability {
 
 namespace {
 internal::Matcher<Expr> callToGet(internal::Matcher<Decl> OnClass) {
   return memberCallExpr(
              on(expr(anyOf(hasType(OnClass),
                            hasType(qualType(pointsTo(decl(OnClass).bind(
                                "ptr_to_ptr")))))).bind("smart_pointer")),
              unless(callee(memberExpr(hasObjectExpression(thisExpr())))),
StringRef SmartptrText = Lexer::getSourceText(
       CharSourceRange::getTokenRange(Smartptr->getSourceRange()),
       *Result.SourceManager, LangOptions());
   // Replace foo->get() with *foo, and foo.get() with foo.
   std::string Replacement = Twine(IsPtrToPtr ? "*" : "", SmartptrText).str();
   diag(GetCall->getLocStart(), "Redundant get() call on smart pointer.")
       << FixItHint::CreateReplacement(GetCall->getSourceRange(), Replacement);
 }
 
+} // namespace readability
 } // namespace tidy
 } // namespace clang

unittests/clang-tidy/MiscModuleTest.cpp

 #include "ClangTidyTest.h"
 #include "misc/ArgumentCommentCheck.h"
-#include "misc/BracesAroundStatementsCheck.h"
+#include "readability/BracesAroundStatementsCheck.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace tidy {
+
+using readability::BracesAroundStatementsCheck;
+
 namespace test {
 
 #define EXPECT_NO_CHANGES(Check, Code)                                         \
   EXPECT_EQ(Code, runCheckOnCode<Check>(Code))
 
 TEST(ArgumentCommentCheckTest, CorrectComments) {
   EXPECT_NO_CHANGES(ArgumentCommentCheck,
                     "void f(int x, int y); void g() { f(/*x=*/0, /*y=*/0); }");