Clean up rewrite_to_chrome_style naming logic.

Clean up rewrite_to_chrome_style naming logic.

Previously, the naming logic was pretty minimal and haphazard.
Identifiers that were named inconsistently with Blink style would often
get rewritten incorrectly. Now, the naming logic always normalizes the
identifier to a list of words before performing any transformations,
ensuring the output is consistent.

BUG=580739, 580740

[dcheng, 2016-01-26 07:04:48]

Description was changed from

==========
Clean up naming logic and move it into a separate module.

Previously, the naming logic was pretty minimal and haphazard.
Identifiers that were named inconsistently with Blink style would often
get rewritten incorrectly. Now, the naming logic always normalizes the
identifier to a list of words before performing any transformations,
ensuring the output is consistent.

BUG=580739,580740
==========

to

==========
Clean up rewrite_to_chrome_style naming logic.

Previously, the naming logic was pretty minimal and haphazard.
Identifiers that were named inconsistently with Blink style would often
get rewritten incorrectly. Now, the naming logic always normalizes the
identifier to a list of words before performing any transformations,
ensuring the output is consistent.

BUG=580739,580740
==========

[dcheng, 2016-01-26 07:04:48]

dcheng@chromium.org changed reviewers:
+ danakj@chromium.org

[dcheng, 2016-01-26 07:07:30]

There's probably a few other naming fixes to make, but those should probably
happen in followups: for instance, handling for DOUBLEACRONYMS.

[danakj, 2016-01-26 21:32:10]

https://codereview.chromium.org/1639663003/diff/1/tools/clang/rewrite_to_chro...
File tools/clang/rewrite_to_chrome_style/Renamer.cpp (right):

