[clang-tidy] use specified type verbatim in modernize-use-using fix

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

@@ -7,9 +7,14 @@
 //===----------------------------------------------------------------------===//
 
 #include "UseUsingCheck.h"
-#include "clang/AST/ASTContext.h"
+#include "../utils/LexerUtils.h"
 #include "clang/AST/DeclGroup.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Lexer.h"
+#include <string>
 
 using namespace clang::ast_matchers;
 namespace {
@@ -83,6 +88,9 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
   if (!ParentDecl)
     return;
 
+  const SourceManager &SM = *Result.SourceManager;
+  const LangOptions &LO = getLangOpts();
+
   // Match CXXRecordDecl only to store the range of the last non-implicit full
   // declaration, to later check whether it's within the typdef itself.
   const auto *MatchedTagDecl = Result.Nodes.getNodeAs<TagDecl>(TagDeclName);
@@ -119,14 +127,51 @@ 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, this, &TL, &SM,
+                               &LO]() -> 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())) {
+      const auto RangeLeftOfIdentifier = CharSourceRange::getCharRange(
+          TypeRange.getBegin(), MatchedDecl->getLocation());
+      const auto RangeRightOfIdentifier = CharSourceRange::getCharRange(
+          Lexer::getLocForEndOfToken(MatchedDecl->getLocation(), 0, SM, LO),
+          Lexer::getLocForEndOfToken(TypeRange.getEnd(), 0, SM, LO));
+      const std::string VerbatimType =
+          (Lexer::getSourceText(RangeLeftOfIdentifier, SM, LO) +
+           Lexer::getSourceText(RangeRightOfIdentifier, SM, LO))
+              .str();
+      return {VerbatimType, ""};
+    }
+
+    StringRef ExtraReference = "";
+    if (MainTypeEndLoc.isValid() && TypeRange.fullyContains(MainTypeEndLoc)) {
+      // Each type introduced in a typedef can specify being a reference or
+      // pointer type seperately, so we need to sigure out if the new using-decl
+      // needs to be to a reference or pointer as well.
+      const SourceLocation Tok = utils::lexer::findPreviousAnyTokenKind(
+          MatchedDecl->getLocation(), SM, LO, tok::TokenKind::star,
+          tok::TokenKind::amp, tok::TokenKind::comma,
+          tok::TokenKind::kw_typedef);
+
+      ExtraReference = Lexer::getSourceText(
+          CharSourceRange::getCharRange(Tok, Tok.getLocWithOffset(1)), SM, LO);
 
-  std::string Type = MatchedDecl->getUnderlyingType().getAsString(PrintPolicy);
-  std::string Name = MatchedDecl->getNameAsString();
+      if (ExtraReference != "*" && ExtraReference != "&")
+        ExtraReference = "";
+
+      TypeRange.setEnd(MainTypeEndLoc);
+    }
+    return {
+        Lexer::getSourceText(CharSourceRange::getTokenRange(TypeRange), SM, LO)
+            .str(),
+        ExtraReference.str()};
+  }();
+  StringRef Name = MatchedDecl->getName();
   SourceRange ReplaceRange = MatchedDecl->getSourceRange();
 
   // typedefs with multiple comma-separated definitions produce multiple
@@ -143,7 +188,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 +199,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();
@@ -171,13 +217,12 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
       LastTagDeclRange->second.isValid() &&
       ReplaceRange.fullyContains(LastTagDeclRange->second)) {
     Type = std::string(Lexer::getSourceText(
-        CharSourceRange::getTokenRange(LastTagDeclRange->second),
-        *Result.SourceManager, getLangOpts()));
+        CharSourceRange::getTokenRange(LastTagDeclRange->second), SM, LO));
     if (Type.empty())
       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,57 @@ 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))
+
+// GH37846 & GH41685
+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);
+
+typedef void (__attribute((cdecl)) *GH41685)(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]
+// CHECK-FIXES: using GH41685 = void (__attribute((cdecl)) *)(int);
+
+namespace GH83568 {
+  typedef int(*name)(int arg1, int arg2);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef' [modernize-use-using]
+// CHECK-FIXES: using name = int(*)(int arg1, int arg2);
+}
+
+#ifdef FOO
+#define GH95716 float
+#else
+#define GH95716 double
+#endif
+
+typedef GH95716 foo;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]
+// CHECK-FIXES: using foo = GH95716;
+
+namespace GH97009 {
+  typedef double PointType[3];
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef' [modernize-use-using]
+  typedef bool (*Function)(PointType, PointType);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef' [modernize-use-using]
+// CHECK-FIXES: using Function = bool (*)(PointType, PointType);
+}