rewrite_to_chrome_style: Improve naming of iterator and type traits.

tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp

Index: tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
diff --git a/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp b/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
index 1f86290a750e6da36918ae9538be4d9bfb5dabab..b9b74bfb0d87df19af6d481c59f79c0ff336bab3 100644
--- a/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
+++ b/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
@@ -74,26 +74,40 @@ AST_MATCHER_P(clang::FunctionTemplateDecl,
   return InnerMatcher.matches(*Node.getTemplatedDecl(), Finder, Builder);
 }
 
+bool IsDeclInBlinkOrWtf(const clang::DeclContext* decl_context,
+                        bool blink,

[dcheng, 2016/03/01 23:58:02]

The double bools here are kind of hard to read, but enums would be pretty clunky
here as well.

Meh, I guess this is fine.

+                        bool wtf) {
+  assert(blink || wtf);  // Else, what's the point?
+  auto* namespace_decl = clang::dyn_cast_or_null<clang::NamespaceDecl>(
+      decl_context->getEnclosingNamespaceContext());
+  return namespace_decl && namespace_decl->getParent()->isTranslationUnit() &&
+         ((blink && namespace_decl->getName() == "blink") ||
+          (wtf && namespace_decl->getName() == "WTF"));
+}
+
 // A method is from Blink if it is from the Blink namespace or overrides a
 // method from the Blink namespace.
-bool IsBlinkMethod(const clang::CXXMethodDecl& decl) {
+bool IsBlinkOrWtfMethod(const clang::CXXMethodDecl& decl,

[dcheng, 2016/03/01 23:58:02]

Wtf or WTF consistently, I think.

[danakj, 2016/03/02 00:32:12]

Done.

+                        bool blink,
+                        bool wtf) {

[dcheng, 2016/03/01 23:58:02]

It looks like blink and wtf are both always true?

[danakj, 2016/03/02 00:32:12]

Yeh.. I was doing something I shouldn't have with this method. I can remove
them.

+  assert(blink || wtf);  // Else, what's the point?
   auto* namespace_decl = clang::dyn_cast_or_null<clang::NamespaceDecl>(

[dcheng, 2016/03/01 23:58:02]

Maybe this can just call out to IsDeclInBlinkOrWtf.

[danakj, 2016/03/02 00:32:12]

Done.

       decl.getParent()->getEnclosingNamespaceContext());
   if (namespace_decl && namespace_decl->getParent()->isTranslationUnit() &&
-      (namespace_decl->getName() == "blink" ||
-       namespace_decl->getName() == "WTF"))
+      ((blink && namespace_decl->getName() == "blink") ||
+       (wtf && namespace_decl->getName() == "WTF")))
     return true;
 
   for (auto it = decl.begin_overridden_methods();
        it != decl.end_overridden_methods(); ++it) {
-    if (IsBlinkMethod(**it))
+    if (IsBlinkOrWtfMethod(**it, blink, wtf))
       return true;
   }
   return false;
 }
 
-AST_MATCHER(clang::CXXMethodDecl, isBlinkMethod) {
-  return IsBlinkMethod(Node);
+AST_MATCHER(clang::CXXMethodDecl, isBlinkOrWTFMethod) {
+  return IsBlinkOrWtfMethod(Node, true, true);
 }
 
 // Helper to convert from a camelCaseName to camel_case_name. It uses some
@@ -220,9 +234,21 @@ bool GetNameForDecl(const clang::CXXMethodDecl& decl,
   StringRef original_name = decl.getName();
 
   if (!decl.isStatic()) {
+    std::string ret_type = decl.getReturnType().getAsString();
+    if (ret_type.find("iterator") != std::string::npos ||
+        ret_type.find("Iterator") != std::string::npos) {
+      // Iterator methods shouldn't be renamed to work with stl and range-for
+      // loops.
+      static const char* kIteratorBlacklist[] = {"begin", "end", "rbegin",
+                                                 "rend"};
+      for (const auto& b : kIteratorBlacklist) {
+        if (original_name == b)
+          return false;
+      }
+    }
+
     // Some methods shouldn't be renamed because reasons.
-    static const char* kBlacklist[] = {"begin", "end",  "rbegin", "rend",
-                                       "trace", "lock", "unlock", "try_lock"};
+    static const char* kBlacklist[] = {"trace", "lock", "unlock", "try_lock"};
     for (const auto& b : kBlacklist) {
       if (original_name == b)
         return false;
@@ -276,6 +302,16 @@ bool GetNameForDecl(const clang::VarDecl& decl,
     if (original_name.size() >= 2 && original_name[0] == 'k' &&
         clang::isUppercase(original_name[1]))
       return false;
+
+    // Struct consts in WTF do not become kFoo cuz stuff like type traits
+    // should stay as lowercase.
+    const clang::DeclContext* decl_context = decl.getDeclContext();
+    bool is_in_wtf = IsDeclInBlinkOrWtf(decl_context, false, true);
+    const clang::CXXRecordDecl* parent =
+        clang::dyn_cast_or_null<clang::CXXRecordDecl>(decl.getDeclContext());

[dcheng, 2016/03/01 23:58:02]

s/decl.getDeclContext()/decl_context/

[danakj, 2016/03/02 00:32:12]

Done.

+    if (is_in_wtf && parent && parent->isStruct())
+      return false;
+
     name = 'k';
     name.append(original_name.data(), original_name.size());
     name[1] = clang::toUppercase(name[1]);
@@ -553,7 +589,7 @@ int main(int argc, const char* argv[]) {
   // matches |g|.
   auto method_decl_matcher =
       id("decl",
-         cxxMethodDecl(isBlinkMethod(),
+         cxxMethodDecl(isBlinkOrWTFMethod(),
                        unless(anyOf(is_generated,
                                     // Overloaded operators have special names
                                     // and should never be renamed.