https://codereview.chromium.org/1639663003/diff/1/tools/clang/rewrite_to_chro...
tools/clang/rewrite_to_chrome_style/Renamer.cpp:41: if (was_lowercase &&
is_uppercase) {
What about V8Stuff?

8 won't be uppercase or lowercase right? But Stuff is a new word. So easiest is
probably to treat numbers as lowercase?

Or else here do !was_uppercase && is_uppercase?

Having 2 bools for upper/lower but then not handling the both-false case seems
off.

https://codereview.chromium.org/1639663003/diff/1/tools/clang/rewrite_to_chro...
tools/clang/rewrite_to_chrome_style/Renamer.cpp:47: --i;  // Already iterated
into the next word, so backtrack.
Should it in_acronym = false?

https://codereview.chromium.org/1639663003/diff/1/tools/clang/rewrite_to_chro...
tools/clang/rewrite_to_chrome_style/Renamer.cpp:52: // probably part of an
acronym.
Heh, except AThing. I wonder if that happens..

It does:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[dcheng, 2016-01-27 00:09:28]

https://codereview.chromium.org/1639663003/diff/1/tools/clang/rewrite_to_chro...
File tools/clang/rewrite_to_chrome_style/Renamer.cpp (right):

https://codereview.chromium.org/1639663003/diff/1/tools/clang/rewrite_to_chro...
tools/clang/rewrite_to_chrome_style/Renamer.cpp:41: if (was_lowercase &&
is_uppercase) {
On 2016/01/26 at 21:32:10, danakj wrote:
> What about V8Stuff?
> 
> 8 won't be uppercase or lowercase right? But Stuff is a new word. So easiest
is probably to treat numbers as lowercase?
> 
> Or else here do !was_uppercase && is_uppercase?
> 
> Having 2 bools for upper/lower but then not handling the both-false case seems
off.

I made a small tweak so this gets handled correctly now (and added some test
cases).

I don't think we necessarily want to treat numbers as lowercase: this means
acronyms with numbers embedded in the middle might get split into two parts.
However, I did a search for acronyms of this type and found that they were only
present in THINGS_NAM3D_LIKE_THIS. So I added a fallback path for the simple
splitting case to just use underscores.

Of course, to make things more complicated, there are identifiers like:
- calculateStringHashAndLengthFromUTF8MaskingTop8Bits
- enableUnsafeES3APIs

I added these to the test set.

https://codereview.chromium.org/1639663003/diff/1/tools/clang/rewrite_to_chro...
tools/clang/rewrite_to_chrome_style/Renamer.cpp:47: --i;  // Already iterated
into the next word, so backtrack.
On 2016/01/26 at 21:32:10, danakj wrote:
> Should it in_acronym = false?

This is scoped to the body of the outermost for loop, so it automatically gets
reset at the next iteration.

https://codereview.chromium.org/1639663003/diff/1/tools/clang/rewrite_to_chro...
tools/clang/rewrite_to_chrome_style/Renamer.cpp:52: // probably part of an
acronym.
On 2016/01/26 at 21:32:10, danakj wrote:
> Heh, except AThing. I wonder if that happens..
> 
> It does:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

This is OK though: we'll mark this as being in an acronym, then we hit the
lowercase letter on the next iteration of the loop. Since we backtrack, this
will clip it the word to one letter. I'll add an explicit test case though.

[danakj, 2016-01-27 00:28:56]

So close!

https://codereview.chromium.org/1639663003/diff/40001/tools/clang/rewrite_to_...
File tools/clang/rewrite_to_chrome_style/tests/variables-expected.cc (right):

https://codereview.chromium.org/1639663003/diff/40001/tools/clang/rewrite_to_...
tools/clang/rewrite_to_chrome_style/tests/variables-expected.cc:29: bool
enable_unsafe_es3_ap_is;
itym es3_apis D:

[dcheng, 2016-01-27 00:38:29]

https://codereview.chromium.org/1639663003/diff/40001/tools/clang/rewrite_to_...
File tools/clang/rewrite_to_chrome_style/tests/variables-expected.cc (right):

https://codereview.chromium.org/1639663003/diff/40001/tools/clang/rewrite_to_...
tools/clang/rewrite_to_chrome_style/tests/variables-expected.cc:29: bool
enable_unsafe_es3_ap_is;
On 2016/01/27 at 00:28:56, danakj wrote:
> itym es3_apis D:

Yeah... previously, this didn't really show up, since we didn't try to fix the
casing on acronyms.

Depending on how many spectacular failures like this there are, I'm thinking of
just special-casing them in the tool in a followup. WDYT?

[danakj, 2016-01-27 00:40:53]

LGTM but file a bug for APIs and we can sort it out/data-collect.

https://codereview.chromium.org/1639663003/diff/40001/tools/clang/rewrite_to_...
File tools/clang/rewrite_to_chrome_style/tests/variables-expected.cc (right):

https://codereview.chromium.org/1639663003/diff/40001/tools/clang/rewrite_to_...
tools/clang/rewrite_to_chrome_style/tests/variables-expected.cc:29: bool
enable_unsafe_es3_ap_is;
On 2016/01/27 00:38:29, dcheng wrote:
> On 2016/01/27 at 00:28:56, danakj wrote:
> > itym es3_apis D:
> 
> Yeah... previously, this didn't really show up, since we didn't try to fix the
> casing on acronyms.
> 
> Depending on how many spectacular failures like this there are, I'm thinking
of
> just special-casing them in the tool in a followup. WDYT?

that'd be fine.

[dcheng, 2016-01-27 00:45:09]

On 2016/01/27 at 00:40:53, danakj wrote:
> LGTM but file a bug for APIs and we can sort it out/data-collect.
> 
>
https://codereview.chromium.org/1639663003/diff/40001/tools/clang/rewrite_to_...
> File tools/clang/rewrite_to_chrome_style/tests/variables-expected.cc (right):
> 
>
https://codereview.chromium.org/1639663003/diff/40001/tools/clang/rewrite_to_...
> tools/clang/rewrite_to_chrome_style/tests/variables-expected.cc:29: bool
enable_unsafe_es3_ap_is;
> On 2016/01/27 00:38:29, dcheng wrote:
> > On 2016/01/27 at 00:28:56, danakj wrote:
> > > itym es3_apis D:
> > 
> > Yeah... previously, this didn't really show up, since we didn't try to fix
the
> > casing on acronyms.
> > 
> > Depending on how many spectacular failures like this there are, I'm thinking
of
> > just special-casing them in the tool in a followup. WDYT?
> 
> that'd be fine.

https://crbug.com/581560

[dcheng, 2016-01-27 00:45:13]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2016-01-27 00:45:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1639663003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1639663003/40001

[commit-bot: I haz the power, 2016-01-27 00:51:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-27 00:51:45]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  android_clang_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[dcheng, 2016-01-29 00:17:47]

Closing this for now, since we've decided to not make our lives harder than they
have to be.