Repository: google/yapf
Pull request #1 (2015-03-22 22:28:00)

Implement configurable styles in yapf.
Idea similar to clang-format: there's a set of supported "known" styles - for
the moment PEP8 and Google. For custom stuff, a configuration file can be
specified.


Patch:
diff --git a/yapf/__init__.py b/yapf/__init__.py
index d6203fc..18d21bd 100644
--- a/yapf/__init__.py
+++ b/yapf/__init__.py
@@ -47,6 +47,10 @@ def main(argv):
     0 if there were no errors, non-zero otherwise.
   """
   parser = argparse.ArgumentParser(description='Formatter for Python code.')
+  parser.add_argument(
+      '--style', action='store', default=None,
+      help=('specify formatting style: either a style name (for example "pep8" '
+            'or "google"), or the name of a file with style settings'))
   diff_inplace_group = parser.add_mutually_exclusive_group()
   diff_inplace_group.add_argument(
       '-d', '--diff', action='store_true',
@@ -90,14 +94,15 @@ def main(argv):
     sys.stdout.write(yapf_api.FormatCode(
         py3compat.unicode('\n'.join(original_source) + '\n'),
         filename='<stdin>',
+        style_config=args.style,
         lines=lines))
     return 0
 
-  FormatFiles(files, lines, args.in_place)
+  FormatFiles(files, lines, style_config=args.style, in_place=args.in_place)
   return 0
 
 
-def FormatFiles(filenames, lines, in_place=False):
+def FormatFiles(filenames, lines, style_config=None, in_place=False):
   """Format a list of files.
 
   Arguments:
@@ -106,11 +111,13 @@ def FormatFiles(filenames, lines, in_place=False):
       that we want to format. The lines are 1-based indexed. This argument
       overrides the 'args.lines'. It can be used by third-party code (e.g.,
       IDEs) when reformatting a snippet of code.
+    style_config: (string) Style name or file path.
     in_place: (bool) Modify the files in place.
   """
   for filename in filenames:
     logging.info('Reformatting %s', filename)
-    reformatted_code = yapf_api.FormatFile(filename, lines)
+    reformatted_code = yapf_api.FormatFile(
+        filename, style_config=style_config, lines=lines)
     if reformatted_code is not None:
       file_resources.WriteReformattedCode(filename, reformatted_code, in_place)
 
diff --git a/yapf/yapflib/py3compat.py b/yapf/yapflib/py3compat.py
index b5b7819..c6e3abd 100644
--- a/yapf/yapflib/py3compat.py
+++ b/yapf/yapflib/py3compat.py
@@ -26,6 +26,8 @@ if PY3:
   range = range
   ifilter = filter
   raw_input = input
+
+  import configparser
 else:
   import __builtin__
   import cStringIO
@@ -36,6 +38,8 @@ else:
   from itertools import ifilter
   raw_input = raw_input
 
+  import ConfigParser as configparser
+
 
 def EncodeForStdout(s):
   """Encode the given string for emission to stdout.
@@ -62,3 +66,12 @@ def unicode(s):
     return s
   else:
     return __builtin__.unicode(s, 'unicode_escape')
+
+
+# In Python 3.2+, readfp is deprecated in favor of read_file, which doesn't
+# exist in Python 2 yet. To avoid deprecation warnings, subclass ConfigParser to
+# fix this - now read_file works across all Python versions we care about.
+class ConfigParser(configparser.ConfigParser):
+  if not PY3:
+    def read_file(self, fp, source=None):
+      self.readfp(fp, filename=source)
diff --git a/yapf/yapflib/style.py b/yapf/yapflib/style.py
index d8702f9..4a6ebd8 100644
--- a/yapf/yapflib/style.py
+++ b/yapf/yapflib/style.py
@@ -11,7 +11,9 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-"""Python format style guidelines."""
+"""Python formatting style settings."""
+
+from yapf.yapflib import py3compat
 
 
 def Get(setting_name):
