diff --git a/clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp --- a/clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp @@ -18,6 +18,7 @@ #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 { @@ -25,67 +26,57 @@ namespace bugprone { void StandaloneEmptyCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { - 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 ClearMatcher = ast_matchers::cxxMethodDecl(ast_matchers::hasName("clear")).bind("clear"); + 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"); 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("clear")){ - SourceLocation ClearLoc = ClearDecl->getBeginLoc(); - diag(ClearLoc, "HERE"); - foundClear = true; - } - - if (const auto *MemberCall = Result.Nodes.getNodeAs("empty")) { + if (const auto *MemberCall = + Result.Nodes.getNodeAs("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("empty")) { + } else if (const auto *NonMemberCall = + Result.Nodes.getNodeAs("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(NonMemberCall->getCalleeDecl())->getQualifiedNameAsString() - << FixItHint::CreateReplacement(ReplacementRange, ReplacementText); + diag(NonMemberLoc, "ignoring the result of '%0'") + << llvm::dyn_cast(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 diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp @@ -6,76 +6,74 @@ struct vector { bool empty(); void clear(); - // CHECK-MESSAGES: HERE }; -// template -// bool empty(T &&); +template +bool empty(T &&); } // namespace std -// namespace absl { -// template -// struct vector { -// bool empty(); -// void clear(); -// // CHEscdcCK-MESSAGES: HERE -// }; +namespace absl { +template +struct vector { + bool empty(); + void clear(); +}; -// template -// bool empty(T &&); -// } // namespace absl +template +bool empty(T &&); +} // namespace absl int test_member_empty() { std::vector 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 w; + absl::vector 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 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 v; - -// empty(v); -// // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty] -// // CHECK-FIXES: {{^ }}v.clear();{{$}} - -// absl::vector 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();{{$}} -// } -// } +int test_qualified_empty() { + absl::vector 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 v; + + empty(v); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty] + // CHECK-FIXES: {{^ }}v.clear();{{$}} + + absl::vector 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();{{$}} + } +}