[clang-tidy] Fix some false positives in google-readability-casting

[alexfh]

Summary

Ignore C-style casts in extern "C" {} sections. Be more careful when
detecting redundant casts between typedefs to the same type - emit a more
specific warning and don't automatically fix them.

[klimek]

lg

clang-tidy/google/AvoidCStyleCastsCheck.cpp

void AvoidCStyleCastsCheck::check(const MatchFinder::MatchResult &Result) {
   if (ParenRange.getBegin().isMacroID() || ParenRange.getEnd().isMacroID())
     return;
 
   // Casting to void is an idiomatic way to mute "unused variable" and similar
   // warnings.
   if (CastExpr->getTypeAsWritten()->isVoidType())
     return;
 
-  QualType SourceType =
-      CastExpr->getSubExprAsWritten()->getType().getCanonicalType();
-  QualType DestType = CastExpr->getTypeAsWritten().getCanonicalType();
+  QualType SourceType = CastExpr->getSubExprAsWritten()->getType();
+  QualType DestType = CastExpr->getTypeAsWritten();
 
   if (SourceType == DestType) {
-    diag(CastExpr->getLocStart(), "Redundant cast to the same type.")
+    diag(CastExpr->getLocStart(), "redundant cast to the same type")
         << FixItHint::CreateRemoval(ParenRange);
     return;
   }
+  SourceType = SourceType.getCanonicalType();
+  DestType = DestType.getCanonicalType();
+  if (SourceType == DestType) {
+    diag(CastExpr->getLocStart(),
+         "possibly redundant cast between typedefs of the same type");
+    return;
+  }
+
 
   // The rest of this check is only relevant to C++.
   if (!Result.Context->getLangOpts().CPlusPlus)
     return;
+  // Ignore code inside extern "C" {} blocks.
+  if (!match(expr(hasAncestor(linkageSpecDecl())), *CastExpr, *Result.Context)
+           .empty())
+    return;
 
   // Leave type spelling exactly as it was (unlike
   // getTypeAsWritten().getAsString() which would spell enum types 'enum X').
   StringRef DestTypeString = Lexer::getSourceText(
       CharSourceRange::getTokenRange(
           CastExpr->getLParenLoc().getLocWithOffset(1),
           CastExpr->getRParenLoc().getLocWithOffset(-1)),
       *Result.SourceManager, Result.Context->getLangOpts());
 
   auto diag_builder =
       diag(CastExpr->getLocStart(), "C-style casts are discouraged. %0");
 
   auto ReplaceWithCast = [&](StringRef CastType) {
-    diag_builder << ("Use " + CastType + ".").str();
+    diag_builder << ("Use " + CastType).str();
 
     const Expr *SubExpr = CastExpr->getSubExprAsWritten()->IgnoreImpCasts();
     std::string CastText = (CastType + "<" + DestTypeString + ">").str();
     if (!isa<ParenExpr>(SubExpr)) {
       CastText.push_back('(');
       diag_builder << FixItHint::CreateInsertion(
           Lexer::getLocForEndOfToken(SubExpr->getLocEnd(), 0,
                                      *Result.SourceManager,
                                      Result.Context->getLangOpts()),
           ")");
     }
     diag_builder << FixItHint::CreateReplacement(ParenRange, CastText);
   };
+
   // Suggest appropriate C++ cast. See [expr.cast] for cast notation semantics.
   switch (CastExpr->getCastKind()) {
   case CK_NoOp:
     if (needsConstCast(SourceType, DestType) &&
         pointedTypesAreEqual(SourceType, DestType)) {
       ReplaceWithCast("const_cast");
       return;
     }
if (!needsConstCast(SourceType, DestType)) {
       ReplaceWithCast("reinterpret_cast");
       return;
     }
     break;
   default:
     break;
   }
 
-  diag_builder << "Use static_cast/const_cast/reinterpret_cast.";
+  diag_builder << "Use static_cast/const_cast/reinterpret_cast";
 }
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang

test/clang-tidy/google-readability-casting.c

 // RUN: $(dirname %s)/check_clang_tidy.sh %s google-readability-casting %t -- -x c
 // REQUIRES: shell
 
 void f(const char *cpc) {
   const char *cpc2 = (const char*)cpc;
-  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: Redundant cast to the same type. [google-readability-casting]
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: redundant cast to the same type [google-readability-casting]
   // CHECK-FIXES: const char *cpc2 = cpc;
   char *pc = (char*)cpc;
 }

test/clang-tidy/google-readability-casting.cpp

 // RUN: $(dirname %s)/check_clang_tidy.sh %s google-readability-casting %t
 // REQUIRES: shell
 
 bool g() { return false; }
 
 enum Enum { Enum1 };
 struct X {};
 struct Y : public X {};
 
 void f(int a, double b, const char *cpc, const void *cpv, X *pX) {
   const char *cpc2 = (const char*)cpc;
-  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: Redundant cast to the same type. [google-readability-casting]
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: redundant cast to the same type [google-readability-casting]
   // CHECK-FIXES: const char *cpc2 = cpc;
 
+  typedef const char *Typedef1;
+  typedef const char *Typedef2;
+  Typedef1 t1;
+  (Typedef2)t1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: possibly redundant cast between typedefs of the same type [google-readability-casting]
+  // CHECK-FIXES: {{^}}  (Typedef2)t1;
+  (const char*)t1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: possibly redundant cast {{.*}}
+  // CHECK-FIXES: {{^}}  (const char*)t1;
+  (Typedef1)cpc;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: possibly redundant cast {{.*}}
+  // CHECK-FIXES: {{^}}  (Typedef1)cpc;
+  (Typedef1)t1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant cast to the same type
+  // CHECK-FIXES: {{^}}  t1;
+
   char *pc = (char*)cpc;
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: C-style casts are discouraged. Use const_cast. {{.*}}
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: C-style casts are discouraged. Use const_cast [google-readability-casting]
   // CHECK-FIXES: char *pc = const_cast<char*>(cpc);
 
   char *pc2 = (char*)(cpc + 33);
-  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: C-style casts are discouraged. Use const_cast. {{.*}}
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}. Use const_cast {{.*}}
   // CHECK-FIXES: char *pc2 = const_cast<char*>(cpc + 33);
 
   const char &crc = *cpc;
   char &rc = (char&)crc;
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: C-style casts are discouraged. Use const_cast. {{.*}}
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: {{.*}}. Use const_cast {{.*}}
   // CHECK-FIXES: char &rc = const_cast<char&>(crc);
 
   char &rc2 = (char&)*cpc;
-  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: C-style casts are discouraged. Use const_cast. {{.*}}
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}. Use const_cast {{.*}}
   // CHECK-FIXES: char &rc2 = const_cast<char&>(*cpc);
 
   char ** const* const* ppcpcpc;
   char ****ppppc = (char****)ppcpcpc;
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: C-style casts are discouraged. Use const_cast. {{.*}}
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}}. Use const_cast {{.*}}
   // CHECK-FIXES: char ****ppppc = const_cast<char****>(ppcpcpc);
 
   char ***pppc = (char***)*(ppcpcpc);
-  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: C-style casts are discouraged. Use const_cast. {{.*}}
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: {{.*}}. Use const_cast {{.*}}
   // CHECK-FIXES: char ***pppc = const_cast<char***>(*(ppcpcpc));
 
   char ***pppc2 = (char***)(*ppcpcpc);
-  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: C-style casts are discouraged. Use const_cast. {{.*}}
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: {{.*}}. Use const_cast {{.*}}
   // CHECK-FIXES: char ***pppc2 = const_cast<char***>(*ppcpcpc);
 
   char *pc5 = (char*)(const char*)(cpv);
-  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: C-style casts are discouraged. Use const_cast. {{.*}}
-  // CHECK-MESSAGES: :[[@LINE-2]]:22: warning: C-style casts are discouraged. Use reinterpret_cast. {{.*}}
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}. Use const_cast {{.*}}
+  // CHECK-MESSAGES: :[[@LINE-2]]:22: warning: {{.*}}. Use reinterpret_cast {{.*}}
   // CHECK-FIXES: char *pc5 = const_cast<char*>(reinterpret_cast<const char*>(cpv));
 
   int b1 = (int)b;
-  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: C-style casts are discouraged. Use static_cast. [google-readability-casting]
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: {{.*}}. Use static_cast {{.*}}
   // CHECK-FIXES: int b1 = static_cast<int>(b);
 
   Y *pB = (Y*)pX;
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: C-style casts are discouraged. Use static_cast/const_cast/reinterpret_cast. [google-readability-casting]
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}}. Use static_cast/const_cast/reinterpret_cast {{.*}}
   Y &rB = (Y&)*pX;
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: C-style casts are discouraged. Use static_cast/const_cast/reinterpret_cast. [google-readability-casting]
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}}. Use static_cast/const_cast/reinterpret_cast {{.*}}
 
   const char *pc3 = (const char*)cpv;
-  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: C-style casts are discouraged. Use reinterpret_cast. [google-readability-casting]
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: {{.*}}. Use reinterpret_cast {{.*}}
   // CHECK-FIXES: const char *pc3 = reinterpret_cast<const char*>(cpv);
 
   char *pc4 = (char*)cpv;
-  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: C-style casts are discouraged. Use static_cast/const_cast/reinterpret_cast. [google-readability-casting]
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}. Use static_cast/const_cast/reinterpret_cast {{.*}}
   // CHECK-FIXES: char *pc4 = (char*)cpv;
 
   b1 = (int)Enum1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: C-style casts are discouraged. Use static_cast. [google-readability-casting]
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: {{.*}}. Use static_cast {{.*}}
   // CHECK-FIXES: b1 = static_cast<int>(Enum1);
 
   Enum e = (Enum)b1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: C-style casts are discouraged. Use static_cast. [google-readability-casting]
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: {{.*}}. Use static_cast {{.*}}
   // CHECK-FIXES: Enum e = static_cast<Enum>(b1);
 
   // CHECK-MESSAGES-NOT: warning:
   int b2 = int(b);
   int b3 = static_cast<double>(b);
   int b4 = b;
   double aa = a;
   (void)b2;
   return (void)g();
 }
 
 template <typename T>
 void template_function(T t, int n) {
   int i = (int)t;
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: C-style casts are discouraged. Use static_cast/const_cast/reinterpret_cast.
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}}. Use static_cast/const_cast/reinterpret_cast {{.*}}
   // CHECK-FIXES: int i = (int)t;
   int j = (int)n;
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Redundant cast to the same type.
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant cast to the same type
   // CHECK-FIXES: int j = n;
 }
 
 template <typename T>
 struct TemplateStruct {
   void f(T t, int n) {
     int k = (int)t;
-    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: C-style casts are discouraged. Use static_cast/const_cast/reinterpret_cast.
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}}. Use static_cast/const_cast/reinterpret_cast
     // CHECK-FIXES: int k = (int)t;
     int l = (int)n;
-    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: Redundant cast to the same type.
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant cast to the same type
     // CHECK-FIXES: int l = n;
   }
 };
 
 void test_templates() {
   template_function(1, 42);
   template_function(1.0, 42);
   TemplateStruct<int>().f(1, 42);
   TemplateStruct<double>().f(1.0, 42);
 }
 
+extern "C" {
+void extern_c_code(const char *cpc) {
+  const char *cpc2 = (const char*)cpc;
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: redundant cast to the same type
+  // CHECK-FIXES: const char *cpc2 = cpc;
+  char *pc = (char*)cpc;
+}
+}
+
 #define CAST(type, value) (type)(value)
 void macros(double d) {
   int i = CAST(int, d);
 }
 
 enum E { E1 = 1 };
 template <E e>
 struct A {
   // Usage of template argument e = E1 is represented as (E)1 in the AST for
   // some reason. We have a special treatment of this case to avoid warnings
   // here.
   static const E ee = e;
 };
 struct B : public A<E1> {};