@@ -95,12 +97,103 @@ def CreateGoogleStyle():
   style['SPACES_BEFORE_COMMENT'] = 2
   style['NO_BLANK_LINE_AFTER_CLASS_OR_DEF'] = False
   style['I18N_COMMENT'] = r'#\..*'
-  style['I18N_FUNCTION_CALL'] = ('N_', '_')
+  style['I18N_FUNCTION_CALL'] = ['N_', '_']
   return style
 
 
+_STYLE_NAME_TO_FACTORY = dict(
+    pep8 = CreatePEP8Style,
+    google = CreateGoogleStyle,
+)
+
+
+def _StringListConverter(s):
+  """Option value converter for a comma-separated list of strings."""
+  return [part.strip() for part in s.split(',')]
+
+
+def _BoolConverter(s):
+  """Option value converter for a boolean."""
+  # borrowed from configparser.
+  BOOLEAN_STATES = {'1': True, 'yes': True, 'true': True, 'on': True,
+                    '0': False, 'no': False, 'false': False, 'off': False}
+  return BOOLEAN_STATES[s.lower()]
+
+
+# Different style options need to have their values interpreted differently when
+# read from the config file. This dict maps an option name to a "converter"
+# function that accepts the string read for the option's value from the file and
+# returns it wrapper in actual Python type that's going to be meaningful to
+# yapf.
+_STYLE_OPTION_VALUE_CONVERTER = dict(
+    COLUMN_LIMIT = int,
+    I18N_COMMENT = str,
+    I18N_FUNCTION_CALL = _StringListConverter,
+    INDENT_WIDTH = int,
+    CONTINUATION_INDENT_WIDTH = int,
+    NO_BLANK_LINE_AFTER_CLASS_OR_DEF = _BoolConverter,
+    SPACES_BEFORE_COMMENT = int,
+    SPLIT_BEFORE_LOGICAL_OPERATOR = _BoolConverter,
+    SPLIT_BEFORE_NAMED_ASSIGNS = _BoolConverter,
+    SPLIT_PENALTY_AFTER_UNARY_OPERATOR = int,
+    SPLIT_PENALTY_EXCESS_CHARACTER = int,
+    SPLIT_PENALTY_LOGICAL_OPERATOR = int,
+    SPLIT_PENALTY_MATCHING_BRACKET = int,
+    SPLIT_PENALTY_AFTER_OPENING_BRACKET = int,
+    SPLIT_PENALTY_FOR_ADDED_LINE_SPLIT = int,
+    USE_TAB = _BoolConverter,
+    TAB_WIDTH = int,
+)
+
+
+def CreateStyleFromConfig(style_config):
+  """Create a style dict from the given config.
+
+  Arguments:
+    style_config: either a style name or a file name. The file is expected to
+    contain settings. It can have a special BASED_ON_STYLE setting naming the
+    style which it derives from. If no such setting is found, it derives from
+    the default style. When style_config is None, the DEFAULT_STYLE_FACTORY
+    config is created.
+  Returns:
+    A style dict.
+  """
+  if style_config is None:
+    return DEFAULT_STYLE_FACTORY()
+  style_factory = _STYLE_NAME_TO_FACTORY.get(style_config.lower())
+  if style_factory is not None:
+    return style_factory()
+  # Unknown style name - assume a file path.
+  with open(style_config) as style_file:
+    config = py3compat.ConfigParser()
+    config.read_file(style_file)
+    if not config.has_section('style'):
+      raise RuntimeError('Unable to find section [style] in {0}'.format(
+          style_config))
+    # Initialize the base style.
+    if config.has_option('style', 'based_on_style'):
+      based_on = config.get('style', 'based_on_style').lower()
+      base_style = _STYLE_NAME_TO_FACTORY[based_on]()
+    else:
+      base_style = DEFAULT_STYLE_FACTORY()
+    # Read all options specified in the file and update the style.
+    for option, value in config.items('style'):
+      if option.lower() == 'based_on_style':
+        # Now skip this one - we've already handled it and it's not one of the
+        # recognized style options.
+        continue
+      option = option.upper()
+      base_style[option] = _STYLE_OPTION_VALUE_CONVERTER[option](value)
+    return base_style
+
+
+# The default style - used if yapf is not invoked without specifically
+# requesting a formatting style.
+DEFAULT_STYLE_FACTORY = CreatePEP8Style
+
+
 # TODO(eliben): For now we're preserving the global presence of a style dict.
 # Refactor this so that the style is passed around through yapf rather than
 # being global.
 _style = {}
