Add --idl-methods <filepath> parameter for skipping renaming of IDL methods.

tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // Changes Blink-style names to Chrome-style names. Currently transforms:
 //   fields:
 //     int m_operationCount => int operation_count_
 //   variables (including parameters):
 //     int mySuperVariable => int my_super_variable
 //   constants:
@@ skipping 16 matching lines @@
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/MacroArgs.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/LineIterator.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/TargetSelect.h"
 
 #include "EditTracker.h"
 
 using namespace clang::ast_matchers;
 using clang::tooling::CommonOptionsParser;
 using clang::tooling::Replacement;
 using llvm::StringRef;
 
 namespace {
 
 const char kBlinkFieldPrefix[] = "m_";
 const char kBlinkStaticMemberPrefix[] = "s_";
 const char kGeneratedFileRegex[] = "^gen/|/gen/";
 const char kGMockMethodNamePrefix[] = "gmock_";
+const char kMethodBlocklistParamName[] = "method-blocklist";
 
 template <typename MatcherType, typename NodeType>
 bool IsMatching(const MatcherType& matcher,
                 const NodeType& node,
                 clang::ASTContext& context) {
   return !match(matcher, node, context).empty();
 }
 
 const clang::ast_matchers::internal::
     VariadicDynCastAllOfMatcher<clang::Expr, clang::UnresolvedMemberExpr>
@@ skipping 50 matching lines @@
     if (potentially_mocked_method->getNumParams() != Node.getNumParams())
       continue;
 
     if (InnerMatcher.matches(*potentially_mocked_method, Finder, Builder))
       return true;
   }
 
   return false;
 }
 
+class MethodBlocklist {
+ public:
+  explicit MethodBlocklist(const std::string& filepath) {
+    if (!filepath.empty())
+      ParseInputFile(filepath);
+  }
+
+  bool Contains(const clang::CXXMethodDecl& method) const {
+    auto it = method_to_class_to_args_.find(method.getName());
+    if (it == method_to_class_to_args_.end())
+      return false;
+
+    const clang::CXXRecordDecl* actual_class = method.getParent();
+    // Hopefully |getParent()| can find the class even for non-inlined method
+    // definitions (where CXXRecordDecl is not a parent AST node of
+    // CXXMethodDecl).
+    assert(actual_class);

[dcheng, 2017/01/17 22:56:10]

Nit: a lot of times, assert messages in LLVM look like:

assert(actual_class && "some descriptive message here");

It might be worth considering doing that here.

[Łukasz Anforowicz, 2017/01/17 23:52:47]

Done.

+
+    const llvm::StringMap<std::set<unsigned>>& class_to_args = it->second;
+    auto it2 = class_to_args.find(actual_class->getName());
+    if (it2 == class_to_args.end())
+      return false;
+
+    const std::set<unsigned>& set_of_expected_number_of_args = it2->second;

[dcheng, 2017/01/17 22:56:10]

Nit: from the context, it's not clear if expected means blocked or unblocked. I
would just name this something sort like |arg_counts| (which matches how things
like class_to_args and method_to_class_to_args are named.

I'm having a trouble keeping expected / actual straight as I read this function.

[Łukasz Anforowicz, 2017/01/17 23:52:47]

Done.

+    unsigned max_actual_number_of_args = method.param_size();
+    unsigned min_actual_number_of_args = max_actual_number_of_args;
+    for (const clang::ParmVarDecl* param : method.parameters()) {
+      if (param->hasInit())
+        min_actual_number_of_args--;
+    }
+    bool got_expected_number_of_args = std::any_of(

[dcheng, 2017/01/17 22:56:11]

Nit: Rename got_expected_to_number_of_args => found_matching_arg_count?

[Łukasz Anforowicz, 2017/01/17 23:52:46]

Done.

+        set_of_expected_number_of_args.begin(),
+        set_of_expected_number_of_args.end(),
+        [min_actual_number_of_args,
+         max_actual_number_of_args](unsigned expected_number_of_args) {
+          return (min_actual_number_of_args <= expected_number_of_args) &&
+                 (expected_number_of_args <= max_actual_number_of_args);
+        });
+    if (!got_expected_number_of_args)
+      return false;
+
+    // No need to verify here that |actual_class| is in the |blink| namespace -
+    // this will be done by other matchers elsewhere.
+
+    // TODO(lukasza): Do we need to consider return type and/or param types?
+
+    return true;

[dcheng, 2017/01/17 22:56:10]

Then just return found_matching_arg_count directly here (we can keep the TODO)

[Łukasz Anforowicz, 2017/01/17 23:52:47]

Done.

+  }
+
+ private:
+  // Each line is expected to have the following format:
+  // <class name>:::<method name>:::<number of arguments>
+  void ParseInputFile(const std::string& filepath) {
+    llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> file_or_err =
+        llvm::MemoryBuffer::getFile(filepath);
+    if (std::error_code err = file_or_err.getError()) {
+      llvm::errs() << "ERROR: Cannot open the file specified in --"
+                   << kMethodBlocklistParamName << " argument: " << filepath
+                   << ": " << err.message() << "\n";
+      assert(false);
+      return;
+    }
+
+    llvm::line_iterator it(**file_or_err, true /* SkipBlanks */, '#');
+    for (; !it.is_at_eof(); ++it) {
+      llvm::StringRef line = it->trim();
+      if (line.empty())
+        continue;
+
+      // Split the line into ':::'-delimited parts.
+      const size_t kExpectedNumberOfParts = 3;
+      llvm::SmallVector<llvm::StringRef, kExpectedNumberOfParts> parts;
+      line.split(parts, ":::");
+      if (parts.size() != kExpectedNumberOfParts) {
+        llvm::errs() << "ERROR: Parsing error - expected "
+                     << kExpectedNumberOfParts
+                     << " ':::'-delimited parts: " << filepath << ":"
+                     << it.line_number() << ": " << line << "\n";
+        assert(false);
+        continue;
+      }
+
+      // Parse individual parts.
+      llvm::StringRef class_name = parts[0];
+      llvm::StringRef method_name = parts[1];
+      unsigned number_of_method_args;
+      if (parts[2].getAsInteger(0, number_of_method_args)) {
+        llvm::errs() << "ERROR: Parsing error - '" << parts[2] << "' "
+                     << "is not an unsigned integer: " << filepath << ":"
+                     << it.line_number() << ": " << line << "\n";
+        assert(false);
+        continue;
+      }
+
+      // Store the new entry.
+      method_to_class_to_args_[method_name][class_name].insert(
+          number_of_method_args);
+    }
+  }
+
+  // Stores methods to blacklist in a map:
+  // method name -> class name -> set of all allowed numbers of arguments.
+  llvm::StringMap<llvm::StringMap<std::set<unsigned>>> method_to_class_to_args_;
+};
+
+AST_MATCHER_P(clang::CXXMethodDecl,
+              internalIsBlocklistedMethod,
+              MethodBlocklist,
+              Blocklist) {
+  return Blocklist.Contains(Node);
+}
+
+clang::ast_matchers::internal::Matcher<clang::CXXMethodDecl>
+isBlocklistedMethod(const std::string& method_blocklist_filepath) {
+  return internalIsBlocklistedMethod(
+      MethodBlocklist(method_blocklist_filepath));

[dcheng, 2017/01/17 22:59:50]

Oh, one other thing: do we need to split this between an internal / non-internal
matcher? Can line 1555 just construct the MethodBlocklist object and pass it in
directly?

[Łukasz Anforowicz, 2017/01/17 23:52:47]

Done.  I did it this way thinking that one day we'll move
isBlocklistedMethod(filename) to a separate header (without exposing
MethodBlocklist class through the header), but you're right that it doesn't make
sense today.

+}
+
 // If |InnerMatcher| matches |top|, then the returned matcher will match:
 // - |top::function|
 // - |top::Class::method|
 // - |top::internal::Class::method|
 AST_MATCHER_P(
     clang::NestedNameSpecifier,
     hasTopLevelPrefix,
     clang::ast_matchers::internal::Matcher<clang::NestedNameSpecifier>,
     InnerMatcher) {
   const clang::NestedNameSpecifier* NodeToMatch = &Node;
@@ skipping 1136 matching lines @@
 
 static llvm::cl::extrahelp common_help(CommonOptionsParser::HelpMessage);
 
 int main(int argc, const char* argv[]) {
   // TODO(dcheng): Clang tooling should do this itself.
   // http://llvm.org/bugs/show_bug.cgi?id=21627
   llvm::InitializeNativeTarget();
   llvm::InitializeNativeTargetAsmParser();
   llvm::cl::OptionCategory category(
       "rewrite_to_chrome_style: convert Blink style to Chrome style.");
+  llvm::cl::opt<std::string> blocklisted_methods_file(
+      kMethodBlocklistParamName, llvm::cl::value_desc("filepath"),
+      llvm::cl::desc("file listing methods to be blocked (not renamed)"));
   CommonOptionsParser options(argc, argv, category);
   clang::tooling::ClangTool tool(options.getCompilations(),
                                  options.getSourcePathList());
 
   MatchFinder match_finder;
   std::set<Replacement> replacements;
 
   // Blink namespace matchers ========
   auto blink_namespace_decl =
       namespaceDecl(anyOf(hasName("blink"), hasName("WTF")),
@@ skipping 131 matching lines @@
   //   struct S {
   //     void g();
   //   };
   // matches |g|.
   // For a method to be considered for rewrite, it must not override something
   // that we're not rewriting. Any methods that we would not normally consider
   // but that override something we are rewriting should also be rewritten. So
   // we use includeAllOverriddenMethods() to check these rules not just for the
   // method being matched but for the methods it overrides also.
   auto is_blink_method = includeAllOverriddenMethods(
-      allOf(in_blink_namespace, unless(isBlacklistedMethod())));
+      allOf(in_blink_namespace, unless(isBlacklistedMethod()),
+            unless(isBlocklistedMethod(blocklisted_methods_file))));
   auto method_decl_matcher = id(
       "decl",
       cxxMethodDecl(
           unless(anyOf(
               // Overloaded operators have special names and should never be
               // renamed.
               isOverloadedOperator(),
               // Similarly, constructors, destructors, and conversion
               // functions should not be considered for renaming.
               cxxConstructorDecl(), cxxDestructorDecl(), cxxConversionDecl())),
@@ skipping 227 matching lines @@
   for (const auto& r : replacements) {
     std::string replacement_text = r.getReplacementText().str();
     std::replace(replacement_text.begin(), replacement_text.end(), '\n', '\0');
     llvm::outs() << "r:::" << r.getFilePath() << ":::" << r.getOffset()
                  << ":::" << r.getLength() << ":::" << replacement_text << "\n";
   }
   llvm::outs() << "==== END EDITS ====\n";
 
   return 0;
 }