[clang-tidy] do not expand macros in modernize-use-using fix

Previously, the implementation used the printed type, which contains the
macro arguments expanded (also deleting comments). Instead, this
check can be more surgical and keep the actual written type as is,
keeping macros unexpanded.

Fixes #33760, #37846

Author: 5chmidti

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

@@ -7,9 +7,12 @@
 //===----------------------------------------------------------------------===//
 
 #include "UseUsingCheck.h"
-#include "clang/AST/ASTContext.h"
+#include "../utils/LexerUtils.h"
 #include "clang/AST/DeclGroup.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Lexer.h"
+#include <string>
 
 using namespace clang::ast_matchers;
 namespace {
@@ -119,14 +122,55 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
     return;
   }
 
-  PrintingPolicy PrintPolicy(getLangOpts());
-  PrintPolicy.SuppressScope = true;
-  PrintPolicy.ConstantArraySizeAsWritten = true;
-  PrintPolicy.UseVoidForZeroParams = false;
-  PrintPolicy.PrintInjectedClassNameWithArguments = false;
+  const TypeLoc TL = MatchedDecl->getTypeSourceInfo()->getTypeLoc();
+
+  auto [Type, QualifierStr] = [MatchedDecl, &Result, this,
+                               &TL]() -> std::pair<std::string, std::string> {
+    SourceRange TypeRange = TL.getSourceRange();
+
+    // Function pointer case, get the left and right side of the identifier
+    // without the identifier.
+    if (TypeRange.fullyContains(MatchedDecl->getLocation())) {
+      return {(Lexer::getSourceText(
+                   CharSourceRange::getCharRange(TL.getBeginLoc(),
+                                                 MatchedDecl->getLocation()),
+                   *Result.SourceManager, getLangOpts()) +
+               Lexer::getSourceText(
+                   CharSourceRange::getCharRange(
+                       Lexer::getLocForEndOfToken(MatchedDecl->getLocation(), 0,
+                                                  *Result.SourceManager,
+                                                  getLangOpts()),
+                       Lexer::getLocForEndOfToken(TL.getEndLoc(), 0,
+                                                  *Result.SourceManager,
+                                                  getLangOpts())),
+                   *Result.SourceManager, getLangOpts()))
+                  .str(),
+              ""};
+    }
+
+    StringRef ExtraReference = "";
+    if (MainTypeEndLoc.isValid() && TypeRange.fullyContains(MainTypeEndLoc)) {
+      const SourceLocation Tok = utils::lexer::findPreviousAnyTokenKind(
+          MatchedDecl->getLocation(), *Result.SourceManager, getLangOpts(),
+          tok::TokenKind::star, tok::TokenKind::amp, tok::TokenKind::comma,
+          tok::TokenKind::kw_typedef);
+
+      ExtraReference = Lexer::getSourceText(
+          CharSourceRange::getCharRange(Tok, Tok.getLocWithOffset(1)),
+          *Result.SourceManager, getLangOpts());
 
-  std::string Type = MatchedDecl->getUnderlyingType().getAsString(PrintPolicy);
-  std::string Name = MatchedDecl->getNameAsString();
+      if (ExtraReference != "*" && ExtraReference != "&")
+        ExtraReference = "";
+
+      if (MainTypeEndLoc.isValid())
+        TypeRange.setEnd(MainTypeEndLoc);
+    }
+    return {Lexer::getSourceText(CharSourceRange::getTokenRange(TypeRange),
+                                 *Result.SourceManager, getLangOpts())
+                .str(),
+            ExtraReference.str()};
+  }();
+  StringRef Name = MatchedDecl->getName();
   SourceRange ReplaceRange = MatchedDecl->getSourceRange();
 
   // typedefs with multiple comma-separated definitions produce multiple
@@ -143,7 +187,8 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
     // This is the first (and possibly the only) TypedefDecl in a typedef. Save
     // Type and Name in case we find subsequent TypedefDecl's in this typedef.
     FirstTypedefType = Type;
-    FirstTypedefName = Name;
+    FirstTypedefName = Name.str();
+    MainTypeEndLoc = TL.getEndLoc();
   } else {
     // This is additional TypedefDecl in a comma-separated typedef declaration.
     // Start replacement *after* prior replacement and separate with semicolon.
@@ -153,10 +198,10 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
     // If this additional TypedefDecl's Type starts with the first TypedefDecl's
     // type, make this using statement refer back to the first type, e.g. make
     // "typedef int Foo, *Foo_p;" -> "using Foo = int;\nusing Foo_p = Foo*;"
-    if (Type.size() > FirstTypedefType.size() &&
-        Type.substr(0, FirstTypedefType.size()) == FirstTypedefType)
-      Type = FirstTypedefName + Type.substr(FirstTypedefType.size() + 1);
+    if (Type == FirstTypedefType && !QualifierStr.empty())
+      Type = FirstTypedefName;
   }
+
   if (!ReplaceRange.getEnd().isMacroID()) {
     const SourceLocation::IntTy Offset =
         MatchedDecl->getFunctionType() ? 0 : Name.size();
@@ -177,7 +222,7 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
       return;
   }
 
