[clang-tidy] Fix false negative in readability-simplify-subscript-expr when subscripting substituted types (#185570)

This check's bespoke method of avoiding matching
in template instantations is overeager. This commit
changes it to just rely on IgnoreUnlessSpelledInSource
traversal instead.  This is the same problem 
as in #185559.

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

@@ -15,30 +15,27 @@ using namespace clang::ast_matchers;
 
 namespace clang::tidy::readability {
 
-static constexpr char KDefaultTypes[] =
+static constexpr char DefaultTypes[] =
     "::std::basic_string;::std::basic_string_view;::std::vector;::std::array;::"
     "std::span";
 
 SimplifySubscriptExprCheck::SimplifySubscriptExprCheck(
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context), Types(utils::options::parseStringList(
-                                         Options.get("Types", KDefaultTypes))) {
-}
+                                         Options.get("Types", DefaultTypes))) {}
 
 void SimplifySubscriptExprCheck::registerMatchers(MatchFinder *Finder) {
   const auto TypesMatcher = hasUnqualifiedDesugaredType(
       recordType(hasDeclaration(cxxRecordDecl(hasAnyName(Types)))));
 
   Finder->addMatcher(
-      arraySubscriptExpr(hasBase(
-          cxxMemberCallExpr(
-              has(memberExpr().bind("member")),
-              on(hasType(qualType(
-                  unless(anyOf(substTemplateTypeParmType(),
-                               hasDescendant(substTemplateTypeParmType()))),
-                  anyOf(TypesMatcher, pointerType(pointee(TypesMatcher)))))),
-              callee(namedDecl(hasName("data"))))
-              .bind("call"))),
+      arraySubscriptExpr(
+          hasBase(cxxMemberCallExpr(
+                      has(memberExpr().bind("member")),
+                      on(hasType(qualType(anyOf(
+                          TypesMatcher, pointerType(pointee(TypesMatcher)))))),
+                      callee(namedDecl(hasName("data"))))
+                      .bind("call"))),
       this);
 }
 

clang-tools-extra/docs/ReleaseNotes.rst

@@ -335,6 +335,12 @@ Changes in existing checks
   <clang-tidy/checks/readability/simplify-boolean-expr>` check to provide valid
   fix suggestions for C23 and later by not using ``static_cast``.
 
+- Improved :doc:`readability-simplify-subscript-expr
+  <clang-tidy/checks/readability/simplify-subscript-expr>` check by fixing
+  missing warnings when subscripting an object held inside a generic
+  container (e.g. subscripting a ``std::string`` held inside a
+  ``std::vector<std::string>``).
+
 - Improved :doc:`readability-suspicious-call-argument
   <clang-tidy/checks/readability/suspicious-call-argument>` check by avoiding a
   crash from invalid ``Abbreviations`` option.

clang-tools-extra/test/clang-tidy/checkers/readability/simplify-subscript-expr.cpp

@@ -74,3 +74,22 @@ void f(int i) {
 
   char c10 = Foo<std::string>{}.bar(i);
 }
+
+template <typename T>
+struct Wrapper {
+  T value;
+};
+
+void g() {
+  // This used to be a false negative.
+  Wrapper<std::string>().value.data()[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: accessing an element
+  // CHECK-FIXES: Wrapper<std::string>().value[10];
+}
+
+template <typename T>
+void dependent() {
+  T().data()[10];
+}
+
+template void dependent<std::string>();