Merge branch 'refactor-decision-engine' into 'master'

Refactor decision logic into its own object

See merge request !190
This commit is contained in:
Ian Cordasco 2017-06-04 20:11:32 +00:00
commit 01f8824490
11 changed files with 520 additions and 405 deletions

View file

@ -106,8 +106,9 @@ Reporting Violations
Next, the application takes the violations from the file checker manager, and
feeds them through the :class:`~flake8.style_guide.StyleGuide`. This
determines whether the particular :term:`error code` is selected or ignored
and then appropriately sends it to the formatter (or not).
relies on a :class:`~flake8.style_guide.DecisionEngine` instance to determine
whether the particular :term:`error code` is selected or ignored and then
appropriately sends it to the formatter (or not).
Reporting Benchmarks
--------------------

View file

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

View file

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

View file

@ -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,178 +68,50 @@ Error = collections.namedtuple(
)
class StyleGuide(object):
"""Manage a Flake8 user's style guide."""
class Violation(_Violation):
"""Class representing a violation reported by Flake8."""
def __init__(self, options, listener_trie, formatter):
"""Initialize our StyleGuide.
def is_inline_ignored(self, disable_noqa):
# type: (Violation) -> bool
"""Determine if an comment has been added to ignore this line.
.. todo:: Add parameter documentation.
"""
self.options = options
self.listener = listener_trie
self.formatter = formatter
self.stats = statistics.Statistics()
self._selected = tuple(options.select)
self._extended_selected = tuple(sorted(
options.extended_default_select,
reverse=True,
))
self._enabled_extensions = tuple(options.enable_extensions)
self._all_selected = tuple(sorted(
self._selected + self._enabled_extensions,
reverse=True,
))
self._ignored = tuple(sorted(options.ignore, reverse=True))
self._using_default_ignore = set(self._ignored) == set(defaults.IGNORE)
self._using_default_select = (
set(self._selected) == set(defaults.SELECT)
)
self._decision_cache = {}
self._parsed_diff = {}
def is_user_selected(self, code):
# type: (str) -> Union[Selected, Ignored]
"""Determine if the code has been selected by the user.
:param str code:
The code for the check that has been run.
:param bool disable_noqa:
Whether or not users have provided ``--disable-noqa``.
:returns:
Selected.Implicitly if the selected list is empty,
Selected.Explicitly if the selected list is not empty and a match
was found,
Ignored.Implicitly if the selected list is not empty but no match
was found.
True if error is ignored in-line, False otherwise.
:rtype:
bool
"""
if self._all_selected and code.startswith(self._all_selected):
return Selected.Explicitly
if (not self._all_selected and
(self._extended_selected and
code.startswith(self._extended_selected))):
# If it was not explicitly selected, it may have been implicitly
# selected because the check comes from a plugin that is enabled by
# default
return Selected.Implicitly
return Ignored.Implicitly
def is_user_ignored(self, code):
# type: (str) -> Union[Selected, Ignored]
"""Determine if the code has been ignored by the user.
:param str code:
The code for the check that has been run.
:returns:
Selected.Implicitly if the ignored list is empty,
Ignored.Explicitly if the ignored list is not empty and a match was
found,
Selected.Implicitly if the ignored list is not empty but no match
was found.
"""
if self._ignored and code.startswith(self._ignored):
return Ignored.Explicitly
return Selected.Implicitly
@contextlib.contextmanager
def processing_file(self, filename):
"""Record the fact that we're processing the file's results."""
self.formatter.beginning(filename)
yield self
self.formatter.finished(filename)
def _decision_for(self, code):
# type: (Error) -> 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)
if select and ignore:
if self._using_default_ignore and not self._using_default_select:
return Decision.Selected
return find_more_specific(select, ignore)
if extra_select and ignore:
return find_more_specific(extra_select, ignore)
if select or (extra_select and self._using_default_select):
return Decision.Selected
if (select is None and
(extra_select is None or not self._using_default_ignore)):
return Decision.Ignored
return Decision.Selected
def should_report_error(self, code):
# type: (str) -> Decision
"""Determine if the error code should be reported or ignored.
This method only cares about the select and ignore rules as specified
by the user in their configuration files and command-line flags.
This method does not look at whether the specific line is being
ignored in the file itself.
:param str code:
The code for the check that has been run.
"""
decision = self._decision_cache.get(code)
if decision is None:
LOG.debug('Deciding if "%s" should be reported', code)
selected = self.is_user_selected(code)
ignored = self.is_user_ignored(code)
LOG.debug('The user configured "%s" to be "%s", "%s"',
code, selected, ignored)
if ((selected is Selected.Explicitly or
selected is Selected.Implicitly) and
ignored is Selected.Implicitly):
decision = Decision.Selected
elif ((selected is Selected.Explicitly and
ignored is Ignored.Explicitly) or
(selected is Ignored.Implicitly and
ignored is Selected.Implicitly)):
decision = self._decision_for(code)
elif (selected is Ignored.Implicitly or
ignored is Ignored.Explicitly):
decision = Decision.Ignored # pylint: disable=R0204
self._decision_cache[code] = decision
LOG.debug('"%s" will be "%s"', code, decision)
return decision
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
physical_line = self.physical_line
# TODO(sigmavirus24): Determine how to handle stdin with linecache
if self.options.disable_noqa:
if 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)
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', error)
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``', error)
LOG.debug('%r is ignored by a blanket ``# noqa``', self)
return True
codes = set(utils.parse_comma_separated_list(codes_str))
if error.code in codes or error.code.startswith(tuple(codes)):
if self.code in codes or self.code.startswith(tuple(codes)):
LOG.debug('%r is ignored specifically inline with ``# noqa: %s``',
error, codes_str)
self, codes_str)
return True
LOG.debug('%r is not ignored inline with ``# noqa: %s``',
error, codes_str)
self, 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.
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
@ -241,7 +130,7 @@ class StyleGuide(object):
:rtype:
bool
"""
if not self._parsed_diff:
if not diff:
return True
# NOTE(sigmavirus24): The parsed diff will be a defaultdict with
@ -250,11 +139,213 @@ class StyleGuide(object):
# 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)
line_numbers = diff.get(self.filename)
if not line_numbers:
return False
return error.line_number in line_numbers
return self.line_number in line_numbers
class DecisionEngine(object):
"""A class for managing the decision process around violations.
This contains the logic for whether a violation should be reported or
ignored.
"""
def __init__(self, options):
"""Initialize the engine."""
self.cache = {}
self.selected = tuple(options.select)
self.extended_selected = tuple(sorted(
options.extended_default_select,
reverse=True,
))
self.enabled_extensions = tuple(options.enable_extensions)
self.all_selected = tuple(sorted(
self.selected + self.enabled_extensions,
reverse=True,
))
self.ignored = tuple(sorted(options.ignore, reverse=True))
self.using_default_ignore = set(self.ignored) == set(defaults.IGNORE)
self.using_default_select = (
set(self.selected) == set(defaults.SELECT)
)
def _in_all_selected(self, code):
return self.all_selected and code.startswith(self.all_selected)
def _in_extended_selected(self, code):
return (self.extended_selected and
code.startswith(self.extended_selected))
def was_selected(self, code):
# type: (str) -> Union[Selected, Ignored]
"""Determine if the code has been selected by the user.
:param str code:
The code for the check that has been run.
:returns:
Selected.Implicitly if the selected list is empty,
Selected.Explicitly if the selected list is not empty and a match
was found,
Ignored.Implicitly if the selected list is not empty but no match
was found.
"""
if self._in_all_selected(code):
return Selected.Explicitly
if not self.all_selected and self._in_extended_selected(code):
# If it was not explicitly selected, it may have been implicitly
# selected because the check comes from a plugin that is enabled by
# default
return Selected.Implicitly
return Ignored.Implicitly
def was_ignored(self, code):
# type: (str) -> Union[Selected, Ignored]
"""Determine if the code has been ignored by the user.
:param str code:
The code for the check that has been run.
:returns:
Selected.Implicitly if the ignored list is empty,
Ignored.Explicitly if the ignored list is not empty and a match was
found,
Selected.Implicitly if the ignored list is not empty but no match
was found.
"""
if self.ignored and code.startswith(self.ignored):
return Ignored.Explicitly
return Selected.Implicitly
def more_specific_decision_for(self, code):
# 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)
if select and ignore:
# If the violation code appears in both the select and ignore
# lists (in some fashion) then if we're using the default ignore
# list and a custom select list we should select the code. An
# example usage looks like this:
# A user has a code that would generate an E126 violation which
# is in our default ignore list and they specify select=E.
# We should be reporting that violation. This logic changes,
# however, if they specify select and ignore such that both match.
# In that case we fall through to our find_more_specific call.
# If, however, the user hasn't specified a custom select, and
# we're using the defaults for both select and ignore then the
# more specific rule must win. In most cases, that will be to
# ignore the violation since our default select list is very
# high-level and our ignore list is highly specific.
if self.using_default_ignore and not self.using_default_select:
return Decision.Selected
return find_more_specific(select, ignore)
if extra_select and ignore:
# At this point, select is false-y. Now we need to check if the
# code is in our extended select list and our ignore list. This is
# a *rare* case as we see little usage of the extended select list
# that plugins can use, so I suspect this section may change to
# look a little like the block above in which we check if we're
# using our default ignore list.
return find_more_specific(extra_select, ignore)
if select or (extra_select and self.using_default_select):
# Here, ignore was false-y and the user has either selected
# explicitly the violation or the violation is covered by
# something in the extended select list and we're using the
# default select list. In either case, we want the violation to be
# selected.
return Decision.Selected
if (select is None and
(extra_select is None or not self.using_default_ignore)):
return Decision.Ignored
return Decision.Selected
def make_decision(self, code):
"""Decide if code should be ignored or selected."""
LOG.debug('Deciding if "%s" should be reported', code)
selected = self.was_selected(code)
ignored = self.was_ignored(code)
LOG.debug('The user configured "%s" to be "%s", "%s"',
code, selected, ignored)
if ((selected is Selected.Explicitly or
selected is Selected.Implicitly) and
ignored is Selected.Implicitly):
decision = Decision.Selected
elif ((selected is Selected.Explicitly and
ignored is Ignored.Explicitly) or
(selected is Ignored.Implicitly and
ignored is Selected.Implicitly)):
decision = self.more_specific_decision_for(code)
elif (selected is Ignored.Implicitly or
ignored is Ignored.Explicitly):
decision = Decision.Ignored # pylint: disable=R0204
return decision
def decision_for(self, code):
# type: (str) -> Decision
"""Return the decision for a specific code.
This method caches the decisions for codes to avoid retracing the same
logic over and over again. We only care about the select and ignore
rules as specified by the user in their configuration files and
command-line flags.
This method does not look at whether the specific line is being
ignored in the file itself.
:param str code:
The code for the check that has been run.
"""
decision = self.cache.get(code)
if decision is None:
decision = self.make_decision(code)
self.cache[code] = decision
LOG.debug('"%s" will be "%s"', code, decision)
return decision
class StyleGuide(object):
"""Manage a Flake8 user's style guide."""
def __init__(self, options, listener_trie, formatter, decider=None):
"""Initialize our StyleGuide.
.. todo:: Add parameter documentation.
"""
self.options = options
self.listener = listener_trie
self.formatter = formatter
self.stats = statistics.Statistics()
self.decider = decider or DecisionEngine(options)
self._parsed_diff = {}
@contextlib.contextmanager
def processing_file(self, filename):
"""Record the fact that we're processing the file's results."""
self.formatter.beginning(filename)
yield self
self.formatter.finished(filename)
def should_report_error(self, code):
# type: (str) -> Decision
"""Determine if the error code should be reported or ignored.
This method only cares about the select and ignore rules as specified
by the user in their configuration files and command-line flags.
This method does not look at whether the specific line is being
ignored in the file itself.
:param str code:
The code for the check that has been run.
"""
return self.decider.decision_for(code)
def handle_error(self, code, filename, line_number, column_number, text,
physical_line=None):
@ -281,17 +372,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)