-SetGlobalStyle(CreatePEP8Style())
+SetGlobalStyle(DEFAULT_STYLE_FACTORY())
diff --git a/yapf/yapflib/yapf_api.py b/yapf/yapflib/yapf_api.py
index fb067fe..9a86919 100644
--- a/yapf/yapflib/yapf_api.py
+++ b/yapf/yapflib/yapf_api.py
@@ -17,6 +17,18 @@ The main APIs that YAPF exposes to drive the reformatting.
 
   FormatFile(): reformat a file.
   FormatCode(): reformat a string of code.
+
+These APIs have some common arguments:
+
+  style_config: (string) Either a style name or a path to a file that contains
+    formatting style settings. If None is specified, use the default style
+    as set in style.DEFAULT_STYLE_FACTORY
+  lines: (list of tuples of integers) A list of tuples of lines, [start, end],
+    that we want to format. The lines are 1-based indexed. It can be used by
+    third-party code (e.g., IDEs) when reformatting a snippet of code rather
+    than a whole file.
+  print_diff: (bool) Instead of returning the reformatted source, return a
+    diff that turns the formatted source into reformatter source.
 """
 
 import difflib
@@ -31,19 +43,16 @@ from yapf.yapflib import pytree_unwrapper
 from yapf.yapflib import pytree_utils
 from yapf.yapflib import reformatter
 from yapf.yapflib import split_penalty
+from yapf.yapflib import style
 from yapf.yapflib import subtype_assigner
 
 
-def FormatFile(filename, lines=None, print_diff=False):
+def FormatFile(filename, style_config=None, lines=None, print_diff=False):
   """Format a single Python file and return the formatted code.
 
   Arguments:
     filename: (unicode) The file to reformat.
-    lines: (list of tuples of integers) A list of tuples of lines, [start, end],
-      that we want to format. The lines are 1-based indexed. This argument
-      overrides the 'FLAGS.lines'. It can be used by third-party code (e.g.,
-      IDEs) when reformatting a snippet of code.
-    print_diff: (bool) Print the diff for the fixed source.
+    style_config, lines, print_diff: see comment at the top of this module.
 
   Returns:
     The reformatted code or None if the file doesn't exist.
@@ -52,12 +61,12 @@ def FormatFile(filename, lines=None, print_diff=False):
   if original_source is None:
     return None
 
-  return FormatCode(original_source, filename=filename, lines=lines,
-                    print_diff=print_diff)
+  return FormatCode(original_source, style_config=style_config,
+                    filename=filename, lines=lines, print_diff=print_diff)
 
 
-def FormatCode(unformatted_source, filename='<unknown>', lines=None,
-               print_diff=False):
+def FormatCode(unformatted_source, filename='<unknown>', style_config=None,
+               lines=None, print_diff=False):
   """Format a string of Python code.
 
   This provides an alternative entry point to YAPF.
@@ -65,15 +74,12 @@ def FormatCode(unformatted_source, filename='<unknown>', lines=None,
   Arguments:
     unformatted_source: (unicode) The code to format.
     filename: (unicode) The name of the file being reformatted.
-    lines: (list of tuples of integers) A list of tuples of lines, [start, end],
-      that we want to format. The lines are 1-based indexed. This argument
-      overrides the 'FLAGS.lines'. It can be used by third-party code (e.g.,
-      IDEs) when reformatting a snippet of code.
-    print_diff: (bool) Print the diff for the fixed source.
+    style_config, lines, print_diff: see comment at the top of this module.
 
   Returns:
     The code reformatted to conform to the desired formatting style.
   """
+  style.SetGlobalStyle(style.CreateStyleFromConfig(style_config))
   tree = pytree_utils.ParseCodeToTree(unformatted_source)
 
   # Run passes on the tree, modifying it in place.


