[clang-tidy] Add misc-sizeof-container check to find sizeof() uses on stl

containers.

Summary:
sizeof(some_std_string) is likely to be an error. This check finds this
pattern and suggests using .size() instead.

Reviewers: djasper, klimek, aaron.ballman

Subscribers: aaron.ballman, cfe-commits

Differential Revision: http://reviews.llvm.org/D12759

llvm-svn: 247297

4 files changed, 122 insertions, 0 deletions

diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
index 0e69a804e7a..6cdd621f81e 100644
--- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -12,6 +12,7 @@ add_clang_library(clangTidyMiscModule
   MiscTidyModule.cpp
   MoveConstructorInitCheck.cpp
   NoexceptMoveConstructorCheck.cpp
+  SizeofContainerCheck.cpp
   StaticAssertCheck.cpp
   SwappedArgumentsCheck.cpp
   UndelegatedConstructor.cpp
diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
index 997613170be..dc9a4eb21a6 100644
--- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -20,6 +20,7 @@
 #include "MacroRepeatedSideEffectsCheck.h"
 #include "MoveConstructorInitCheck.h"
 #include "NoexceptMoveConstructorCheck.h"
+#include "SizeofContainerCheck.h"
 #include "StaticAssertCheck.h"
 #include "SwappedArgumentsCheck.h"
 #include "UndelegatedConstructor.h"
@@ -54,6 +55,8 @@ public:
         "misc-move-constructor-init");
     CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
         "misc-noexcept-move-constructor");
+    CheckFactories.registerCheck<SizeofContainerCheck>(
+        "misc-sizeof-container");
     CheckFactories.registerCheck<StaticAssertCheck>(
         "misc-static-assert");
     CheckFactories.registerCheck<SwappedArgumentsCheck>(
diff --git a/clang-tools-extra/clang-tidy/misc/SizeofContainerCheck.cpp b/clang-tools-extra/clang-tidy/misc/SizeofContainerCheck.cpp
new file mode 100644
index 00000000000..44753b93382
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/misc/SizeofContainerCheck.cpp
@@ -0,0 +1,83 @@
+//===--- SizeofContainerCheck.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 "SizeofContainerCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+namespace {
+
+bool needsParens(const Expr *E) {
+  E = E->IgnoreImpCasts();
+  if (isa<BinaryOperator>(E) || isa<ConditionalOperator>(E))
+    return true;
+  if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(E)) {
+    return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call &&
+           Op->getOperator() != OO_Subscript;
+  }
+  return false;
+}
+
+} // anonymous namespace
+
+void SizeofContainerCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      expr(unless(isInTemplateInstantiation()),
+           expr(sizeOfExpr(has(expr(hasType(hasCanonicalType(hasDeclaration(
+                    recordDecl(matchesName("^(::std::|::string)"),
+                               hasMethod(methodDecl(hasName("size"), isPublic(),
+                                                    isConst()))))))))))
+               .bind("sizeof"),
+           // Ignore ARRAYSIZE(<array of containers>) pattern.
+           unless(hasAncestor(binaryOperator(
+               anyOf(hasOperatorName("/"), hasOperatorName("%")),
+               hasLHS(ignoringParenCasts(sizeOfExpr(expr()))),
+               hasRHS(ignoringParenCasts(equalsBoundNode("sizeof"))))))),
+      this);
+}
+
+void SizeofContainerCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *SizeOf =
+      Result.Nodes.getNodeAs<UnaryExprOrTypeTraitExpr>("sizeof");
+
+  SourceLocation SizeOfLoc = SizeOf->getLocStart();
+  auto Diag = diag(SizeOfLoc, "sizeof() doesn't return the size of the "
+                              "container; did you mean .size()?");
+
+  // Don't generate fixes for macros.
+  if (SizeOfLoc.isMacroID())
+    return;
+
+  SourceLocation RParenLoc = SizeOf->getRParenLoc();
+
+  // sizeof argument is wrapped in a single ParenExpr.
+  const auto *Arg = cast<ParenExpr>(SizeOf->getArgumentExpr());
+
+  if (needsParens(Arg->getSubExpr())) {
+    Diag << FixItHint::CreateRemoval(
+                CharSourceRange::getTokenRange(SizeOfLoc, SizeOfLoc))
+         << FixItHint::CreateInsertion(RParenLoc.getLocWithOffset(1),
+                                       ".size()");
+  } else {
+    Diag << FixItHint::CreateRemoval(
+                CharSourceRange::getTokenRange(SizeOfLoc, Arg->getLParen()))
+         << FixItHint::CreateReplacement(
+                CharSourceRange::getTokenRange(RParenLoc, RParenLoc),
+                ".size()");
+  }
+}
+
+} // namespace tidy
+} // namespace clang
+
diff --git a/clang-tools-extra/clang-tidy/misc/SizeofContainerCheck.h b/clang-tools-extra/clang-tidy/misc/SizeofContainerCheck.h
new file mode 100644
index 00000000000..eae5a06320d
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/misc/SizeofContainerCheck.h
@@ -0,0 +1,35 @@
+//===--- SizeofContainerCheck.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_SIZEOF_CONTAINER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SIZEOF_CONTAINER_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// Find usages of sizeof on expressions of STL container types. Most likely the
+/// user wanted to use `.size()` instead.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-sizeof-container.html
+class SizeofContainerCheck : public ClangTidyCheck {
+public:
+  SizeofContainerCheck(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_SIZEOF_CONTAINER_H
+