Remove 'using base::string16;' from base/strings/string16.h and remove temporary

Remove 'using base::string16;' from base/strings/string16.h and remove temporary
PRESUBMIT.py _CheckForString16().

R=brettw@chromium.org, maruel@chromium.org
TBR=sky@chromium.org, tsepez@chromium.org
BUG=329295
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=242733

[Dan Beam, 2013-12-20 22:38:47]

depends on:
https://chromiumcodereview.appspot.com/119713003/
https://chromiumcodereview.appspot.com/107803004/
https://chromiumcodereview.appspot.com/113653003/

[M-A Ruel, 2013-12-20 22:55:13]

PRESUBMIT Rubberstamp lgtm
Le 2013-12-20 17:38, <dbeam@chromium.org> a écrit :

> Reviewers: brettw, M-A Ruel,
>
> Message:
> depends on:
> https://chromiumcodereview.appspot.com/119713003/
> https://chromiumcodereview.appspot.com/107803004/
> https://chromiumcodereview.appspot.com/113653003/
>
> Description:
> Remove 'using base::string16;' from base/strings/string16.h and remove
> temporary
> PRESUBMIT.py _CheckForString16().
>
> R=brettw@chromium.org,maruel@chromium.org
> BUG=329295
>
> Please review this at https://chromiumcodereview.appspot.com/93903011/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files (+2, -91 lines):
>   M PRESUBMIT.py
>   M PRESUBMIT_test.py
>   M base/strings/string16.h
>
>
> Index: PRESUBMIT.py
> diff --git a/PRESUBMIT.py b/PRESUBMIT.py
> index 9bb1471cbed6ae9c37ed36022b878fe58fc95d21..
> 04cea10dffee5aed22c9e0372b8bae7fb6e03974 100644
> --- a/PRESUBMIT.py
> +++ b/PRESUBMIT.py
> @@ -1037,7 +1037,6 @@ 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(
> @@ -1180,53 +1179,6 @@ 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'^android_webview[\\\/]',
> -          r'^apps[\\\/]',
> -          r'^ash[\\\/]',
> -          r'^chrome[\\\/]',
> -          r'^chromeos[\\\/]',
> -          r'^components[\\\/]',
> -          r'^content[\\\/]',
> -          r'^device[\\\/]',
> -          r'^ipc[\\\/]',
> -          r'^net[\\\/]',
> -          r'^ppapi[\\\/]',
> -          r'^printing[\\\/]',
> -          r'^rlz[\\\/]',
> -          r'^skia[\\\/]',
> -          r'^tools[\\\/]',
> -          r'^ui[\\\/]',
> -          r'^webkit[\\\/]',
> -      ),
> -      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))
> Index: PRESUBMIT_test.py
> diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py
> index 9c455e90924cb3038e8e0873f50e6bb6513cfa41..
> 1c7990ce898bb824b1b7068d7787826b9250a0d1 100755
> --- a/PRESUBMIT_test.py
> +++ b/PRESUBMIT_test.py
> @@ -393,46 +393,6 @@ 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 = [
> Index: base/strings/string16.h
> diff --git a/base/strings/string16.h b/base/strings/string16.h
> index fd98f1b5be79b20a9af02f31fb3d9ed2ec772b3b..
> da424c16e7652f0e09e6183a5b3f2dac19bffa91 100644
> --- a/base/strings/string16.h
> +++ b/base/strings/string16.h
> @@ -181,9 +181,8 @@ class BASE_EXPORT std::basic_string<base::char16,
> base::string16_char_traits>;
>
>  #endif  // WCHAR_T_IS_UTF32
>
> -// TODO(brettw) update users of string16 to use the namespace and remove
> -// this "using".
> +// TODO(brettw) update users of char16 to use the namespace and remove
> this
> +// "using".
>  using base::char16;
> -using base::string16;
>
>  #endif  // BASE_STRINGS_STRING16_H_
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[brettw, 2013-12-20 23:03:58]

lgtm

[commit-bot: I haz the power, 2013-12-21 01:02:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/93903011/1

[commit-bot: I haz the power, 2013-12-21 01:02:28]

Failed to apply patch for PRESUBMIT.py:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file PRESUBMIT.py
  Hunk #2 FAILED at 1179.
  1 out of 2 hunks FAILED -- saving rejects to file PRESUBMIT.py.rej

Patch:       PRESUBMIT.py
Index: PRESUBMIT.py
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index
9bb1471cbed6ae9c37ed36022b878fe58fc95d21..04cea10dffee5aed22c9e0372b8bae7fb6e03974
100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -1037,7 +1037,6 @@ 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(
@@ -1180,53 +1179,6 @@ 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'^android_webview[\\\/]',
-          r'^apps[\\\/]',
-          r'^ash[\\\/]',
-          r'^chrome[\\\/]',
-          r'^chromeos[\\\/]',
-          r'^components[\\\/]',
-          r'^content[\\\/]',
-          r'^device[\\\/]',
-          r'^ipc[\\\/]',
-          r'^net[\\\/]',
-          r'^ppapi[\\\/]',
-          r'^printing[\\\/]',
-          r'^rlz[\\\/]',
-          r'^skia[\\\/]',
-          r'^tools[\\\/]',
-          r'^ui[\\\/]',
-          r'^webkit[\\\/]',
-      ),
-      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))

[commit-bot: I haz the power, 2013-12-21 01:25:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/93903011/100001

[commit-bot: I haz the power, 2013-12-21 01:44:45]

Retried try job too often on mac_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[Dan Beam, 2013-12-30 21:43:05]

Committed patchset #9 manually as r242733 (presubmit successful).