[clang-tools-extra] [clang-tidy] Modernize-use-string-view: fix todo for ternary (PR #209008)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tidy

Author: Zinovy Nis (irishrover)

<details>
<summary>Changes</summary>

Add a support for any-depth ternary expressions

---
Full diff: https://github.com/llvm/llvm-project/pull/209008.diff


2 Files Affected:

- (modified) clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.cpp (+13-4) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view.cpp (+19-1) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.cpp
index 5209846aca567..3464c6ace8305 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.cpp
@@ -22,6 +22,17 @@ using namespace clang::ast_matchers;
 namespace clang::tidy::modernize {
 
 namespace {
+AST_MATCHER(Expr, isStringLiteralOrTernary) {
+  const auto Matches = [](const auto &Self, const Expr &Expression) -> bool {
+    const Expr *Unwrapped = Expression.IgnoreParenImpCasts();
+    if (const auto *Ternary = dyn_cast<ConditionalOperator>(Unwrapped))
+      return Self(Self, *Ternary->getTrueExpr()) &&
+             Self(Self, *Ternary->getFalseExpr());
+    return isa<StringLiteral>(Unwrapped);
+  };
+  return Matches(Matches, Node);
+}
+
 AST_MATCHER(FunctionDecl, isOverloaded) {
   const DeclarationName Name = Node.getDeclName();
   // Sanity check
@@ -106,9 +117,6 @@ void UseStringViewCheck::registerMatchers(MatchFinder *Finder) {
   const auto IsStdStringView = getStringTypeMatcher("::std::basic_string_view");
   const auto IgnoredFunctionsMatcher =
       matchers::matchesAnyListedRegexName(IgnoredFunctions);
-  const auto TernaryOperator = conditionalOperator(
-      hasTrueExpression(ignoringParenImpCasts(stringLiteral())),
-      hasFalseExpression(ignoringParenImpCasts(stringLiteral())));
   const auto VirtualOrOperator =
       cxxMethodDecl(anyOf(cxxConversionDecl(), isVirtual()));
   const auto CheckOverloaded =
@@ -121,7 +129,8 @@ void UseStringViewCheck::registerMatchers(MatchFinder *Finder) {
                        ast_matchers::isExplicitTemplateSpecialization())),
           returns(IsStdString), hasDescendant(returnStmt()),
           unless(hasDescendant(returnStmt(hasReturnValue(unless(
-              anyOf(stringLiteral(), hasType(IsStdStringView), TernaryOperator,
+              anyOf(stringLiteral(), hasType(IsStdStringView),
+                    isStringLiteralOrTernary(),
                     cxxConstructExpr(anyOf(
                         allOf(hasType(IsStdString), argumentCountIs(0)),
                         allOf(isListInitialization(),
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view.cpp
index 26a72c1c242e1..1189d690664d4 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view.cpp
@@ -136,10 +136,21 @@ std::string ternary(bool flag) {
 }
 
 std::string nested(int x) {
-  // TODO: support for nested ternary
+// CHECK-MESSAGES:[[@LINE-1]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view]
+// CHECK-FIXES: std::string_view nested(int x) {
   return x < 0 ? "neg" : (x == 0 ? "zero" : "pos");
 }
 
+std::string deeplyNested(int x) {
+// CHECK-MESSAGES:[[@LINE-1]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view]
+// CHECK-FIXES: std::string_view deeplyNested(int x) {
+  return x < 0 ? "negative"
+               : (x == 0 ? "zero"
+                         : (x == 1 ? "one"
+                                   : (x == 2 ? "two"
+                                             : (x == 3 ? "three" : "many"))));
+}
+
 class A {
   std::string classMethodInt() { return "internal"; }
 // CHECK-MESSAGES:[[@LINE-1]]:3: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view]
@@ -207,6 +218,13 @@ MyString<wchar_t> aliasedWChar() {
 // Negative tests
 // ==========================================================
 
+std::string nestedTernaryWithLocalString(int x) {
+  std::string local = "local";
+  // Must not be converted because one branch returns a local string.
+  return x < 0 ? "negative"
+               : (x == 0 ? "zero" : (x == 1 ? local : "positive"));
+}
+
 std::string toString() {
   return "ignored by default options";
 }

``````````

</details>


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