Test patch:
diff --git a/yapftests/__init__.py b/yapftests/__init__.py
index 6eceb4e..42e3c56 100644
--- a/yapftests/__init__.py
+++ b/yapftests/__init__.py
@@ -18,6 +18,7 @@ import unittest
 from yapftests import blank_line_calculator_test
 from yapftests import blank_line_calculator_test
 from yapftests import comment_splicer_test
+from yapftests import file_resources_test
 from yapftests import format_decision_state_test
 from yapftests import format_token_test
 from yapftests import line_joiner_test
@@ -26,6 +27,7 @@ from yapftests import pytree_utils_test
 from yapftests import pytree_visitor_test
 from yapftests import reformatter_test
 from yapftests import split_penalty_test
+from yapftests import style_test
 from yapftests import subtype_assigner_test
 from yapftests import unwrapped_line_test
 from yapftests import yapf_test
@@ -36,6 +38,7 @@ def suite():
   result.addTests(blank_line_calculator_test.suite())
   result.addTests(blank_line_calculator_test.suite())
   result.addTests(comment_splicer_test.suite())
+  result.addTests(file_resources_test.suite())
   result.addTests(format_decision_state_test.suite())
   result.addTests(format_token_test.suite())
   result.addTests(line_joiner_test.suite())
@@ -44,6 +47,7 @@ def suite():
   result.addTests(pytree_visitor_test.suite())
   result.addTests(reformatter_test.suite())
   result.addTests(split_penalty_test.suite())
+  result.addTests(style_test.suite())
   result.addTests(subtype_assigner_test.suite())
   result.addTests(unwrapped_line_test.suite())
   result.addTests(yapf_test.suite())
diff --git a/yapftests/file_resources_test.py b/yapftests/file_resources_test.py
index c22d935..0413426 100644
--- a/yapftests/file_resources_test.py
+++ b/yapftests/file_resources_test.py
@@ -37,11 +37,13 @@ def stdout_redirector(stream):
 
 class WriteReformattedCodeTest(unittest.TestCase):
 
-  def setUp(self):
-    self.test_tmpdir = tempfile.mkdtemp()
+  @classmethod
+  def setUpClass(cls):
+    cls.test_tmpdir = tempfile.mkdtemp()
 
-  def tearDown(self):
-    shutil.rmtree(self.test_tmpdir)
+  @classmethod
+  def tearDownClass(cls):
+    shutil.rmtree(cls.test_tmpdir)
 
   def testWriteToFile(self):
     s = u'foobar'
@@ -60,5 +62,11 @@ class WriteReformattedCodeTest(unittest.TestCase):
     self.assertEqual(stream.getvalue(), s)
 
 
+def suite():
+  result = unittest.TestSuite()
+  result.addTests(unittest.makeSuite(WriteReformattedCodeTest))
+  return result
+
+
 if __name__ == '__main__':
   unittest.main()
