added clear check functionality

[cjdb]

A great start, thanks! I understand this CL to have been sent out a few minutes in advance, so as a note to other reviewers: maybe check back in in ~20 minutes please?

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp

52–55

Note to other reviewers: I suggested this because we couldn't work out a better way to identify if .clear() exists (Abraham can best expand on the issues regarding how a matcher wasn't working).

75–78

It looks as though this is unconditionally suggesting clear (even if it doesn't exist), and doesn't suggest it in the message.

clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp

5–9

Please remove clear from vector, and add vector_with_clear. We need to make sure that this works both when clear is absent and present.

16–21

As above. Please also rename absl::vector to something like absl::string (and drop the template altogether).

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp

 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 
 namespace clang {
 namespace tidy {
 namespace bugprone {
 
 void StandaloneEmptyCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
-  const auto CallMatcher = ast_matchers::callExpr(
-                         ast_matchers::callee(ast_matchers::functionDecl(
+  const auto CallMatcher =
+      ast_matchers::callExpr(ast_matchers::callee(ast_matchers::functionDecl(
                                  ast_matchers::hasName("empty"))))
-                         .bind("empty");
-  const auto MemberMatcher = ast_matchers::cxxMemberCallExpr(
-                         ast_matchers::callee(ast_matchers::cxxMethodDecl(
-                             ast_matchers::hasName("empty"))))
-                         .bind("empty");
+  const auto MemberMatcher =
+      ast_matchers::cxxMemberCallExpr(
+          ast_matchers::callee(
+              ast_matchers::cxxMethodDecl(ast_matchers::hasName("empty"))))
 
-  const auto ClearMatcher = ast_matchers::cxxMethodDecl(ast_matchers::hasName("clear")).bind("clear");
-
   Finder->addMatcher(MemberMatcher, this);
   Finder->addMatcher(CallMatcher, this);
-  Finder->addMatcher(ClearMatcher, this);
-
-  // Finder->addMatcher(ast_matchers::expr(
-  //                     ast_matchers::anyOf(CallMatcher, MemberMatcher)),
-  //                    this);
 }
 
 void StandaloneEmptyCheck::check(
     const ast_matchers::MatchFinder::MatchResult &Result) {
-  
-  bool foundClear = false;
-  if (const auto *ClearDecl = Result.Nodes.getNodeAs<CXXMethodDecl>("clear")){
-      SourceLocation ClearLoc = ClearDecl->getBeginLoc();
-      diag(ClearLoc, "HERE");
-      foundClear = true;
-    }
-
-  if (const auto *MemberCall = Result.Nodes.getNodeAs<CXXMemberCallExpr>("empty")) {
+  if (const auto *MemberCall =
+          Result.Nodes.getNodeAs<CXXMemberCallExpr>("empty")) {
     SourceLocation MemberLoc = MemberCall->getBeginLoc();
     SourceLocation ReplacementLoc = MemberCall->getExprLoc();
     SourceRange ReplacementRange = SourceRange(ReplacementLoc, ReplacementLoc);
-    DiagnosticBuilder builder = diag(MemberLoc, "ignoring the result of 'empty()', did you mean 'clear()'? ");
-    if (foundClear) {
-      builder << FixItHint::CreateReplacement(ReplacementRange, "clear");
-    }
-    
-    
+    auto Methods = MemberCall->getRecordDecl()->methods();
+    auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) {
+      return F->getDeclName().getAsString() == "clear" &&
+             F->getMinRequiredArguments() == 0;
+    });
+
+    if (Clear != Methods.end()) {
+      DiagnosticBuilder Builder =
+          diag(MemberLoc,
+               "ignoring the result of 'empty()', did you mean 'clear()'? ");
+      Builder << FixItHint::CreateReplacement(ReplacementRange, "clear");
+    }
 
-  } else if (const auto *NonMemberCall = Result.Nodes.getNodeAs<CallExpr>("empty")) {
+  } else if (const auto *NonMemberCall =
+                 Result.Nodes.getNodeAs<CallExpr>("empty")) {
     SourceLocation NonMemberLoc = NonMemberCall->getExprLoc();
     SourceLocation NonMemberEndLoc = NonMemberCall->getEndLoc();
-    const Expr* arg = NonMemberCall->getArg(0);
-    std::string ReplacementText = std::string(Lexer::getSourceText(
-        CharSourceRange::getTokenRange(arg->getSourceRange()),
-        *Result.SourceManager, getLangOpts())) + ".clear()";
+    const Expr *Arg = NonMemberCall->getArg(0);
+    std::string ReplacementText =
+        std::string(Lexer::getSourceText(
+            CharSourceRange::getTokenRange(Arg->getSourceRange()),
+            *Result.SourceManager, getLangOpts())) +
+        ".clear()";
     SourceRange ReplacementRange = SourceRange(NonMemberLoc, NonMemberEndLoc);
-    diag(NonMemberLoc, "ignoring the result of '%0'") << llvm::dyn_cast<NamedDecl>(NonMemberCall->getCalleeDecl())->getQualifiedNameAsString()
+    diag(NonMemberLoc, "ignoring the result of '%0'")
+        << llvm::dyn_cast<NamedDecl>(NonMemberCall->getCalleeDecl())
+               ->getQualifiedNameAsString()
         << FixItHint::CreateReplacement(ReplacementRange, ReplacementText);
   }
-
-
-  // diag(MatchedDecl->getLocation(), "insert 'awesome'", DiagnosticIDs::Note)
-  //     << FixItHint::CreateInsertion(MatchedDecl->getLocation(), "awesome_");
-
- 
-
-  // diag(NonMemberLoc, "alternate message");
-  
 }
 
 } // namespace bugprone
 } // namespace tidy
 } // namespace clang

clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp

 // RUN: %check_clang_tidy %s bugprone-standalone-empty %t
 
 namespace std {
 
 template <typename T>
 struct vector {
   bool empty();
   void clear();
-  // CHECK-MESSAGES: HERE
 };
 
-// template <typename T>
-// bool empty(T &&);
+template <typename T>
+bool empty(T &&);
 
 } // namespace std
 
-// namespace absl {
-// template <typename T>
-// struct vector {
-//   bool empty();
-//   void clear();
-//   // CHEscdcCK-MESSAGES: HERE
-// };
-
-// template <class T>
-// bool empty(T &&);
-// } // namespace absl
+namespace absl {
+template <typename T>
+struct vector {
+  bool empty();
+  void clear();
+};
+
+template <class T>
+bool empty(T &&);
+} // namespace absl
 
 int test_member_empty() {
   std::vector<int> v;
 
   v.empty();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
-  // CvfdafHECK-FIXES: {{^  }}v.clear();{{$}}
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
 
-  // absl::vector<int> w;
+  absl::vector<int> w;
 
-  // w.empty();
+  w.empty();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
-  // ChafdafHECK-FIXES: {{^  }}w.clear();{{$}}
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
 }
 
-// int test_qualified_empty() {
-//   absl::vector<int> v;
+int test_qualified_empty() {
+  absl::vector<int> v;
 
-//   std::empty(v);
-//   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
-//   // CHECK-FIXES: {{^  }}v.clear();{{$}}
-
-//   absl::empty(v);
-//   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
-//   // CHECK-FIXES: {{^  }}v.clear();{{$}}
-// }
-
-// int test_unqualified_empty() {
-//   std::vector<int> v;
-
-//   empty(v);
-//   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
-//   // CHECK-FIXES: {{^  }}v.clear();{{$}}
-
-//   absl::vector<int> w;
-
-//   empty(w);
-//   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
-//   // CHECK-FIXES: {{^  }}w.clear();{{$}}
-
-//   {
-//     using std::empty;
-//     empty(v);
-//     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
-//     // CHECK-FIXES: {{^  }}  v.clear();{{$}}
-//   }
-
-//   {
-//     using absl::empty;
-//     empty(w);
-//     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
-//     // CHECK-FIXES: {{^  }}  w.clear();{{$}}
-//   }
-// }
+  std::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+}
+
+int test_unqualified_empty() {
+  std::vector<int> v;
+
+  empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::vector<int> w;
+
+  empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  {
+    using std::empty;
+    empty(v);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+    using absl::empty;
+    empty(w);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  w.clear();{{$}}
+  }
+}