diff --git a/docs/source/release-notes/3.2.1.rst b/docs/source/release-notes/3.2.1.rst index dead1db..5fd3375 100644 --- a/docs/source/release-notes/3.2.1.rst +++ b/docs/source/release-notes/3.2.1.rst @@ -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 diff --git a/src/flake8/defaults.py b/src/flake8/defaults.py index e3ac9fb..340e8a9 100644 --- a/src/flake8/defaults.py +++ b/src/flake8/defaults.py @@ -1,9 +1,28 @@ """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']) diff --git a/src/flake8/main/options.py b/src/flake8/main/options.py index 7e9b79e..9780378 100644 --- a/src/flake8/main/options.py +++ b/src/flake8/main/options.py @@ -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)', diff --git a/src/flake8/style_guide.py b/src/flake8/style_guide.py index 9f3b86f..a531b39 100644 --- a/src/flake8/style_guide.py +++ b/src/flake8/style_guide.py @@ -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 + 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 selected is None: + 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 select is None and extra_select is None and ignore is not None: return Decision.Ignored - - if ignored is None: - return Decision.Selected - - if selected.startswith(ignored) and selected != ignored: - return Decision.Selected - 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 diff --git a/tests/unit/test_style_guide.py b/tests/unit/test_style_guide.py index c11fd1d..7357b25 100644 --- a/tests/unit/test_style_guide.py +++ b/tests/unit/test_style_guide.py @@ -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),