-  std::string Replacement = Using + Name + " = " + Type;
+  std::string Replacement = (Using + Name + " = " + Type + QualifierStr).str();
   Diag << FixItHint::CreateReplacement(ReplaceRange, Replacement);
 }
 } // namespace clang::tidy::modernize

clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h

@@ -26,6 +26,7 @@ class UseUsingCheck : public ClangTidyCheck {
 
   std::string FirstTypedefType;
   std::string FirstTypedefName;
+  SourceLocation MainTypeEndLoc;
 
 public:
   UseUsingCheck(StringRef Name, ClangTidyContext *Context);

clang-tools-extra/docs/ReleaseNotes.rst

@@ -228,6 +228,9 @@ Changes in existing checks
   member function calls too and to only expand macros starting with ``PRI``
   and ``__PRI`` from ``<inttypes.h>`` in the format string.
 
+- Improved :doc:`modernize-use-using
+  <clang-tidy/checks/modernize/use-using>` check by not expanding macros.
+
 - Improved :doc:`performance-avoid-endl
   <clang-tidy/checks/performance/avoid-endl>` check to use ``std::endl`` as
   placeholder when lexer cannot get source text.

clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp

@@ -80,7 +80,7 @@ typedef Test<my_class *> another;
 
 typedef int* PInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: using PInt = int *;
+// CHECK-FIXES: using PInt = int*;
 
 typedef int bla1, bla2, bla3;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
@@ -112,7 +112,7 @@ TYPEDEF Foo Bak;
 typedef FOO Bam;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
 // CHECK-FIXES: #define FOO Foo
-// CHECK-FIXES: using Bam = Foo;
+// CHECK-FIXES: using Bam = FOO;
 
 typedef struct Foo Bap;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
@@ -247,7 +247,7 @@ typedef Q<T{0 < 0}.b> Q3_t;
 
 typedef TwoArgTemplate<TwoArgTemplate<int, Q<T{0 < 0}.b> >, S<(0 < 0), Q<b[0 < 0]> > > Nested_t;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: using Nested_t = TwoArgTemplate<TwoArgTemplate<int, Q<T{0 < 0}.b>>, S<(0 < 0), Q<b[0 < 0]>>>;
+// CHECK-FIXES: using Nested_t = TwoArgTemplate<TwoArgTemplate<int, Q<T{0 < 0}.b> >, S<(0 < 0), Q<b[0 < 0]> > >;
 
 template <typename a>
 class TemplateKeyword {
@@ -265,12 +265,12 @@ class Variadic {};
 
 typedef Variadic<Variadic<int, bool, Q<T{0 < 0}.b> >, S<(0 < 0), Variadic<Q<b[0 < 0]> > > > Variadic_t;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: using Variadic_t = Variadic<Variadic<int, bool, Q<T{0 < 0}.b>>, S<(0 < 0), Variadic<Q<b[0 < 0]>>>>
+// CHECK-FIXES: using Variadic_t = Variadic<Variadic<int, bool, Q<T{0 < 0}.b> >, S<(0 < 0), Variadic<Q<b[0 < 0]> > > >
 
 typedef Variadic<Variadic<int, bool, Q<T{0 < 0}.b> >, S<(0 < 0), Variadic<Q<b[0 < 0]> > > > Variadic_t, *Variadic_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
 // CHECK-MESSAGES: :[[@LINE-2]]:103: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: using Variadic_t = Variadic<Variadic<int, bool, Q<T{0 < 0}.b>>, S<(0 < 0), Variadic<Q<b[0 < 0]>>>>;
+// CHECK-FIXES: using Variadic_t = Variadic<Variadic<int, bool, Q<T{0 < 0}.b> >, S<(0 < 0), Variadic<Q<b[0 < 0]> > > >;
 // CHECK-FIXES-NEXT: using Variadic_p = Variadic_t*;
 
 typedef struct { int a; } R_t, *R_p;
@@ -383,3 +383,29 @@ namespace ISSUE_72179
   // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: use 'using' instead of 'typedef' [modernize-use-using]
   // CHECK-FIXES: const auto foo4 = [](int a){using d = int;};
 }
+
+
+typedef int* int_ptr, *int_ptr_ptr;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:21: warning: use 'using' instead of 'typedef' [modernize-use-using]
+// CHECK-FIXES: using int_ptr = int*;
+// CHECK-FIXES-NEXT: using int_ptr_ptr = int_ptr*;
+
+#ifndef SpecialMode
+#define SomeMacro(x) x
+#else
+#define SomeMacro(x) SpecialType
+#endif
+
+class SomeMacro(GH33760) { };
+
+typedef void(SomeMacro(GH33760)::* FunctionType)(float, int);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]
+// CHECK-FIXES: using FunctionType = void(SomeMacro(GH33760)::* )(float, int);
+
+#define CDECL __attribute((cdecl))
+
+typedef void (CDECL *GH37846)(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]
+// CHECK-FIXES: using GH37846 = void (CDECL *)(int);
+