From 61f75a9e043179f2b9224459d6ef2aa19bf5f73e Mon Sep 17 00:00:00 2001 From: Graham Lyons Date: Mon, 12 Dec 2016 17:25:02 +0000 Subject: [PATCH 1/7] Utilise autoflake's ability to remove all unused imports --- pyformat.py | 9 +++++---- test_pyformat.py | 6 ++++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/pyformat.py b/pyformat.py index c7c9b7c..528ba87 100755 --- a/pyformat.py +++ b/pyformat.py @@ -41,10 +41,10 @@ __version__ = '0.6' -def formatters(aggressive, apply_config, filename=''): +def formatters(aggressive, apply_config, filename='', remove_all_unused_imports=False): """Return list of code formatters.""" if aggressive: - yield autoflake.fix_code + yield lambda code: autoflake.fix_code(code, remove_all_unused_imports=remove_all_unused_imports) autopep8_options = autopep8.parse_args( [filename] + int(aggressive) * ['--aggressive'], apply_config=apply_config) @@ -57,11 +57,12 @@ def formatters(aggressive, apply_config, filename=''): yield unify.format_code -def format_code(source, aggressive=False, apply_config=False, filename=''): +def format_code(source, aggressive=False, apply_config=False, filename='', + remove_all_unused_imports=False): """Return formatted source code.""" formatted_source = source - for fix in formatters(aggressive, apply_config, filename): + for fix in formatters(aggressive, apply_config, filename, remove_all_unused_imports): formatted_source = fix(formatted_source) return formatted_source diff --git a/test_pyformat.py b/test_pyformat.py index c036974..2ea35a4 100755 --- a/test_pyformat.py +++ b/test_pyformat.py @@ -60,6 +60,12 @@ def test_format_code_with_unicode(self): pyformat.format_code( 'x = "abc" \\\n"ö"\n')) + def test_format_code_with_remove_all_unused_imports(self): + self.assertEqual("x = 'abc' \\\n 'ö'\n", + pyformat.format_code( + 'import os\nx = "abc" \\\n"ö"\n', aggressive=True, + remove_all_unused_imports=True)) + def test_format_multiple_files(self): with temporary_file('''\ if True: From 1bd92e6cdc687fa773313bb9e3207abeb1f18e57 Mon Sep 17 00:00:00 2001 From: Graham Lyons Date: Tue, 13 Dec 2016 16:10:55 +0000 Subject: [PATCH 2/7] Allow removal of unused imports to be specified via the CLI --- pyformat.py | 5 ++++- test_pyformat.py | 24 +++++++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/pyformat.py b/pyformat.py index 528ba87..48f80bc 100755 --- a/pyformat.py +++ b/pyformat.py @@ -85,7 +85,8 @@ def format_file(filename, args, standard_out): formatted_source = format_code(source, aggressive=args.aggressive, apply_config=args.config, - filename=filename) + filename=filename, + remove_all_unused_imports=args.remove_all_unused_imports) if source != formatted_source: if args.in_place: @@ -161,6 +162,8 @@ def parse_args(argv): help='drill down directories recursively') parser.add_argument('-a', '--aggressive', action='count', default=0, help='use more aggressive formatters') + parser.add_argument('--remove-all-unused-imports', action='store_true', + help='remove all unused imports, not just standard library') parser.add_argument('-j', '--jobs', type=int, metavar='n', default=1, help='number of parallel jobs; ' 'match CPU count if value is less than 1') diff --git a/test_pyformat.py b/test_pyformat.py index 2ea35a4..abc1af5 100755 --- a/test_pyformat.py +++ b/test_pyformat.py @@ -64,7 +64,7 @@ def test_format_code_with_remove_all_unused_imports(self): self.assertEqual("x = 'abc' \\\n 'ö'\n", pyformat.format_code( 'import os\nx = "abc" \\\n"ö"\n', aggressive=True, - remove_all_unused_imports=True)) + remove_all_unused_imports=True)) def test_format_multiple_files(self): with temporary_file('''\ @@ -309,6 +309,28 @@ def test_exclude(self): '', output_file.getvalue().strip()) + def test_remove_all_unused_imports(self): + with temporary_file("""\ +import mymodule + +def test(): + return 42 +""") as filename: + output_file = io.StringIO() + pyformat._main(argv=['my_fake_program', + '--in-place', + '--aggressive', + '--remove-all-unused-imports', + filename], + standard_out=output_file, + standard_error=None) + with open(filename) as f: + self.assertEqual('''\ + +def test(): + return 42 +''', f.read()) + def test_end_to_end(self): with temporary_file("""\ import os From 38375b030de401cdc2cb4810c6d9e82f07cd35e2 Mon Sep 17 00:00:00 2001 From: Graham Lyons Date: Tue, 13 Dec 2016 16:12:34 +0000 Subject: [PATCH 3/7] The os module would have been removed anyway - make this test valid --- test_pyformat.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_pyformat.py b/test_pyformat.py index abc1af5..639b6be 100755 --- a/test_pyformat.py +++ b/test_pyformat.py @@ -63,7 +63,7 @@ def test_format_code_with_unicode(self): def test_format_code_with_remove_all_unused_imports(self): self.assertEqual("x = 'abc' \\\n 'ö'\n", pyformat.format_code( - 'import os\nx = "abc" \\\n"ö"\n', aggressive=True, + 'import mymodule\nx = "abc" \\\n"ö"\n', aggressive=True, remove_all_unused_imports=True)) def test_format_multiple_files(self): From 3ee87e3d1e6b112d2f7c2494be173bab392f3607 Mon Sep 17 00:00:00 2001 From: Graham Lyons Date: Tue, 13 Dec 2016 16:39:57 +0000 Subject: [PATCH 4/7] Allow unused variables to be removed --- pyformat.py | 14 ++++++++++---- test_pyformat.py | 6 ++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/pyformat.py b/pyformat.py index 48f80bc..015c199 100755 --- a/pyformat.py +++ b/pyformat.py @@ -41,10 +41,14 @@ __version__ = '0.6' -def formatters(aggressive, apply_config, filename='', remove_all_unused_imports=False): +def formatters(aggressive, apply_config, filename='', remove_all_unused_imports=False, + remove_unused_variables=False): """Return list of code formatters.""" if aggressive: - yield lambda code: autoflake.fix_code(code, remove_all_unused_imports=remove_all_unused_imports) + yield lambda code: autoflake.fix_code( + code, + remove_all_unused_imports=remove_all_unused_imports, + remove_unused_variables=remove_unused_variables) autopep8_options = autopep8.parse_args( [filename] + int(aggressive) * ['--aggressive'], apply_config=apply_config) @@ -58,11 +62,13 @@ def formatters(aggressive, apply_config, filename='', remove_all_unused_imports= def format_code(source, aggressive=False, apply_config=False, filename='', - remove_all_unused_imports=False): + remove_all_unused_imports=False, remove_unused_variables=False): """Return formatted source code.""" formatted_source = source - for fix in formatters(aggressive, apply_config, filename, remove_all_unused_imports): + for fix in formatters( + aggressive, apply_config, filename, + remove_all_unused_imports, remove_unused_variables): formatted_source = fix(formatted_source) return formatted_source diff --git a/test_pyformat.py b/test_pyformat.py index 639b6be..9ac55a8 100755 --- a/test_pyformat.py +++ b/test_pyformat.py @@ -66,6 +66,12 @@ def test_format_code_with_remove_all_unused_imports(self): 'import mymodule\nx = "abc" \\\n"ö"\n', aggressive=True, remove_all_unused_imports=True)) + def test_format_code_with_remove_unused_variables(self): + self.assertEqual("def test():\n return 42\n", + pyformat.format_code( + 'def test():\n x = 4\n return 42', aggressive=True, + remove_unused_variables=True)) + def test_format_multiple_files(self): with temporary_file('''\ if True: From f096e99aee678fbc1a037a41297ce720ef878db7 Mon Sep 17 00:00:00 2001 From: Graham Lyons Date: Tue, 13 Dec 2016 16:45:44 +0000 Subject: [PATCH 5/7] Hook remove-unused-variables up to the CLI --- pyformat.py | 7 +++++-- test_pyformat.py | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/pyformat.py b/pyformat.py index 015c199..218ad51 100755 --- a/pyformat.py +++ b/pyformat.py @@ -92,7 +92,8 @@ def format_file(filename, args, standard_out): aggressive=args.aggressive, apply_config=args.config, filename=filename, - remove_all_unused_imports=args.remove_all_unused_imports) + remove_all_unused_imports=args.remove_all_unused_imports, + remove_unused_variables=args.remove_unused_variables) if source != formatted_source: if args.in_place: @@ -169,7 +170,9 @@ def parse_args(argv): parser.add_argument('-a', '--aggressive', action='count', default=0, help='use more aggressive formatters') parser.add_argument('--remove-all-unused-imports', action='store_true', - help='remove all unused imports, not just standard library') + help='remove all unused imports, not just standard library (requires "aggresive")') + parser.add_argument('--remove-unused-variables', action='store_true', + help='remove unused variables (requires "aggresive")') parser.add_argument('-j', '--jobs', type=int, metavar='n', default=1, help='number of parallel jobs; ' 'match CPU count if value is less than 1') diff --git a/test_pyformat.py b/test_pyformat.py index 9ac55a8..d8f6547 100755 --- a/test_pyformat.py +++ b/test_pyformat.py @@ -333,6 +333,26 @@ def test(): with open(filename) as f: self.assertEqual('''\ +def test(): + return 42 +''', f.read()) + + def test_remove_unused_variables(self): + with temporary_file("""\ +def test(): + x = 43 + return 42 +""") as filename: + output_file = io.StringIO() + pyformat._main(argv=['my_fake_program', + '--in-place', + '--aggressive', + '--remove-unused-variables', + filename], + standard_out=output_file, + standard_error=None) + with open(filename) as f: + self.assertEqual('''\ def test(): return 42 ''', f.read()) From 76948a0a428e4d397452635eabf37605b5bdfcd9 Mon Sep 17 00:00:00 2001 From: Graham Lyons Date: Tue, 13 Dec 2016 16:54:07 +0000 Subject: [PATCH 6/7] Shorten lines flagged by flake8 --- pyformat.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/pyformat.py b/pyformat.py index 218ad51..c39d546 100755 --- a/pyformat.py +++ b/pyformat.py @@ -41,8 +41,8 @@ __version__ = '0.6' -def formatters(aggressive, apply_config, filename='', remove_all_unused_imports=False, - remove_unused_variables=False): +def formatters(aggressive, apply_config, filename='', + remove_all_unused_imports=False, remove_unused_variables=False): """Return list of code formatters.""" if aggressive: yield lambda code: autoflake.fix_code( @@ -62,7 +62,8 @@ def formatters(aggressive, apply_config, filename='', remove_all_unused_imports= def format_code(source, aggressive=False, apply_config=False, filename='', - remove_all_unused_imports=False, remove_unused_variables=False): + remove_all_unused_imports=False, + remove_unused_variables=False): """Return formatted source code.""" formatted_source = source @@ -88,12 +89,13 @@ def format_file(filename, args, standard_out): if not source: return False - formatted_source = format_code(source, - aggressive=args.aggressive, - apply_config=args.config, - filename=filename, - remove_all_unused_imports=args.remove_all_unused_imports, - remove_unused_variables=args.remove_unused_variables) + formatted_source = format_code( + source, + aggressive=args.aggressive, + apply_config=args.config, + filename=filename, + remove_all_unused_imports=args.remove_all_unused_imports, + remove_unused_variables=args.remove_unused_variables) if source != formatted_source: if args.in_place: @@ -170,7 +172,8 @@ def parse_args(argv): parser.add_argument('-a', '--aggressive', action='count', default=0, help='use more aggressive formatters') parser.add_argument('--remove-all-unused-imports', action='store_true', - help='remove all unused imports, not just standard library (requires "aggresive")') + help='remove all unused imports, not just standard library \ +(requires "aggresive")') parser.add_argument('--remove-unused-variables', action='store_true', help='remove unused variables (requires "aggresive")') parser.add_argument('-j', '--jobs', type=int, metavar='n', default=1, From a7da9342242e5dead4b522425048211f33c17f05 Mon Sep 17 00:00:00 2001 From: Graham Lyons Date: Wed, 14 Dec 2016 13:52:04 +0000 Subject: [PATCH 7/7] Prompt user when options requiring --aggressive are used --- pyformat.py | 13 ++++++++++--- test_pyformat.py | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/pyformat.py b/pyformat.py index c39d546..58d8dca 100755 --- a/pyformat.py +++ b/pyformat.py @@ -172,10 +172,11 @@ def parse_args(argv): parser.add_argument('-a', '--aggressive', action='count', default=0, help='use more aggressive formatters') parser.add_argument('--remove-all-unused-imports', action='store_true', - help='remove all unused imports, not just standard library \ -(requires "aggresive")') + help='remove all unused imports, ' + 'not just standard library ' + '(requires "aggressive")') parser.add_argument('--remove-unused-variables', action='store_true', - help='remove unused variables (requires "aggresive")') + help='remove unused variables (requires "aggressive")') parser.add_argument('-j', '--jobs', type=int, metavar='n', default=1, help='number of parallel jobs; ' 'match CPU count if value is less than 1') @@ -201,6 +202,12 @@ def parse_args(argv): import multiprocessing args.jobs = multiprocessing.cpu_count() + if args.remove_all_unused_imports and not args.aggressive: + parser.error('--remove-all-unused-imports requires --aggressive') + + if args.remove_unused_variables and not args.aggressive: + parser.error('--remove-unused-variables requires --aggressive') + return args diff --git a/test_pyformat.py b/test_pyformat.py index d8f6547..feec01f 100755 --- a/test_pyformat.py +++ b/test_pyformat.py @@ -254,6 +254,22 @@ def test_jobs_less_than_one_should_default_to_cpu_count(self): self.assertGreater(args.jobs, 0) + def test_remove_all_unused_imports_requires_aggressive(self): + _stderr = sys.stderr + sys.stderr = io.StringIO() + self.assertRaises( + SystemExit, pyformat.parse_args, + ['my_fake_program', '--remove-all-unused-imports', __file__]) + sys.stderr = _stderr + + def test_remove_unused_variables_requires_aggressive(self): + _stderr = sys.stderr + sys.stderr = io.StringIO() + self.assertRaises( + SystemExit, pyformat.parse_args, + ['my_fake_program', '--remove-unused-variables', __file__]) + sys.stderr = _stderr + def test_ignore_hidden_directories(self): with temporary_directory() as directory: with temporary_directory(prefix='.',