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