[clang-tidy] Include std::basic_string_view in readability-redundant-string-init.

[ckennelly]

Summary

std::string_view("") produces a string_view instance that compares
equal to std::string_view(), but requires more complex initialization
(storing the address of the string literal, rather than zeroing).

[njames93]

You don't really need to duplicate every single std::string test. Just enough so that it detects it, kind of like ::out::TestString. Also can you update the docs and release notes to explain this behaviour.

[ckennelly]

In D91009#2381590, @njames93 wrote:

You don't really need to duplicate every single std::string test. Just enough so that it detects it, kind of like ::out::TestString. Also can you update the docs and release notes to explain this behaviour.

Done

[ckennelly]

Applied feedback from review

• reduced test cases to verify functionality
• updated release notes
• updated docs

[ckennelly]

Ping

[aaron.ballman]

clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp

25

nit: using size_type = decltype(sizeof(0));

30

This constructor doesn't exist according to the standard: http://eel.is/c++draft/string.view.template.general

65

Can you also add tests for wstring_view?

Also, perhaps one for std::string_view foo{}; (to remove the spurious {})

[ckennelly]

Applied review feedback

clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp

65

Done for wstring_view

re std::string_view foo{}, it looks like we don't even match std::string{}, https://godbolt.org/z/zaafrb

[aaron.ballman]

LGTM!

clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp

 
 using namespace clang::ast_matchers;
 using namespace clang::tidy::matchers;
 
 namespace clang {
 namespace tidy {
 namespace readability {
 
-const char DefaultStringNames[] = "::std::basic_string";
+const char DefaultStringNames[] =
+    "::std::basic_string_view;::std::basic_string";
 
 static ast_matchers::internal::Matcher<NamedDecl>
 hasAnyNameStdString(std::vector<std::string> Names) {
   return ast_matchers::internal::Matcher<NamedDecl>(
       new ast_matchers::internal::HasNameMatcher(std::move(Names)));
 }
 
 static std::vector<std::string>

clang-tools-extra/docs/ReleaseNotes.rst

- Improved :doc:`readability-identifier-naming
   style violation.
   
   Added support for specifying the style of scoped ``enum`` constants. If 
   unspecified, will fall back to the style for regular ``enum`` constants.
 
 - Removed `google-runtime-references` check because the rule it checks does
   not exist in the Google Style Guide anymore.
 
+- Improved :doc:`readability-redundant-string-init
+  <clang-tidy/checks/readability-redundant-string-init>` check.
+
+  Added `std::basic_string_view` to default list of ``string``-like types.
+
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 
 Improvements to clang-include-fixer
 -----------------------------------
 

clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-init.rst

.. code-block:: c++
   std::string a = "";
   std::string b("");
 
   // becomes
 
   std::string a;
   std::string b;
 
+  // Initializing a string_view with an empty string literal produces an
+  // instance that compares equal to string_view().
+  std::string_view a = "";
+  std::string_view b("");
+
+  // becomes
+  std::string_view a;
+  std::string_view b;
+
 Options
 -------
 
 .. option:: StringNames
 
-    Default is `::std::basic_string`.
+    Default is `::std::basic_string;::std::basic_string_view`.
 
     Semicolon-delimited list of class names to apply this check to.
     By default `::std::basic_string` applies to ``std::string`` and
     ``std::wstring``. Set to e.g. `::std::basic_string;llvm::StringRef;QString`
     to perform this check on custom classes.

clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp

 // RUN: %check_clang_tidy -std=c++11,c++14 %s readability-redundant-string-init %t \
 // RUN:   -config="{CheckOptions: \
 // RUN:             [{key: readability-redundant-string-init.StringNames, \
-// RUN:               value: '::std::basic_string;our::TestString'}] \
+// RUN:               value: '::std::basic_string;::std::basic_string_view;our::TestString'}] \
 // RUN:             }"
 // FIXME: Fix the checker to work in C++17 mode.
 
 namespace std {
 template <typename T>
 class allocator {};
 template <typename T>
 class char_traits {};
 template <typename C, typename T = std::char_traits<C>, typename A = std::allocator<C>>
 struct basic_string {
   basic_string();
   basic_string(const basic_string&);
   basic_string(const C *, const A &a = A());
   ~basic_string();
 };
 typedef basic_string<char> string;
 typedef basic_string<wchar_t> wstring;
+
+template <typename C, typename T = std::char_traits<C>, typename A = std::allocator<C>>
+struct basic_string_view {
+  using size_type = unsigned;
+
+  basic_string_view();
+  basic_string_view(const basic_string_view &);
+  basic_string_view(const C *, size_type);
+  basic_string_view(const C *, const A &a = A());
+  template <class It, class End>
+  basic_string_view(It, End);
+};
+typedef basic_string_view<char> string_view;
+typedef basic_string_view<wchar_t> wstring_view;
 }
 
 void f() {
   std::string a = "";
   // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization [readability-redundant-string-init]
   // CHECK-FIXES: std::string a;
   std::string b("");
   // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
void f() {
 
   std::string u = "u";
   std::string w("w");
   std::string x = R"(x)";
   std::string y(R"(y)");
   std::string z;
 }
 
+void fview() {
+  std::string_view a = "";
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: redundant string initialization [readability-redundant-string-init]
+  // CHECK-FIXES: std::string_view a;
+  std::string_view b("");
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: redundant string initialization
+  // CHECK-FIXES: std::string_view b;
+  std::string_view c = R"()";
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: redundant string initialization
+  // CHECK-FIXES: std::string_view c;
+  std::string_view d(R"()");
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: redundant string initialization
+  // CHECK-FIXES: std::string_view d;
+  std::string_view e{""};
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: redundant string initialization
+  // CHECK-FIXES: std::string_view e;
+  std::string_view f = {""};
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: redundant string initialization
+  // CHECK-FIXES: std::string_view f;
+
+  std::string_view u = "u";
+  std::string_view w("w");
+  std::string_view x = R"(x)";
+  std::string_view y(R"(y)");
+  std::string_view z;
+}
+
 void g() {
   std::wstring a = L"";
   // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
   // CHECK-FIXES: std::wstring a;
   std::wstring b(L"");
   // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
   // CHECK-FIXES: std::wstring b;
   std::wstring c = LR"()";
public:
   // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
   // CHECK-FIXES:  Foo(short)  {}
 
   Foo(float) : A{""}, B{""} {}
   // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
   // CHECK-MESSAGES: [[@LINE-2]]:23: warning: redundant string initialization
   // CHECK-FIXES:  Foo(float)  {}
 
-
   // Check how it handles removing some redundant initializers while leaving
   // valid initializers intact.
   Foo(std::string Arg) : A(Arg), B(""), C("NonEmpty"), D(R"()"), E("") {}
   // CHECK-MESSAGES: [[@LINE-1]]:34: warning: redundant string initialization
   // CHECK-MESSAGES: [[@LINE-2]]:56: warning: redundant string initialization
   // CHECK-MESSAGES: [[@LINE-3]]:66: warning: redundant string initialization
   // CHECK-FIXES:  Foo(std::string Arg) : A(Arg),  C("NonEmpty"),  E() {}
 };