fix performance issue due to search in tokens

flake8_eradicate.py

@@ -1,7 +1,8 @@
 # -*- coding: utf-8 -*-
 
+import ast
 import tokenize
-from typing import Iterable, Tuple
+from typing import Iterable, Iterator, Tuple, Type
 
 import pkg_resources
 from eradicate import Eradicator
@@ -25,14 +26,17 @@ class Checker(object):
 
     options = None
 
-    def __init__(self, physical_line, tokens) -> None:
+    def __init__(self, tree: ast.AST, filename: str):
         """
-        Creates new checker instance.
+        ``flake8`` plugin constructor.
+
+        Arguments:
+            tree: the file abstract syntax tree.
+            filename: the name of the file to process
 
-        When performance will be an issue - we can refactor it.
         """
-        self._physical_line = physical_line
-        self._tokens = tokens
+        self.filename = filename
+
         self._options = {
             'aggressive': self.options.eradicate_aggressive,  # type: ignore
         }
@@ -103,14 +107,14 @@ def parse_options(cls, options) -> None:
         """Parses registered options for providing them to each visitor."""
         cls.options = options
 
-    def __iter__(self) -> Iterable[Tuple[int, str]]:
+    def run(self) -> Iterator[Tuple[int, int, str, Type['Checker']]]:
         """Runs on each step of flake8."""
-        if self._contains_commented_out_code():
-            yield (1, self._error_template)
+        for line_no in self._lines_with_commented_out_code():
+            yield line_no, 0, self._error_template, type(self)
 
-    def _contains_commented_out_code(self) -> bool:
+    def _lines_with_commented_out_code(self) -> Iterable[int]:
         """
-        Check if the current physical line contains commented out code.
+        Yield the physical line number that contain commented out code.
 
         This test relies on eradicate function to remove commented out code
         from a physical line.
@@ -121,19 +125,28 @@ def _contains_commented_out_code(self) -> bool:
         To prevent this false-positive, the tokens of the physical line are
         checked for a comment. The eradicate function is only invokes,
         when the tokens indicate a comment in the physical line.
-
         """
-        comment_in_line = any(
-            token_type == tokenize.COMMENT
-            for token_type, _, _, _, _ in self._tokens
-        )
-
-        if comment_in_line:
-            filtered_source = ''.join(
-                self._eradicator.filter_commented_out_code(
-                    self._physical_line,
-                    self._options['aggressive'],
-                ),
+        with open(self.filename) as f:
+            # Skip python commented encoding line
+            first_line = f.readline()
+            if not first_line.startswith('# -*- coding:'):
+                # rewind as we don't want to skip it during tokenization
+                f.seek(0)
+
+            file_tokens = tokenize.generate_tokens(f.readline)
+            comment_in_file = any(
+                token.type == tokenize.COMMENT
+                for token in file_tokens
             )
-            return self._physical_line != filtered_source
-        return False
+
+            if comment_in_file:
+                f.seek(0)
+                for line_no, line in enumerate(f.readlines(), start=1):
+                    filtered_source = ''.join(
+                        self._eradicator.filter_commented_out_code(
+                            line,
+                            self._options['aggressive'],
+                        ),
+                    )
+                    if line != filtered_source:
+                        yield line_no

tests/fixtures/correct_no_comment.py

@@ -0,0 +1,16 @@
+# -*- coding: utf-8 -*-
+
+class Some(object):
+
+    property_name = 1
+
+    other_property: int = 2
+
+    def some_method(self) -> None:
+        print('not True and not False')
+
+        print(12 + 23 / 3)
+
+
+def some_function():
+    return "something"

tests/test_comments.py

@@ -2,8 +2,11 @@
 
 import subprocess
 import sys
+from collections import namedtuple
 
-PY36 = sys.version_info >= (3, 6)
+from flake8_eradicate import Checker
+
+PY_GTE_36 = sys.version_info >= (3, 6)
 
 
 def test_correct_fixture(absolute_path):
@@ -43,9 +46,9 @@ def test_incorrect_fixture(absolute_path):
     )
     stdout, _ = process.communicate()
 
-    assert stdout.count(b'E800') == 6 + int(PY36)
+    assert stdout.count(b'E800') == 6 + int(PY_GTE_36)
     assert b'# property_name = 1' in stdout
-    if PY36:
+    if PY_GTE_36:
         assert b'# typed_property: int = 10' in stdout
 
 
@@ -66,9 +69,9 @@ def test_incorrect_fixture_aggressive(absolute_path):
         stderr=subprocess.PIPE,
     )
     stdout, _ = process.communicate()
-    assert stdout.count(b'E800') == 12 + int(PY36)
+    assert stdout.count(b'E800') == 12 + int(PY_GTE_36)
     assert b'# property_name = 1' in stdout
-    if PY36:
+    if PY_GTE_36:
         assert b'# typed_property: int = 10' in stdout
     assert b'# def function_name():' in stdout
     assert b'# class CommentedClass(object):' in stdout
@@ -93,9 +96,9 @@ def test_incorrect_fixture_whitelist(absolute_path):
     )
     stdout, _ = process.communicate()
 
-    assert stdout.count(b'E800') == 6 + int(PY36) * 3
+    assert stdout.count(b'E800') == 6 + int(PY_GTE_36) * 3
     assert b'# property_name = 1' in stdout
-    if PY36:
+    if PY_GTE_36:
         assert b'# typed_property: int = 10' in stdout
         assert b'# fmt: on' in stdout
         assert b'# fmt: off' in stdout
@@ -120,8 +123,51 @@ def test_incorrect_fixture_whitelist_extend(absolute_path):
     )
     stdout, _ = process.communicate()
 
-    assert stdout.count(b'E800') == 3 + int(PY36)
+    assert stdout.count(b'E800') == 3 + int(PY_GTE_36)
     assert b'# property_name = 1' in stdout
     assert b'return' not in stdout
-    if PY36:
+    if PY_GTE_36:
         assert b'# typed_property: int = 10' in stdout
+
+
+def test_lines_with_commented_out_code_incorrect_fixture_output(absolute_path):
+    """Verify central underlying method is returning correct output."""
+    filename = absolute_path('fixtures', 'incorrect.py')
+
+    OptionsStub = namedtuple(
+        'Options',
+        'eradicate_aggressive eradicate_whitelist eradicate_whitelist_extend',
+    )
+    Checker.options = OptionsStub(
+        eradicate_aggressive=True,
+        eradicate_whitelist=False,
+        eradicate_whitelist_extend=False,
+    )
+
+    checker = Checker(tree=None, filename=filename)
+    output = list(checker._lines_with_commented_out_code())
+    if PY_GTE_36:
+        assert output == [3, 4, 9, 10, 14, 15, 16, 18, 19, 21, 22, 24, 25]
+    else:
+        assert output == [3, 9, 10, 14, 15, 16, 18, 19, 21, 22, 24, 25]
+
+
+def test_lines_with_commented_out_code_file_without_comment_output(
+    absolute_path,
+):
+    """Make sure file without comment are ignored."""
+    filename = absolute_path('fixtures', 'correct_no_comment.py')
+
+    OptionsStub = namedtuple(
+        'Options',
+        'eradicate_aggressive eradicate_whitelist eradicate_whitelist_extend',
+    )
+    Checker.options = OptionsStub(
+        eradicate_aggressive=True,
+        eradicate_whitelist=False,
+        eradicate_whitelist_extend=False,
+    )
+
+    checker = Checker(tree=None, filename=filename)
+    output = list(checker._lines_with_commented_out_code())
+    assert output == []