[clang-tidy] Update RedundantVoidArgCheck to check for type aliases in addition to typedefs

[courbet]

Summary

RedundantVoidArgCheck will now warn on:

using f_t = int (*(void))(void);

too.

(Depends on http://reviews.llvm.org/D19113)

[aaron.ballman]

clang-tidy/modernize/RedundantVoidArgCheck.cpp

109–111

Is this a drive-by formatting fix from clang-format?

187

Use of "typedef" is incorrect here, it should probably be "typedef" or "type alias", depending on the results of an isa<> test.

[courbet]

clang-tidy/modernize/RedundantVoidArgCheck.cpp

109–111

Yes. What's the policy here (I'm a new contributor) ? Should I not touch it ?

[courbet]

Fix error message in case of type alias declaration.

[aaron.ballman]

LGTM, thank you for working on this! I can submit the patch on your behalf if you would like.

clang-tidy/modernize/RedundantVoidArgCheck.cpp

109–111

Generally, you run clang-format just over the patch so that you're only formatting the code you've touched. However, if you ran clang-format over the entire file and this is the only change it produced, I think it's fine as a drive-by. If it produced more changes, then I would say submit all of the formatting changes as a secondary patch.

[courbet]

Thanks for the review !

I can submit the patch on your behalf if you would like.

That'd be great, thank you.

clang-tidy/modernize/RedundantVoidArgCheck.cpp

109–111

Thanks, that's what I though. The test file had a ton of reformatting changes, so I left it out.

[aaron.ballman]

Commit in r266358

clang-tidy/modernize/RedundantVoidArgCheck.h

public:
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
 
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
   void processFunctionDecl(const ast_matchers::MatchFinder::MatchResult &Result,
                            const FunctionDecl *Function);
 
-  void processTypedefDecl(const ast_matchers::MatchFinder::MatchResult &Result,
-                          const TypedefDecl *Typedef);
+  void
+  processTypedefNameDecl(const ast_matchers::MatchFinder::MatchResult &Result,
+                         const TypedefNameDecl *Typedef);
 
   void processFieldDecl(const ast_matchers::MatchFinder::MatchResult &Result,
                         const FieldDecl *Member);
 
   void processVarDecl(const ast_matchers::MatchFinder::MatchResult &Result,
                       const VarDecl *Var);
 
   void

