Add a hardcoded list of methods that need a "Get" prefix.

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 97d9ebd309706e2e64d64d88f1452ea64400d3ed..c19d2e1bdd45c44a56f88b393aaecbe64eb41264 100644
--- a/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
+++ b/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
@@ -340,41 +340,115 @@ AST_MATCHER_P(clang::QualType, hasString, std::string, ExpectedString) {
   return ExpectedString == Node.getAsString();
 }
 
+bool ShouldPrefixMethodName(std::string old_method_name) {

[dcheng, 2016/12/20 00:42:02]

StringRef or const std::string&

[Łukasz Anforowicz, 2016/12/20 00:58:58]

Done - thanks for pointing this out (I initially had |const std::string&| here
THEN changed to |std::string| to clobber the argument value to always change the
first character to lower case THEN reverted the cloberring in response to code
review, but forgot to change the parameter type).

+  // Methods that are named similarily to a type - they should be prefixed
+  // with a "get" prefix.
+  static const char* kConflictingMethods[] = {
+      "animationWorklet",
+      "audioWorklet",
+      "binaryType",
+      "blob",
+      "channelCountMode",
+      "color",
+      "counterDirectives",
+      "document",
+      "emptyChromeClient",
+      "emptyEditorClient",
+      "emptySpellCheckerClient",
+      "entryType",
+      "error",
+      "fileUtilities",
+      "font",
+      "iconURL",
+      "inputMethodController",
+      "inputType",
+      "layout",
+      "layoutBlock",
+      "layoutSize",
+      "length",
+      "lineCap",
+      "lineJoin",
+      "matchedProperties",
+      "name",
+      "navigationType",
+      "node",
+      "outcome",
+      "pagePopup",
+      "paintWorklet",
+      "path",
+      "processingInstruction",
+      "readyState",
+      "screenInfo",
+      "scrollAnimator",
+      "settings",
+      "signalingState",
+      "state",
+      "string",
+      "text",
+      "textAlign",
+      "textBaseline",
+      "theme",
+      "timing",
+      "topLevelBlameContext",

[dcheng, 2016/12/20 00:42:02]

This one looks surprising, as I don't see a type with this name in the Blink
namespace.

[Łukasz Anforowicz, 2016/12/20 00:58:58]

Ack.  Please see the error message in my other reply.

+      "widget",
+  };
+  for (const auto& conflicting_method : kConflictingMethods) {
+    if (old_method_name == conflicting_method)
+      return true;
+  }
+
+  return false;
+}
+
 bool GetNameForDecl(const clang::FunctionDecl& decl,
                     clang::ASTContext& context,
                     std::string& name) {
   name = decl.getName().str();
+
+  // Need to call ShouldPrefixMethodName on the *old* name.
+  bool should_prefix_method_name = ShouldPrefixMethodName(name);
+
+  // Calculate the new name.
   name[0] = clang::toUppercase(name[0]);
 
-  // Given
-  //   class Foo {};
-  //   class DerivedFoo : class Foo;
-  //   using Bar = Foo;
-  //   Bar f1();  // <- |Bar| would be matched by hasString("Bar") below.
-  //   Bar f2();  // <- |Bar| would be matched by hasName("Foo") below.
-  //   DerivedFoo f3();  // <- |DerivedFoo| matched by isDerivedFrom(...) below.
-  // |type_with_same_name_as_function| matcher matches Bar and Foo return types.
-  auto type_with_same_name_as_function = qualType(anyOf(
-      // hasString matches the type as spelled (Bar above).
-      hasString(name),
-      // hasDeclaration matches resolved type (Foo or DerivedFoo above).
-      hasDeclaration(namedDecl(hasName(name))),
-      hasDeclaration(cxxRecordDecl(isDerivedFrom(namedDecl(hasName(name)))))));
-
-  // |type_containing_same_name_as_function| matcher will match all of the
-  // return types below:
-  // - Foo foo()  // Direct application of |type_with_same_name_as_function|.
-  // - Foo* foo()  // |hasDescendant| traverses references/pointers.
-  // - RefPtr<Foo> foo()  // |hasDescendant| traverses template arguments.
-  auto type_containing_same_name_as_function =
-      qualType(anyOf(type_with_same_name_as_function,
-                     hasDescendant(type_with_same_name_as_function)));
-  // https://crbug.com/582312: Prepend "Get" if method name conflicts with
-  // return type.
-  auto conflict_matcher =
-      functionDecl(returns(type_containing_same_name_as_function));
-  if (IsMatching(conflict_matcher, decl, context))
+  // See if the new name conflicts with a type ~inside~ the return type.
+  if (!should_prefix_method_name) {
+    // Given
+    //   class Foo {};
+    //   class DerivedFoo : class Foo;
+    //   using Bar = Foo;
+    // |type_with_same_name_as_function| would match:
+    //   Bar bar();  // <- |Bar| would be matched by hasString("Bar") below.
+    //   Bar foo();  // <- |Bar| would be matched by hasName("Foo") below.
+    //   DerivedFoo foo();  // <- |DerivedFoo| matched by isDerivedFrom(...)
+    //                      //    below.
+    auto type_with_same_name_as_function = qualType(anyOf(
+        // hasString matches the type as spelled (Bar above).
+        hasString(name),
+        // hasDeclaration matches resolved type (Foo or DerivedFoo above).
+        hasDeclaration(namedDecl(hasName(name))),
+        hasDeclaration(
+            cxxRecordDecl(isDerivedFrom(namedDecl(hasName(name)))))));
+
+    // |type_containing_same_name_as_function| matcher will match all of the
+    // return types below:
+    // - Foo foo()  // Direct application of |type_with_same_name_as_function|.
+    // - Foo* foo()  // |hasDescendant| traverses references/pointers.
+    // - RefPtr<Foo> foo()  // |hasDescendant| traverses template arguments.
+    auto type_containing_same_name_as_function =
+        qualType(anyOf(type_with_same_name_as_function,
+                       hasDescendant(type_with_same_name_as_function)));
+    // https://crbug.com/582312: Prepend "Get" if method name conflicts with
+    // return type.
+    auto conflict_matcher =
+        functionDecl(returns(type_containing_same_name_as_function));
+    if (IsMatching(conflict_matcher, decl, context))
+      should_prefix_method_name = true;
+  }
+
+  if (should_prefix_method_name) {
     name = "Get" + name;
+  }
 
   return true;
 }
@@ -829,6 +903,8 @@ class UnresolvedRewriterBase : public RewriterBase<TargetNode> {
         !IsBlacklistedFunctionOrMethodName(old_name)) {
       new_name = old_name;
       new_name[0] = clang::toUppercase(old_name[0]);
+      if (ShouldPrefixMethodName(old_name))
+        new_name = "Get" + new_name;
       return true;
     }