View file

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

View file

@ -0,0 +1,184 @@
"""Tests for the flake8.style_guide.DecisionEngine class."""
import optparse
import pytest
from flake8 import defaults
from flake8 import style_guide
def create_options(**kwargs):
"""Create and return an instance of optparse.Values."""
kwargs.setdefault('select', [])
kwargs.setdefault('extended_default_select', [])
kwargs.setdefault('ignore', [])
kwargs.setdefault('disable_noqa', False)
kwargs.setdefault('enable_extensions', [])
return optparse.Values(kwargs)
@pytest.mark.parametrize('ignore_list,error_code', [
(['E111', 'E121'], 'E111'),
(['E111', 'E121'], 'E121'),
(['E11', 'E12'], 'E121'),
(['E2', 'E12'], 'E121'),
(['E2', 'E12'], 'E211'),
])
def test_was_ignored_ignores_errors(ignore_list, error_code):
"""Verify we detect users explicitly ignoring an error."""
decider = style_guide.DecisionEngine(create_options(ignore=ignore_list))
assert decider.was_ignored(error_code) is style_guide.Ignored.Explicitly
@pytest.mark.parametrize('ignore_list,error_code', [
(['E111', 'E121'], 'E112'),
(['E111', 'E121'], 'E122'),
(['E11', 'E12'], 'W121'),
(['E2', 'E12'], 'E112'),
(['E2', 'E12'], 'E111'),
])
def test_was_ignored_implicitly_selects_errors(ignore_list, error_code):
"""Verify we detect users does not explicitly ignore an error."""
decider = style_guide.DecisionEngine(create_options(ignore=ignore_list))
assert decider.was_ignored(error_code) is style_guide.Selected.Implicitly
@pytest.mark.parametrize('select_list,enable_extensions,error_code', [
(['E111', 'E121'], [], 'E111'),
(['E111', 'E121'], [], 'E121'),
(['E11', 'E12'], [], 'E121'),
(['E2', 'E12'], [], 'E121'),
(['E2', 'E12'], [], 'E211'),
(['E1'], ['E2'], 'E211'),
([], ['E2'], 'E211'),
])
def test_was_selected_selects_errors(select_list, enable_extensions,
error_code):
"""Verify we detect users explicitly selecting an error."""
decider = style_guide.DecisionEngine(
options=create_options(select=select_list,
enable_extensions=enable_extensions),
)
assert decider.was_selected(error_code) is style_guide.Selected.Explicitly
def test_was_selected_implicitly_selects_errors():
"""Verify we detect users implicitly selecting an error."""
select_list = []
error_code = 'E121'
decider = style_guide.DecisionEngine(
create_options(
select=select_list,
extended_default_select=['E'],
),
)
assert decider.was_selected(error_code) is style_guide.Selected.Implicitly
@pytest.mark.parametrize('select_list,error_code', [
(['E111', 'E121'], 'E112'),
(['E111', 'E121'], 'E122'),
(['E11', 'E12'], 'E132'),
(['E2', 'E12'], 'E321'),
(['E2', 'E12'], 'E410'),
])
def test_was_selected_excludes_errors(select_list, error_code):
"""Verify we detect users implicitly excludes an error."""
decider = style_guide.DecisionEngine(create_options(select=select_list))
assert decider.was_selected(error_code) is style_guide.Ignored.Implicitly
@pytest.mark.parametrize('select_list,ignore_list,error_code,expected', [
(['E111', 'E121'], [], 'E111', style_guide.Decision.Selected),
(['E111', 'E121'], [], 'E112', style_guide.Decision.Ignored),
(['E111', 'E121'], [], 'E121', style_guide.Decision.Selected),
(['E111', 'E121'], [], 'E122', style_guide.Decision.Ignored),
(['E11', 'E12'], [], 'E132', style_guide.Decision.Ignored),
(['E2', 'E12'], [], 'E321', style_guide.Decision.Ignored),
(['E2', 'E12'], [], 'E410', style_guide.Decision.Ignored),
(['E11', 'E121'], ['E1'], 'E112', style_guide.Decision.Selected),
(['E111', 'E121'], ['E2'], 'E122', style_guide.Decision.Ignored),
(['E11', 'E12'], ['E13'], 'E132', style_guide.Decision.Ignored),
(['E1', 'E3'], ['E32'], 'E321', style_guide.Decision.Ignored),
([], ['E2', 'E12'], 'E410', style_guide.Decision.Ignored),
(['E4'], ['E2', 'E12', 'E41'], 'E410', style_guide.Decision.Ignored),
(['E41'], ['E2', 'E12', 'E4'], 'E410', style_guide.Decision.Selected),
(['E'], ['F'], 'E410', style_guide.Decision.Selected),
(['F'], [], 'E410', style_guide.Decision.Ignored),
(['E'], defaults.IGNORE, 'E126', style_guide.Decision.Selected),
(['W'], defaults.IGNORE, 'E126', style_guide.Decision.Ignored),
(['E'], defaults.IGNORE, 'W391', style_guide.Decision.Ignored),
(['E', 'W'], ['E13'], 'E131', style_guide.Decision.Ignored),
(defaults.SELECT, ['E13'], 'E131', style_guide.Decision.Ignored),
(defaults.SELECT, defaults.IGNORE, 'E126', style_guide.Decision.Ignored),
(defaults.SELECT, defaults.IGNORE, 'W391', style_guide.Decision.Selected),
])
def test_decision_for(select_list, ignore_list, error_code, expected):
"""Verify we decide when to report an error."""
decider = style_guide.DecisionEngine(create_options(select=select_list,
ignore=ignore_list))
assert decider.decision_for(error_code) is expected
@pytest.mark.parametrize(
'select,ignore,extend_select,enabled_extensions,error_code,expected', [
(defaults.SELECT, [], ['I1'], [], 'I100',
style_guide.Decision.Selected),
(defaults.SELECT, [], ['I1'], [], 'I201',
style_guide.Decision.Ignored),
(defaults.SELECT, ['I2'], ['I1'], [], 'I101',
style_guide.Decision.Selected),
(defaults.SELECT, ['I2'], ['I1'], [], 'I201',
style_guide.Decision.Ignored),
(defaults.SELECT, ['I1'], ['I10'], [], 'I101',
style_guide.Decision.Selected),
(defaults.SELECT, ['I10'], ['I1'], [], 'I101',
style_guide.Decision.Ignored),
(defaults.SELECT, [], [], ['U4'], 'U401',
style_guide.Decision.Selected),
(defaults.SELECT, ['U401'], [], ['U4'], 'U401',
style_guide.Decision.Ignored),
(defaults.SELECT, ['U401'], [], ['U4'], 'U402',
style_guide.Decision.Selected),
(['E', 'W'], ['E13'], [], [], 'E131', style_guide.Decision.Ignored),
(['E', 'W'], ['E13'], [], [], 'E126', style_guide.Decision.Selected),
(['E2'], ['E21'], [], [], 'E221', style_guide.Decision.Selected),
(['E2'], ['E21'], [], [], 'E212', style_guide.Decision.Ignored),
(['F', 'W'], ['C90'], ['I1'], [], 'C901',
style_guide.Decision.Ignored),
(['E', 'W'], ['C'], [], [], 'E131',
style_guide.Decision.Selected),
(defaults.SELECT, defaults.IGNORE, [], ['I'], 'I101',
style_guide.Decision.Selected),
(defaults.SELECT, defaults.IGNORE, ['G'], ['I'], 'G101',
style_guide.Decision.Selected),
(defaults.SELECT, ['G1'], ['G'], ['I'], 'G101',
style_guide.Decision.Ignored),
(defaults.SELECT, ['E126'], [], ['I'], 'I101',
style_guide.Decision.Selected),
# This next one should exercise the catch-all return and yes, this is
# a *very* odd combination but users find much odder combinations
# anyway.
(['E', 'W'], defaults.IGNORE, ['I'], [], 'I101',
style_guide.Decision.Selected),
]
)
def test_more_specific_decision_for_logic(select, ignore, extend_select,
enabled_extensions, error_code,
expected):
"""Verify the logic of DecisionEngine.more_specific_decision_for."""
decider = style_guide.DecisionEngine(
create_options(
select=select, ignore=ignore,
extended_default_select=extend_select,
enable_extensions=enabled_extensions,
),
)
assert decider.more_specific_decision_for(error_code) is expected

