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

Refactor decision logic into its own object

See merge request !190

11 files changed, 492 insertions, 377 deletions

diff --git a/docs/source/internal/start-to-finish.rst b/docs/source/internal/start-to-finish.rst
index 381ef91..faed9bd 100644
--- a/docs/source/internal/start-to-finish.rst
+++ b/docs/source/internal/start-to-finish.rst
@@ -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
 --------------------
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 ebc01ed..91423de 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,37 +68,118 @@ 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.
+        :param bool disable_noqa:
+            Whether or not users have provided ``--disable-noqa``.
+        :returns:
+            True if error is ignored in-line, False otherwise.
+        :rtype:
+            bool
         """
-        self.options = options
-        self.listener = listener_trie
-        self.formatter = formatter
-        self.stats = statistics.Statistics()
-        self._selected = tuple(options.select)
-        self._extended_selected = tuple(sorted(
+        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.
+
+    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,
+        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.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):
+    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.
 
@@ -94,12 +192,10 @@ class StyleGuide(object):
             Ignored.Implicitly if the selected list is not empty but no match
             was found.
         """
-        if self._all_selected and code.startswith(self._all_selected):
+        if self._in_all_selected(code):
             return Selected.Explicitly
 
-        if (not self._all_selected and
-            (self._extended_selected and
-                code.startswith(self._extended_selected))):
+        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
@@ -107,7 +203,7 @@ class StyleGuide(object):
 
         return Ignored.Implicitly
 
-    def is_user_ignored(self, code):
+    def was_ignored(self, code):
         # type: (str) -> Union[Selected, Ignored]
         """Determine if the code has been ignored by the user.
 
@@ -120,43 +216,85 @@ class StyleGuide(object):
             Selected.Implicitly if the ignored list is not empty but no match
             was found.
         """
-        if self._ignored and code.startswith(self._ignored):
+        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)
+    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 self._using_default_ignore and not self._using_default_select:
+            # 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):
+        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)):
+                (extra_select is None or not self.using_default_ignore)):
             return Decision.Ignored
         return Decision.Selected
 
-    def should_report_error(self, code):
+    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
-        """Determine if the error code should be reported or ignored.
+        """Return the decision for a specific code.
 
-        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 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.
@@ -164,97 +302,50 @@ class StyleGuide(object):
         :param str code:
             The code for the check that has been run.
         """
-        decision = self._decision_cache.get(code)
+        decision = self.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
+            decision = self.make_decision(code)
+            self.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
-        # 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
+class StyleGuide(object):
+    """Manage a Flake8 user's style guide."""
 
-        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
+    def __init__(self, options, listener_trie, formatter, decider=None):
+        """Initialize our StyleGuide.
 
-        LOG.debug('%r is not ignored inline with ``# noqa: %s``',
-                  error, codes_str)
-        return False
+        .. 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 = {}
 
-    def is_in_diff(self, error):
-        # type: (Error) -> bool
-        """Determine if an error is included in a diff's line ranges.
+    @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)
 
-        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.
+    def should_report_error(self, code):
+        # type: (str) -> Decision
+        """Determine if the error code should be reported or ignored.
 
-        :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
+        This method only cares about the select and ignore rules as specified
+        by the user in their configuration files and command-line flags.
 
-        # 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
+        This method does not look at whether the specific line is being
+        ignored in the file itself.
 
-        return error.line_number in line_numbers
+        :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)
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_decision_engine.py b/tests/unit/test_decision_engine.py
new file mode 100644
index 0000000..db9c270
--- /dev/null
+++ b/tests/unit/test_decision_engine.py
@@ -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
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 5c24048..84c47ff 100644
--- a/tests/unit/test_style_guide.py
+++ b/tests/unit/test_style_guide.py
@@ -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)
 
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