From 2baaf00e83073a749238798983de4b8161e3e328 Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Sat, 27 May 2017 19:22:38 -0500 Subject: [PATCH 1/5] Further refine our logic handling selection There was a *very* subtle bug in how we handle blanket select statements with error codes that are in our DEFAULT_IGNORE. In the specific case, users were specifying ``--select E`` and E126 was not being properly reported. This unveiled further logic bugs as we refined this. Closes #318 --- src/flake8/style_guide.py | 13 +++++++++---- tests/unit/test_style_guide.py | 5 ++++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/flake8/style_guide.py b/src/flake8/style_guide.py index fb0df3a..38fc29a 100644 --- a/src/flake8/style_guide.py +++ b/src/flake8/style_guide.py @@ -74,6 +74,10 @@ class StyleGuide(object): 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 = {} @@ -135,14 +139,15 @@ class StyleGuide(object): 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._selected == defaults.SELECT): + if select or (extra_select and self._using_default_select): return Decision.Selected - 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: + if ((select is None and extra_select is None) or + (not self._using_default_ignore and select is None)): return Decision.Ignored return Decision.Selected diff --git a/tests/unit/test_style_guide.py b/tests/unit/test_style_guide.py index 7357b25..10550f8 100644 --- a/tests/unit/test_style_guide.py +++ b/tests/unit/test_style_guide.py @@ -125,6 +125,9 @@ def test_is_user_selected_excludes_errors(select_list, error_code): (['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), ]) def test_should_report_error(select_list, ignore_list, error_code, expected): """Verify we decide when to report an error.""" @@ -141,7 +144,7 @@ def test_should_report_error(select_list, ignore_list, error_code, expected): (defaults.SELECT, [], ['I1'], [], 'I100', style_guide.Decision.Selected), (defaults.SELECT, [], ['I1'], [], 'I201', - style_guide.Decision.Selected), + style_guide.Decision.Ignored), (defaults.SELECT, ['I2'], ['I1'], [], 'I101', style_guide.Decision.Selected), (defaults.SELECT, ['I2'], ['I1'], [], 'I201', From 804eef43681e376155e15fc37a2b7785181d42a7 Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Sat, 27 May 2017 19:31:10 -0500 Subject: [PATCH 2/5] Upgrade dependencies when dogfooding --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index ba2e43b..69d01e6 100644 --- a/tox.ini +++ b/tox.ini @@ -25,7 +25,7 @@ deps = wheel commands = python setup.py -qq bdist_wheel - pip install --pre --find-links ./dist/ flake8 + pip install -U --pre --find-links ./dist/ flake8 flake8 --version flake8 src/flake8/ tests/ setup.py From 72a6425ad3e1e6b48773093e052f8081e2d7c496 Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Sat, 27 May 2017 19:39:33 -0500 Subject: [PATCH 3/5] Add release note for bug 318 --- docs/source/release-notes/3.4.0.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/source/release-notes/3.4.0.rst b/docs/source/release-notes/3.4.0.rst index e1c42ea..8c08160 100644 --- a/docs/source/release-notes/3.4.0.rst +++ b/docs/source/release-notes/3.4.0.rst @@ -3,6 +3,9 @@ You can view the `3.4.0 milestone`_ on GitLab for more details. +- Refine logic around ``--select`` and ``--ignore`` when combined with the + default values for each. (See also `GitLab#318`_) + - Handle spaces as an alternate separate for error codes, e.g., ``--ignore 'E123 E234'``. (See also `GitLab#329`_) @@ -14,6 +17,8 @@ You can view the `3.4.0 milestone`_ on GitLab for more details. https://gitlab.com/pycqa/flake8/milestones/18 .. issue links +.. _GitLab#318: + https://gitlab.com/pycqa/flake8/issues/318 .. _GitLab#329: https://gitlab.com/pycqa/flake8/issues/329 .. _GitLab#330: From 178092954d2b6d5293f6211bd4e2f9f37bb8c525 Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Sat, 27 May 2017 20:16:15 -0500 Subject: [PATCH 4/5] Add extra test cases for violation decisions --- tests/unit/test_style_guide.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unit/test_style_guide.py b/tests/unit/test_style_guide.py index 10550f8..7ae77d7 100644 --- a/tests/unit/test_style_guide.py +++ b/tests/unit/test_style_guide.py @@ -128,6 +128,10 @@ def test_is_user_selected_excludes_errors(select_list, error_code): (['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.""" @@ -159,10 +163,20 @@ def test_should_report_error(select_list, ignore_list, error_code, expected): 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), + (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, From aefa79535f985bd19c375332abea6f3712815a0a Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Sat, 27 May 2017 20:34:04 -0500 Subject: [PATCH 5/5] Simplify conditoinal in decision logic Add test to cover branch of decision logic we were not previously exercising --- src/flake8/style_guide.py | 4 ++-- tests/unit/test_style_guide.py | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/flake8/style_guide.py b/src/flake8/style_guide.py index 38fc29a..ebc01ed 100644 --- a/src/flake8/style_guide.py +++ b/src/flake8/style_guide.py @@ -146,8 +146,8 @@ class StyleGuide(object): 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 and select is None)): + if (select is None and + (extra_select is None or not self._using_default_ignore)): return Decision.Ignored return Decision.Selected diff --git a/tests/unit/test_style_guide.py b/tests/unit/test_style_guide.py index 7ae77d7..5c24048 100644 --- a/tests/unit/test_style_guide.py +++ b/tests/unit/test_style_guide.py @@ -169,6 +169,8 @@ def test_should_report_error(select_list, ignore_list, error_code, expected): (['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',