From 7fef0af0f5521a8cec8451cb8dd01952acd78408 Mon Sep 17 00:00:00 2001
From: Ian Cordasco <graffatcolmingov@gmail.com>
Date: Thu, 1 Jun 2017 20:36:37 -0500
Subject: [PATCH 1/8] Refactor decision logic into its own object

In dealing with the decision logic in the StyleGuide recently I
recognized that the logic really doesn't belong strictly on the
StyleGuide. A separate object makes perfect sense especially from the
perspective of testability. This is a minor refactor intended solely to
facilitate further testing and perhaps making the logic easier to
understand for others.
---
 src/flake8/style_guide.py      | 156 +++++++++++++++++++++++----------
 tests/unit/test_style_guide.py |  12 +--
 2 files changed, 112 insertions(+), 56 deletions(-)

diff --git a/src/flake8/style_guide.py b/src/flake8/style_guide.py
index ebc01ed..24b87c3 100644
--- a/src/flake8/style_guide.py
+++ b/src/flake8/style_guide.py
@@ -51,37 +51,40 @@ Error = collections.namedtuple(
 )
 
 
-class StyleGuide(object):
-    """Manage a Flake8 user's style guide."""
+class DecisionEngine(object):
+    """A class for managing the decision process around violations.
 
-    def __init__(self, options, listener_trie, formatter):
-        """Initialize our StyleGuide.
+    This contains the logic for whether a violation should be reported or
+    ignored.
+    """
 
-        .. 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(
+    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 +97,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 +108,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,34 +121,27 @@ 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):
+    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)
+        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 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):
+        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)):
+                (extra_select is None or not self.using_default_ignore)):
             return Decision.Ignored
         return Decision.Selected
 
@@ -164,11 +158,11 @@ 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)
+            selected = self.was_selected(code)
+            ignored = self.was_ignored(code)
             LOG.debug('The user configured "%s" to be "%s", "%s"',
                       code, selected, ignored)
 
@@ -180,15 +174,83 @@ class StyleGuide(object):
                   ignored is Ignored.Explicitly) or
                   (selected is Ignored.Implicitly and
                    ignored is Selected.Implicitly)):
-                decision = self._decision_for(code)
+                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
+            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):
+        """Initialize our StyleGuide.
+
+        .. todo:: Add parameter documentation.
+        """
+        self.options = options
+        self.listener = listener_trie
+        self.formatter = formatter
+        self.stats = statistics.Statistics()
+        self.decider = DecisionEngine(options)
+        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.
+        :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.
+        """
+        return self.decider.was_selected(code)
+
+    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.
+        """
+        return self.decider.was_ignored(code)
+
+    @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.should_report_error(code)
+
     def is_inline_ignored(self, error):
         # type: (Error) -> bool
         """Determine if an comment has been added to ignore this line."""
diff --git a/tests/unit/test_style_guide.py b/tests/unit/test_style_guide.py
index 5c24048..bfef893 100644
--- a/tests/unit/test_style_guide.py
+++ b/tests/unit/test_style_guide.py
@@ -183,22 +183,16 @@ def test_should_report_error(select_list, ignore_list, error_code, expected):
 )
 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(
+    """Verify the complicated logic of DecisionEngine.decision_for."""
+    decider = style_guide.DecisionEngine(
         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
+    assert decider.decision_for(error_code) is expected
 
 
 @pytest.mark.parametrize('error_code,physical_line,expected_result', [

From 583fda7a70eb49dcf3b7b83dba0dd646d224592f Mon Sep 17 00:00:00 2001
From: Ian Cordasco <graffatcolmingov@gmail.com>
Date: Fri, 2 Jun 2017 19:16:28 -0500
Subject: [PATCH 2/8] Move unit tests for StyleGuide decision logic

Convert it to test the DecisionEngine class directly and put them in
their own file.
---
 tests/unit/test_decision_engine.py | 178 +++++++++++++++++++++++++++++
 tests/unit/test_style_guide.py     | 176 ----------------------------
 2 files changed, 178 insertions(+), 176 deletions(-)
 create mode 100644 tests/unit/test_decision_engine.py

diff --git a/tests/unit/test_decision_engine.py b/tests/unit/test_decision_engine.py
new file mode 100644
index 0000000..18aca78
--- /dev/null
+++ b/tests/unit/test_decision_engine.py
@@ -0,0 +1,178 @@
+"""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_should_report_error(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.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 DecisionEngine.decision_for."""
+    decider = style_guide.DecisionEngine(
+        create_options(
+            select=select, ignore=ignore,
+            extended_default_select=extend_select,
+            enable_extensions=enabled_extensions,
+        ),
+    )
+
+    assert decider.decision_for(error_code) is expected
diff --git a/tests/unit/test_style_guide.py b/tests/unit/test_style_guide.py
index bfef893..b3641c6 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,181 +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 DecisionEngine.decision_for."""
-    decider = style_guide.DecisionEngine(
-        create_options(
-            select=select, ignore=ignore,
-            extended_default_select=extend_select,
-            enable_extensions=enabled_extensions,
-        ),
-    )
-
-    assert decider.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),

From 65107a5624eb6eb4b1ca78e081b6c66879b77655 Mon Sep 17 00:00:00 2001
From: Ian Cordasco <graffatcolmingov@gmail.com>
Date: Sat, 3 Jun 2017 19:20:46 -0500
Subject: [PATCH 3/8] Rename methods on the new DecisionEngine

Rename `decision_for` to `more_specific_decision_for` and
`should_report_error` to `decision_for`.
---
 src/flake8/style_guide.py          | 42 +++++-------------------------
 tests/unit/test_decision_engine.py | 13 ++++-----
 2 files changed, 13 insertions(+), 42 deletions(-)

diff --git a/src/flake8/style_guide.py b/src/flake8/style_guide.py
index 24b87c3..07d9e06 100644
--- a/src/flake8/style_guide.py
+++ b/src/flake8/style_guide.py
@@ -126,7 +126,7 @@ class DecisionEngine(object):
 
         return Selected.Implicitly
 
-    def decision_for(self, code):
+    def more_specific_decision_for(self, code):
         # type: (Error) -> Decision
         select = find_first_match(code, self.all_selected)
         extra_select = find_first_match(code, self.extended_selected)
@@ -145,7 +145,7 @@ class DecisionEngine(object):
             return Decision.Ignored
         return Decision.Selected
 
-    def should_report_error(self, code):
+    def decision_for(self, code):
         # type: (str) -> Decision
         """Determine if the error code should be reported or ignored.
 
@@ -174,7 +174,7 @@ class DecisionEngine(object):
                   ignored is Ignored.Explicitly) or
                   (selected is Ignored.Implicitly and
                    ignored is Selected.Implicitly)):
-                decision = self.decision_for(code)
+                decision = self.more_specific_decision_for(code)
             elif (selected is Ignored.Implicitly or
                   ignored is Ignored.Explicitly):
                 decision = Decision.Ignored  # pylint: disable=R0204
@@ -187,7 +187,7 @@ class DecisionEngine(object):
 class StyleGuide(object):
     """Manage a Flake8 user's style guide."""
 
-    def __init__(self, options, listener_trie, formatter):
+    def __init__(self, options, listener_trie, formatter, decider=None):
         """Initialize our StyleGuide.
 
         .. todo:: Add parameter documentation.
@@ -196,39 +196,9 @@ class StyleGuide(object):
         self.listener = listener_trie
         self.formatter = formatter
         self.stats = statistics.Statistics()
-        self.decider = DecisionEngine(options)
+        self.decider = decider or DecisionEngine(options)
         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.
-        :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.
-        """
-        return self.decider.was_selected(code)
-
-    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.
-        """
-        return self.decider.was_ignored(code)
-
     @contextlib.contextmanager
     def processing_file(self, filename):
         """Record the fact that we're processing the file's results."""
@@ -249,7 +219,7 @@ class StyleGuide(object):
         :param str code:
             The code for the check that has been run.
         """
-        return self.decider.should_report_error(code)
+        return self.decider.decision_for(code)
 
     def is_inline_ignored(self, error):
         # type: (Error) -> bool
diff --git a/tests/unit/test_decision_engine.py b/tests/unit/test_decision_engine.py
index 18aca78..aad0176 100644
--- a/tests/unit/test_decision_engine.py
+++ b/tests/unit/test_decision_engine.py
@@ -118,12 +118,12 @@ def test_was_selected_excludes_errors(select_list, error_code):
     (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):
+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.should_report_error(error_code) is expected
+    assert decider.decision_for(error_code) is expected
 
 
 @pytest.mark.parametrize(
@@ -164,9 +164,10 @@ def test_should_report_error(select_list, ignore_list, error_code, expected):
             style_guide.Decision.Selected),
     ]
 )
-def test_decision_for_logic(select, ignore, extend_select, enabled_extensions,
-                            error_code, expected):
-    """Verify the complicated logic of DecisionEngine.decision_for."""
+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,
@@ -175,4 +176,4 @@ def test_decision_for_logic(select, ignore, extend_select, enabled_extensions,
         ),
     )
 
-    assert decider.decision_for(error_code) is expected
+    assert decider.more_specific_decision_for(error_code) is expected

From 92c367dee4e5908745822902adfda0bd915b1380 Mon Sep 17 00:00:00 2001
From: Ian Cordasco <graffatcolmingov@gmail.com>
Date: Sun, 4 Jun 2017 07:57:28 -0500
Subject: [PATCH 4/8] Rename style_guide.Error to style_guide.Violation

Move all Violation related methods from the StyleGuide to our Violation
class.
---
 src/flake8/formatting/base.py             |  15 +-
 src/flake8/statistics.py                  |   9 +-
 src/flake8/style_guide.py                 | 177 +++++++++++++---------
 tests/unit/test_base_formatter.py         |   8 +-
 tests/unit/test_filenameonly_formatter.py |   7 +-
 tests/unit/test_nothing_formatter.py      |   4 +-
 tests/unit/test_statistics.py             |   2 +-
 tests/unit/test_style_guide.py            |  44 +-----
 tests/unit/test_violation.py              |  55 +++++++
 9 files changed, 185 insertions(+), 136 deletions(-)
 create mode 100644 tests/unit/test_violation.py

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

From 3921afccff66702101e3e8dd99bc184eed77e962 Mon Sep 17 00:00:00 2001
From: Ian Cordasco <graffatcolmingov@gmail.com>
Date: Sun, 4 Jun 2017 13:43:26 -0500
Subject: [PATCH 5/8] Update internal documentation around StyleGuide

---
 docs/source/internal/start-to-finish.rst | 5 +++--
 1 file changed, 3 insertions(+), 2 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
 --------------------

From 86e1fb6e64ecc76d8c94416a92686d56d0fe5978 Mon Sep 17 00:00:00 2001
From: Ian Cordasco <graffatcolmingov@gmail.com>
Date: Sun, 4 Jun 2017 14:50:20 -0500
Subject: [PATCH 6/8] Pull decision making out of decision_for

Also, this further highlights why naming methods is so hard. I can't
think of a better name for 'more_specific_decision_for' that isn't
wildly long and unnecessarily verbose.
---
 src/flake8/style_guide.py | 50 ++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/src/flake8/style_guide.py b/src/flake8/style_guide.py
index 124c7d5..f47a628 100644
--- a/src/flake8/style_guide.py
+++ b/src/flake8/style_guide.py
@@ -240,12 +240,36 @@ class DecisionEngine(object):
             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
-        """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.
@@ -255,25 +279,7 @@ class DecisionEngine(object):
         """
         decision = self.cache.get(code)
         if decision is None:
-            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
-
+            decision = self.make_decision(code)
             self.cache[code] = decision
             LOG.debug('"%s" will be "%s"', code, decision)
         return decision

From e8c6a1e2f5aa98273474ada751613dbf7c39d813 Mon Sep 17 00:00:00 2001
From: Ian Cordasco <graffatcolmingov@gmail.com>
Date: Sun, 4 Jun 2017 14:51:01 -0500
Subject: [PATCH 7/8] Get *one* more line of test coverage

This was mostly to prove to myself that we could possibly reach that
return more than actually covering that return. This just shows how
gnarly this logic actually is. I wish there were a better way to write
it.
---
 tests/unit/test_decision_engine.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/unit/test_decision_engine.py b/tests/unit/test_decision_engine.py
index aad0176..58ce54c 100644
--- a/tests/unit/test_decision_engine.py
+++ b/tests/unit/test_decision_engine.py
@@ -162,6 +162,9 @@ def test_decision_for(select_list, ignore_list, error_code, expected):
             style_guide.Decision.Ignored),
         (defaults.SELECT, ['E126'], [], ['I'], 'I101',
             style_guide.Decision.Selected),
+        # This next one should exercise the catch-all return
+        (['E', 'W'], defaults.IGNORE, ['I'], [], 'I101',
+            style_guide.Decision.Selected),
     ]
 )
 def test_more_specific_decision_for_logic(select, ignore, extend_select,

From ff07ca3ed9a650b7f23866e11860bad9583e8acf Mon Sep 17 00:00:00 2001
From: Ian Cordasco <graffatcolmingov@gmail.com>
Date: Sun, 4 Jun 2017 15:07:10 -0500
Subject: [PATCH 8/8] Add some better comments for decision logic

---
 src/flake8/style_guide.py          | 25 +++++++++++++++++++++++++
 tests/unit/test_decision_engine.py |  4 +++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/src/flake8/style_guide.py b/src/flake8/style_guide.py
index f47a628..91423de 100644
--- a/src/flake8/style_guide.py
+++ b/src/flake8/style_guide.py
@@ -228,12 +228,37 @@ class DecisionEngine(object):
         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)):
diff --git a/tests/unit/test_decision_engine.py b/tests/unit/test_decision_engine.py
index 58ce54c..db9c270 100644
--- a/tests/unit/test_decision_engine.py
+++ b/tests/unit/test_decision_engine.py
@@ -162,7 +162,9 @@ def test_decision_for(select_list, ignore_list, error_code, expected):
             style_guide.Decision.Ignored),
         (defaults.SELECT, ['E126'], [], ['I'], 'I101',
             style_guide.Decision.Selected),
-        # This next one should exercise the catch-all return
+        # 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),
     ]