Automatically sort headers in files affected by move_source_file.py

tools/git/move_source_file.py

Index: tools/git/move_source_file.py
diff --git a/tools/git/move_source_file.py b/tools/git/move_source_file.py
index 148cc0ddc1b1ca30b8698a8e8482aafec801fe6e..4e56854a87805171f4e604ca17ebceafdb118545 100755
--- a/tools/git/move_source_file.py
+++ b/tools/git/move_source_file.py
@@ -4,18 +4,16 @@
 # found in the LICENSE file.
 
 """Moves a C++ file to a new location, updating any include paths that
-point to it.  Updates include guards in moved header files.  Assumes
-Chromium coding style.
+point to it, and re-ordering headers as needed.  Updates include
+guards in moved header files.  Assumes Chromium coding style.
 
-Does not reorder headers; instead, use this after committing all of
-your moves:
-  ./tools/git/for-all-touched-files.py -c "tools/sort-headers.py [[FILENAME]]"
+Attempts to update paths used in .gyp(i) files, but does not reorder
+or restructure .gyp(i) files in any way.
 
-Updates paths used in .gyp(i) files, but does not reorder or
-restructure .gyp(i) files in any way.
+Updates full-path references to files in // comments in source files.
 
-Must run in a git checkout, as it relies on git for a fast way to find
-files that reference the moved file.
+Must run in a git checkout, as it relies on git grep for a fast way to
+find files that reference the moved file.
 """
 
 
@@ -24,6 +22,13 @@ import re
 import subprocess
 import sys
 
+if __name__ == '__main__':
+  # Need to add the directory containing sort-headers.py to the Python
+  # classpath.
+  sys.path.append(os.path.abspath(os.path.join(sys.path[0], '..')))
+sort_headers = __import__('sort-headers')
+
+
 HANDLED_EXTENSIONS = ['.cc', '.mm', '.h', '.hh']
 
 
@@ -54,47 +59,42 @@ def MoveFile(from_path, to_path):
 
 def MultiFileFindReplace(original,
                          replacement,
-                         grep_pattern,
-                         file_globs,
-                         guard_formats):
-  """Implements fast multi-file find and replace with optional guards.
+                         file_globs):
+  """Implements fast multi-file find and replace.
 
-  Given an |original| string and a |replacement| string, search for
-  them by formatting |grep_pattern| with |original| and running
-  git grep on the result, for files matching any of |file_globs|.
+  Given an |original| string and a |replacement| string, find matching
+  files by running git grep on |original| in files matching any
+  pattern in |file_globs|.
 
-  Once files are found, the function searches for any of
-  |guard_formats| formatted with |original| and replaces each match
-  with the same guard format as matched, formatted with |replacement|.
+  Once files are found, |re.sub| is run to replace |original| with
+  |replacement|.  |replacement| may use capture group back-references.
 
   Args:
-    original: 'chrome/browser/ui/browser.h'
-    replacement: 'chrome/browser/ui/browser/browser.h'
-    grep_pattern: r'#(include|import)\s*["<]%s[>"]'
+    original: '(#(include|import)\s*["<])chrome/browser/ui/browser.h([>"])'
+    replacement: '\1chrome/browser/ui/browser/browser.h\3'
     file_globs: ['*.cc', '*.h', '*.m', '*.mm']
-    guard_formats: None or ('"%s"', '<%s>')
+
+  Returns the list of files modified.
 
   Raises an exception on error.
   """
   out, err = subprocess.Popen(
-    ['git', 'grep', '-E', '--name-only',
-     grep_pattern % re.escape(original), '--'] + file_globs,
-    stdout=subprocess.PIPE).communicate()
+      ['git', 'grep', '-E', '--name-only', original, '--'] + file_globs,
+      stdout=subprocess.PIPE).communicate()
   referees = out.splitlines()
 
   for referee in referees:
     with open(referee) as f:
       original_contents = f.read()
-      contents = original_contents
-      for guard_format in guard_formats or []:
-        contents = contents.replace(guard_format % original,
-                                    guard_format % replacement)
+      contents = re.sub(original, replacement, original_contents)
       if contents == original_contents:
         raise Exception('No change in file %s although matched in grep' %
                         referee)
       with open(referee, 'w') as f:
         f.write(contents)
 
+  return referees
+
 
 def UpdatePostMove(from_path, to_path):
   """Given a file that has moved from |from_path| to |to_path|,
@@ -110,12 +110,21 @@ def UpdatePostMove(from_path, to_path):
     UpdateIncludeGuard(from_path, to_path)
 
     # Update include/import references.
-    MultiFileFindReplace(
-      from_path,
-      to_path,
-      r'#(include|import)\s*["<]%s[>"]',
-      ['*.cc', '*.h', '*.m', '*.mm'],
-      ['"%s"', '<%s>'])
+    files_with_changed_includes = MultiFileFindReplace(
+        r'(#(include|import)\s*["<])%s([>"])' % re.escape(from_path),
+        r'\1%s\3' % to_path,
+        ['*.cc', '*.h', '*.m', '*.mm'])
+
+    # Reorder headers in files that changed.
+    for changed_file in files_with_changed_includes:

[Nico, 2012/11/21 18:35:36]

I'd do `def auto_confirm(a, b): return true; FixFileWithConfrim(changed_file,
always_confirm)`, that's a bit more readable than the lambda imo.

[Jói, 2012/11/22 09:42:50]

Done.

+      sort_headers.FixFileWithConfirmFunction(changed_file, lambda a, b: True)
+
+  # Update comments; only supports // comments, which are primarily
+  # used in our code.
+  MultiFileFindReplace(
+      r'(//.*)%s' % re.escape(from_path),
+      r'\1%s' % to_path,
+      ['*.cc', '*.h', '*.m', '*.mm'])

[Nico, 2012/11/21 18:35:36]

I'd omit this as I said. It only finds comments that contain the full file path,
and comments with filenames aren't super common. Imho this isn't worth 20%.

(But searching crbug.com for "//.*[/].*\w\.cc -file:v8 -file:third_party" finds
more than I thought it would, so up to you.)

[Jói, 2012/11/22 09:42:50]

I'll leave it in since it's safe and convenient and updates something you might
not remember to search for and update yourself.  I've added a comment
documenting that comment handling is somewhat slow and the script could be sped
up by making it optional under a flag; time will tell if it is fast enough or
not.

 
   # Update references in .gyp(i) files.
   def PathMinusFirstComponent(path):
@@ -125,11 +134,10 @@ def UpdatePostMove(from_path, to_path):
       return parts[1]
     else:
       return parts[0]
-  MultiFileFindReplace(PathMinusFirstComponent(from_path),
-                       PathMinusFirstComponent(to_path),
-                       r'[\'"]%s[\'"]',
-                       ['*.gyp*'],
-                       ["'%s'", '"%s"'])
+  MultiFileFindReplace(
+      r'([\'"])%s([\'"])' % re.escape(PathMinusFirstComponent(from_path)),
+      r'\1%s\2' % PathMinusFirstComponent(to_path),
+      ['*.gyp*'])
 
 
 def MakeIncludeGuardName(path_from_root):