Adds PRESUBMIT.py check to look for unprefixed string16.

R=maruel@chromium.org
BUG=329295
TEST=PRESUBMIT_test.py
NOTRY=true

Review URL: https://codereview.chromium.org/98143005

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

2 files changed, 70 insertions, 0 deletions

diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index dc30cbc..4e41b4f 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -1027,6 +1027,7 @@ def _CommonChecks(input_api, output_api):
   results.extend(_CheckForAnonymousVariables(input_api, output_api))
   results.extend(_CheckCygwinShell(input_api, output_api))
   results.extend(_CheckJavaStyle(input_api, output_api))
+  results.extend(_CheckForString16(input_api, output_api))
 
   if any('PRESUBMIT.py' == f.LocalPath() for f in input_api.AffectedFiles()):
     results.extend(input_api.canned_checks.RunUnitTestsInDirectory(
@@ -1169,6 +1170,35 @@ def _CheckForInvalidOSMacros(input_api, output_api):
       'or add your macro to src/PRESUBMIT.py.', bad_macros)]
 
 
+def _CheckForString16InFile(input_api, f):
+  """Check for string16 without base:: in front."""
+  reg = input_api.re.compile(r'\b(?<!base::)string16\b')
+  use = 'using base::string16;'
+  include = '#include "base/strings/string16.h"'
+  results = []
+  for lnum, line in f.ChangedContents():
+    if reg.search(line) and not include in line and not use in f.NewContents():
+      results.append('    %s:%d' % (f.LocalPath(), lnum))
+  return results
+
+
+def _CheckForString16(input_api, output_api):
+  file_filter = lambda f: input_api.FilterSourceFile(f,
+      white_list=(r'^chrome[\\\/]browser[\\\/]', ),
+      black_list=(_EXCLUDED_PATHS + _TEST_CODE_EXCLUDED_PATHS +
+                  input_api.DEFAULT_BLACK_LIST))
+
+  unprefixed = []
+  for f in input_api.AffectedFiles(file_filter=file_filter):
+    unprefixed.extend(_CheckForString16InFile(input_api, f))
+
+  if not unprefixed:
+    return []
+
+  return [output_api.PresubmitPromptWarning(
+      'string16 should be prefixed with base:: namespace.', unprefixed)]
+
+
 def CheckChangeOnUpload(input_api, output_api):
   results = []
   results.extend(_CommonChecks(input_api, output_api))
diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py
index 1c7990c..9c455e9 100755
--- a/PRESUBMIT_test.py
+++ b/PRESUBMIT_test.py
@@ -393,6 +393,46 @@ class InvalidOSMacroNamesTest(unittest.TestCase):
     self.assertEqual(0, len(errors))
 
 
+class String16Test(unittest.TestCase):
+  def testUnprefixedGivesWarnings(self):
+    lines = ['string16 GetName() const;',
+             'void SetName(const string16& name) OVERRIDE {}',
+             'const string16& GetNameRef() const = 0;',
+             'string16 name;',
+             'string16 foo = ASCIIToUTF16("bar");',
+             'string16 blah(ASCIIToUTF16("blee"));',
+             'std::vector<string16> names;',
+             'string16 var_with_string16_in_name;']
+    warnings = PRESUBMIT._CheckForString16InFile(
+        MockInputApi(), MockFile('chrome/browser/name_getter_bad.cc', lines))
+    self.assertEqual(len(lines), len(warnings))
+
+  def testPrefixedSkipped(self):
+    lines = ['#include "base/strings/string16.h"',
+             'void SetName(const base::string16& name) OVERRIDE {}',
+             'const base::string16& GetNameRef() const = 0;',
+             'base::string16 name;',
+             'base::string16 foo = ASCIIToUTF16("bar");',
+             'base::string16 blah(ASCIIToUTF16("blee"));',
+             'std::vector<base::string16> names;',
+             'base::string16 var_with_string16_in_name;']
+    warnings = PRESUBMIT._CheckForString16InFile(
+        MockInputApi(), MockFile('chrome/browser/name_getter_good.cc', lines))
+    self.assertEqual(0, len(warnings))
+
+  def testUsingYieldsNoWarnings(self):
+    lines = ['#include "base/strings/string16.h',
+             'namespace {',
+             'using base::string16;',
+             'string16 SayHiOnlyToEd(const string16& name) {',
+             '  string16 first = name.substr(0, 2);  // Only "Ed" gets a "Hi".',
+             '  return first == "Ed" ? first : string16();',
+             '}']
+    warnings = PRESUBMIT._CheckForString16InFile(
+        MockInputApi(), MockFile('chrome/browser/say_hi_to_ed.cc', lines))
+    self.assertEqual(0, len(warnings))
+
+
 class CheckAddedDepsHaveTetsApprovalsTest(unittest.TestCase):
   def testDepsFilesToCheck(self):
     changed_lines = [