diff --git a/yapftests/style_test.py b/yapftests/style_test.py
new file mode 100644
index 0000000..d69a175
--- /dev/null
+++ b/yapftests/style_test.py
@@ -0,0 +1,153 @@
+# -*- coding: utf-8 -*-
+# Copyright 2015 Google Inc. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Tests for yapf.style"""
+
+import contextlib
+import shutil
+import tempfile
+import textwrap
+import unittest
+
+from yapf.yapflib import style
+
+
+class UtilsTest(unittest.TestCase):
+
+  def test_StringListConverter(self):
+    self.assertEqual(style._StringListConverter('foo, bar'), ['foo', 'bar'])
+    self.assertEqual(style._StringListConverter('foo,bar'), ['foo', 'bar'])
+    self.assertEqual(style._StringListConverter('  foo'), ['foo'])
+    self.assertEqual(style._StringListConverter('joe  ,foo,  bar'),
+                     ['joe', 'foo', 'bar'])
+
+  def test_BoolConverter(self):
+    self.assertEqual(style._BoolConverter('true'), True)
+    self.assertEqual(style._BoolConverter('1'), True)
+    self.assertEqual(style._BoolConverter('false'), False)
+    self.assertEqual(style._BoolConverter('0'), False)
+
+
+def _LooksLikeGoogleStyle(cfg):
+  return (cfg['INDENT_WIDTH'] == 2 and
+          not cfg['NO_BLANK_LINE_AFTER_CLASS_OR_DEF'])
+
+
+def _LooksLikePEP8Style(cfg):
+  return cfg['INDENT_WIDTH'] == 4 and cfg['NO_BLANK_LINE_AFTER_CLASS_OR_DEF']
+  
+
+class PredefinedStylesByNameTest(unittest.TestCase):
+
+  def test_Default(self):
+    # default is PEP8
+    cfg = style.CreateStyleFromConfig(None)
+    self.assertTrue(_LooksLikePEP8Style(cfg))
+
+  def test_GoogleByName(self):
+    for google_name in ('google', 'Google', 'GOOGLE'):
+      cfg = style.CreateStyleFromConfig(google_name)
+      self.assertTrue(_LooksLikeGoogleStyle(cfg))
+
+  def test_PEP8ByName(self):
+    for pep8_name in ('PEP8', 'pep8', 'Pep8'):
+      cfg = style.CreateStyleFromConfig(pep8_name)
+      self.assertTrue(_LooksLikePEP8Style(cfg))
+
+
+@contextlib.contextmanager
+def _TempFileContents(dir, contents):
+  with tempfile.NamedTemporaryFile(dir=dir, mode='w') as f:
+    f.write(contents)
+    f.flush()
+    yield f
+  
+
+class StyleFromFileTest(unittest.TestCase):
+
+  @classmethod
+  def setUpClass(cls):
+    cls.test_tmpdir = tempfile.mkdtemp()
+
+  @classmethod
+  def tearDownClass(cls):
+    shutil.rmtree(cls.test_tmpdir)
+
+  def test_DefaultBasedOnStyle(self):
+    cfg = textwrap.dedent('''\
+        [style]
+        tab_width = 20
+        ''')
+    with _TempFileContents(self.test_tmpdir, cfg) as f:
+      cfg = style.CreateStyleFromConfig(f.name)
+      self.assertTrue(_LooksLikePEP8Style(cfg))
+      self.assertEqual(cfg['TAB_WIDTH'], 20)
+
+  def test_DefaultBasedOnPEP8Style(self):
+    cfg = textwrap.dedent('''\
+        [style]
+        based_on_style = pep8
+        tab_width = 40
+        ''')
+    with _TempFileContents(self.test_tmpdir, cfg) as f:
+      cfg = style.CreateStyleFromConfig(f.name)
+      self.assertTrue(_LooksLikePEP8Style(cfg))
+      self.assertEqual(cfg['TAB_WIDTH'], 40)
+
+  def test_DefaultBasedOnGoogleStyle(self):
+    cfg = textwrap.dedent('''\
+        [style]
+        based_on_style = google
+        split_penalty_matching_bracket = 33
+        ''')
+    with _TempFileContents(self.test_tmpdir, cfg) as f:
+      cfg = style.CreateStyleFromConfig(f.name)
+      self.assertTrue(_LooksLikeGoogleStyle(cfg))
+      self.assertEqual(cfg['SPLIT_PENALTY_MATCHING_BRACKET'], 33)
+
+  def test_BoolOptionValue(self):
+    cfg = textwrap.dedent('''\
+        [style]
+        based_on_style = google
+        SPLIT_BEFORE_NAMED_ASSIGNS = False
+        split_before_logical_operator = true
+        ''')
+    with _TempFileContents(self.test_tmpdir, cfg) as f:
+      cfg = style.CreateStyleFromConfig(f.name)
+      self.assertTrue(_LooksLikeGoogleStyle(cfg))
+      self.assertEqual(cfg['SPLIT_BEFORE_NAMED_ASSIGNS'], False)
+      self.assertEqual(cfg['SPLIT_BEFORE_LOGICAL_OPERATOR'], True)
+
+  def test_StringListOptionValue(self):
+    cfg = textwrap.dedent('''\
+        [style]
+        based_on_style = google
+        I18N_FUNCTION_CALL = N_, V_, T_
+        ''')
+    with _TempFileContents(self.test_tmpdir, cfg) as f:
+      cfg = style.CreateStyleFromConfig(f.name)
+      self.assertTrue(_LooksLikeGoogleStyle(cfg))
+      self.assertEqual(cfg['I18N_FUNCTION_CALL'], ['N_', 'V_', 'T_'])
+
+
+def suite():
+  result = unittest.TestSuite()
+  result.addTests(unittest.makeSuite(UtilsTest))
+  result.addTests(unittest.makeSuite(PredefinedStylesByNameTest))
+  result.addTests(unittest.makeSuite(StyleFromFileTest))
+  return result
+
+
+if __name__ == '__main__':
+  unittest.main()
diff --git a/yapftests/yapf_test.py b/yapftests/yapf_test.py
index 896862c..af48e56 100644
--- a/yapftests/yapf_test.py
+++ b/yapftests/yapf_test.py
@@ -48,11 +48,13 @@ class YapfTest(unittest.TestCase):
 class CommandLineTest(unittest.TestCase):
   """Test how calling yapf from the command line acts."""
 
