From 92c367dee4e5908745822902adfda0bd915b1380 Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Sun, 4 Jun 2017 07:57:28 -0500 Subject: [PATCH] Rename style_guide.Error to style_guide.Violation Move all Violation related methods from the StyleGuide to our Violation class. --- src/flake8/formatting/base.py | 15 +- src/flake8/statistics.py | 9 +- src/flake8/style_guide.py | 177 +++++++++++++--------- tests/unit/test_base_formatter.py | 8 +- tests/unit/test_filenameonly_formatter.py | 7 +- tests/unit/test_nothing_formatter.py | 4 +- tests/unit/test_statistics.py | 2 +- tests/unit/test_style_guide.py | 44 +----- tests/unit/test_violation.py | 55 +++++++ 9 files changed, 185 insertions(+), 136 deletions(-) create mode 100644 tests/unit/test_violation.py diff --git a/src/flake8/formatting/base.py b/src/flake8/formatting/base.py index abad254..c4c67d5 100644 --- a/src/flake8/formatting/base.py +++ b/src/flake8/formatting/base.py @@ -78,9 +78,10 @@ class BaseFormatter(object): method. :param error: - This will be an instance of :class:`~flake8.style_guide.Error`. + This will be an instance of + :class:`~flake8.style_guide.Violation`. :type error: - flake8.style_guide.Error + flake8.style_guide.Violation """ line = self.format(error) source = self.show_source(error) @@ -92,9 +93,10 @@ class BaseFormatter(object): This method **must** be implemented by subclasses. :param error: - This will be an instance of :class:`~flake8.style_guide.Error`. + This will be an instance of + :class:`~flake8.style_guide.Violation`. :type error: - flake8.style_guide.Error + flake8.style_guide.Violation :returns: The formatted error string. :rtype: @@ -144,9 +146,10 @@ class BaseFormatter(object): is reported as generating the problem. :param error: - This will be an instance of :class:`~flake8.style_guide.Error`. + This will be an instance of + :class:`~flake8.style_guide.Violation`. :type error: - flake8.style_guide.Error + flake8.style_guide.Violation :returns: The formatted error string if the user wants to show the source. If the user does not want to show the source, this will return diff --git a/src/flake8/statistics.py b/src/flake8/statistics.py index d555df2..d39750a 100644 --- a/src/flake8/statistics.py +++ b/src/flake8/statistics.py @@ -23,9 +23,10 @@ class Statistics(object): """Add the fact that the error was seen in the file. :param error: - The Error instance containing the information about the violation. + The Violation instance containing the information about the + violation. :type error: - flake8.style_guide.Error + flake8.style_guide.Violation """ key = Key.create_from(error) if key not in self._store: @@ -73,7 +74,7 @@ class Key(collections.namedtuple('Key', ['filename', 'code'])): @classmethod def create_from(cls, error): - """Create a Key from :class:`flake8.style_guide.Error`.""" + """Create a Key from :class:`flake8.style_guide.Violation`.""" return cls( filename=error.filename, code=error.code, @@ -115,7 +116,7 @@ class Statistic(object): @classmethod def create_from(cls, error): - """Create a Statistic from a :class:`flake8.style_guide.Error`.""" + """Create a Statistic from a :class:`flake8.style_guide.Violation`.""" return cls( error_code=error.code, filename=error.filename, diff --git a/src/flake8/style_guide.py b/src/flake8/style_guide.py index 07d9e06..124c7d5 100644 --- a/src/flake8/style_guide.py +++ b/src/flake8/style_guide.py @@ -2,6 +2,7 @@ import collections import contextlib import enum +import functools import linecache import logging @@ -16,6 +17,17 @@ __all__ = ( LOG = logging.getLogger(__name__) +try: + lru_cache = functools.lru_cache +except AttributeError: + def lru_cache(maxsize=128, typed=False): + """Stub for missing lru_cache.""" + def fake_decorator(func): + return func + + return fake_decorator + + # TODO(sigmavirus24): Determine if we need to use enum/enum34 class Selected(enum.Enum): """Enum representing an explicitly or implicitly selected code.""" @@ -38,8 +50,13 @@ class Decision(enum.Enum): Selected = 'selected error' -Error = collections.namedtuple( - 'Error', +@lru_cache(maxsize=512) +def find_noqa(physical_line): + return defaults.NOQA_INLINE_REGEXP.search(physical_line) + + +_Violation = collections.namedtuple( + 'Violation', [ 'code', 'filename', @@ -51,6 +68,84 @@ Error = collections.namedtuple( ) +class Violation(_Violation): + """Class representing a violation reported by Flake8.""" + + def is_inline_ignored(self, disable_noqa): + # type: (Violation) -> bool + """Determine if an comment has been added to ignore this line. + + :param bool disable_noqa: + Whether or not users have provided ``--disable-noqa``. + :returns: + True if error is ignored in-line, False otherwise. + :rtype: + bool + """ + physical_line = self.physical_line + # TODO(sigmavirus24): Determine how to handle stdin with linecache + if disable_noqa: + return False + + if physical_line is None: + physical_line = linecache.getline(self.filename, + self.line_number) + noqa_match = find_noqa(physical_line) + if noqa_match is None: + LOG.debug('%r is not inline ignored', self) + return False + + codes_str = noqa_match.groupdict()['codes'] + if codes_str is None: + LOG.debug('%r is ignored by a blanket ``# noqa``', self) + return True + + codes = set(utils.parse_comma_separated_list(codes_str)) + if self.code in codes or self.code.startswith(tuple(codes)): + LOG.debug('%r is ignored specifically inline with ``# noqa: %s``', + self, codes_str) + return True + + LOG.debug('%r is not ignored inline with ``# noqa: %s``', + self, codes_str) + return False + + def is_in(self, diff): + """Determine if the violation is included in a diff's line ranges. + + This function relies on the parsed data added via + :meth:`~StyleGuide.add_diff_ranges`. If that has not been called and + we are not evaluating files in a diff, then this will always return + True. If there are diff ranges, then this will return True if the + line number in the error falls inside one of the ranges for the file + (and assuming the file is part of the diff data). If there are diff + ranges, this will return False if the file is not part of the diff + data or the line number of the error is not in any of the ranges of + the diff. + + :returns: + True if there is no diff or if the error is in the diff's line + number ranges. False if the error's line number falls outside + the diff's line number ranges. + :rtype: + bool + """ + if not diff: + return True + + # NOTE(sigmavirus24): The parsed diff will be a defaultdict with + # a set as the default value (if we have received it from + # flake8.utils.parse_unified_diff). In that case ranges below + # could be an empty set (which is False-y) or if someone else + # is using this API, it could be None. If we could guarantee one + # or the other, we would check for it more explicitly. + line_numbers = diff.get(self.filename) + if not line_numbers: + return False + + return self.line_number in line_numbers + + class DecisionEngine(object): """A class for managing the decision process around violations. @@ -127,7 +222,7 @@ class DecisionEngine(object): return Selected.Implicitly def more_specific_decision_for(self, code): - # type: (Error) -> Decision + # type: (Violation) -> Decision select = find_first_match(code, self.all_selected) extra_select = find_first_match(code, self.extended_selected) ignore = find_first_match(code, self.ignored) @@ -221,73 +316,6 @@ class StyleGuide(object): """ return self.decider.decision_for(code) - def is_inline_ignored(self, error): - # type: (Error) -> bool - """Determine if an comment has been added to ignore this line.""" - physical_line = error.physical_line - # TODO(sigmavirus24): Determine how to handle stdin with linecache - if self.options.disable_noqa: - return False - - if physical_line is None: - physical_line = linecache.getline(error.filename, - error.line_number) - noqa_match = defaults.NOQA_INLINE_REGEXP.search(physical_line) - if noqa_match is None: - LOG.debug('%r is not inline ignored', error) - return False - - codes_str = noqa_match.groupdict()['codes'] - if codes_str is None: - LOG.debug('%r is ignored by a blanket ``# noqa``', error) - return True - - codes = set(utils.parse_comma_separated_list(codes_str)) - if error.code in codes or error.code.startswith(tuple(codes)): - LOG.debug('%r is ignored specifically inline with ``# noqa: %s``', - error, codes_str) - return True - - LOG.debug('%r is not ignored inline with ``# noqa: %s``', - error, codes_str) - return False - - def is_in_diff(self, error): - # type: (Error) -> bool - """Determine if an error is included in a diff's line ranges. - - This function relies on the parsed data added via - :meth:`~StyleGuide.add_diff_ranges`. If that has not been called and - we are not evaluating files in a diff, then this will always return - True. If there are diff ranges, then this will return True if the - line number in the error falls inside one of the ranges for the file - (and assuming the file is part of the diff data). If there are diff - ranges, this will return False if the file is not part of the diff - data or the line number of the error is not in any of the ranges of - the diff. - - :returns: - True if there is no diff or if the error is in the diff's line - number ranges. False if the error's line number falls outside - the diff's line number ranges. - :rtype: - bool - """ - if not self._parsed_diff: - return True - - # NOTE(sigmavirus24): The parsed diff will be a defaultdict with - # a set as the default value (if we have received it from - # flake8.utils.parse_unified_diff). In that case ranges below - # could be an empty set (which is False-y) or if someone else - # is using this API, it could be None. If we could guarantee one - # or the other, we would check for it more explicitly. - line_numbers = self._parsed_diff.get(error.filename) - if not line_numbers: - return False - - return error.line_number in line_numbers - def handle_error(self, code, filename, line_number, column_number, text, physical_line=None): # type: (str, str, int, int, str) -> int @@ -313,17 +341,18 @@ class StyleGuide(object): :rtype: int """ + disable_noqa = self.options.disable_noqa # NOTE(sigmavirus24): Apparently we're provided with 0-indexed column # numbers so we have to offset that here. Also, if a SyntaxError is # caught, column_number may be None. if not column_number: column_number = 0 - error = Error(code, filename, line_number, column_number + 1, text, - physical_line) + error = Violation(code, filename, line_number, column_number + 1, + text, physical_line) error_is_selected = (self.should_report_error(error.code) is Decision.Selected) - is_not_inline_ignored = self.is_inline_ignored(error) is False - is_included_in_diff = self.is_in_diff(error) + is_not_inline_ignored = error.is_inline_ignored(disable_noqa) is False + is_included_in_diff = error.is_in(self._parsed_diff) if (error_is_selected and is_not_inline_ignored and is_included_in_diff): self.formatter.handle(error) diff --git a/tests/unit/test_base_formatter.py b/tests/unit/test_base_formatter.py index bb54358..87b5fee 100644 --- a/tests/unit/test_base_formatter.py +++ b/tests/unit/test_base_formatter.py @@ -51,7 +51,7 @@ def test_show_source_returns_nothing_when_not_showing_source(): """Ensure we return nothing when users want nothing.""" formatter = base.BaseFormatter(options(show_source=False)) assert formatter.show_source( - style_guide.Error('A000', 'file.py', 1, 1, 'error text', 'line') + style_guide.Violation('A000', 'file.py', 1, 1, 'error text', 'line') ) is '' @@ -59,7 +59,7 @@ def test_show_source_returns_nothing_when_there_is_source(): """Ensure we return nothing when there is no line.""" formatter = base.BaseFormatter(options(show_source=True)) assert formatter.show_source( - style_guide.Error('A000', 'file.py', 1, 1, 'error text', None) + style_guide.Violation('A000', 'file.py', 1, 1, 'error text', None) ) is '' @@ -71,7 +71,7 @@ def test_show_source_returns_nothing_when_there_is_source(): def test_show_source_updates_physical_line_appropriately(line, column): """Ensure the error column is appropriately indicated.""" formatter = base.BaseFormatter(options(show_source=True)) - error = style_guide.Error('A000', 'file.py', 1, column, 'error', line) + error = style_guide.Violation('A000', 'file.py', 1, column, 'error', line) output = formatter.show_source(error) _, pointer = output.rsplit('\n', 1) assert pointer.count(' ') == (column - 1) @@ -149,7 +149,7 @@ def test_handle_formats_the_error(): """Verify that a formatter will call format from handle.""" formatter = FormatFormatter(options(show_source=False)) filemock = formatter.output_fd = mock.Mock() - error = style_guide.Error( + error = style_guide.Violation( code='A001', filename='example.py', line_number=1, diff --git a/tests/unit/test_filenameonly_formatter.py b/tests/unit/test_filenameonly_formatter.py index caa0ae3..b423ee3 100644 --- a/tests/unit/test_filenameonly_formatter.py +++ b/tests/unit/test_filenameonly_formatter.py @@ -17,14 +17,15 @@ def test_caches_filenames_already_printed(): formatter = default.FilenameOnly(options()) assert formatter.filenames_already_printed == set() - formatter.format(style_guide.Error('code', 'file.py', 1, 1, 'text', 'l')) + formatter.format( + style_guide.Violation('code', 'file.py', 1, 1, 'text', 'l')) assert formatter.filenames_already_printed == {'file.py'} def test_only_returns_a_string_once_from_format(): """Verify format ignores the second error with the same filename.""" formatter = default.FilenameOnly(options()) - error = style_guide.Error('code', 'file.py', 1, 1, 'text', '1') + error = style_guide.Violation('code', 'file.py', 1, 1, 'text', '1') assert formatter.format(error) == 'file.py' assert formatter.format(error) is None @@ -33,6 +34,6 @@ def test_only_returns_a_string_once_from_format(): def test_show_source_returns_nothing(): """Verify show_source returns nothing.""" formatter = default.FilenameOnly(options()) - error = style_guide.Error('code', 'file.py', 1, 1, 'text', '1') + error = style_guide.Violation('code', 'file.py', 1, 1, 'text', '1') assert formatter.show_source(error) is None diff --git a/tests/unit/test_nothing_formatter.py b/tests/unit/test_nothing_formatter.py index ce8087f..a1fd683 100644 --- a/tests/unit/test_nothing_formatter.py +++ b/tests/unit/test_nothing_formatter.py @@ -15,7 +15,7 @@ def options(**kwargs): def test_format_returns_nothing(): """Verify Nothing.format returns None.""" formatter = default.Nothing(options()) - error = style_guide.Error('code', 'file.py', 1, 1, 'text', '1') + error = style_guide.Violation('code', 'file.py', 1, 1, 'text', '1') assert formatter.format(error) is None @@ -23,6 +23,6 @@ def test_format_returns_nothing(): def test_show_source_returns_nothing(): """Verify Nothing.show_source returns None.""" formatter = default.Nothing(options()) - error = style_guide.Error('code', 'file.py', 1, 1, 'text', '1') + error = style_guide.Violation('code', 'file.py', 1, 1, 'text', '1') assert formatter.show_source(error) is None diff --git a/tests/unit/test_statistics.py b/tests/unit/test_statistics.py index f95c638..31d6276 100644 --- a/tests/unit/test_statistics.py +++ b/tests/unit/test_statistics.py @@ -11,7 +11,7 @@ DEFAULT_TEXT = 'Default text' def make_error(**kwargs): """Create errors with a bunch of default values.""" - return style_guide.Error( + return style_guide.Violation( code=kwargs.pop('code', DEFAULT_ERROR_CODE), filename=kwargs.pop('filename', DEFAULT_FILENAME), line_number=kwargs.pop('line_number', 1), diff --git a/tests/unit/test_style_guide.py b/tests/unit/test_style_guide.py index b3641c6..84c47ff 100644 --- a/tests/unit/test_style_guide.py +++ b/tests/unit/test_style_guide.py @@ -19,46 +19,6 @@ def create_options(**kwargs): return optparse.Values(kwargs) -@pytest.mark.parametrize('error_code,physical_line,expected_result', [ - ('E111', 'a = 1', False), - ('E121', 'a = 1 # noqa: E111', False), - ('E121', 'a = 1 # noqa: E111,W123,F821', False), - ('E111', 'a = 1 # noqa: E111,W123,F821', True), - ('W123', 'a = 1 # noqa: E111,W123,F821', True), - ('E111', 'a = 1 # noqa: E11,W123,F821', True), - ('E111', 'a = 1 # noqa, analysis:ignore', True), - ('E111', 'a = 1 # noqa analysis:ignore', True), - ('E111', 'a = 1 # noqa - We do not care', True), - ('E111', 'a = 1 # noqa: We do not care', True), -]) -def test_is_inline_ignored(error_code, physical_line, expected_result): - """Verify that we detect inline usage of ``# noqa``.""" - guide = style_guide.StyleGuide(create_options(select=['E', 'W', 'F']), - listener_trie=None, - formatter=None) - error = style_guide.Error(error_code, 'filename.py', 1, 1, 'error text', - None) - # We want `None` to be passed as the physical line so we actually use our - # monkey-patched linecache.getline value. - - with mock.patch('linecache.getline', return_value=physical_line): - assert guide.is_inline_ignored(error) is expected_result - - -def test_disable_is_inline_ignored(): - """Verify that is_inline_ignored exits immediately if disabling NoQA.""" - guide = style_guide.StyleGuide(create_options(disable_noqa=True), - listener_trie=None, - formatter=None) - error = style_guide.Error('E121', 'filename.py', 1, 1, 'error text', - 'line') - - with mock.patch('linecache.getline') as getline: - assert guide.is_inline_ignored(error) is False - - assert getline.called is False - - @pytest.mark.parametrize('select_list,ignore_list,error_code', [ (['E111', 'E121'], [], 'E111'), (['E111', 'E121'], [], 'E121'), @@ -76,8 +36,8 @@ def test_handle_error_notifies_listeners(select_list, ignore_list, error_code): with mock.patch('linecache.getline', return_value=''): guide.handle_error(error_code, 'stdin', 1, 0, 'error found') - error = style_guide.Error(error_code, 'stdin', 1, 1, 'error found', - None) + error = style_guide.Violation( + error_code, 'stdin', 1, 1, 'error found', None) listener_trie.notify.assert_called_once_with(error_code, error) formatter.handle.assert_called_once_with(error) diff --git a/tests/unit/test_violation.py b/tests/unit/test_violation.py new file mode 100644 index 0000000..fa75278 --- /dev/null +++ b/tests/unit/test_violation.py @@ -0,0 +1,55 @@ +"""Tests for the flake8.style_guide.Violation class.""" +import mock +import pytest + +from flake8 import style_guide + + +@pytest.mark.parametrize('error_code,physical_line,expected_result', [ + ('E111', 'a = 1', False), + ('E121', 'a = 1 # noqa: E111', False), + ('E121', 'a = 1 # noqa: E111,W123,F821', False), + ('E111', 'a = 1 # noqa: E111,W123,F821', True), + ('W123', 'a = 1 # noqa: E111,W123,F821', True), + ('E111', 'a = 1 # noqa: E11,W123,F821', True), + ('E111', 'a = 1 # noqa, analysis:ignore', True), + ('E111', 'a = 1 # noqa analysis:ignore', True), + ('E111', 'a = 1 # noqa - We do not care', True), + ('E111', 'a = 1 # noqa: We do not care', True), +]) +def test_is_inline_ignored(error_code, physical_line, expected_result): + """Verify that we detect inline usage of ``# noqa``.""" + error = style_guide.Violation( + error_code, 'filename.py', 1, 1, 'error text', None) + # We want `None` to be passed as the physical line so we actually use our + # monkey-patched linecache.getline value. + + with mock.patch('linecache.getline', return_value=physical_line): + assert error.is_inline_ignored(False) is expected_result + + +def test_disable_is_inline_ignored(): + """Verify that is_inline_ignored exits immediately if disabling NoQA.""" + error = style_guide.Violation( + 'E121', 'filename.py', 1, 1, 'error text', 'line') + + with mock.patch('linecache.getline') as getline: + assert error.is_inline_ignored(True) is False + + assert getline.called is False + + +@pytest.mark.parametrize('violation_file,violation_line,diff,expected', [ + ('file.py', 10, {}, True), + ('file.py', 1, {'file.py': range(1, 2)}, True), + ('file.py', 10, {'file.py': range(1, 2)}, False), + ('file.py', 1, {'other.py': range(1, 2)}, False), + ('file.py', 10, {'other.py': range(1, 2)}, False), +]) +def test_violation_is_in_diff(violation_file, violation_line, diff, expected): + """Verify that we find violations within a diff.""" + violation = style_guide.Violation( + 'E001', violation_file, violation_line, 1, 'warning', 'line', + ) + + assert violation.is_in(diff) is expected