clang-tidy/modernize/RedundantVoidArgCheck.cpp

 namespace tidy {
 namespace modernize {
 
 void RedundantVoidArgCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(functionDecl(parameterCountIs(0), unless(isImplicit()),
                                   unless(isExternC()))
                          .bind(FunctionId),
                      this);
-  Finder->addMatcher(typedefDecl().bind(TypedefId), this);
+  Finder->addMatcher(typedefNameDecl().bind(TypedefId), this);
   auto ParenFunctionType = parenType(innerType(functionType()));
   auto PointerToFunctionType = pointee(ParenFunctionType);
   auto FunctionOrMemberPointer =
       anyOf(hasType(pointerType(PointerToFunctionType)),
             hasType(memberPointerType(PointerToFunctionType)));
   Finder->addMatcher(fieldDecl(FunctionOrMemberPointer).bind(FieldId), this);
   Finder->addMatcher(varDecl(FunctionOrMemberPointer).bind(VarId), this);
   auto CastDestinationIsFunction =

 void RedundantVoidArgCheck::check(const MatchFinder::MatchResult &Result) {
   if (!Result.Context->getLangOpts().CPlusPlus) {
     return;
   }
 
   const BoundNodes &Nodes = Result.Nodes;
   if (const auto *Function = Nodes.getNodeAs<FunctionDecl>(FunctionId)) {
     processFunctionDecl(Result, Function);
-  } else if (const auto *Typedef = Nodes.getNodeAs<TypedefDecl>(TypedefId)) {
-    processTypedefDecl(Result, Typedef);
+  } else if (const auto *TypedefName =
+                 Nodes.getNodeAs<TypedefNameDecl>(TypedefId)) {
+    processTypedefNameDecl(Result, TypedefName);
   } else if (const auto *Member = Nodes.getNodeAs<FieldDecl>(FieldId)) {
     processFieldDecl(Result, Member);
   } else if (const auto *Var = Nodes.getNodeAs<VarDecl>(VarId)) {
     processVarDecl(Result, Var);
   } else if (const auto *NamedCast =
                  Nodes.getNodeAs<CXXNamedCastExpr>(NamedCastId)) {
     processNamedCastExpr(Result, NamedCast);
   } else if (const auto *CStyleCast =
                  Nodes.getNodeAs<CStyleCastExpr>(CStyleCastId)) {
     processExplicitCastExpr(Result, CStyleCast);
   } else if (const auto *ExplicitCast =
                  Nodes.getNodeAs<ExplicitCastExpr>(ExplicitCastId)) {
     processExplicitCastExpr(Result, ExplicitCast);
   } else if (const auto *Lambda = Nodes.getNodeAs<LambdaExpr>(LambdaId)) {
     processLambdaExpr(Result, Lambda);
   }
 }
 
 void RedundantVoidArgCheck::processFunctionDecl(
     const MatchFinder::MatchResult &Result, const FunctionDecl *Function) {
   if (Function->isThisDeclarationADefinition()) {
     const Stmt *Body = Function->getBody();
     SourceLocation Start = Function->getLocStart();
-    SourceLocation End = Body ? Body->getLocStart().getLocWithOffset(-1) :
-                                Function->getLocEnd();
+    SourceLocation End =
+        Body ? Body->getLocStart().getLocWithOffset(-1) : Function->getLocEnd();
     removeVoidArgumentTokens(Result, SourceRange(Start, End),
                              "function definition");
   } else {
     removeVoidArgumentTokens(Result, Function->getSourceRange(),
                              "function declaration");
   }
 }
 
 void RedundantVoidArgCheck::removeVoidArgumentTokens(

 void RedundantVoidArgCheck::removeVoidToken(Token VoidToken,
                                             StringRef Diagnostic) {
   SourceLocation VoidLoc(VoidToken.getLocation());
   auto VoidRange =
       CharSourceRange::getTokenRange(VoidLoc, VoidLoc.getLocWithOffset(3));
   diag(VoidLoc, Diagnostic) << FixItHint::CreateRemoval(VoidRange);
 }
 
-void RedundantVoidArgCheck::processTypedefDecl(
-    const MatchFinder::MatchResult &Result, const TypedefDecl *Typedef) {
-  if (protoTypeHasNoParms(Typedef->getUnderlyingType())) {
-    removeVoidArgumentTokens(Result, Typedef->getSourceRange(), "typedef");
+void RedundantVoidArgCheck::processTypedefNameDecl(
+    const MatchFinder::MatchResult &Result,
+    const TypedefNameDecl *TypedefName) {
+  if (protoTypeHasNoParms(TypedefName->getUnderlyingType())) {
+    removeVoidArgumentTokens(Result, TypedefName->getSourceRange(),
+                             isa<TypedefDecl>(TypedefName) ? "typedef"
+                                                           : "type alias");
   }
 }
 
 void RedundantVoidArgCheck::processFieldDecl(
     const MatchFinder::MatchResult &Result, const FieldDecl *Member) {
   if (protoTypeHasNoParms(Member->getType())) {
     removeVoidArgumentTokens(Result, Member->getSourceRange(),
                              "field declaration");

test/clang-tidy/modernize-redundant-void-arg.cpp

 // CHECK-MESSAGES: :[[@LINE-2]]:34: warning: {{.*}} in function declaration
 // CHECK-FIXES: {{^}}int (*returns_fn_void_int())();{{$}}
 
 typedef int (*returns_fn_void_int_t(void))(void);
 // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: {{.*}} in typedef
 // CHECK-MESSAGES: :[[@LINE-2]]:44: warning: {{.*}} in typedef
 // CHECK-FIXES: {{^}}typedef int (*returns_fn_void_int_t())();{{$}}
 
+// Should work for type aliases as well as typedef.
+using returns_fn_void_int_t2 = int (*(void))(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:39: warning: {{.*}} in type alias
+// CHECK-MESSAGES: :[[@LINE-2]]:46: warning: {{.*}} in type alias
+// CHECK-FIXES: {{^}}using returns_fn_void_int_t2 = int (*())();{{$}}
+
 int (*returns_fn_void_int(void))(void) {
 // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: {{.*}} in function definition
 // CHECK-MESSAGES: :[[@LINE-2]]:34: warning: {{.*}} in function definition
 // CHECK-FIXES: {{^}}int (*returns_fn_void_int())() {{{$}}
   return nullptr;
 }
 
 // A function taking void and returning a pointer to a function taking void