-  def setUp(self):
-    self.test_tmpdir = tempfile.mkdtemp()
+  @classmethod
+  def setUpClass(cls):
+    cls.test_tmpdir = tempfile.mkdtemp()
 
-  def tearDown(self):
-    shutil.rmtree(self.test_tmpdir)
+  @classmethod
+  def tearDownClass(cls):
+    shutil.rmtree(cls.test_tmpdir)
 
   def testUnicodeEncodingPipedToFile(self):
     unformatted_code = textwrap.dedent(u"""\
@@ -110,6 +112,53 @@ class CommandLineTest(unittest.TestCase):
     self.assertIsNone(stderrdata)
     self.assertEqual(reformatted_code.decode('utf-8'), expected_formatted_code)
 
+  def testSetGoogleStyle(self):
+    unformatted_code = textwrap.dedent(u"""\
+        def foo(): # trail
+            x = 37 
+        """)
+    expected_formatted_code = textwrap.dedent(u"""\
+        def foo():  # trail
+          x = 37
+        """)
+
+    p = subprocess.Popen(YAPF_BINARY + ['--style=Google'],
+                         stdout=subprocess.PIPE,
+                         stdin=subprocess.PIPE,
+                         stderr=subprocess.STDOUT)
+    reformatted_code, stderrdata = p.communicate(
+        unformatted_code.encode('utf-8'))
+    self.assertIsNone(stderrdata)
+    self.assertEqual(reformatted_code.decode('utf-8'), expected_formatted_code)
+
+  def testSetCustomStyleBasedOnGoogle(self):
+    unformatted_code = textwrap.dedent(u"""\
+        def foo(): # trail
+            x = 37 
+        """)
+    expected_formatted_code = textwrap.dedent(u"""\
+        def foo():    # trail
+          x = 37
+        """)
+
+    with tempfile.NamedTemporaryFile(dir=self.test_tmpdir, mode='w') as f:
+      f.write(textwrap.dedent('''\
+          [style]
+          based_on_style = Google
+          SPACES_BEFORE_COMMENT = 4
+          '''))
+      f.flush()
+
+      p = subprocess.Popen(YAPF_BINARY + ['--style={0}'.format(f.name)],
+                           stdout=subprocess.PIPE,
+                           stdin=subprocess.PIPE,
+                           stderr=subprocess.STDOUT)
+      reformatted_code, stderrdata = p.communicate(
+          unformatted_code.encode('utf-8'))
+      self.assertIsNone(stderrdata)
+      self.assertEqual(reformatted_code.decode('utf-8'),
+                       expected_formatted_code)
+
   def testReadSingleLineCodeFromStdin(self):
     unformatted_code = textwrap.dedent(u"""\
         if True: pass