View file

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

View file

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

View file

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

View file

@ -4,7 +4,6 @@ import optparse
import mock
import pytest
from flake8 import defaults
from flake8 import style_guide
from flake8.formatting import base
from flake8.plugins import notifier
@ -20,227 +19,6 @@ def create_options(**kwargs):
return optparse.Values(kwargs)
@pytest.mark.parametrize('ignore_list,error_code', [
(['E111', 'E121'], 'E111'),
(['E111', 'E121'], 'E121'),
(['E11', 'E12'], 'E121'),
(['E2', 'E12'], 'E121'),
(['E2', 'E12'], 'E211'),
])
def test_is_user_ignored_ignores_errors(ignore_list, error_code):
"""Verify we detect users explicitly ignoring an error."""
guide = style_guide.StyleGuide(create_options(ignore=ignore_list),
listener_trie=None,
formatter=None)
assert guide.is_user_ignored(error_code) is style_guide.Ignored.Explicitly
@pytest.mark.parametrize('ignore_list,error_code', [
(['E111', 'E121'], 'E112'),
(['E111', 'E121'], 'E122'),
(['E11', 'E12'], 'W121'),
(['E2', 'E12'], 'E112'),
(['E2', 'E12'], 'E111'),
])
def test_is_user_ignored_implicitly_selects_errors(ignore_list, error_code):
"""Verify we detect users does not explicitly ignore an error."""
guide = style_guide.StyleGuide(create_options(ignore=ignore_list),
listener_trie=None,
formatter=None)
assert guide.is_user_ignored(error_code) is style_guide.Selected.Implicitly
@pytest.mark.parametrize('select_list,enable_extensions,error_code', [
(['E111', 'E121'], [], 'E111'),
(['E111', 'E121'], [], 'E121'),
(['E11', 'E12'], [], 'E121'),
(['E2', 'E12'], [], 'E121'),
(['E2', 'E12'], [], 'E211'),
(['E1'], ['E2'], 'E211'),
([], ['E2'], 'E211'),
])
def test_is_user_selected_selects_errors(select_list, enable_extensions,
error_code):
"""Verify we detect users explicitly selecting an error."""
guide = style_guide.StyleGuide(
options=create_options(select=select_list,
enable_extensions=enable_extensions),
listener_trie=None,
formatter=None,
)
assert (guide.is_user_selected(error_code) is
style_guide.Selected.Explicitly)
def test_is_user_selected_implicitly_selects_errors():
"""Verify we detect users implicitly selecting an error."""
select_list = []
error_code = 'E121'
guide = style_guide.StyleGuide(
create_options(
select=select_list,
extended_default_select=['E'],
),
listener_trie=None,
formatter=None,
)
assert (guide.is_user_selected(error_code) is
style_guide.Selected.Implicitly)
@pytest.mark.parametrize('select_list,error_code', [
(['E111', 'E121'], 'E112'),
(['E111', 'E121'], 'E122'),
(['E11', 'E12'], 'E132'),
(['E2', 'E12'], 'E321'),
(['E2', 'E12'], 'E410'),
])
def test_is_user_selected_excludes_errors(select_list, error_code):
"""Verify we detect users implicitly excludes an error."""
guide = style_guide.StyleGuide(create_options(select=select_list),
listener_trie=None,
formatter=None)
assert guide.is_user_selected(error_code) is style_guide.Ignored.Implicitly
@pytest.mark.parametrize('select_list,ignore_list,error_code,expected', [
(['E111', 'E121'], [], 'E111', style_guide.Decision.Selected),
(['E111', 'E121'], [], 'E112', style_guide.Decision.Ignored),
(['E111', 'E121'], [], 'E121', style_guide.Decision.Selected),
(['E111', 'E121'], [], 'E122', style_guide.Decision.Ignored),
(['E11', 'E12'], [], 'E132', style_guide.Decision.Ignored),
(['E2', 'E12'], [], 'E321', style_guide.Decision.Ignored),
(['E2', 'E12'], [], 'E410', style_guide.Decision.Ignored),
(['E11', 'E121'], ['E1'], 'E112', style_guide.Decision.Selected),
(['E111', 'E121'], ['E2'], 'E122', style_guide.Decision.Ignored),
(['E11', 'E12'], ['E13'], 'E132', style_guide.Decision.Ignored),
(['E1', 'E3'], ['E32'], 'E321', style_guide.Decision.Ignored),
([], ['E2', 'E12'], 'E410', style_guide.Decision.Ignored),
(['E4'], ['E2', 'E12', 'E41'], 'E410', style_guide.Decision.Ignored),
(['E41'], ['E2', 'E12', 'E4'], 'E410', style_guide.Decision.Selected),
(['E'], ['F'], 'E410', style_guide.Decision.Selected),
(['F'], [], 'E410', style_guide.Decision.Ignored),
(['E'], defaults.IGNORE, 'E126', style_guide.Decision.Selected),
(['W'], defaults.IGNORE, 'E126', style_guide.Decision.Ignored),
(['E'], defaults.IGNORE, 'W391', style_guide.Decision.Ignored),
(['E', 'W'], ['E13'], 'E131', style_guide.Decision.Ignored),
(defaults.SELECT, ['E13'], 'E131', style_guide.Decision.Ignored),
(defaults.SELECT, defaults.IGNORE, 'E126', style_guide.Decision.Ignored),
(defaults.SELECT, defaults.IGNORE, 'W391', style_guide.Decision.Selected),
])
def test_should_report_error(select_list, ignore_list, error_code, expected):
"""Verify we decide when to report an error."""
guide = style_guide.StyleGuide(create_options(select=select_list,
ignore=ignore_list),
listener_trie=None,
formatter=None)
assert guide.should_report_error(error_code) is expected
@pytest.mark.parametrize(
'select,ignore,extend_select,enabled_extensions,error_code,expected', [
(defaults.SELECT, [], ['I1'], [], 'I100',
style_guide.Decision.Selected),
(defaults.SELECT, [], ['I1'], [], 'I201',
style_guide.Decision.Ignored),
(defaults.SELECT, ['I2'], ['I1'], [], 'I101',
style_guide.Decision.Selected),
(defaults.SELECT, ['I2'], ['I1'], [], 'I201',
style_guide.Decision.Ignored),
(defaults.SELECT, ['I1'], ['I10'], [], 'I101',
style_guide.Decision.Selected),
(defaults.SELECT, ['I10'], ['I1'], [], 'I101',
style_guide.Decision.Ignored),
(defaults.SELECT, [], [], ['U4'], 'U401',
style_guide.Decision.Selected),
(defaults.SELECT, ['U401'], [], ['U4'], 'U401',
style_guide.Decision.Ignored),
(defaults.SELECT, ['U401'], [], ['U4'], 'U402',
style_guide.Decision.Selected),
(['E', 'W'], ['E13'], [], [], 'E131', style_guide.Decision.Ignored),
(['E', 'W'], ['E13'], [], [], 'E126', style_guide.Decision.Selected),
(['E2'], ['E21'], [], [], 'E221', style_guide.Decision.Selected),
(['E2'], ['E21'], [], [], 'E212', style_guide.Decision.Ignored),
(['F', 'W'], ['C90'], ['I1'], [], 'C901',
style_guide.Decision.Ignored),
(['E', 'W'], ['C'], [], [], 'E131',
style_guide.Decision.Selected),
(defaults.SELECT, defaults.IGNORE, [], ['I'], 'I101',
style_guide.Decision.Selected),
(defaults.SELECT, defaults.IGNORE, ['G'], ['I'], 'G101',
style_guide.Decision.Selected),
(defaults.SELECT, ['G1'], ['G'], ['I'], 'G101',
style_guide.Decision.Ignored),
(defaults.SELECT, ['E126'], [], ['I'], 'I101',
style_guide.Decision.Selected),
]
)
def test_decision_for_logic(select, ignore, extend_select, enabled_extensions,
error_code, expected):
"""Verify the complicated logic of StyleGuide._decision_for.
I usually avoid testing private methods, but this one is very important in
our conflict resolution work in Flake8.
"""
guide = style_guide.StyleGuide(
create_options(
select=select, ignore=ignore,
extended_default_select=extend_select,
enable_extensions=enabled_extensions,
),
listener_trie=None,
formatter=None,
)
assert guide._decision_for(error_code) is expected
@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'),
@ -258,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)

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