[clang-tidy] Update bugprone-stringview-nullptr to consistently prefe…

…r the empty string when passing arguments to constructors/functions 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. Differential Revision: https://reviews.llvm.org/D117840
llvm · Jan 20, 2022 · a568411 · a568411
1 parent 5e88f52
commit a568411
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 22 deletions.
diff --git a/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
@@ -44,6 +44,9 @@ RewriteRule StringviewNullptrCheckImpl() {
   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");
@@ -53,9 +56,6 @@ RewriteRule StringviewNullptrCheckImpl() {
   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(
@@ -211,11 +211,12 @@ RewriteRule StringviewNullptrCheckImpl() {
       remove(node("null_arg_expr")), construction_warning);
 
   // `function(null_arg_expr)`
-  auto HandleFunctionArgumentInitialization = makeRule(
-      callExpr(hasAnyArgument(
-                   ignoringImpCasts(BasicStringViewConstructingFromNullExpr)),
-               unless(cxxOperatorCallExpr())),
-      changeTo(node("construct_expr"), cat("{}")), construction_warning);
+  auto HandleFunctionArgumentInitialization =
+      makeRule(callExpr(hasAnyArgument(ignoringImpCasts(
+                            BasicStringViewConstructingFromNullExpr)),
+                        unless(cxxOperatorCallExpr())),
+               changeTo(node("construct_expr"), cat("\"\"")),
+               argument_construction_warning);
 
   // `sv = null_arg_expr`
   auto HandleAssignment = makeRule(
@@ -268,7 +269,7 @@ RewriteRule StringviewNullptrCheckImpl() {
                                       BasicStringViewConstructingFromNullExpr)),
                    unless(HasBasicStringViewType)),
                changeTo(node("construct_expr"), cat("\"\"")),
-               constructor_argument_warning);
+               argument_construction_warning);
 
   return applyFirst(
       {HandleTemporaryCXXFunctionalCastExpr,

diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-stringview-nullptr.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-stringview-nullptr.rst
@@ -43,9 +43,9 @@ is translated into...
   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
 

diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
@@ -1039,24 +1039,24 @@ 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
@@ -1599,7 +1599,7 @@ 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 */;