[clang-tidy] Fix typedefs handling in bugprone-dangling-handle

[PiotrZSL]

Summary

Using 'hasUnqualifiedDesugaredType' to skip all type aliases when
checking for handle.

Fixes #38779

[carlosgalvezp]

clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp

55–57

Right now the test will no longer test classes that have the conversion operator written explicitly instead of via typedef. Do you think it's worth keeping the other implementation as well, or can we safely assume that basic_string is always implemented like this?

[PiotrZSL]

clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp

55–57

Both GCC (sv_type) & Clang (self_view) implement this using typedef.
But code will work fine also without typedef.

[carlosgalvezp]

clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp

55–57

But code will work fine also without typedef.

I guess we can't know if this will hold in the future unless we have an explicit test for it :) But I agree it's probably unlikely and if we had implemented this from scratch copying the pattern from GCC or Clang directly we would have ended up with this result.

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

 namespace {
 
 ast_matchers::internal::BindableMatcher<Stmt>
 handleFrom(const ast_matchers::internal::Matcher<RecordDecl> &IsAHandle,
            const ast_matchers::internal::Matcher<Expr> &Arg) {
   return expr(
       anyOf(cxxConstructExpr(hasDeclaration(cxxMethodDecl(ofClass(IsAHandle))),
                              hasArgument(0, Arg)),
-            cxxMemberCallExpr(hasType(cxxRecordDecl(IsAHandle)),
+            cxxMemberCallExpr(hasType(hasUnqualifiedDesugaredType(recordType(
+                                  hasDeclaration(cxxRecordDecl(IsAHandle))))),
                               callee(memberExpr(member(cxxConversionDecl()))),
                               on(Arg))));
 }
 
 ast_matchers::internal::Matcher<Stmt> handleFromTemporaryValue(
     const ast_matchers::internal::Matcher<RecordDecl> &IsAHandle) {
   // If a ternary operator returns a temporary value, then both branches hold a
   // temporary value. If one of them is not a temporary then it must be copied

clang-tools-extra/docs/ReleaseNotes.rst

 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 - Improved :doc:`readability-redundant-string-cstr
   <clang-tidy/checks/readability/redundant-string-cstr>` check to recognise
   unnecessary ``std::string::c_str()`` and ``std::string::data()`` calls in
   arguments to ``std::print``, ``std::format`` or other functions listed in
   the ``StringParameterFunction`` check option.
 
+- Improved :doc:`bugprone-dangling-handle
+  <clang-tidy/checks/bugprone/dangling-handle>` check enhancing detection of
+  handles behind type aliases.
+
 - Deprecated check-local options `HeaderFileExtensions`
   in :doc:`bugprone-dynamic-static-initializers
   <clang-tidy/checks/bugprone/dynamic-static-initializers>` check.
   Global options of the same name should be used instead.
 
 - Improved :doc:`bugprone-fold-init-type
   <clang-tidy/checks/bugprone/fold-init-type>` to handle iterators that do not
   define `value_type` type aliases.

clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp

 
 class basic_string_view;
 
 class basic_string {
  public:
   basic_string();
   basic_string(const char*);
 
-  operator basic_string_view() const noexcept;
+  typedef basic_string_view str_view;
+  operator str_view() const noexcept;
 
   ~basic_string();
 };
 
 typedef basic_string string;
 
 class basic_string_view {
  public:
   basic_string_view(const char*);

 void TakesAStringView(std::string_view);
 
 void Negatives(std::string_view default_arg = ReturnsAString()) {
   std::string str;
   std::string_view view = str;
 
   TakesAStringView(std::string());
 }
+