[clang-tidy] Enhance modernize-use-auto to casts

Summary:
Extend modernize-use-auto to cases when a variable is assigned with a cast.

e.g.
Type *Ptr1 = dynamic_cast<Type*>(Ptr2);

http://llvm.org/PR25499

Reviewers: angelgarcia, aaron.ballman, klimek, Prazek, alexfh

Subscribers: Prazek, Eugene.Zelenko, cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D25316

llvm-svn: 285579

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

@@ -23,6 +23,7 @@ namespace {
 
 const char IteratorDeclStmtId[] = "iterator_decl";
 const char DeclWithNewId[] = "decl_new";
+const char DeclWithCastId[] = "decl_cast";
 
 /// \brief Matches variable declarations that have explicit initializers that
 /// are not initializer lists.
@@ -208,27 +209,18 @@ TypeMatcher iteratorFromUsingDeclaration() {
 /// \brief This matcher returns declaration statements that contain variable
 /// declarations with written non-list initializer for standard iterators.
 StatementMatcher makeIteratorDeclMatcher() {
-  return declStmt(
-             // At least one varDecl should be a child of the declStmt to ensure
-             // it's a declaration list and avoid matching other declarations,
-             // e.g. using directives.
-             has(varDecl()),
-             unless(has(varDecl(anyOf(
-                 unless(hasWrittenNonListInitializer()), hasType(autoType()),
-                 unless(hasType(
-                     isSugarFor(anyOf(typedefIterator(), nestedIterator(),
-                                      iteratorFromUsingDeclaration())))))))))
+  return declStmt(unless(has(
+                      varDecl(anyOf(unless(hasWrittenNonListInitializer()),
+                                    unless(hasType(isSugarFor(anyOf(
+                                        typedefIterator(), nestedIterator(),
+                                        iteratorFromUsingDeclaration())))))))))
       .bind(IteratorDeclStmtId);
 }
 
 StatementMatcher makeDeclWithNewMatcher() {
   return declStmt(
-             has(varDecl()),
              unless(has(varDecl(anyOf(
                  unless(hasInitializer(ignoringParenImpCasts(cxxNewExpr()))),
-                 // Skip declarations that are already using auto.
-                 anyOf(hasType(autoType()),
-                       hasType(pointerType(pointee(autoType())))),
                  // FIXME: TypeLoc information is not reliable where CV
                  // qualifiers are concerned so these types can't be
                  // handled for now.
@@ -243,6 +235,26 @@ StatementMatcher makeDeclWithNewMatcher() {
       .bind(DeclWithNewId);
 }
 
+StatementMatcher makeDeclWithCastMatcher() {
+  return declStmt(
+             unless(has(varDecl(unless(hasInitializer(explicitCastExpr()))))))
+      .bind(DeclWithCastId);
+}
+
+StatementMatcher makeCombinedMatcher() {
+  return declStmt(
+      // At least one varDecl should be a child of the declStmt to ensure
+      // it's a declaration list and avoid matching other declarations,
+      // e.g. using directives.
+      has(varDecl()),
+      // Skip declarations that are already using auto.
+      unless(has(varDecl(anyOf(hasType(autoType()),
+                               hasType(pointerType(pointee(autoType()))),
+                               hasType(referenceType(pointee(autoType()))))))),
+      anyOf(makeIteratorDeclMatcher(), makeDeclWithNewMatcher(),
+            makeDeclWithCastMatcher()));
+}
+
 } // namespace
 
 UseAutoCheck::UseAutoCheck(StringRef Name, ClangTidyContext *Context)
@@ -257,8 +269,7 @@ void UseAutoCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matchers for C++; the functionality currently does not
   // provide any benefit to other languages, despite being benign.
   if (getLangOpts().CPlusPlus) {
-    Finder->addMatcher(makeIteratorDeclMatcher(), this);
-    Finder->addMatcher(makeDeclWithNewMatcher(), this);
+    Finder->addMatcher(makeCombinedMatcher(), this);
   }
 }
 
@@ -313,7 +324,9 @@ void UseAutoCheck::replaceIterators(const DeclStmt *D, ASTContext *Context) {
       << FixItHint::CreateReplacement(Range, "auto");
 }
 
-void UseAutoCheck::replaceNew(const DeclStmt *D, ASTContext *Context) {
+void UseAutoCheck::replaceExpr(const DeclStmt *D, ASTContext *Context,
+                               std::function<QualType(const Expr *)> GetType,
+                               StringRef Message) {
   const auto *FirstDecl = dyn_cast<VarDecl>(*D->decl_begin());
   // Ensure that there is at least one VarDecl within the DeclStmt.
   if (!FirstDecl)
@@ -328,13 +341,13 @@ void UseAutoCheck::replaceNew(const DeclStmt *D, ASTContext *Context) {
     if (!V)
       return;
 
-    const auto *NewExpr = cast<CXXNewExpr>(V->getInit()->IgnoreParenImpCasts());
-    // Ensure that every VarDecl has a CXXNewExpr initializer.
-    if (!NewExpr)
+    const auto *Expr = V->getInit()->IgnoreParenImpCasts();
+    // Ensure that every VarDecl has an initializer.
+    if (!Expr)
       return;
 
     // If VarDecl and Initializer have mismatching unqualified types.
-    if (!Context->hasSameUnqualifiedType(V->getType(), NewExpr->getType()))
+    if (!Context->hasSameUnqualifiedType(V->getType(), GetType(Expr)))
       return;
 
     // All subsequent variables in this declaration should have the same
@@ -367,9 +380,13 @@ void UseAutoCheck::replaceNew(const DeclStmt *D, ASTContext *Context) {
            Loc.getTypeLocClass() == TypeLoc::Qualified)
       Loc = Loc.getNextTypeLoc();
   }
+  while (Loc.getTypeLocClass() == TypeLoc::LValueReference ||
+         Loc.getTypeLocClass() == TypeLoc::RValueReference ||
+         Loc.getTypeLocClass() == TypeLoc::Qualified) {
+    Loc = Loc.getNextTypeLoc();
+  }
   SourceRange Range(Loc.getSourceRange());
-  auto Diag = diag(Range.getBegin(), "use auto when initializing with new"
-                                     " to avoid duplicating the type name");
+  auto Diag = diag(Range.getBegin(), Message);
 
   // Space after 'auto' to handle cases where the '*' in the pointer type is
   // next to the identifier. This avoids changing 'int *p' into 'autop'.
@@ -382,7 +399,19 @@ void UseAutoCheck::check(const MatchFinder::MatchResult &Result) {
     replaceIterators(Decl, Result.Context);
   } else if (const auto *Decl =
                  Result.Nodes.getNodeAs<DeclStmt>(DeclWithNewId)) {
-    replaceNew(Decl, Result.Context);
+    replaceExpr(Decl, Result.Context,
+                [](const Expr *Expr) { return Expr->getType(); },
+                "use auto when initializing with new to avoid "
+                "duplicating the type name");
+  } else if (const auto *Decl =
+                 Result.Nodes.getNodeAs<DeclStmt>(DeclWithCastId)) {
+    replaceExpr(
+        Decl, Result.Context,
+        [](const Expr *Expr) {
+          return cast<ExplicitCastExpr>(Expr)->getTypeAsWritten();
+        },
+        "use auto when initializing with a cast to avoid duplicating the type "
+        "name");
   } else {
     llvm_unreachable("Bad Callback. No node provided.");
   }

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

@@ -25,7 +25,9 @@ public:
 
 private:
   void replaceIterators(const DeclStmt *D, ASTContext *Context);
-  void replaceNew(const DeclStmt *D, ASTContext *Context);
+  void replaceExpr(const DeclStmt *D, ASTContext *Context,
+                   std::function<QualType(const Expr *)> GetType,
+                   StringRef Message);
 
   const bool RemoveStars;
 };

clang-tools-extra/docs/ReleaseNotes.rst

@@ -81,6 +81,10 @@ Improvements to clang-tidy
   Warns if an object is used after it has been moved, without an intervening
   reinitialization.
 
+- The `modernize-use-auto
+  <http://clang.llvm.org/extra/clang-tidy/checks/modernize.html>`_ check
+  now warns about variable declarations that are initialized with a cast.
+
 - New `mpi-buffer-deref
   <http://clang.llvm.org/extra/clang-tidy/checks/mpi-buffer-deref.html>`_ check
 

clang-tools-extra/docs/clang-tidy/checks/modernize-use-auto.rst

@@ -114,7 +114,7 @@ New expressions
 ---------------
 
 Frequently, when a pointer is declared and initialized with ``new``, the
-pointee type has to be written twice: in the declaration type and in the
+pointee type is written twice: in the declaration type and in the
 ``new`` expression. In this cases, the declaration type can be replaced with
 ``auto`` improving readability and maintainability.
 
@@ -143,6 +143,25 @@ the following conditions are satisfied:
 
   auto *my_first_pointer = new TypeName, *my_second_pointer = new TypeName;
 
+Cast expressions
+----------------
+
+Frequently, when a variable is declared and initialized with a cast, the
+variable type is written twice: in the declaration type and in the
+cast expression. In this cases, the declaration type can be replaced with
+``auto`` improving readability and maintainability.
+
+.. code-block:: c++
+
+  TypeName *my_pointer = static_cast<TypeName>(my_param);
+
+  // becomes
+
+  auto *my_pointer = static_cast<TypeName>(my_param);
+
+The check handles ``static_cast``, ``dynamic_cast``, ``const_cast``,
+``reinterpret_cast``, functional casts and C-style casts.
+
 Known Limitations
 -----------------
 
@@ -151,6 +170,9 @@ Known Limitations
 
 * User-defined iterators are not handled at this time.
 
+* Function templates that behave as casts, such as ``llvm::dyn_cast``,
+  ``boost::lexical_cast`` or ``gsl::narrow_cast`` are not handled.
+
 Options
 -------
 

clang-tools-extra/test/clang-tidy/modernize-use-auto-cast-remove-stars.cpp

@@ -0,0 +1,118 @@
+// RUN: %check_clang_tidy %s modernize-use-auto %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-auto.RemoveStars, value: '1'}]}" \
+// RUN:   -- -std=c++11
+
+struct A {
+  virtual ~A() {}
+};
+
+struct B : public A {};
+
+struct C {};
+
+void f_static_cast() {
+  long l = 1;
+  int i1 = static_cast<int>(l);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto  i1 = static_cast<int>(l);
+
+  const int i2 = static_cast<int>(l);
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: const auto  i2 = static_cast<int>(l);
+
+  long long ll = static_cast<long long>(l);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto  ll = static_cast<long long>(l);
+  unsigned long long ull = static_cast<unsigned long long>(l);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto  ull = static_cast<unsigned long long>(l);
+  unsigned int ui = static_cast<unsigned int>(l);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto  ui = static_cast<unsigned int>(l);
+  long double ld = static_cast<long double>(l);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto  ld = static_cast<long double>(l);
+
+  A *a = new B();
+  B *b1 = static_cast<B *>(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto b1 = static_cast<B *>(a);
+
+  B *const b2 = static_cast<B *>(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto const b2 = static_cast<B *>(a);
+
+  const B *b3 = static_cast<const B *>(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto b3 = static_cast<const B *>(a);
+
+  B &b4 = static_cast<B &>(*a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto  &b4 = static_cast<B &>(*a);
+
+  const B &b5 = static_cast<const B &>(*a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: const auto  &b5 = static_cast<const B &>(*a);
+
+  B &b6 = static_cast<B &>(*a), &b7 = static_cast<B &>(*a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto  &b6 = static_cast<B &>(*a), &b7 = static_cast<B &>(*a);
+
+  // Don't warn when auto is already being used.
+  auto i3 = static_cast<int>(l);
+  auto *b8 = static_cast<B *>(a);
+  auto &b9 = static_cast<B &>(*a);
+}
+
+void f_dynamic_cast() {
+  A *a = new B();
+  B *b1 = dynamic_cast<B *>(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto b1 = dynamic_cast<B *>(a);
+
+  B &b2 = dynamic_cast<B &>(*a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto  &b2 = dynamic_cast<B &>(*a);
+}
+
+void f_reinterpret_cast() {
+  auto *a = new A();
+  C *c1 = reinterpret_cast<C *>(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto c1 = reinterpret_cast<C *>(a);
+
+  C &c2 = reinterpret_cast<C &>(*a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto  &c2 = reinterpret_cast<C &>(*a);
+}
+
+void f_const_cast() {
+  const A *a1 = new A();
+  A *a2 = const_cast<A *>(a1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto a2 = const_cast<A *>(a1);
+  A &a3 = const_cast<A &>(*a1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto  &a3 = const_cast<A &>(*a1);
+}
+
+void f_cstyle_cast() {
+  auto *a = new A();
+  C *c1 = (C *)a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto c1 = (C *)a;
+
+  C &c2 = (C &)*a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto  &c2 = (C &)*a;
+}
+
+void f_functional_cast() {
+  long l = 1;
+  int i1 = int(l);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto  i1 = int(l);
+
+  B b;
+  A a = A(b);
+}

clang-tools-extra/test/clang-tidy/modernize-use-auto-cast.cpp

@@ -0,0 +1,120 @@
+// RUN: %check_clang_tidy %s modernize-use-auto %t -- -- \
+// RUN:   -std=c++11 -I %S/Inputs/modernize-use-auto
+
+struct A {
+  virtual ~A() {}
+};
+
+struct B : public A {};
+
+struct C {};
+
+void f_static_cast() {
+  long l = 1;
+  int i1 = static_cast<int>(l);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto i1 = static_cast<int>(l);
+
+  const int i2 = static_cast<int>(l);
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: const auto i2 = static_cast<int>(l);
+
+  long long ll = static_cast<long long>(l);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto ll = static_cast<long long>(l);
+  unsigned long long ull = static_cast<unsigned long long>(l);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto ull = static_cast<unsigned long long>(l);
+  unsigned int ui = static_cast<unsigned int>(l);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto ui = static_cast<unsigned int>(l);
+  long double ld = static_cast<long double>(l);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto ld = static_cast<long double>(l);
+
+  A *a = new B();
+  B *b1 = static_cast<B *>(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto *b1 = static_cast<B *>(a);
+
+  B *const b2 = static_cast<B *>(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto *const b2 = static_cast<B *>(a);
+
+  const B *b3 = static_cast<const B *>(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: const auto *b3 = static_cast<const B *>(a);
+
+  B &b4 = static_cast<B &>(*a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto &b4 = static_cast<B &>(*a);
+
+  const B &b5 = static_cast<const B &>(*a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: const auto &b5 = static_cast<const B &>(*a);
+
+  B &b6 = static_cast<B &>(*a), &b7 = static_cast<B &>(*a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto &b6 = static_cast<B &>(*a), &b7 = static_cast<B &>(*a);
+
+  // Don't warn when non-cast involved
+  long double cast = static_cast<long double>(l), noncast = 5;
+
+  // Don't warn when auto is already being used.
+  auto i3 = static_cast<int>(l);
+  auto *b8 = static_cast<B *>(a);
+  auto &b9 = static_cast<B &>(*a);
+}
+
+void f_dynamic_cast() {
+  A *a = new B();
+  B *b1 = dynamic_cast<B *>(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto *b1 = dynamic_cast<B *>(a);
+
+  B &b2 = dynamic_cast<B &>(*a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto &b2 = dynamic_cast<B &>(*a);
+}
+
+void f_reinterpret_cast() {
+  auto *a = new A();
+  C *c1 = reinterpret_cast<C *>(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto *c1 = reinterpret_cast<C *>(a);
+
+  C &c2 = reinterpret_cast<C &>(*a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto &c2 = reinterpret_cast<C &>(*a);
+}
+
+void f_const_cast() {
+  const A *a1 = new A();
+  A *a2 = const_cast<A *>(a1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto *a2 = const_cast<A *>(a1);
+  A &a3 = const_cast<A &>(*a1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto &a3 = const_cast<A &>(*a1);
+}
+
+void f_cstyle_cast() {
+  auto *a = new A();
+  C *c1 = (C *)a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto *c1 = (C *)a;
+
+  C &c2 = (C &)*a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto &c2 = (C &)*a;
+}
+
+void f_functional_cast() {
+  long l = 1;
+  int i1 = int(l);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto i1 = int(l);
+
+  B b;
+  A a = A(b);
+}

clang-tools-extra/test/clang-tidy/modernize-use-auto-new.cpp

@@ -15,6 +15,10 @@ void auto_new() {
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use auto when initializing with new
   // CHECK-FIXES: static auto *a_static = new MyType();
 
+  long long *ll = new long long();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with new
+  // CHECK-FIXES: auto *ll = new long long();
+
   MyType *derived = new MyDerivedType();
 
   void *vd = new MyType();