Fixes to modernize-use-using

[krystyna]

Summary

• removed unnessacary namespaces
• added option to print warning in macros
• no fix for typedef with array
• removed "void" word from functions with 0 parameters

Fixes some bugs from bug http://llvm.org/PR28334. Rest of bugs moved to new bug http://llvm.org/PR31793.

[Prazek]

Please mention in comments which bugs it solves. Also mention this review in the bugs on buganizer.
after fixing the nits, it LGTM, but wait for Alex approval.

clang-tidy/modernize/UseUsingCheck.cpp

99

Start with space and capital letter, end with dot.

clang-tidy/modernize/UseUsingCheck.h

24

please move it under the functions declarations, adding private:

28

probably move it to source file to keep convention.

[krystyna]

Style fixes.

[krystyna]

ping

[alexfh]

LG with a few comments.

clang-tidy/modernize/UseUsingCheck.h

24

nit: empty line

35

nit: MacroWarning doesn't sound right to me. Maybe WarnInMacros or even IgnoreMacros (defaulting to true)?

test/clang-tidy/modernize-use-using.cpp

154

Why no trailing semicolon?

[alexfh]

Krystyna, do you need help committing the patch after you address the outstanding comments?

[krystyna]

I have plan to finish this patch next week, when I finish academic year at my school. If I will have any issues with submitting, Prazek offered to help me.

[alexfh]

In D29262#709351, @krystyna wrote:

I have plan to finish this patch next week, when I finish academic year at my school. If I will have any issues with submitting, Prazek offered to help me.

Sure, no stress. Good luck with your studies!

clang-tidy/modernize/UseUsingCheck.h

 namespace tidy {
 namespace modernize {
 
 /// Check finds typedefs and replaces it with usings.
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-using.html
 class UseUsingCheck : public ClangTidyCheck {
+  const bool MacroWarning;
+
 public:
-  UseUsingCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  UseUsingCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override {
+    Options.store(Opts, "MacroWarning", MacroWarning);
+  }
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 };
 
 } // namespace modernize
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_USING_H

clang-tidy/modernize/UseUsingCheck.cpp

 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace modernize {
 
+UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      MacroWarning(Options.get("MacroWarning", false)) {}
+
 void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus11)
     return;
   Finder->addMatcher(typedefDecl().bind("typedef"), this);
 }
 
 // Checks if 'typedef' keyword can be removed - we do it only if
 // it is the only declaration in a declaration chain.
void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *MatchedDecl = Result.Nodes.getNodeAs<TypedefDecl>("typedef");
   if (MatchedDecl->getLocation().isInvalid())
     return;
 
   auto &Context = *Result.Context;
   auto &SM = *Result.SourceManager;
 
   if (auto *D = MatchedDecl->getUnderlyingType()->getAsCXXRecordDecl()) {
-    //TypeLoc TL = MatchedDecl->getTypeSourceInfo()->getTypeLoc();
+    // TypeLoc TL = MatchedDecl->getTypeSourceInfo()->getTypeLoc();
     llvm::errs() << D->getNameAsString() << "\n";
   }
 
+  SourceLocation StartLoc = MatchedDecl->getLocStart();
+
+  if (StartLoc.isMacroID() && !MacroWarning)
+    return;
+
   auto Diag =
       diag(MatchedDecl->getLocStart(), "use 'using' instead of 'typedef'");
 
-  SourceLocation StartLoc = MatchedDecl->getLocStart();
-  if (StartLoc.isMacroID())
+  //do not fix if there is macro or array 
+  if (MatchedDecl->getUnderlyingType()->isArrayType() || StartLoc.isMacroID())
     return;
 
   if (CheckRemoval(SM, StartLoc, Context)) {
+    auto printPolicy = PrintingPolicy(getLangOpts());
+    printPolicy.SuppressScope = true;
+    printPolicy.ConstantArraySizeAsWritten = true;
+    printPolicy.UseVoidForZeroParams = false;
+
     Diag << FixItHint::CreateReplacement(
         MatchedDecl->getSourceRange(),
         "using " + MatchedDecl->getNameAsString() + " = " +
-            MatchedDecl->getUnderlyingType().getAsString(getLangOpts()));
+            MatchedDecl->getUnderlyingType().getAsString(printPolicy));
   }
 }
 
 } // namespace modernize
 } // namespace tidy
 } // namespace clang

test/clang-tidy/modernize-use-using-macros.cpp

+// RUN: %check_clang_tidy %s modernize-use-using %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-using.MacroWarning, value: 1}]}" \
+// RUN:   -- -std=c++11 -I %S/Inputs/modernize-use-using
+
+#define CODE typedef int INT
+
+CODE;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: #define CODE typedef int INT
+// CHECK-FIXES: CODE;
+
+struct Foo;
+#define Bar Baz
+typedef Foo Bar;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: #define Bar Baz
+// CHECK-FIXES: using Baz = Foo;
+
+#define TYPEDEF typedef
+TYPEDEF Foo Bak;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: #define TYPEDEF typedef
+// CHECK-FIXES: TYPEDEF Foo Bak;

test/clang-tidy/modernize-use-using.cpp

 
 typedef int bla1, bla2, bla3;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
 // CHECK-FIXES: typedef int bla1, bla2, bla3;
 
 #define CODE typedef int INT
 
 CODE;
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
 // CHECK-FIXES: #define CODE typedef int INT
 // CHECK-FIXES: CODE;
 
 struct Foo;
 #define Bar Baz
 typedef Foo Bar;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
 // CHECK-FIXES: #define Bar Baz
 // CHECK-FIXES: using Baz = Foo;
 
 #define TYPEDEF typedef
 TYPEDEF Foo Bak;
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
 // CHECK-FIXES: #define TYPEDEF typedef
 // CHECK-FIXES: TYPEDEF Foo Bak;
 
 #define FOO Foo
 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-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
 // CHECK-FIXES: typedef struct { int b; } S2;
 struct Q2 { int c; } typedef S3;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
 // CHECK-FIXES: struct Q2 { int c; } typedef S3;
 struct { int d; } typedef S4;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
 // CHECK-FIXES: struct { int d; } typedef S4;
+
+namespace my_space {
+  class my_cclass {};
+  typedef my_cclass FuncType;
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using FuncType = my_cclass
+}
+
+#define lol 4
+typedef unsigned Map[lol]; 
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+
+typedef void (*fun_type)();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using fun_type = void (*)();