Commit ab20cc08 authored by dpranke@chromium.org's avatar dpranke@chromium.org

Add a new 'format-webkitpy' command that will reformat code to the style guide.

This is an experimental utility based on autopep8 and lib2to3. 

It is capable of reformatting to either the Blink or Chromium style guides (the difference between the two is that Blink uses 132c lines, unix_hacker naming for functions and methods, a 4-space indent, and prefers single-quoted string literals, whereas Chromium uses 80c lines, InitialCaps for functions and methods, a 2-space indent, and double-quoted literals).

Note that format-webkitpy does *not* convert method names between the two conventions, since that is a potentially unsafe thing to do. It is probably possible to write a lib2to3 fixer that does do this relatively safely, but that was more work than I felt like doing for an initial go.

As part of this, we add autopep8 into webkitpy third_party (lib2to3 is part of the standard lib) and update pep8, which hadn't been updated for years.

This patch also adds some simple helper methods to Executive() to do parallel map calls using multiprocessing (and to mock them out properly) so that client code doesn't need to be aware of the hoop-jumping you need to do since hosts aren't picklable. This part of the code is still a little crufty.

BUG=

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

git-svn-id: svn://svn.chromium.org/blink/trunk@181690 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent b7f25a14
#!/usr/bin/env python
# Copyright 2014 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import sys
from webkitpy.formatter.main import main
sys.exit(main())
...@@ -471,14 +471,17 @@ class Executive(object): ...@@ -471,14 +471,17 @@ class Executive(object):
def run_in_parallel(self, command_lines_and_cwds, processes=None): def run_in_parallel(self, command_lines_and_cwds, processes=None):
"""Runs a list of (cmd_line list, cwd string) tuples in parallel and returns a list of (retcode, stdout, stderr) tuples.""" """Runs a list of (cmd_line list, cwd string) tuples in parallel and returns a list of (retcode, stdout, stderr) tuples."""
assert len(command_lines_and_cwds) assert len(command_lines_and_cwds)
return self.map(_run_command_thunk, command_lines_and_cwds, processes)
if sys.platform in ('cygwin', 'win32'): def map(self, thunk, arglist, processes=None):
return map(_run_command_thunk, command_lines_and_cwds) if sys.platform in ('cygwin', 'win32') or len(arglist) == 1:
pool = multiprocessing.Pool(processes=processes) return map(thunk, arglist)
results = pool.map(_run_command_thunk, command_lines_and_cwds) pool = multiprocessing.Pool(processes=(processes or multiprocessing.cpu_count()))
pool.close() try:
pool.join() return pool.map(thunk, arglist)
return results finally:
pool.close()
pool.join()
def _run_command_thunk(cmd_line_and_cwd): def _run_command_thunk(cmd_line_and_cwd):
......
...@@ -168,6 +168,9 @@ class MockExecutive(object): ...@@ -168,6 +168,9 @@ class MockExecutive(object):
self.calls.append(new_calls) self.calls.append(new_calls)
return command_outputs return command_outputs
def map(self, thunk, arglist, processes=None):
return map(thunk, arglist)
class MockExecutive2(MockExecutive): class MockExecutive2(MockExecutive):
"""MockExecutive2 is like MockExecutive except it doesn't log anything.""" """MockExecutive2 is like MockExecutive except it doesn't log anything."""
......
...@@ -41,6 +41,9 @@ class SystemHost(object): ...@@ -41,6 +41,9 @@ class SystemHost(object):
self.user = user.User() self.user = user.User()
self.platform = platforminfo.PlatformInfo(sys, platform, self.executive) self.platform = platforminfo.PlatformInfo(sys, platform, self.executive)
self.workspace = workspace.Workspace(self.filesystem, self.executive) self.workspace = workspace.Workspace(self.filesystem, self.executive)
self.stdin = sys.stdin
self.stdout = sys.stdout
self.stderr = sys.stderr
def copy_current_environment(self): def copy_current_environment(self):
return environment.Environment(os.environ.copy()) return environment.Environment(os.environ.copy())
...@@ -48,8 +51,5 @@ class SystemHost(object): ...@@ -48,8 +51,5 @@ class SystemHost(object):
def print_(self, *args, **kwargs): def print_(self, *args, **kwargs):
sep = kwargs.get('sep', ' ') sep = kwargs.get('sep', ' ')
end = kwargs.get('end', '\n') end = kwargs.get('end', '\n')
file = kwargs.get('file', None) stream = kwargs.get('stream', self.stdout)
stderr = kwargs.get('stderr', False) stream.write(sep.join([str(arg) for arg in args]) + end)
file = file or (sys.stderr if stderr else sys.stdout)
file.write(sep.join([str(arg) for arg in args]) + end)
...@@ -51,6 +51,7 @@ class MockSystemHost(object): ...@@ -51,6 +51,7 @@ class MockSystemHost(object):
# FIXME: Should this take pointers to the filesystem and the executive? # FIXME: Should this take pointers to the filesystem and the executive?
self.workspace = MockWorkspace() self.workspace = MockWorkspace()
self.stdin = StringIO()
self.stdout = StringIO() self.stdout = StringIO()
self.stderr = StringIO() self.stderr = StringIO()
...@@ -60,8 +61,5 @@ class MockSystemHost(object): ...@@ -60,8 +61,5 @@ class MockSystemHost(object):
def print_(self, *args, **kwargs): def print_(self, *args, **kwargs):
sep = kwargs.get('sep', ' ') sep = kwargs.get('sep', ' ')
end = kwargs.get('end', '\n') end = kwargs.get('end', '\n')
file = kwargs.get('file', None) stream = kwargs.get('stream', self.stdout)
stderr = kwargs.get('stderr', False) stream.write(sep.join([str(arg) for arg in args]) + end)
file = file or (self.stderr if stderr else self.stdout)
file.write(sep.join([str(arg) for arg in args]) + end)
# Copyright 2014 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import sys
from webkitpy.formatter.main import main
if __name__ == '__main__':
sys.exit(main())
# Copyright 2014 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
"""
A 2to3 fixer that converts all string literals to use double quotes.
Strings that contain double quotes will not be modified. Prefixed string
literals will also not be modified. This affects both single-quoted strings
and triple-single-quoted strings.
"""
from lib2to3.fixer_base import BaseFix
from lib2to3.pgen2 import token
class FixDoubleQuoteStrings(BaseFix):
explicit = True
_accept_type = token.STRING
def match(self, node):
res = node.value.startswith("'") and '"' not in node.value[1:-1]
return res
def transform(self, node, results):
node.value = node.value.replace("'", '"')
node.changed()
# Copyright 2014 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
"""
A 2to3 fixer that converts all string literals to use single quotes.
Strings that contain single quotes will not be modified. Prefixed string
literals will also not be modified. This affect double-quoted strings but
not triple-double-quote strings.
"""
from lib2to3.fixer_base import BaseFix
from lib2to3.pgen2 import token
class FixSingleQuoteStrings(BaseFix):
explicit = True
_accept_type = token.STRING
def match(self, node):
res = node.value.startswith('"') and not node.value.startswith('"""') and "'" not in node.value[1:-1]
return res
def transform(self, node, results):
node.value = node.value.replace('"', "'")
node.changed()
# Copyright 2014 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import argparse
import lib2to3.refactor
from webkitpy.common.system.systemhost import SystemHost
from webkitpy.thirdparty import autopep8
def parse_args(args=None):
parser = argparse.ArgumentParser()
parser.add_argument('--chromium', action='store_const', dest='style', const='chromium', default='blink',
help="format according to Chromium's Python coding styles instead of Blink's")
parser.add_argument('--no-backups', action='store_false', default=True, dest='backup',
help='do not back up files before overwriting them')
parser.add_argument('-j', '--jobs', metavar='n', type=int, default=0,
help='number of parallel jobs; match CPU count if less than 1')
parser.add_argument('files', nargs='*', default=['-'],
help="files to format or '-' for standard in")
return parser.parse_args(args=args)
def main(host=None, args=None):
options = parse_args(args)
if options.files == ['-']:
host = host or SystemHost()
host.print_(reformat_source(host.stdin.read(), options.style, '<stdin>'), end='')
return
# We create the arglist before checking if we need to create a Host, because a
# real host is non-picklable and can't be passed to host.executive.map().
arglist = [(host, name, options.style, options.backup) for name in options.files]
host = host or SystemHost()
host.executive.map(_reformat_thunk, arglist, processes=options.jobs)
def _reformat_thunk(args):
reformat_file(*args)
def reformat_file(host, name, style, should_backup_file):
host = host or SystemHost()
source = host.filesystem.read_text_file(name)
dest = reformat_source(source, style, name)
if dest != source:
if should_backup_file:
host.filesystem.write_text_file(name + '.bak', source)
host.filesystem.write_text_file(name, dest)
def reformat_source(source, style, name):
options = _autopep8_options_for_style(style)
tmp_str = autopep8.fix_code(source, options)
fixers = _fixers_for_style(style)
tool = lib2to3.refactor.RefactoringTool(fixer_names=fixers,
explicit=fixers)
return unicode(tool.refactor_string(tmp_str, name=name))
def _autopep8_options_for_style(style):
if style == 'chromium':
max_line_length = 80
indent_size = 2
else:
max_line_length = 132
indent_size = 4
return autopep8.parse_args(['--aggressive',
'--max-line-length', str(max_line_length),
'--indent-size', str(indent_size),
''])
def _fixers_for_style(style):
if style == 'chromium':
return ['webkitpy.formatter.fix_double_quote_strings']
else:
return ['webkitpy.formatter.fix_single_quote_strings']
# Copyright 2014 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import StringIO
import unittest
from webkitpy.common.system.systemhost_mock import MockSystemHost
from webkitpy.formatter.main import main
ACTUAL_INPUT = '''
def foo():
"""triple-quoted docstring"""
try:
bar = "bar"
long_list = ['this is a list of strings that should be wrapped', "and consistently quoted"]
longer_list = ['this is a list of strings that should be wrapped', "and consistently quoted", "because it's important to test quoting"]
except Exception, e:
pass
'''
EXPECTED_BLINK_OUTPUT = '''
def foo():
"""triple-quoted docstring"""
try:
bar = 'bar'
long_list = ['this is a list of strings that should be wrapped', 'and consistently quoted']
longer_list = [
'this is a list of strings that should be wrapped',
'and consistently quoted',
"because it's important to test quoting"]
except Exception as e:
pass
'''
EXPECTED_CHROMIUM_OUTPUT = '''
def foo():
"""triple-quoted docstring"""
try:
bar = "bar"
long_list = [
"this is a list of strings that should be wrapped",
"and consistently quoted"]
longer_list = [
"this is a list of strings that should be wrapped",
"and consistently quoted",
"because it's important to test quoting"]
except Exception as e:
pass
'''
class TestMain(unittest.TestCase):
maxDiff = 4096
def test_stdin_blink(self):
host = MockSystemHost()
host.stdin = StringIO.StringIO(ACTUAL_INPUT)
main(host, ['-'])
self.assertMultiLineEqual(host.stdout.getvalue(), EXPECTED_BLINK_OUTPUT)
def test_stdin_chromium(self):
host = MockSystemHost()
host.stdin = StringIO.StringIO(ACTUAL_INPUT)
main(host, ['--chromium', '-'])
self.assertMultiLineEqual(host.stdout.getvalue(), EXPECTED_CHROMIUM_OUTPUT)
def test_files_blink(self):
host = MockSystemHost()
host.filesystem.files = {
'test.py': ACTUAL_INPUT}
main(host, ['test.py'])
self.assertEqual(host.filesystem.files, {
'test.py': EXPECTED_BLINK_OUTPUT,
'test.py.bak': ACTUAL_INPUT})
def test_files_blink_no_backup(self):
host = MockSystemHost()
host.filesystem.files = {
'test.py': ACTUAL_INPUT}
main(host, ['--no-backups', 'test.py'])
self.assertEqual(host.filesystem.files, {
'test.py': EXPECTED_BLINK_OUTPUT})
...@@ -6,6 +6,16 @@ an entry in this file that refers to reliable documentation of the project's ...@@ -6,6 +6,16 @@ an entry in this file that refers to reliable documentation of the project's
license terms on the web (and add a note pointing here in the README file in license terms on the web (and add a note pointing here in the README file in
that directory). that directory).
Name: autopep8
Short Name: autopep8
URL: https://pypi.python.org/packages/source/a/autopep8/autopep8-1.0.3.tar.gz#md5=7c16d385cf9ad7c1d7fbcfcea2588a56
Version: 1.0.3
License: MIT
License File: NOT_SHIPPED
Security Critical: no
Description: Used to reformat python code via format-webkitpy
Local Modifications: None
Name: BeautifulSoup - HTML parser Name: BeautifulSoup - HTML parser
Short Name: BeautifulSoup Short Name: BeautifulSoup
URL: http://www.crummy.com/software/BeautifulSoup/download/3.x/BeautifulSoup-3.2.1.tar.gz (?) URL: http://www.crummy.com/software/BeautifulSoup/download/3.x/BeautifulSoup-3.2.1.tar.gz (?)
...@@ -48,12 +58,12 @@ Local Modifications: None ...@@ -48,12 +58,12 @@ Local Modifications: None
Name: pep8 - A Python style guide checker Name: pep8 - A Python style guide checker
Short Name: pep8 Short Name: pep8
URL: http://pypi.python.org/packages/source/p/pep8/pep8-0.5.0.tar.gz#md5=512a818af9979290cd619cce8e9c2e2b URL: https://pypi.python.org/packages/source/p/pep8/pep8-1.5.7.tar.gz#md5=f6adbdd69365ecca20513c709f9b7c93
Version: 0.5.0 Version: 1.5.7
License: MIT License: MIT
License File: NOT_SHIPPED License File: NOT_SHIPPED
Security Critical: no Security Critical: no
Description: Used during presubmit checks and via lint-webkitpy. There is Description: Used during presubmit checks and via lint-webkitpy and format-webkitpy. There is
overlap between pep8 and pylint, but pep8 catches a bunch of stylistic overlap between pep8 and pylint, but pep8 catches a bunch of stylistic
issues that pylint doesn't (e.g., warning about blank lines, various whitespace issues, etc.). issues that pylint doesn't (e.g., warning about blank lines, various whitespace issues, etc.).
Local Modifications: None Local Modifications: None
This source diff could not be displayed because it is too large. You can view the blob instead.
This source diff could not be displayed because it is too large. You can view the blob instead.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment