[clang-tidy] Add IgnoreAliasing option to readability-qualified-auto check (#147060)

`readability-qualified-auto` check currently looks at the unsugared
type, skipping any typedefs, to determine if the variable is a
pointer-type. This may not be the desired behaviour, in particular when
the type depends on compilation flags.
For example

```
 #if CONDITION
      using Handler = int *;
  #else
      using Handler = uint64_t;
  #endif
```

A more common example is some implementations of `std::array` use
pointers as iterators.

This introduces the IgnoreAliasing option so that
`readability-qualified-auto` does not look beyond typedefs.

---------

Co-authored-by: juanbesa <[email protected]>
Co-authored-by: Kazu Hirata <[email protected]>
Co-authored-by: EugeneZelenko <[email protected]>
Co-authored-by: Baranov Victor <[email protected]>

Author: 5 people

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

@@ -106,12 +106,14 @@ QualifiedAutoCheck::QualifiedAutoCheck(StringRef Name,
     : ClangTidyCheck(Name, Context),
       AddConstToQualified(Options.get("AddConstToQualified", true)),
       AllowedTypes(
-          utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
+          utils::options::parseStringList(Options.get("AllowedTypes", ""))),
+      IgnoreAliasing(Options.get("IgnoreAliasing", true)) {}
 
 void QualifiedAutoCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "AddConstToQualified", AddConstToQualified);
   Options.store(Opts, "AllowedTypes",
                 utils::options::serializeStringList(AllowedTypes));
+  Options.store(Opts, "IgnoreAliasing", IgnoreAliasing);
 }
 
 void QualifiedAutoCheck::registerMatchers(MatchFinder *Finder) {
@@ -134,16 +136,30 @@ void QualifiedAutoCheck::registerMatchers(MatchFinder *Finder) {
 
   auto IsBoundToType = refersToType(equalsBoundNode("type"));
   auto UnlessFunctionType = unless(hasUnqualifiedDesugaredType(functionType()));
-  auto IsAutoDeducedToPointer = [](const std::vector<StringRef> &AllowedTypes,
-                                   const auto &...InnerMatchers) {
-    return autoType(hasDeducedType(
-        hasUnqualifiedDesugaredType(pointerType(pointee(InnerMatchers...))),
-        unless(hasUnqualifiedType(
-            matchers::matchesAnyListedTypeName(AllowedTypes, false))),
-        unless(pointerType(pointee(hasUnqualifiedType(
-            matchers::matchesAnyListedTypeName(AllowedTypes, false)))))));
+
+  auto IsPointerType = [this](const auto &...InnerMatchers) {
+    if (this->IgnoreAliasing) {
+      return qualType(
+          hasUnqualifiedDesugaredType(pointerType(pointee(InnerMatchers...))));
+    } else {
+      return qualType(
+          anyOf(qualType(pointerType(pointee(InnerMatchers...))),
+                qualType(substTemplateTypeParmType(hasReplacementType(
+                    pointerType(pointee(InnerMatchers...)))))));
+    }
   };
 
+  auto IsAutoDeducedToPointer =
+      [IsPointerType](const std::vector<StringRef> &AllowedTypes,
+                      const auto &...InnerMatchers) {
+        return autoType(hasDeducedType(
+            IsPointerType(InnerMatchers...),
+            unless(hasUnqualifiedType(
+                matchers::matchesAnyListedTypeName(AllowedTypes, false))),
+            unless(pointerType(pointee(hasUnqualifiedType(
+                matchers::matchesAnyListedTypeName(AllowedTypes, false)))))));
+      };
+
   Finder->addMatcher(
       ExplicitSingleVarDecl(
           hasType(IsAutoDeducedToPointer(AllowedTypes, UnlessFunctionType)),

clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.h

@@ -32,6 +32,7 @@ class QualifiedAutoCheck : public ClangTidyCheck {
 private:
   const bool AddConstToQualified;
   const std::vector<StringRef> AllowedTypes;
+  const bool IgnoreAliasing;
 };
 
 } // namespace clang::tidy::readability

clang-tools-extra/docs/ReleaseNotes.rst

@@ -154,6 +154,10 @@ Changes in existing checks
   <clang-tidy/checks/portability/template-virtual-member-function>` check to
   avoid false positives on pure virtual member functions.
 
+- Improved :doc:`readability-qualified-auto
+  <clang-tidy/checks/readability/qualified-auto>` check by adding the option
+  `IgnoreAliasing`, that allows not looking at underlying types of type aliases.
+
 Removed checks
 ^^^^^^^^^^^^^^
 

clang-tools-extra/docs/clang-tidy/checks/readability/qualified-auto.rst

@@ -96,3 +96,45 @@ Note in the LLVM alias, the default value is `false`.
   matched against only the type name (i.e. ``Type``). E.g. to suppress reports
   for ``std::array`` iterators use `std::array<.*>::(const_)?iterator` string.
   The default is an empty string.
+
+.. option:: IgnoreAliasing
+
+  If set to `true` the check will use the underlying type to determine the type
+  that ``auto`` is deduced to. If set to `false` the check will not look beyond
+  the first type alias.
+  Default value is `true`.
+
+  .. code-block:: c++
+
+    using IntPtr = int*;
+    IntPtr foo();
+
+    auto bar = foo();
+
+  If :option:`IgnoreAliasing` is set to `true`, it will be transformed into:
+
+  .. code-block:: c++
+
+    auto *bar = foo();
+
+  Otherwise no changes will occur.
+
+Limitations
+-----------
+
+When :option:`IgnoreAliasing` is set to `false`, there are cases where
+Clang has not preserved the type alias and the underlying type will be used so
+false positives may occur.
+
+For example:
+
+.. code-block:: c++
+
+  using IntPtr = int *;
+
+  void loopPtr(const std::vector<IntPtr> &VectorIntPtr) {
+
+    // May fail for IgnoreAliasing==false as AST does not have the 'IntPtr'
+    for (auto Data : VectorIntPtr) {
+    }
+  }