Fix E265 and E266 confusion and overlap

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,37 @@ 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)
+        line = target.lstrip(' \t')
+        pos = next((index for index, c in enumerate(line) if c != '#'))
+        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
+
+    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
@@ -1685,48 +1716,6 @@ 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
-    """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.
 

test/test_autopep8.py

@@ -202,38 +202,38 @@ def test_shorten_comment_should_not_modify_special_comments(self):
     def test_format_block_comments(self):
         self.assertEqual(
             '# abc',
-            autopep8.fix_e265('#abc'))
+            fix_e265_and_e266('#abc'))
 
         self.assertEqual(
             '# abc',
-            autopep8.fix_e265('####abc'))
+            fix_e265_and_e266('####abc'))
 
         self.assertEqual(
             '# abc',
-            autopep8.fix_e265('##   #   ##abc'))
+            fix_e265_and_e266('##   #   ##abc'))
 
         self.assertEqual(
             '# abc "# noqa"',
-            autopep8.fix_e265('# abc "# noqa"'))
+            fix_e265_and_e266('# abc "# noqa"'))
 
         self.assertEqual(
             '# *abc',
-            autopep8.fix_e265('#*abc'))
+            fix_e265_and_e266('#*abc'))
 
     def test_format_block_comments_should_leave_outline_alone(self):
         line = """\
 ###################################################################
 ##   Some people like these crazy things. So leave them alone.   ##
 ###################################################################
 """
-        self.assertEqual(line, autopep8.fix_e265(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_e265(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_e265("""\
+            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_e265('#: abc'))
+            fix_e265_and_e266('#: abc'))
 
         self.assertEqual(
             '#!/bin/bash\n',
-            autopep8.fix_e265('#!/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_e265(commented_out_code))
+            fix_e265_and_e266(commented_out_code))
 
     def test_fix_file(self):
         self.assertIn(
@@ -2227,17 +2227,51 @@ 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_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"
+        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 = "#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)
 
+    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'
@@ -7506,6 +7540,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: