From e0fdc4c5e8a917e89b0aeab2dfd6a65a19017d8e Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 12 Jan 2023 16:17:27 -0500 Subject: [PATCH] Avoid SIM110/SIM110 errors with else statements (#1832) Closes #1831. --- .../test/fixtures/flake8_simplify/SIM110.py | 38 ++++++++++++-- .../test/fixtures/flake8_simplify/SIM111.py | 52 +++++++++++++++++-- src/flake8_simplify/rules/ast_for.rs | 12 ++++- ...ke8_simplify__tests__SIM110_SIM110.py.snap | 10 ++-- ...ke8_simplify__tests__SIM111_SIM111.py.snap | 18 +++---- 5 files changed, 108 insertions(+), 22 deletions(-) diff --git a/resources/test/fixtures/flake8_simplify/SIM110.py b/resources/test/fixtures/flake8_simplify/SIM110.py index 7f7daa05b0..dbf3e64420 100644 --- a/resources/test/fixtures/flake8_simplify/SIM110.py +++ b/resources/test/fixtures/flake8_simplify/SIM110.py @@ -1,5 +1,6 @@ def f(): - for x in iterable: # SIM110 + # SIM110 + for x in iterable: if check(x): return True return False @@ -20,14 +21,16 @@ def f(): def f(): - for x in iterable: # SIM111 + # SIM111 + for x in iterable: if check(x): return False return True def f(): - for x in iterable: # SIM111 + # SIM111 + for x in iterable: if not x.is_empty(): return False return True @@ -45,3 +48,32 @@ def f(): if check(x): return "foo" return "bar" + + +def f(): + for x in iterable: + if check(x): + return True + elif x.is_empty(): + return True + return False + + +def f(): + for x in iterable: + if check(x): + return True + else: + return True + return False + + +def f(): + for x in iterable: + if check(x): + return True + elif x.is_empty(): + return True + else: + return True + return False diff --git a/resources/test/fixtures/flake8_simplify/SIM111.py b/resources/test/fixtures/flake8_simplify/SIM111.py index eb92c7fdce..dbf3e64420 100644 --- a/resources/test/fixtures/flake8_simplify/SIM111.py +++ b/resources/test/fixtures/flake8_simplify/SIM111.py @@ -1,10 +1,18 @@ def f(): - for x in iterable: # SIM110 + # SIM110 + for x in iterable: if check(x): return True return False +def f(): + for x in iterable: + if check(x): + return True + return True + + def f(): for el in [1, 2, 3]: if is_true(el): @@ -13,21 +21,59 @@ def f(): def f(): - for x in iterable: # SIM111 + # SIM111 + for x in iterable: if check(x): return False return True def f(): - for x in iterable: # SIM 111 + # SIM111 + for x in iterable: if not x.is_empty(): return False return True +def f(): + for x in iterable: + if check(x): + return False + return False + + def f(): for x in iterable: if check(x): return "foo" return "bar" + + +def f(): + for x in iterable: + if check(x): + return True + elif x.is_empty(): + return True + return False + + +def f(): + for x in iterable: + if check(x): + return True + else: + return True + return False + + +def f(): + for x in iterable: + if check(x): + return True + elif x.is_empty(): + return True + else: + return True + return False diff --git a/src/flake8_simplify/rules/ast_for.rs b/src/flake8_simplify/rules/ast_for.rs index 530ef9e5b6..faae275ead 100644 --- a/src/flake8_simplify/rules/ast_for.rs +++ b/src/flake8_simplify/rules/ast_for.rs @@ -25,6 +25,7 @@ fn return_values<'a>(stmt: &'a Stmt, sibling: &'a Stmt) -> Option> { body, target, iter, + orelse, .. } = &stmt.node else { return None; @@ -35,16 +36,23 @@ fn return_values<'a>(stmt: &'a Stmt, sibling: &'a Stmt) -> Option> { if body.len() != 1 { return None; } + // TODO(charlie): If we have `else: return True` or `else: return False`, we + // should still be able to simplify. + if !orelse.is_empty() { + return None; + } let StmtKind::If { body: nested_body, - test: nested_test, - .. + test: nested_test, orelse: nested_orelse, } = &body[0].node else { return None; }; if nested_body.len() != 1 { return None; } + if !nested_orelse.is_empty() { + return None; + } let StmtKind::Return { value } = &nested_body[0].node else { return None; }; diff --git a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM110_SIM110.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM110_SIM110.py.snap index ac46076449..05cc0c3996 100644 --- a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM110_SIM110.py.snap +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM110_SIM110.py.snap @@ -1,22 +1,22 @@ --- source: src/flake8_simplify/mod.rs -expression: checks +expression: diagnostics --- - kind: ConvertLoopToAny: return any(check(x) for x in iterable) location: - row: 2 + row: 3 column: 4 end_location: - row: 4 + row: 5 column: 23 fix: content: return any(check(x) for x in iterable) location: - row: 2 + row: 3 column: 4 end_location: - row: 5 + row: 6 column: 16 parent: ~ diff --git a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM111_SIM111.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM111_SIM111.py.snap index 05cbdbd38a..95fc7e6cfa 100644 --- a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM111_SIM111.py.snap +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM111_SIM111.py.snap @@ -1,39 +1,39 @@ --- source: src/flake8_simplify/mod.rs -expression: checks +expression: diagnostics --- - kind: ConvertLoopToAll: return all(not check(x) for x in iterable) location: - row: 16 + row: 25 column: 4 end_location: - row: 18 + row: 27 column: 24 fix: content: return all(not check(x) for x in iterable) location: - row: 16 + row: 25 column: 4 end_location: - row: 19 + row: 28 column: 15 parent: ~ - kind: ConvertLoopToAll: return all(x.is_empty() for x in iterable) location: - row: 23 + row: 33 column: 4 end_location: - row: 25 + row: 35 column: 24 fix: content: return all(x.is_empty() for x in iterable) location: - row: 23 + row: 33 column: 4 end_location: - row: 26 + row: 36 column: 15 parent: ~