[clang-tidy] Update bugprone-stringview-nullptr to consistently prefer the empty string when passing arguments to constructors/functions

[CJ-Johnson]

Summary

Previously, function(nullptr) would have been fixed with function({}). This unfortunately can change overload resolution and even become ambiguous. T(nullptr) was already being fixed with T(""), so this change just brings function calls in line with that.

clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp

 
 RewriteRule StringviewNullptrCheckImpl() {
   auto construction_warning =
       cat("constructing basic_string_view from null is undefined; replace with "
           "the default constructor");
   auto static_cast_warning =
       cat("casting to basic_string_view from null is undefined; replace with "
           "the empty string");
+  auto argument_construction_warning =
+      cat("passing null as basic_string_view is undefined; replace with the "
+          "empty string");
   auto assignment_warning =
       cat("assignment to basic_string_view from null is undefined; replace "
           "with the default constructor");
   auto relative_comparison_warning =
       cat("comparing basic_string_view to null is undefined; replace with the "
           "empty string");
   auto equality_comparison_warning =
       cat("comparing basic_string_view to null is undefined; replace with the "
           "emptiness query");
-  auto constructor_argument_warning =
-      cat("passing null as basic_string_view is undefined; replace with the "
-          "empty string");
 
   // Matches declarations and expressions of type `basic_string_view`
   auto HasBasicStringViewType = hasType(hasUnqualifiedDesugaredType(recordType(
       hasDeclaration(cxxRecordDecl(hasName("::std::basic_string_view"))))));
 
   // Matches `nullptr` and `(nullptr)` binding to a pointer
   auto NullLiteral = implicitCastExpr(
       hasCastKind(clang::CK_NullToPointer),
RewriteRule StringviewNullptrCheckImpl() {
   // `new std::string_view{null_arg_expr}`
   auto HandleHeapDirectListInitialization = makeRule(
       cxxNewExpr(has(cxxConstructExpr(BasicStringViewConstructingFromNullExpr,
                                       isListInitialization())),
                  unless(isArray()), unless(hasAnyPlacementArg(anything()))),
       remove(node("null_arg_expr")), construction_warning);
 
   // `function(null_arg_expr)`
-  auto HandleFunctionArgumentInitialization = makeRule(
-      callExpr(hasAnyArgument(
-                   ignoringImpCasts(BasicStringViewConstructingFromNullExpr)),
+  auto HandleFunctionArgumentInitialization =
+      makeRule(callExpr(hasAnyArgument(ignoringImpCasts(
+                            BasicStringViewConstructingFromNullExpr)),
                         unless(cxxOperatorCallExpr())),
-      changeTo(node("construct_expr"), cat("{}")), construction_warning);
+               changeTo(node("construct_expr"), cat("\"\"")),
+               argument_construction_warning);
 
   // `sv = null_arg_expr`
   auto HandleAssignment = makeRule(
       cxxOperatorCallExpr(hasOverloadedOperatorName("="),
                           hasRHS(materializeTemporaryExpr(
                               has(BasicStringViewConstructingFromNullExpr)))),
       changeTo(node("construct_expr"), cat("{}")), assignment_warning);
 
RewriteRule StringviewNullptrCheckImpl() {
   // `T(null_arg_expr)`
   auto HandleConstructorInvocation =
       makeRule(cxxConstructExpr(
                    hasAnyArgument(/* `hasArgument` would skip over parens */
                                   ignoringImpCasts(
                                       BasicStringViewConstructingFromNullExpr)),
                    unless(HasBasicStringViewType)),
                changeTo(node("construct_expr"), cat("\"\"")),
-               constructor_argument_warning);
+               argument_construction_warning);
 
   return applyFirst(
       {HandleTemporaryCXXFunctionalCastExpr,
        HandleTemporaryCXXTemporaryObjectExprAndCompoundLiteralExpr,
        HandleTemporaryCStyleCastExpr,
        HandleTemporaryCXXStaticCastExpr,
        HandleStackCopyInitialization,
        HandleStackCopyListInitialization,

clang-tools-extra/docs/clang-tidy/checks/bugprone-stringview-nullptr.rst

.. code-block:: c++
 
   std::string_view sv = {};
 
   sv = {};
 
   bool is_empty = sv.empty();
   bool isnt_empty = !sv.empty();
 
-  accepts_sv({});
+  accepts_sv("");
 
-  accepts_sv({});  // A
+  accepts_sv("");  // A
 
   accepts_sv({nullptr, 0});  // B
 
 .. note::
 
   The source pattern with trailing comment "A" selects the ``(const CharT*)``
   constructor overload and then value-initializes the pointer, causing a null
   dereference. It happens to not include the ``nullptr`` literal, but it is
   still within the scope of this ClangTidy check.
 
 .. note::
 
   The source pattern with trailing comment "B" selects the
   ``(const CharT*, size_type)`` constructor which is perfectly valid, since the
   length argument is ``0``. It is not changed by this ClangTidy check.

clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp

// Direct List Initialization With Temporary
     // CHECK-FIXES: {{^}}    (void)(new std::string_view{static_cast<SV>("")}) /* e30 */;
   }
 }
 
 void function_argument_initialization() /* f */ {
   // Function Argument Initialization
   {
     function(nullptr) /* f1 */;
-    // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: constructing{{.*}}default
-    // CHECK-FIXES: {{^}}    function({}) /* f1 */;
+    // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: passing null as basic_string_view is undefined; replace with the empty string
+    // CHECK-FIXES: {{^}}    function("") /* f1 */;
 
     function((nullptr)) /* f2 */;
-    // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: constructing{{.*}}default
-    // CHECK-FIXES: {{^}}    function({}) /* f2 */;
+    // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: passing{{.*}}empty string
+    // CHECK-FIXES: {{^}}    function("") /* f2 */;
 
     function({nullptr}) /* f3 */;
-    // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: constructing{{.*}}default
-    // CHECK-FIXES: {{^}}    function({}) /* f3 */;
+    // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: passing{{.*}}empty string
+    // CHECK-FIXES: {{^}}    function("") /* f3 */;
 
     function({(nullptr)}) /* f4 */;
-    // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: constructing{{.*}}default
-    // CHECK-FIXES: {{^}}    function({}) /* f4 */;
+    // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: passing{{.*}}empty string
+    // CHECK-FIXES: {{^}}    function("") /* f4 */;
 
     function({{}}) /* f5 */; // Default `const CharT*`
-    // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: constructing{{.*}}default
-    // CHECK-FIXES: {{^}}    function({}) /* f5 */;
+    // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: passing{{.*}}empty string
+    // CHECK-FIXES: {{^}}    function("") /* f5 */;
   }
 
   // Function Argument Initialization With Temporary
   {
     function(std::string_view(nullptr)) /* f6 */;
     // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: constructing{{.*}}default
     // CHECK-FIXES: {{^}}    function(std::string_view()) /* f6 */;
 
// Return Statement With Temporary
     // CHECK-FIXES: {{^}}    []() -> SV { return static_cast<SV>(""); } /* q12 */;
   }
 }
 
 void constructor_invocation() /* r */ {
   struct AcceptsSV {
     explicit AcceptsSV(std::string_view) {}
   } r1(nullptr);
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: passing null as basic_string_view is undefined; replace with the empty string
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: passing{{.*}}empty string
   // CHECK-FIXES: {{^}}  } r1("");
 
   (void)(AcceptsSV{nullptr}) /* r2 */;
   // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: passing{{.*}}empty string
   // CHECK-FIXES: {{^}}  (void)(AcceptsSV{""}) /* r2 */;
 
   AcceptsSV r3{nullptr};
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: passing{{.*}}empty string
   // CHECK-FIXES: {{^}}  AcceptsSV r3{""};
 }