[Clang-tidy] Add check "modernize use using"

[staronj]

clang-tidy/modernize/UseUsingCheck.h

20

Fix the FIXME.

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

85

Add end of line at the end of file.

[Prazek]

docs/clang-tidy/checks/modernize-use-using.rst

14

add cases with pointers to function / members

[Eugene.Zelenko]

Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).

[Prazek]

clang-tidy/modernize/UseUsingCheck.cpp

27

add static

28

Stick to the LLVM coding style - local variables starts with capital letter.
Also add const here

33–36

you can extract this to another static function
static void replaceAll(std::string &s, const std::string &search, const std::string replace);

49

check if it is clang-formatted

[hokein]

clang-tidy/modernize/ModernizeTidyModule.cpp

49

You can put it in one-line, which will not exceed 80 characters.

clang-tidy/modernize/UseUsingCheck.cpp

23

Add a blank line.

24

A blank between /// and F, please use clang-format to make sure your code align with LLVM style.

25

replace parameter can be const std::string&.

26

code indentation.

38

const auto &p

41

You can use replaceAll(subject, p.first, p.second).

46

You need to filter out the non-c++11 code here.

clang-tidy/modernize/UseUsingCheck.h

19

Please use a complete sentence.

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

2

Remove the extra blank line.

9

No need to check the whole message except the first one. So here you can use warning: use using instead of typedef.

[krystyna]

hide done comments

[Prazek]

lgtm, but I'd rather see Hokein acceptance.

[Eugene.Zelenko]

Check is still not mentioned in docs/ReleaseNotes.rst.

[hokein]

LGTM with some nits.

clang-tidy/modernize/UseUsingCheck.cpp

23

Should be CplusPlus11 here.

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

29

code indentation.

30

The same.

45

The same.

[Prazek]

clang-tidy/modernize/UseUsingCheck.cpp

23

BTW is there any policy about that? I see that some checks from modernize require C++11 (e.g. modernize-make-unique which is in C++14) and other require just C++ (modernize-loop-convert), and it even has a comment

// Only register the matchers for C++. Because this checker is used for

// modernization, it is reasonable to run it on any C++ standard with the
// assumption the user is trying to modernize their codebase.
if (!getLangOpts().CPlusPlus)
  return;

I have 2 thoughts for this:

1. there should be note in documentation about it, so the user won't spend time debuging why the check doesn't do anything. e.g. "This check requires to compile code with C++11 or higher"
2. I would suggest modernize checks to require standard version the same or higher for C++ standars that doesn't break backwards compatibility:

e.g. loop-convert should require C++11, make-shared C++14, this check also C++11,
but for modernize-increment-bool, that is deprecated in C++17, it should require just C++ (because if someone need it, the he wont be able to compile his code with C++17).

[hokein]

clang-tidy/modernize/UseUsingCheck.cpp

23

This is a good point.

As far as I know, we don't have detailed policy about the modernized checks. It depends on the check author. Basically the modern words means "C++11" feature.

I'm +1 on adding a note in each modernized check's document.

[Prazek]

clang-tidy/modernize/UseUsingCheck.cpp

23

Cool. I sent email to mailing list to ask other devs for opinion.

Krystyna, please add note at the end of documentation

"This check requires using C++11 or higher to run."

[curdeius]

I'm really interested in the manner this check works when a typedef has multiple declarations in it (same example as in the comment):

typedef int m_int, *m_int_p, &m_int_r, m_int_arr[10], (&m_int_fun)(int, int);

I tried to implement such a check once, but this was the hard part. FYI, that's my stub: https://github.com/llvm-mirror/clang-tools-extra/compare/master...mkurdej:feature/use-using.

docs/clang-tidy/checks/modernize-use-using.rst

22

Typo: varible -> variable.

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

2

Could you add tests for the case where there are multiple type declarations in one statement? E.g.:

typedef int m_int, *m_int_p, &m_int_r, m_int_arr[10], (&m_int_fun)(int, int);

Would it be transformed to multiple using statements or only a warning will be emitted?

[Prazek]

In D18919#419902, @curdeius wrote:

I'm really interested in the manner this check works when a typedef has multiple declarations in it (same example as in the comment):

typedef int m_int, *m_int_p, &m_int_r, m_int_arr[10], (&m_int_fun)(int, int);

I tried to implement such a check once, but this was the hard part. FYI, that's my stub: https://github.com/llvm-mirror/clang-tools-extra/compare/master...mkurdej:feature/use-using.

I don't think that this featcher is necessary. I don't know a lot of code using it. I am curious how this check will behave.

[Prazek]

clang-tidy/modernize/UseUsingCheck.cpp

14

Is this required?

[Eugene.Zelenko]

clang-tidy/modernize/UseUsingCheck.cpp

14

It'll be reasonable to run Include What You Use at least on new files.

[etienneb]

clang-tidy/modernize/UseUsingCheck.cpp

33

I think 'using' should be quote.
This message is hard to read and understand from a user point of view.

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

49

Could you add test with macro:

#define CODE \

typedef int INT; \
[....]

CODE;

[Prazek]

LGTM

clang-tidy/modernize/UseUsingCheck.cpp

72

There should be method called 'isInvalid()'

[Eugene.Zelenko]

Please see PR28334.

I'm not sure if Krystyna has Bugzilla account, so I report problem here.

clang-tidy/modernize/CMakeLists.txt

   UseDefaultCheck.cpp
   UseNullptrCheck.cpp
   UseOverrideCheck.cpp
+  UseUsingCheck.cpp
 
   LINK_LIBS
   clangAST

clang-tidy/modernize/ModernizeTidyModule.cpp

 #include "UseDefaultCheck.h"
 #include "UseNullptrCheck.h"
 #include "UseOverrideCheck.h"
