[clang-tools-extra] [clang-tidy] Add `bugprone-missing-end-comparison` check (PR #182543)

[via cfe-commits]

================
@@ -0,0 +1,280 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "MissingEndComparisonCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+constexpr llvm::StringRef IteratorAlgorithms[] = {
+    "::std::find",          "::std::find_if",
+    "::std::find_if_not",   "::std::search",
+    "::std::search_n",      "::std::find_end",
+    "::std::find_first_of", "::std::lower_bound",
+    "::std::upper_bound",   "::std::partition_point",
+    "::std::min_element",   "::std::max_element",
+    "::std::adjacent_find", "::std::is_sorted_until"};
+
+constexpr llvm::StringRef RangeAlgorithms[] = {
+    "::std::ranges::find",          "::std::ranges::find_if",
+    "::std::ranges::find_if_not",   "::std::ranges::lower_bound",
+    "::std::ranges::upper_bound",   "::std::ranges::min_element",
+    "::std::ranges::max_element",   "::std::ranges::find_first_of",
+    "::std::ranges::adjacent_find", "::std::ranges::is_sorted_until"};
+} // namespace
+
+MissingEndComparisonCheck::MissingEndComparisonCheck(StringRef Name,
+                                                     ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      ExtraAlgorithms(
+          utils::options::parseStringList(Options.get("ExtraAlgorithms", ""))) {
+}
+
+void MissingEndComparisonCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "ExtraAlgorithms",
+                utils::options::serializeStringList(ExtraAlgorithms));
+}
+
+static bool isConditionVar(const DeclStmt *S, ASTContext &Ctx) {
+  const auto &Parents = Ctx.getParents(*S);
+  if (Parents.empty())
+    return false;
+
+  const auto *ParentStmt = Parents[0].get<Stmt>();
+  if (!ParentStmt)
+    return false;
+
+  if (const auto *If = dyn_cast<IfStmt>(ParentStmt))
+    return If->getConditionVariableDeclStmt() == S;
+  if (const auto *While = dyn_cast<WhileStmt>(ParentStmt))
+    return While->getConditionVariableDeclStmt() == S;
+  if (const auto *For = dyn_cast<ForStmt>(ParentStmt))
+    return For->getConditionVariableDeclStmt() == S;
+
+  return false;
+}
+
+static std::optional<std::string>
+getRangesEndText(const MatchFinder::MatchResult &Result, const CallExpr *Call) {
+  const FunctionDecl *Callee = Call->getDirectCallee();
+  assert(Callee && Callee->getNumParams() > 0 &&
+         "Matcher should ensure Callee has parameters");
+
+  // Range overloads take a reference (R&&), Iterator overloads pass by value.
+  const bool IsIterPair =
+      !Callee->getParamDecl(0)->getType()->isReferenceType();
+
+  if (IsIterPair) {
+    if (Call->getNumArgs() < 3)
+      return std::nullopt;
+    // find(CPO, Iter, Sent, Val...) -> Sent is Arg 2.
+    const Expr *EndArg = Call->getArg(2);
+    return tooling::fixit::getText(*EndArg, *Result.Context).str();
+  }
+
+  if (Call->getNumArgs() < 2)
+    return std::nullopt;
+  // find(CPO, Range, Val, Proj) -> Range is Arg 1.
+  const Expr *RangeArg = Call->getArg(1);
+  // Avoid potential side-effects
+  const Expr *InnerRange = RangeArg->IgnoreParenImpCasts();
+  if (isa<DeclRefExpr>(InnerRange) || isa<MemberExpr>(InnerRange)) {
+    const StringRef RangeText =
+        tooling::fixit::getText(*RangeArg, *Result.Context);
+    if (!RangeText.empty())
+      return ("std::ranges::end(" + RangeText + ")").str();
+  }
+  return "";
+}
+
+static std::optional<std::string>
+getStandardEndText(const MatchFinder::MatchResult &Result,
+                   const CallExpr *Call) {
+  if (Call->getNumArgs() < 2)
+    return std::nullopt;
+
+  // Heuristic: if the first argument is a record type and the types of the
+  // first two arguments are distinct, we assume it's a range algorithm.
+  if (Call->getNumArgs() == 2) {
+    const Expr *Arg0 = Call->getArg(0);
+    const Expr *Arg1 = Call->getArg(1);
+    const QualType T0 = Arg0->getType().getCanonicalType();
+    const QualType T1 = Arg1->getType().getCanonicalType();
+
+    if (T0.getNonReferenceType() != T1.getNonReferenceType() &&
+        T0.getNonReferenceType()->isRecordType()) {
+      const StringRef ContainerText =
+          tooling::fixit::getText(*Arg0, *Result.Context);
+      if (!ContainerText.empty())
+        return ("std::end(" + ContainerText + ")").str();
+    }
+  }
+
+  unsigned EndIdx = 1;
+  const Expr *FirstArg = Call->getArg(0);
+  if (const auto *Record =
+          FirstArg->getType().getNonReferenceType()->getAsCXXRecordDecl()) {
+    if (Record->getName().ends_with("_policy"))
+      EndIdx = 2;
+  }
+
+  if (Call->getNumArgs() <= EndIdx)
+    return std::nullopt;
+
+  const Expr *EndArg = Call->getArg(EndIdx);
+  // Filters nullptr, we assume the intent might be a valid check against null
+  if (EndArg->IgnoreParenCasts()->isNullPointerConstant(
+          *Result.Context, Expr::NPC_ValueDependentIsNull))
+    return std::nullopt;
+
+  return tooling::fixit::getText(*EndArg, *Result.Context).str();
+}
+
+static const UnaryOperator *getParentLogicalNot(const Expr *E,
----------------
zeyi2 wrote:

This helper is used to identify whether the algorithm's result is being negated, with this helper we can provide cleaner fix-its:

- `if (it)` -> `if (it != end)`
- `if (!it)` -> `if (it == end)`

https://github.com/llvm/llvm-project/pull/182543