From 7b3f0bbb979a24163d81020feae537b3a02f49f4 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Fri, 16 Sep 2022 18:03:58 +0100 Subject: [PATCH 1/7] Fix tests for E265 and E266 to be the right way around E265 is "block comment should start with '# '" E266 is "too many leading '#' for block comment" --- test/test_autopep8.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/test_autopep8.py b/test/test_autopep8.py index f98c92de..6a42765a 100755 --- a/test/test_autopep8.py +++ b/test/test_autopep8.py @@ -2227,14 +2227,14 @@ def test_e262_more_complex(self): self.assertEqual(fixed, result) def test_e265(self): - line = "## comment\n123\n" - fixed = "# comment\n123\n" + line = "#A comment\n123\n" + fixed = "# A comment\n123\n" with autopep8_context(line) as result: self.assertEqual(fixed, result) def test_e266(self): - line = "#1 comment\n123\n" - fixed = "# 1 comment\n123\n" + line = "## comment\n123\n" + fixed = "# comment\n123\n" with autopep8_context(line) as result: self.assertEqual(fixed, result) From 1ad18df567d411e6ef1182d55a1be2530406f8b3 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Fri, 16 Sep 2022 18:41:05 +0100 Subject: [PATCH 2/7] Rename E266 fixer to have the right name It still ends up actually fixing E265 as well (it always did both) however that one is going to be easier to reproduce as a standard fixer. --- autopep8.py | 2 +- test/test_autopep8.py | 22 +++++++++++----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/autopep8.py b/autopep8.py index 87acfa19..5143f3a7 100755 --- a/autopep8.py +++ b/autopep8.py @@ -1685,7 +1685,7 @@ def split_and_strip_non_empty_lines(text): return [line.strip() for line in text.splitlines() if line.strip()] -def fix_e265(source, aggressive=False): # pylint: disable=unused-argument +def fix_e266(source, aggressive=False): # pylint: disable=unused-argument """Format block comments.""" if '#' not in source: # Optimization. diff --git a/test/test_autopep8.py b/test/test_autopep8.py index 6a42765a..33bc0c38 100755 --- a/test/test_autopep8.py +++ b/test/test_autopep8.py @@ -202,23 +202,23 @@ def test_shorten_comment_should_not_modify_special_comments(self): def test_format_block_comments(self): self.assertEqual( '# abc', - autopep8.fix_e265('#abc')) + autopep8.fix_e266('#abc')) self.assertEqual( '# abc', - autopep8.fix_e265('####abc')) + autopep8.fix_e266('####abc')) self.assertEqual( '# abc', - autopep8.fix_e265('## # ##abc')) + autopep8.fix_e266('## # ##abc')) self.assertEqual( '# abc "# noqa"', - autopep8.fix_e265('# abc "# noqa"')) + autopep8.fix_e266('# abc "# noqa"')) self.assertEqual( '# *abc', - autopep8.fix_e265('#*abc')) + autopep8.fix_e266('#*abc')) def test_format_block_comments_should_leave_outline_alone(self): line = """\ @@ -226,14 +226,14 @@ def test_format_block_comments_should_leave_outline_alone(self): ## Some people like these crazy things. So leave them alone. ## ################################################################### """ - self.assertEqual(line, autopep8.fix_e265(line)) + self.assertEqual(line, autopep8.fix_e266(line)) line = """\ ################################################################# # Some people like these crazy things. So leave them alone. # ################################################################# """ - self.assertEqual(line, autopep8.fix_e265(line)) + self.assertEqual(line, autopep8.fix_e266(line)) def test_format_block_comments_with_multiple_lines(self): self.assertEqual( @@ -247,7 +247,7 @@ def test_format_block_comments_with_multiple_lines(self): #do not modify strings''' # """, - autopep8.fix_e265("""\ + autopep8.fix_e266("""\ # abc #blah blah #four space indentation @@ -261,17 +261,17 @@ def test_format_block_comments_with_multiple_lines(self): def test_format_block_comments_should_not_corrupt_special_comments(self): self.assertEqual( '#: abc', - autopep8.fix_e265('#: abc')) + autopep8.fix_e266('#: abc')) self.assertEqual( '#!/bin/bash\n', - autopep8.fix_e265('#!/bin/bash\n')) + autopep8.fix_e266('#!/bin/bash\n')) def test_format_block_comments_should_only_touch_real_comments(self): commented_out_code = '#x = 1' self.assertEqual( commented_out_code, - autopep8.fix_e265(commented_out_code)) + autopep8.fix_e266(commented_out_code)) def test_fix_file(self): self.assertIn( From 2fbeaaa58c7d7b92d930231f13eb6c6a22830397 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Fri, 16 Sep 2022 18:44:08 +0100 Subject: [PATCH 3/7] Implement a dedicated fixer for E265 The E266 fixer will soon only fix E266, so E265 will need its own handling. --- autopep8.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/autopep8.py b/autopep8.py index 5143f3a7..0c185d0c 100755 --- a/autopep8.py +++ b/autopep8.py @@ -809,7 +809,7 @@ def fix_e251(self, result): self.source[result['line'] - 1] = fixed def fix_e262(self, result): - """Fix spacing after comment hash.""" + """Fix spacing after inline comment hash.""" target = self.source[result['line'] - 1] offset = result['column'] @@ -820,6 +820,17 @@ def fix_e262(self, result): self.source[result['line'] - 1] = fixed + def fix_e265(self, result): + """Fix spacing after block comment hash.""" + target = self.source[result['line'] - 1] + + indent = _get_indentation(target) + comment = target.lstrip(' \t#') + + fixed = indent + ('# ' + comment if comment.strip() else '\n') + + self.source[result['line'] - 1] = fixed + def fix_e271(self, result): """Fix extraneous whitespace around keywords.""" line_index = result['line'] - 1 From 7c10dd336b138353f263f6a01815251b386120a7 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Fri, 16 Sep 2022 19:12:20 +0100 Subject: [PATCH 4/7] Route testing of block comment fixes through the normal test utils This detatches this from the implementation function, clearing the way for changing the implementation. --- test/test_autopep8.py | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/test/test_autopep8.py b/test/test_autopep8.py index 33bc0c38..7db4a246 100755 --- a/test/test_autopep8.py +++ b/test/test_autopep8.py @@ -202,23 +202,23 @@ def test_shorten_comment_should_not_modify_special_comments(self): def test_format_block_comments(self): self.assertEqual( '# abc', - autopep8.fix_e266('#abc')) + fix_e265_and_e266('#abc')) self.assertEqual( '# abc', - autopep8.fix_e266('####abc')) + fix_e265_and_e266('####abc')) self.assertEqual( '# abc', - autopep8.fix_e266('## # ##abc')) + fix_e265_and_e266('## # ##abc')) self.assertEqual( '# abc "# noqa"', - autopep8.fix_e266('# abc "# noqa"')) + fix_e265_and_e266('# abc "# noqa"')) self.assertEqual( '# *abc', - autopep8.fix_e266('#*abc')) + fix_e265_and_e266('#*abc')) def test_format_block_comments_should_leave_outline_alone(self): line = """\ @@ -226,14 +226,14 @@ def test_format_block_comments_should_leave_outline_alone(self): ## Some people like these crazy things. So leave them alone. ## ################################################################### """ - self.assertEqual(line, autopep8.fix_e266(line)) + self.assertEqual(line, fix_e265_and_e266(line)) line = """\ ################################################################# # Some people like these crazy things. So leave them alone. # ################################################################# """ - self.assertEqual(line, autopep8.fix_e266(line)) + self.assertEqual(line, fix_e265_and_e266(line)) def test_format_block_comments_with_multiple_lines(self): self.assertEqual( @@ -247,7 +247,7 @@ def test_format_block_comments_with_multiple_lines(self): #do not modify strings''' # """, - autopep8.fix_e266("""\ + fix_e265_and_e266("""\ # abc #blah blah #four space indentation @@ -261,17 +261,17 @@ def test_format_block_comments_with_multiple_lines(self): def test_format_block_comments_should_not_corrupt_special_comments(self): self.assertEqual( '#: abc', - autopep8.fix_e266('#: abc')) + fix_e265_and_e266('#: abc')) self.assertEqual( '#!/bin/bash\n', - autopep8.fix_e266('#!/bin/bash\n')) + fix_e265_and_e266('#!/bin/bash\n')) def test_format_block_comments_should_only_touch_real_comments(self): commented_out_code = '#x = 1' self.assertEqual( commented_out_code, - autopep8.fix_e266(commented_out_code)) + fix_e265_and_e266(commented_out_code)) def test_fix_file(self): self.assertIn( @@ -7506,6 +7506,11 @@ def test_e501_experimental_with_in(self): self.assertEqual(fixed, result) +def fix_e265_and_e266(source): + with autopep8_context(source, options=['--select=E265,E266']) as result: + return result + + @contextlib.contextmanager def autopep8_context(line, options=None): if not options: From a029be947094f556464842e66c49ba934aeeb886 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Fri, 16 Sep 2022 19:29:11 +0100 Subject: [PATCH 5/7] Convert the E266 fixer to be a more usual one This still ends up fixing cases of E265 which overlap with E266 but doesn't go out of its way to do so if E265 is disabled. This ought to be fine since pycodestyle doesn't seem to ever complain about E266 on any lines affected by E265. --- autopep8.py | 55 +++++++++++++---------------------------------------- 1 file changed, 13 insertions(+), 42 deletions(-) diff --git a/autopep8.py b/autopep8.py index 0c185d0c..b9d3efb0 100755 --- a/autopep8.py +++ b/autopep8.py @@ -831,6 +831,19 @@ def fix_e265(self, result): self.source[result['line'] - 1] = fixed + def fix_e266(self, result): + """Fix too many block comment hashes.""" + target = self.source[result['line'] - 1] + + # Leave stylistic outlined blocks alone. + if target.strip().endswith('#'): + return + + indentation = _get_indentation(target) + fixed = indentation + '# ' + target.lstrip('# \t') + + self.source[result['line'] - 1] = fixed + def fix_e271(self, result): """Fix extraneous whitespace around keywords.""" line_index = result['line'] - 1 @@ -1696,48 +1709,6 @@ def split_and_strip_non_empty_lines(text): return [line.strip() for line in text.splitlines() if line.strip()] -def fix_e266(source, aggressive=False): # pylint: disable=unused-argument - """Format block comments.""" - if '#' not in source: - # Optimization. - return source - - ignored_line_numbers = multiline_string_lines( - source, - include_docstrings=True) | set(commented_out_code_lines(source)) - - fixed_lines = [] - sio = io.StringIO(source) - for (line_number, line) in enumerate(sio.readlines(), start=1): - if ( - line.lstrip().startswith('#') and - line_number not in ignored_line_numbers and - not pycodestyle.noqa(line) - ): - indentation = _get_indentation(line) - line = line.lstrip() - - # Normalize beginning if not a shebang. - if len(line) > 1: - pos = next((index for index, c in enumerate(line) - if c != '#')) - if ( - # Leave multiple spaces like '# ' alone. - (line[:pos].count('#') > 1 or line[1].isalnum() or - not line[1].isspace()) and - line[1] not in ':!' and - # Leave stylistic outlined blocks alone. - not line.rstrip().endswith('#') - ): - line = '# ' + line.lstrip('# \t') - - fixed_lines.append(indentation + line) - else: - fixed_lines.append(line) - - return ''.join(fixed_lines) - - def refactor(source, fixer_names, ignore=None, filename=''): """Return refactored code using lib2to3. From 7af08a3cb13c43efdf017dfd65a74e6173e1dc6c Mon Sep 17 00:00:00 2001 From: Peter Law Date: Fri, 16 Sep 2022 19:50:38 +0100 Subject: [PATCH 6/7] Teach the E265 fixer to fix only E265 Previously it would end up changing E266 as well, which the user may not want it to do (for example if they've disabled E266). This also adds a battery of tests to excercise the differences between these errors. --- autopep8.py | 7 +++++-- test/test_autopep8.py | 24 ++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/autopep8.py b/autopep8.py index b9d3efb0..21990159 100755 --- a/autopep8.py +++ b/autopep8.py @@ -825,9 +825,12 @@ def fix_e265(self, result): target = self.source[result['line'] - 1] indent = _get_indentation(target) - comment = target.lstrip(' \t#') + line = target.lstrip(' \t') + pos = next((index for index, c in enumerate(line) if c != '#')) + hashes = line[:pos] + comment = line[pos:].lstrip(' \t') - fixed = indent + ('# ' + comment if comment.strip() else '\n') + fixed = indent + hashes + (' ' + comment if comment.strip() else '\n') self.source[result['line'] - 1] = fixed diff --git a/test/test_autopep8.py b/test/test_autopep8.py index 7db4a246..7aa8ca0a 100755 --- a/test/test_autopep8.py +++ b/test/test_autopep8.py @@ -2232,12 +2232,36 @@ def test_e265(self): with autopep8_context(line) as result: self.assertEqual(fixed, result) + def test_e265_only(self): + line = "##A comment\n#B comment\n123\n" + fixed = "## A comment\n# B comment\n123\n" + with autopep8_context(line, options=['--select=E265']) as result: + self.assertEqual(fixed, result) + + def test_ignore_e265(self): + line = "## A comment\n#B comment\n123\n" + fixed = "# A comment\n#B comment\n123\n" + with autopep8_context(line, options=['--ignore=E265']) as result: + self.assertEqual(fixed, result) + def test_e266(self): line = "## comment\n123\n" fixed = "# comment\n123\n" with autopep8_context(line) as result: self.assertEqual(fixed, result) + def test_e266_only(self): + line = "## A comment\n#B comment\n123\n" + fixed = "# A comment\n#B comment\n123\n" + with autopep8_context(line, options=['--select=E266']) as result: + self.assertEqual(fixed, result) + + def test_ignore_e266(self): + line = "##A comment\n#B comment\n123\n" + fixed = "## A comment\n# B comment\n123\n" + with autopep8_context(line, options=['--ignore=E266']) as result: + self.assertEqual(fixed, result) + def test_e271(self): line = 'True and False\n' fixed = 'True and False\n' From f4a40b13ba9efc7dba803c662472c09f8070b9b2 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Fri, 16 Sep 2022 20:32:16 +0100 Subject: [PATCH 7/7] Teach E265 fixer to ignore special comments even in the middle of files This appears to be expected for compatibility, though is a somewhat unexpected behaviour given that these comments are almost certainly not actually 'shebang' comments. --- autopep8.py | 4 ++++ test/test_autopep8.py | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/autopep8.py b/autopep8.py index 21990159..d856f711 100755 --- a/autopep8.py +++ b/autopep8.py @@ -830,6 +830,10 @@ def fix_e265(self, result): hashes = line[:pos] comment = line[pos:].lstrip(' \t') + # Ignore special comments, even in the middle of the file. + if comment.startswith('!'): + return + fixed = indent + hashes + (' ' + comment if comment.strip() else '\n') self.source[result['line'] - 1] = fixed diff --git a/test/test_autopep8.py b/test/test_autopep8.py index 7aa8ca0a..d54423f5 100755 --- a/test/test_autopep8.py +++ b/test/test_autopep8.py @@ -2232,6 +2232,16 @@ def test_e265(self): with autopep8_context(line) as result: self.assertEqual(fixed, result) + def test_e265_ignores_special_comments(self): + line = "#!python\n456\n" + with autopep8_context(line) as result: + self.assertEqual(line, result) + + def test_e265_ignores_special_comments_in_middle_of_file(self): + line = "123\n\n#!python\n456\n" + with autopep8_context(line) as result: + self.assertEqual(line, result) + def test_e265_only(self): line = "##A comment\n#B comment\n123\n" fixed = "## A comment\n# B comment\n123\n"