[clang-tools-extra] [clang-tidy] Add readability-string-view-substr check (PR #120055)

[Julian Schmidt via cfe-commits]

================
@@ -0,0 +1,217 @@
+//===--- StringViewSubstrCheck.cpp - clang-tidy------------------*- C++ -*-===//
+//
+// 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 "StringViewSubstrCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+void StringViewSubstrCheck::registerMatchers(MatchFinder *Finder) {
+  // Match string_view type
+  const auto StringViewDecl = recordDecl(hasName("::std::basic_string_view"));
+  const auto IsStringView = qualType(
+      hasUnqualifiedDesugaredType(recordType(hasDeclaration(StringViewDecl))));
+
+  // Match substr() call on string_views
+  const auto SubstrCall = cxxMemberCallExpr(
+      callee(memberExpr(hasDeclaration(cxxMethodDecl(hasName("substr"))))),
+      on(expr(hasType(IsStringView)).bind("source")));
+
+  // Match assignment to string_view's substr, but exclude assignments in
+  // expressions
+  Finder->addMatcher(
+      stmt(cxxOperatorCallExpr(
+               unless(isInTemplateInstantiation()),
+               hasOverloadedOperatorName("="),
+               hasArgument(0, expr(hasType(IsStringView)).bind("target")),
+               hasArgument(1, SubstrCall.bind("substr_call")))
+               .bind("assignment"),
+           // Exclude assignments used in larger expressions
+           unless(hasAncestor(varDecl())), unless(hasAncestor(callExpr()))),
+      this);
+}
+
+void StringViewSubstrCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Assignment =
+      Result.Nodes.getNodeAs<CXXOperatorCallExpr>("assignment");
+  const auto *Target = Result.Nodes.getNodeAs<Expr>("target");
+  const auto *Source = Result.Nodes.getNodeAs<Expr>("source");
+  const auto *SubstrCall =
+      Result.Nodes.getNodeAs<CXXMemberCallExpr>("substr_call");
+
+  if (!Assignment || !Target || !Source || !SubstrCall) {
+    return;
+  }
+
+  // Get the DeclRefExpr for the target and source
+  const auto *TargetDRE = dyn_cast<DeclRefExpr>(Target->IgnoreParenImpCasts());
+  const auto *SourceDRE = dyn_cast<DeclRefExpr>(Source->IgnoreParenImpCasts());
+
+  if (!TargetDRE || !SourceDRE) {
+    return;
+  }
+
+  const Expr *StartArg = SubstrCall->getArg(0);
+  const Expr *LengthArg =
+      SubstrCall->getNumArgs() > 1 ? SubstrCall->getArg(1) : nullptr;
+
+  std::string StartText =
+      Lexer::getSourceText(
+          CharSourceRange::getTokenRange(StartArg->getSourceRange()),
+          *Result.SourceManager, Result.Context->getLangOpts())
+          .str();
+
+  const bool IsSameVar = (TargetDRE->getDecl() == SourceDRE->getDecl());
+
+  // Case 1: Check for remove_prefix pattern
+  if (!LengthArg || isa<CXXDefaultArgExpr>(LengthArg)) {
+    if (IsSameVar) {
+      std::string Replacement = TargetDRE->getNameInfo().getAsString() +
+                                ".remove_prefix(" + StartText + ")";
+      diag(Assignment->getBeginLoc(), "prefer 'remove_prefix' over 'substr' "
+                                      "for removing characters from the start")
+          << FixItHint::CreateReplacement(Assignment->getSourceRange(),
+                                          Replacement);
+    }
+    return;
+  }
+
+  // Check if StartArg resolves to 0
+  bool IsZero = false;
+
+  // Handle cases where StartArg is an integer literal
+  if (const auto *IL =
+          dyn_cast<IntegerLiteral>(StartArg->IgnoreParenImpCasts())) {
+    IsZero = IL->getValue() == 0;
+  }
+
+  // Optional: Handle cases where StartArg evaluates to zero
+  if (!IsZero) {
+    // Add logic for other constant evaluation (e.g., constexpr evaluation)
+    const auto &EvalResult = StartArg->EvaluateKnownConstInt(*Result.Context);
+    IsZero = !EvalResult.isNegative() && EvalResult == 0;
+  }
+
+  // If StartArg resolves to 0, handle the case
+  if (IsZero) {
----------------
5chmidti wrote:

Please invert the condition and return early for better readability

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