+#include "UseUsingCheck.h"
 
 using namespace clang::ast_matchers;
 

     CheckFactories.registerCheck<UseDefaultCheck>("modernize-use-default");
     CheckFactories.registerCheck<UseNullptrCheck>("modernize-use-nullptr");
     CheckFactories.registerCheck<UseOverrideCheck>("modernize-use-override");
+    CheckFactories.registerCheck<UseUsingCheck>(
+        "modernize-use-using");
   }
 
   ClangTidyOptions getModuleOptions() override {

clang-tidy/modernize/UseUsingCheck.h

+//===--- UseUsingCheck.h - clang-tidy----------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_USING_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_USING_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Finds typedefs and replaces it with using
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-using.html
+class UseUsingCheck : public ClangTidyCheck {
+public:
+  UseUsingCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  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

+//===--- UseUsingCheck.cpp - clang-tidy------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseUsingCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <utility>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(typedefDecl().bind("typedef"), this);
+}
+///Function which replace all substrings "search" with "replace" in string "s"
+static void replaceAll(std::string &s, const std::string &search, const std::string replace){
+  size_t pos = 0;
+    while ((pos = s.find(search, pos)) != std::string::npos) {
+      s.replace(pos, search.length(), replace);
+      pos += replace.length();
+    }
+}
+
+/// Function which removes words "class", "struct", "union" and "void" from 
+/// AST representation of type.
+static std::string removeExtraASTWords(std::string subject) {
+  const std::pair<std::string, std::string> Subs[] = {
+      {" class ", ""}, {"(class ", "("}, {"struct ", ""}, {"union ", ""}, {"(void)", "()"}};
+  for (auto p : Subs) {
+    std::string search = p.first;
+    std::string replace = p.second;
+    replaceAll(subject, search, replace);
+  }
+  return subject;
+}
+
+void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<TypedefDecl>("typedef");
+  if (!MatchedDecl->getLocation().isValid())
+    return;
+
+  diag(MatchedDecl->getLocStart(), "use using instead of typedef")
+      << FixItHint::CreateReplacement(
+             MatchedDecl->getSourceRange(),
+             "using " + MatchedDecl->getNameAsString() + " = " +
+                 removeExtraASTWords(
+                     MatchedDecl->getUnderlyingType().getAsString()));
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang

docs/clang-tidy/checks/list.rst

    modernize-use-default
    modernize-use-nullptr
    modernize-use-override
+   modernize-use-using
    performance-faster-string-find
    performance-for-range-copy
    performance-implicit-cast-in-loop

docs/clang-tidy/checks/modernize-use-using.rst

+.. title:: clang-tidy - modernize-use-using
+
+modernize-use-using
+===================
+
+Use C++11's ``using`` instead of ``typedef``.
+
+Before:
+
+.. code:: c++
+
+  typedef int variable;
+
+  class Class{};
+  typedef void (Class::* MyPtrType)() const;
+
+After:
+
+.. code:: c++
+
+  using varible = int;
+
+  class Class{};
+  using MyPtrType = void (Class::*)() const;

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

+// RUN: %check_clang_tidy %s modernize-use-using %t
+
+
+typedef int Type;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use using instead of typedef [modernize-use-using]
+// CHECK-FIXES: using Type = int;
+
+typedef long LL;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use using instead of typedef [modernize-use-using]
+// CHECK-FIXES: using LL = long;
+
+typedef int Bla;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use using instead of typedef [modernize-use-using]
+// CHECK-FIXES: using Bla = int;
+
+typedef Bla Bla2;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use using instead of typedef [modernize-use-using]
+// CHECK-FIXES: using Bla2 = Bla;
+
+typedef void (*type)(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use using instead of typedef [modernize-use-using]
+// CHECK-FIXES: using type = void (*)(int);
+
+typedef void (*type2)();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use using instead of typedef [modernize-use-using]
+// CHECK-FIXES: using type2 = void (*)();
+
+class Class {
+    typedef long long Type;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use using instead of typedef [modernize-use-using]
+  // CHECK-FIXES: using Type = long long;
+};
+
+typedef void (Class::* MyPtrType)(Bla) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use using instead of typedef [modernize-use-using]
+// CHECK-FIXES: using MyPtrType = void (Class::*)(Bla) const;
+
+class Iterable {
+public:
+  class Iterator {};
+};
+
+template<typename T>
+class Test {
+    typedef typename T::iterator Iter;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use using instead of typedef [modernize-use-using]
+  // CHECK-FIXES: using Iter = typename T::iterator;
+};
+
+using balba = long long;
+
+union A {};
+
+typedef void (A::*PtrType)(int, int) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use using instead of typedef [modernize-use-using]
+// CHECK-FIXES: using PtrType = void (A::*)(int, int) const;
+
+typedef Class some_class;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use using instead of typedef [modernize-use-using]
+// CHECK-FIXES: using some_class = Class;
+
+typedef Class Cclass;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use using instead of typedef [modernize-use-using]
+// CHECK-FIXES: using Cclass = Class;
+typedef Cclass cclass2;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use using instead of typedef [modernize-use-using]
+// CHECK-FIXES: using cclass2 = Cclass;
+
+class cclass {
+
+};
+
+typedef void (cclass::* MyPtrType3)(Bla);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use using instead of typedef [modernize-use-using]
+// CHECK-FIXES: using MyPtrType3 = void (cclass::*)(Bla);
+
+using my_class = int;
+
+//FIXME: below test fails
+typedef Test<my_class *> another;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use using instead of typedef [modernize-use-using]
+// CHECK-FIXES: using another = Test<my_class *>;
+
+