Fix subtle reporting bug for default on plugins

When we refactored our decision process to fix #239, we broke a subtle
corner case where extensions that are not off-by-default are to be
reported.

This further refactors that logic and adds specific tests around it to
ensure it works as expected and doesn't regress.

Closes #257
parent 8e490527
......@@ -3,6 +3,11 @@
You can view the `3.2.1 milestone`_ on GitLab for more details.
- Fix subtle bug when deciding whether to report an on-by-default's violation
(See also `GitLab#257`_)
.. links
.. _3.2.1 milestone:
https://gitlab.com/pycqa/flake8/milestones/15
.. _GitLab#257:
https://gitlab.com/pycqa/flake8/issues/257
"""Constants that define defaults."""
import re
EXCLUDE = '.svn,CVS,.bzr,.hg,.git,__pycache__,.tox,.eggs,*.egg'
IGNORE = 'E121,E123,E126,E226,E24,E704,W503,W504'
SELECT = 'E,F,W,C90'
EXCLUDE = (
'.svn',
'CVS',
'.bzr',
'.hg',
'.git',
'__pycache__',
'.tox',
'.eggs',
'*.egg',
)
IGNORE = (
'E121',
'E123',
'E126',
'E226',
'E24',
'E704',
'W503',
'W504',
)
SELECT = ('E', 'F', 'W', 'C90')
MAX_LINE_LENGTH = 79
TRUTHY_VALUES = set(['true', '1', 't'])
......
......@@ -63,7 +63,7 @@ def register_default_options(option_manager):
)
add_option(
'--exclude', metavar='patterns', default=defaults.EXCLUDE,
'--exclude', metavar='patterns', default=','.join(defaults.EXCLUDE),
comma_separated_list=True, parse_from_config=True,
normalize_paths=True,
help='Comma-separated list of files or directories to exclude.'
......@@ -102,7 +102,7 @@ def register_default_options(option_manager):
)
add_option(
'--ignore', metavar='errors', default=defaults.IGNORE,
'--ignore', metavar='errors', default=','.join(defaults.IGNORE),
parse_from_config=True, comma_separated_list=True,
help='Comma-separated list of errors and warnings to ignore (or skip).'
' For example, ``--ignore=E4,E51,W234``. (Default: %default)',
......@@ -116,7 +116,7 @@ def register_default_options(option_manager):
)
add_option(
'--select', metavar='errors', default=defaults.SELECT,
'--select', metavar='errors', default=','.join(defaults.SELECT),
parse_from_config=True, comma_separated_list=True,
help='Comma-separated list of errors and warnings to enable.'
' For example, ``--select=E4,E51,W234``. (Default: %default)',
......
......@@ -63,10 +63,16 @@ class StyleGuide(object):
self.formatter = formatter
self.stats = statistics.Statistics()
self._selected = tuple(options.select)
self._extended_selected = tuple(options.extended_default_select)
self._extended_selected = tuple(sorted(
options.extended_default_select,
reverse=True,
))
self._enabled_extensions = tuple(options.enable_extensions)
self._all_selected = self._selected + self._enabled_extensions
self._ignored = tuple(options.ignore)
self._all_selected = tuple(sorted(
self._selected + self._enabled_extensions,
reverse=True,
))
self._ignored = tuple(sorted(options.ignore, reverse=True))
self._decision_cache = {}
self._parsed_diff = {}
......@@ -116,25 +122,21 @@ class StyleGuide(object):
def _decision_for(self, code):
# type: (Error) -> Decision
startswith = code.startswith
try:
selected = sorted([s for s in self._selected if startswith(s)])[0]
except IndexError:
selected = None
try:
ignored = sorted([i for i in self._ignored if startswith(i)])[0]
except IndexError:
ignored = None
if selected is None:
return Decision.Ignored
if ignored is None:
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:
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._selected == defaults.SELECT):
return Decision.Selected
if selected.startswith(ignored) and selected != ignored:
return Decision.Selected
return Decision.Ignored
if select is None and extra_select is None and ignore is not None:
return Decision.Ignored
if self._selected != defaults.SELECT and select is None:
return Decision.Ignored
return Decision.Selected
def should_report_error(self, code):
# type: (str) -> Decision
......@@ -295,3 +297,19 @@ class StyleGuide(object):
Dictionary mapping filenames to sets of line number ranges.
"""
self._parsed_diff = diffinfo
def find_more_specific(selected, ignored):
if selected.startswith(ignored) and selected != ignored:
return Decision.Selected
return Decision.Ignored
def find_first_match(error_code, code_list):
startswith = error_code.startswith
for code in code_list:
if startswith(code):
break
else:
return None
return code
......@@ -4,6 +4,7 @@ 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
......@@ -135,6 +136,52 @@ def test_should_report_error(select_list, ignore_list, error_code, expected):
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.Selected),
(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),
(['E2'], ['E21'], [], [], 'E221', style_guide.Decision.Selected),
(['E2'], ['E21'], [], [], 'E212', style_guide.Decision.Ignored),
(['F', 'W'], ['C90'], ['I1'], [], 'C901',
style_guide.Decision.Ignored),
]
)
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),
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment