Rename style_guide.Error to style_guide.Violation

Move all Violation related methods from the StyleGuide to our Violation
class.
This commit is contained in:
Ian Cordasco 2017-06-04 07:57:28 -05:00
parent 65107a5624
commit 92c367dee4
No known key found for this signature in database
GPG key ID: 656D3395E4A9791A
9 changed files with 185 additions and 136 deletions

View file

@ -78,9 +78,10 @@ class BaseFormatter(object):
method. method.
:param error: :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: :type error:
flake8.style_guide.Error flake8.style_guide.Violation
""" """
line = self.format(error) line = self.format(error)
source = self.show_source(error) source = self.show_source(error)
@ -92,9 +93,10 @@ class BaseFormatter(object):
This method **must** be implemented by subclasses. This method **must** be implemented by subclasses.
:param error: :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: :type error:
flake8.style_guide.Error flake8.style_guide.Violation
:returns: :returns:
The formatted error string. The formatted error string.
:rtype: :rtype:
@ -144,9 +146,10 @@ class BaseFormatter(object):
is reported as generating the problem. is reported as generating the problem.
:param error: :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: :type error:
flake8.style_guide.Error flake8.style_guide.Violation
:returns: :returns:
The formatted error string if the user wants to show the source. 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 If the user does not want to show the source, this will return

View file

@ -23,9 +23,10 @@ class Statistics(object):
"""Add the fact that the error was seen in the file. """Add the fact that the error was seen in the file.
:param error: :param error:
The Error instance containing the information about the violation. The Violation instance containing the information about the
violation.
:type error: :type error:
flake8.style_guide.Error flake8.style_guide.Violation
""" """
key = Key.create_from(error) key = Key.create_from(error)
if key not in self._store: if key not in self._store:
@ -73,7 +74,7 @@ class Key(collections.namedtuple('Key', ['filename', 'code'])):
@classmethod @classmethod
def create_from(cls, error): 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( return cls(
filename=error.filename, filename=error.filename,
code=error.code, code=error.code,
@ -115,7 +116,7 @@ class Statistic(object):
@classmethod @classmethod
def create_from(cls, error): 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( return cls(
error_code=error.code, error_code=error.code,
filename=error.filename, filename=error.filename,

View file

@ -2,6 +2,7 @@
import collections import collections
import contextlib import contextlib
import enum import enum
import functools
import linecache import linecache
import logging import logging
@ -16,6 +17,17 @@ __all__ = (
LOG = logging.getLogger(__name__) 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 # TODO(sigmavirus24): Determine if we need to use enum/enum34
class Selected(enum.Enum): class Selected(enum.Enum):
"""Enum representing an explicitly or implicitly selected code.""" """Enum representing an explicitly or implicitly selected code."""
@ -38,8 +50,13 @@ class Decision(enum.Enum):
Selected = 'selected error' Selected = 'selected error'
Error = collections.namedtuple( @lru_cache(maxsize=512)
'Error', def find_noqa(physical_line):
return defaults.NOQA_INLINE_REGEXP.search(physical_line)
_Violation = collections.namedtuple(
'Violation',
[ [
'code', 'code',
'filename', '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): class DecisionEngine(object):
"""A class for managing the decision process around violations. """A class for managing the decision process around violations.
@ -127,7 +222,7 @@ class DecisionEngine(object):
return Selected.Implicitly return Selected.Implicitly
def more_specific_decision_for(self, code): def more_specific_decision_for(self, code):
# type: (Error) -> Decision # type: (Violation) -> Decision
select = find_first_match(code, self.all_selected) select = find_first_match(code, self.all_selected)
extra_select = find_first_match(code, self.extended_selected) extra_select = find_first_match(code, self.extended_selected)
ignore = find_first_match(code, self.ignored) ignore = find_first_match(code, self.ignored)
@ -221,73 +316,6 @@ class StyleGuide(object):
""" """
return self.decider.decision_for(code) 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, def handle_error(self, code, filename, line_number, column_number, text,
physical_line=None): physical_line=None):
# type: (str, str, int, int, str) -> int # type: (str, str, int, int, str) -> int
@ -313,17 +341,18 @@ class StyleGuide(object):
:rtype: :rtype:
int int
""" """
disable_noqa = self.options.disable_noqa
# NOTE(sigmavirus24): Apparently we're provided with 0-indexed column # NOTE(sigmavirus24): Apparently we're provided with 0-indexed column
# numbers so we have to offset that here. Also, if a SyntaxError is # numbers so we have to offset that here. Also, if a SyntaxError is
# caught, column_number may be None. # caught, column_number may be None.
if not column_number: if not column_number:
column_number = 0 column_number = 0
error = Error(code, filename, line_number, column_number + 1, text, error = Violation(code, filename, line_number, column_number + 1,
physical_line) text, physical_line)
error_is_selected = (self.should_report_error(error.code) is error_is_selected = (self.should_report_error(error.code) is
Decision.Selected) Decision.Selected)
is_not_inline_ignored = self.is_inline_ignored(error) is False is_not_inline_ignored = error.is_inline_ignored(disable_noqa) is False
is_included_in_diff = self.is_in_diff(error) is_included_in_diff = error.is_in(self._parsed_diff)
if (error_is_selected and is_not_inline_ignored and if (error_is_selected and is_not_inline_ignored and
is_included_in_diff): is_included_in_diff):
self.formatter.handle(error) self.formatter.handle(error)

View file

@ -51,7 +51,7 @@ def test_show_source_returns_nothing_when_not_showing_source():
"""Ensure we return nothing when users want nothing.""" """Ensure we return nothing when users want nothing."""
formatter = base.BaseFormatter(options(show_source=False)) formatter = base.BaseFormatter(options(show_source=False))
assert formatter.show_source( 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 '' ) is ''
@ -59,7 +59,7 @@ def test_show_source_returns_nothing_when_there_is_source():
"""Ensure we return nothing when there is no line.""" """Ensure we return nothing when there is no line."""
formatter = base.BaseFormatter(options(show_source=True)) formatter = base.BaseFormatter(options(show_source=True))
assert formatter.show_source( 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 '' ) 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): def test_show_source_updates_physical_line_appropriately(line, column):
"""Ensure the error column is appropriately indicated.""" """Ensure the error column is appropriately indicated."""
formatter = base.BaseFormatter(options(show_source=True)) 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) output = formatter.show_source(error)
_, pointer = output.rsplit('\n', 1) _, pointer = output.rsplit('\n', 1)
assert pointer.count(' ') == (column - 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.""" """Verify that a formatter will call format from handle."""
formatter = FormatFormatter(options(show_source=False)) formatter = FormatFormatter(options(show_source=False))
filemock = formatter.output_fd = mock.Mock() filemock = formatter.output_fd = mock.Mock()
error = style_guide.Error( error = style_guide.Violation(
code='A001', code='A001',
filename='example.py', filename='example.py',
line_number=1, line_number=1,

View file

@ -17,14 +17,15 @@ def test_caches_filenames_already_printed():
formatter = default.FilenameOnly(options()) formatter = default.FilenameOnly(options())
assert formatter.filenames_already_printed == set() 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'} assert formatter.filenames_already_printed == {'file.py'}
def test_only_returns_a_string_once_from_format(): def test_only_returns_a_string_once_from_format():
"""Verify format ignores the second error with the same filename.""" """Verify format ignores the second error with the same filename."""
formatter = default.FilenameOnly(options()) 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) == 'file.py'
assert formatter.format(error) is None 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(): def test_show_source_returns_nothing():
"""Verify show_source returns nothing.""" """Verify show_source returns nothing."""
formatter = default.FilenameOnly(options()) 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 assert formatter.show_source(error) is None

View file

@ -15,7 +15,7 @@ def options(**kwargs):
def test_format_returns_nothing(): def test_format_returns_nothing():
"""Verify Nothing.format returns None.""" """Verify Nothing.format returns None."""
formatter = default.Nothing(options()) 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 assert formatter.format(error) is None
@ -23,6 +23,6 @@ def test_format_returns_nothing():
def test_show_source_returns_nothing(): def test_show_source_returns_nothing():
"""Verify Nothing.show_source returns None.""" """Verify Nothing.show_source returns None."""
formatter = default.Nothing(options()) 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 assert formatter.show_source(error) is None

View file

@ -11,7 +11,7 @@ DEFAULT_TEXT = 'Default text'
def make_error(**kwargs): def make_error(**kwargs):
"""Create errors with a bunch of default values.""" """Create errors with a bunch of default values."""
return style_guide.Error( return style_guide.Violation(
code=kwargs.pop('code', DEFAULT_ERROR_CODE), code=kwargs.pop('code', DEFAULT_ERROR_CODE),
filename=kwargs.pop('filename', DEFAULT_FILENAME), filename=kwargs.pop('filename', DEFAULT_FILENAME),
line_number=kwargs.pop('line_number', 1), line_number=kwargs.pop('line_number', 1),

View file

@ -19,46 +19,6 @@ def create_options(**kwargs):
return optparse.Values(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', [ @pytest.mark.parametrize('select_list,ignore_list,error_code', [
(['E111', 'E121'], [], 'E111'), (['E111', 'E121'], [], 'E111'),
(['E111', 'E121'], [], 'E121'), (['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=''): with mock.patch('linecache.getline', return_value=''):
guide.handle_error(error_code, 'stdin', 1, 0, 'error found') guide.handle_error(error_code, 'stdin', 1, 0, 'error found')
error = style_guide.Error(error_code, 'stdin', 1, 1, 'error found', error = style_guide.Violation(
None) error_code, 'stdin', 1, 1, 'error found', None)
listener_trie.notify.assert_called_once_with(error_code, error) listener_trie.notify.assert_called_once_with(error_code, error)
formatter.handle.assert_called_once_with(error) formatter.handle.assert_called_once_with(error)

View file

@ -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