From d06d85dffd604a706c28f2ced215c63eb85ed075 Mon Sep 17 00:00:00 2001 From: Martin Thoma Date: Thu, 10 Feb 2022 07:50:18 +0100 Subject: [PATCH] SIM106: Remove false-positives (#95) Don't trigger if the 'else' block ends with a ValueError or a NotImplementedError. Closes #87 --- flake8_simplify.py | 12 ++++++++++++ tests/test_simplify.py | 29 ++++++++++++++++++++++++----- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/flake8_simplify.py b/flake8_simplify.py index c2da734..a49f270 100644 --- a/flake8_simplify.py +++ b/flake8_simplify.py @@ -421,6 +421,18 @@ def _get_sim106(node: ast.If) -> List[Tuple[int, int, str]]: ) if not (just_one or many): return errors + ast_raise = node.orelse[-1] + if not isinstance(ast_raise, ast.Raise): + return errors + ast_raised = ast_raise.exc + if ( + isinstance(ast_raised, ast.Call) + and ast_raised.func + and isinstance(ast_raised.func, ast.Name) + and ast_raised.func.id in ["ValueError", "NotImplementedError"] + ): + return errors + errors.append((node.lineno, node.col_offset, SIM106)) return errors diff --git a/tests/test_simplify.py b/tests/test_simplify.py index b429d04..d59ba34 100644 --- a/tests/test_simplify.py +++ b/tests/test_simplify.py @@ -195,14 +195,33 @@ def test_sim106(): assert ret == {"1:0 SIM106 Handle error-cases first"} -def test_sim106_no(): - ret = _results( +@pytest.mark.parametrize( + "s", + ( + """if image_extension in ['.jpg', '.jpeg']: + return 'JPEG' +elif image_extension in ['.png']: + return 'PNG' +else: + raise ValueError("Unknwon image extension {image extension}")""", + """if image_extension in ['.jpg', '.jpeg']: + return 'JPEG' +elif image_extension in ['.png']: + return 'PNG' +else: + logger.error("Unknwon image extension {image extension}") + raise ValueError("Unknwon image extension {image extension}")""", """if cond: raise Exception else: - raise Exception""" - ) - assert ret == set() + raise Exception""", + ), + ids=["ValueError", "ValueError-with-logging", "TwoExceptions"], +) +def test_sim106_false_positive(s): + ret = _results(s) + for el in ret: + assert "SIM106" not in el def test_sim107():