Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix performance issue due to search in tokens #210

Merged
merged 6 commits into from
Jun 22, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 22 additions & 27 deletions flake8_eradicate.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,9 @@ class Checker(object):

options = None

def __init__(self, physical_line, tokens) -> None:
"""
Creates new checker instance.
def __init__(self, tree, filename: str):
self.filename = filename

When performance will be an issue - we can refactor it.
"""
self._physical_line = physical_line
self._tokens = tokens
self._options = {
'aggressive': self.options.eradicate_aggressive, # type: ignore
}
Expand Down Expand Up @@ -103,14 +98,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) -> Iterable[Tuple[int, str]]:
"""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.
Expand All @@ -121,19 +116,19 @@ 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'],
),
)
return self._physical_line != filtered_source
return False
with open(self.filename) as f:
bagerard marked this conversation as resolved.
Show resolved Hide resolved
file_tokens = tokenize.generate_tokens(f.readline)
comment_in_file = any(token.type == tokenize.COMMENT for token in file_tokens)

if comment_in_file:
f.seek(0) # rewind file
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
35 changes: 26 additions & 9 deletions tests/test_comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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


Expand All @@ -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
Expand All @@ -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
Expand All @@ -120,8 +123,22 @@ 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):
bagerard marked this conversation as resolved.
Show resolved Hide resolved
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]