Change how we detect test code so we bail on virtual checks when we see gmock internals.

(There are maybe ten more things to cleanup in the test code itself after this
patch, but I want to get to the path handling problems.)

Also, per popular request, s/chrome-style/chromium-style/; in the error messages.

BUG=none

Review URL: http://codereview.chromium.org/6524005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@74891 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 21 insertions, 25 deletions

diff --git a/tools/clang/plugins/ChromeClassTester.cpp b/tools/clang/plugins/ChromeClassTester.cpp
index 106f16e..d98a44c 100644
--- a/tools/clang/plugins/ChromeClassTester.cpp
+++ b/tools/clang/plugins/ChromeClassTester.cpp
@@ -7,8 +7,6 @@
 
 #include "ChromeClassTester.h"
 
-#include "clang/Basic/FileManager.h"
-
 using namespace clang;
 
 namespace {
@@ -18,6 +16,9 @@ bool starts_with(const std::string& one, const std::string& two) {
 }
 
 bool ends_with(const std::string& one, const std::string& two) {
+  if (two.size() > one.size())
+    return false;
+
   return one.substr(one.size() - two.size(), two.size()) == two;
 }
 
@@ -59,6 +60,13 @@ ChromeClassTester::ChromeClassTester(CompilerInstance& instance)
   ignored_record_names_.push_back("AutoTaskRunner");
   ignored_record_names_.push_back("AutoCallbackRunner");
 
+  // Has a UNIT_TEST only constructor. Isn't *terribly* complex...
+  ignored_record_names_.push_back("AutocompleteController");
+  ignored_record_names_.push_back("HistoryURLProvider");
+
+  // Because of chrome frame
+  ignored_record_names_.push_back("ReliabilityTestSuite");
+
   // Part of our public interface that nacl and friends use. (Arguably, this
   // should mean that this is a higher priority but fixing this looks hard.)
   ignored_record_names_.push_back("PluginVersionInfo");
@@ -84,23 +92,18 @@ void ChromeClassTester::HandleTagDeclDefinition(TagDecl* tag) {
     if (InBannedDirectory(record_location))
       return;
 
-    // For now, due to large amounts of inlining in our unit test code, there's
-    // no way in hell we'll pass our checks but we want to deploy clang plugins
-    // now.
-    //
-    // TODO(erg): Deinline all our test code, and only perform something like
-    // this check for the "virtual" inlining tests on GMOCK code (since all of
-    // that is autogenerated and doesn't specify the "virtual" keyword).
-    if (IsTestCode(record)) {
-      return;
-    }
-
     // We sadly need to maintain a blacklist of types that violate these
     // rules, but do so for good reason or due to limitations of this
     // checker (i.e., we don't handle extern templates very well).
-    if (IsIgnoredType(record))
+    std::string base_name = record->getNameAsString();
+    if (IsIgnoredType(base_name))
       return;
 
+    // We ignore all classes that end with "Matcher" because they're probably
+    // GMock artifacts.
+    if (ends_with(base_name, "Matcher"))
+        return;
+
     CheckChromeClass(record_location, record);
   }
 }
@@ -108,20 +111,14 @@ void ChromeClassTester::HandleTagDeclDefinition(TagDecl* tag) {
 void ChromeClassTester::emitWarning(SourceLocation loc, const char* raw_error) {
   FullSourceLoc full(loc, instance().getSourceManager());
   std::string err;
-  err = "[chrome-style] ";
+  err = "[chromium-style] ";
   err += raw_error;
   unsigned id = diagnostic().getCustomDiagID(Diagnostic::Warning, err);
   DiagnosticBuilder B = diagnostic().Report(full, id);
 }
 
-bool ChromeClassTester::IsTestCode(Decl* record) {
-  if (instance_.hasSourceManager()) {
-    SourceManager& m = instance_.getSourceManager();
-    std::string name = m.getFileEntryForID(m.getMainFileID())->getName();
-    return name.find("test") != name.npos ||
-        name.find("mock") != name.npos;
-  }
-  return false;
+bool ChromeClassTester::InTestingNamespace(Decl* record) {
+  return GetNamespace(record).find("testing") != std::string::npos;
 }
 
 bool ChromeClassTester::InBannedNamespace(Decl* record) {
@@ -195,8 +192,7 @@ bool ChromeClassTester::InBannedDirectory(const SourceLocation& loc) {
   return false;
 }
 
-bool ChromeClassTester::IsIgnoredType(RecordDecl* record) {
-  std::string base_name = record->getNameAsString();
+bool ChromeClassTester::IsIgnoredType(const std::string& base_name) {
   for (std::vector<std::string>::const_iterator it =
            ignored_record_names_.begin();
        it != ignored_record_names_.end(); ++it) {