[clang-tidy] fix modernize-use-auto incorrect fix hints for pointer

clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp

@@ -8,10 +8,12 @@
 
 #include "UseAutoCheck.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/TypeLoc.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Tooling/FixIt.h"
+#include "llvm/ADT/STLExtras.h"
 
 using namespace clang;
 using namespace clang::ast_matchers;
@@ -333,6 +335,25 @@ void UseAutoCheck::replaceIterators(const DeclStmt *D, ASTContext *Context) {
       << FixItHint::CreateReplacement(Range, "auto");
 }
 
+static void ignoreTypeLocClasses(
+    TypeLoc &Loc,
+    std::initializer_list<TypeLoc::TypeLocClass> const &LocClasses) {
+  while (llvm::is_contained(LocClasses, Loc.getTypeLocClass()))
+    Loc = Loc.getNextTypeLoc();
+}
+
+static bool isMutliLevelPointerToTypeLocClasses(
+    TypeLoc Loc,
+    std::initializer_list<TypeLoc::TypeLocClass> const &LocClasses) {
+  ignoreTypeLocClasses(Loc, {TypeLoc::Paren, TypeLoc::Qualified});
+  TypeLoc::TypeLocClass TLC = Loc.getTypeLocClass();
+  if (TLC != TypeLoc::Pointer && TLC != TypeLoc::MemberPointer)
+    return false;
+  ignoreTypeLocClasses(Loc, {TypeLoc::Paren, TypeLoc::Qualified,
+                             TypeLoc::Pointer, TypeLoc::MemberPointer});
+  return llvm::is_contained(LocClasses, Loc.getTypeLocClass());
+}
+
 void UseAutoCheck::replaceExpr(
     const DeclStmt *D, ASTContext *Context,
     llvm::function_ref<QualType(const Expr *)> GetType, StringRef Message) {
@@ -342,6 +363,10 @@ void UseAutoCheck::replaceExpr(
     return;
 
   const QualType FirstDeclType = FirstDecl->getType().getCanonicalType();
+  TypeSourceInfo *TSI = FirstDecl->getTypeSourceInfo();
+
+  if (TSI == nullptr)
+    return;
 
   std::vector<FixItHint> StarRemovals;
   for (const auto *Dec : D->decls()) {
@@ -383,17 +408,11 @@ void UseAutoCheck::replaceExpr(
   // is the same as the initializer, just more CV-qualified. However, TypeLoc
   // information is not reliable where CV qualifiers are concerned so we can't
   // do anything about this case for now.
-  TypeLoc Loc = FirstDecl->getTypeSourceInfo()->getTypeLoc();
-  if (!RemoveStars) {
-    while (Loc.getTypeLocClass() == TypeLoc::Pointer ||
-           Loc.getTypeLocClass() == TypeLoc::Qualified)
-      Loc = Loc.getNextTypeLoc();
-  }
-  while (Loc.getTypeLocClass() == TypeLoc::LValueReference ||
-         Loc.getTypeLocClass() == TypeLoc::RValueReference ||
-         Loc.getTypeLocClass() == TypeLoc::Qualified) {
-    Loc = Loc.getNextTypeLoc();
-  }
+  TypeLoc Loc = TSI->getTypeLoc();
+  if (!RemoveStars)
+    ignoreTypeLocClasses(Loc, {TypeLoc::Pointer, TypeLoc::Qualified});
+  ignoreTypeLocClasses(Loc, {TypeLoc::LValueReference, TypeLoc::RValueReference,
+                             TypeLoc::Qualified});
   SourceRange Range(Loc.getSourceRange());
 
   if (MinTypeNameLength != 0 &&
@@ -405,12 +424,19 @@ void UseAutoCheck::replaceExpr(
 
   auto Diag = diag(Range.getBegin(), Message);
 
+  bool ShouldReplenishVariableName = isMutliLevelPointerToTypeLocClasses(
+      TSI->getTypeLoc(), {TypeLoc::FunctionProto, TypeLoc::ConstantArray});
+
   // Space after 'auto' to handle cases where the '*' in the pointer type is
   // next to the identifier. This avoids changing 'int *p' into 'autop'.
-  // FIXME: This doesn't work for function pointers because the variable name
-  // is inside the type.
-  Diag << FixItHint::CreateReplacement(Range, RemoveStars ? "auto " : "auto")
-       << StarRemovals;
+  llvm::StringRef Auto = ShouldReplenishVariableName
+                             ? (RemoveStars ? "auto " : "auto *")
+                             : (RemoveStars ? "auto " : "auto");
+  std::string ReplenishedVariableName =
+      ShouldReplenishVariableName ? FirstDecl->getQualifiedNameAsString() : "";
+  std::string Replacement =
+      (Auto + llvm::StringRef{ReplenishedVariableName}).str();
+  Diag << FixItHint::CreateReplacement(Range, Replacement) << StarRemovals;
 }
 
 void UseAutoCheck::check(const MatchFinder::MatchResult &Result) {

clang-tools-extra/docs/ReleaseNotes.rst

@@ -422,6 +422,10 @@ Changes in existing checks
   false-positives when constructing the container with ``count`` copies of
   elements with value ``value``.
 
+- Improved :doc:`modernize-use-auto
+  <clang-tidy/checks/modernize/use-auto>` to avoid create incorrect fix hints
+  for pointer to array type and pointer to function type.
+
 - Improved :doc:`modernize-use-emplace
   <clang-tidy/checks/modernize/use-emplace>` to not replace aggregates that
   ``emplace`` cannot construct with aggregate initialization.

clang-tools-extra/test/clang-tidy/checkers/modernize/use-auto-for-pointer.cpp

@@ -0,0 +1,29 @@
+// RUN: %check_clang_tidy -check-suffix=REMOVE %s modernize-use-auto %t -- \
+// RUN:   -config="{CheckOptions: {modernize-use-auto.RemoveStars: 'true', modernize-use-auto.MinTypeNameLength: '0'}}"
+// RUN: %check_clang_tidy %s modernize-use-auto %t -- \
+// RUN:   -config="{CheckOptions: {modernize-use-auto.RemoveStars: 'false', modernize-use-auto.MinTypeNameLength: '0'}}"
+
+void pointerToFunction() {
+  void (*(*(f1)))() = static_cast<void (**)()>(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing
+  // CHECK-FIXES-REMOVE: auto f1 =
+  // CHECK-FIXES: auto *f1 =
+}
+
+void pointerToArray() {
+  int(*a1)[2] = new int[10][2];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing
+  // CHECK-FIXES-REMOVE: auto a1 =
+  // CHECK-FIXES: auto *a1 =
+}
+
+void memberFunctionPointer() {
+  class A {
+    void f();
+  };
+  void(A::* a1)() = static_cast<void(A::*)()>(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing
+  // CHECK-FIXES-REMOVE: auto a1 =
+  // CHECK-FIXES: auto *a1 =
+}
+