[clang-tools-extra] [clang-tidy] Improve `container-data-pointer` check to use `c_str()` (PR #71304)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tidy

Author: Eli Black (neoncube2)

<details>
<summary>Changes</summary>

Improve clang-tidy's `container-data-pointer` check to use `c_str()`, when available.

Fixes #<!-- -->55026

This is my first Clang PR! :)

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


4 Files Affected:

- (modified) clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp (+16-10) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) 
- (modified) clang-tools-extra/docs/clang-tidy/checks/readability/container-data-pointer.rst (+2-6) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp (+6-6) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
index a05e228520c9ef1..c20050063338281 100644
--- a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
@@ -40,8 +40,10 @@ void ContainerDataPointerCheck::registerMatchers(MatchFinder *Finder) {
       cxxRecordDecl(
           unless(matchers::matchesAnyListedName(IgnoredContainers)),
           isSameOrDerivedFrom(
-              namedDecl(
-                  has(cxxMethodDecl(isPublic(), hasName("data")).bind("data")))
+              namedDecl(anyOf(has(cxxMethodDecl(isPublic(), hasName("c_str"))
+                                      .bind("c_str")),
+                              has(cxxMethodDecl(isPublic(), hasName("data"))
+                                      .bind("data"))))
                   .bind("container")))
           .bind("record");
 
@@ -93,6 +95,8 @@ void ContainerDataPointerCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *DCE = Result.Nodes.getNodeAs<Expr>(DerefContainerExprName);
   const auto *ACE = Result.Nodes.getNodeAs<Expr>(AddrOfContainerExprName);
 
+  const auto *CStrMethod = Result.Nodes.getNodeAs<CXXMethodDecl>("c_str");
+
   if (!UO || !CE)
     return;
 
@@ -111,16 +115,18 @@ void ContainerDataPointerCheck::check(const MatchFinder::MatchResult &Result) {
            MemberExpr>(CE))
     ReplacementText = "(" + ReplacementText + ")";
 
-  if (CE->getType()->isPointerType())
-    ReplacementText += "->data()";
-  else
-    ReplacementText += ".data()";
+  ReplacementText += CE->getType()->isPointerType() ? "->" : ".";
+  ReplacementText += CStrMethod != NULL ? "c_str()" : "data()";
+
+  std::string Description =
+      CStrMethod != NULL
+          ? "'c_str' should be used instead of taking the address of the 0-th "
+            "element"
+          : "'data' should be used for accessing the data pointer instead of "
+            "taking the address of the 0-th element";
 
   FixItHint Hint =
       FixItHint::CreateReplacement(UO->getSourceRange(), ReplacementText);
-  diag(UO->getBeginLoc(),
-       "'data' should be used for accessing the data pointer instead of taking "
-       "the address of the 0-th element")
-      << Hint;
+  diag(UO->getBeginLoc(), Description) << Hint;
 }
 } // namespace clang::tidy::readability
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index ecfb3aa9267f140..6477f2243d31e18 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -238,6 +238,10 @@ Changes in existing checks
   Casting to ``void`` no longer suppresses issues by default, control this
   behavior with the new `AllowCastToVoid` option.
 
+- Improved :doc:`container-data-pointer
+  <clang-tidy/checks/readability/container-data-pointer>` check
+  to use ``c_str()`` when it's present on a container.
+
 - Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
   <clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables>` check
   to ignore ``static`` variables declared within the scope of
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/container-data-pointer.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/container-data-pointer.rst
index 0d10829ed3c2f9b..8a6b48c58005bf2 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/container-data-pointer.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/container-data-pointer.rst
@@ -3,13 +3,9 @@
 readability-container-data-pointer
 ==================================
 
-Finds cases where code could use ``data()`` rather than the address of the
-element at index 0 in a container. This pattern is commonly used to materialize
-a pointer to the backing data of a container. ``std::vector`` and
-``std::string`` provide a ``data()`` accessor to retrieve the data pointer which
-should be preferred.
+Finds cases where code references the address of the element at index 0 in a container and replaces them with calls to ``data()`` or ``c_str()``.
 
-This also ensures that in the case that the container is empty, the data pointer
+Using ``data()`` or ``c_str()`` is more readable and ensures that if the container is empty, the data pointer
 access does not perform an errant memory access.
 
 Options
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
index 7d8500ed4affe36..a53f303eba1f5d3 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
@@ -52,15 +52,15 @@ void g(size_t s) {
 void h() {
   std::string s;
   f(&((s).operator[]((z))));
-  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES-CLASSIC: {{^  }}f(s.data());{{$}}
-  // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'c_str' should be used instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(s.c_str());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'c_str' should be used instead of taking the address of the 0-th element [readability-container-data-pointer]
 
   std::wstring w;
   f(&((&(w))->operator[]((z))));
-  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES-CLASSIC: {{^  }}f(w.data());{{$}}
-  // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'c_str' should be used instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(w.c_str());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'c_str' should be used instead of taking the address of the 0-th element [readability-container-data-pointer]
 }
 
 template <typename T, typename U,

``````````

</details>


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