From 9d8c6ba6718c41dcd378ffe3773f949bf4a8392b Mon Sep 17 00:00:00 2001 From: Florian Best Date: Wed, 1 Feb 2023 14:16:47 +0100 Subject: [PATCH] more builtin name checks when autofixing (#2430) --- .../test/fixtures/flake8_simplify/SIM101.py | 7 ++++ .../test/fixtures/flake8_simplify/SIM110.py | 30 ++++++++++++++ .../test/fixtures/flake8_simplify/SIM111.py | 40 +++++++++++++++++++ .../test/fixtures/flake8_simplify/SIM210.py | 8 ++-- .../flake8_simplify/rules/ast_bool_op.rs | 3 ++ src/rules/flake8_simplify/rules/ast_for.rs | 4 +- ...ke8_simplify__tests__SIM110_SIM110.py.snap | 15 ++++++- ...ke8_simplify__tests__SIM111_SIM111.py.snap | 30 ++++++++++++++ ...ke8_simplify__tests__SIM210_SIM210.py.snap | 8 ++-- 9 files changed, 134 insertions(+), 11 deletions(-) diff --git a/resources/test/fixtures/flake8_simplify/SIM101.py b/resources/test/fixtures/flake8_simplify/SIM101.py index ca3125ff2c..9d3d31753b 100644 --- a/resources/test/fixtures/flake8_simplify/SIM101.py +++ b/resources/test/fixtures/flake8_simplify/SIM101.py @@ -21,3 +21,10 @@ if isinstance(a, int) and isinstance(b, bool) or isinstance(a, float): if isinstance(a, bool) or isinstance(b, str): pass + +def f(): + # OK + def isinstance(a, b): + return False + if isinstance(a, int) or isinstance(a, float): + pass diff --git a/resources/test/fixtures/flake8_simplify/SIM110.py b/resources/test/fixtures/flake8_simplify/SIM110.py index f06af587bb..30ce25bb0e 100644 --- a/resources/test/fixtures/flake8_simplify/SIM110.py +++ b/resources/test/fixtures/flake8_simplify/SIM110.py @@ -117,6 +117,26 @@ def f(): return False +def f(): + def any(exp): + pass + + for x in iterable: + if check(x): + return True + return False + + +def f(): + def all(exp): + pass + + for x in iterable: + if check(x): + return False + return True + + def f(): x = 1 @@ -125,3 +145,13 @@ def f(): if check(x): return True return False + + +def f(): + x = 1 + + # SIM111 + for x in iterable: + if check(x): + return False + return True diff --git a/resources/test/fixtures/flake8_simplify/SIM111.py b/resources/test/fixtures/flake8_simplify/SIM111.py index ddf6f82d6c..30ce25bb0e 100644 --- a/resources/test/fixtures/flake8_simplify/SIM111.py +++ b/resources/test/fixtures/flake8_simplify/SIM111.py @@ -115,3 +115,43 @@ def f(): else: return True return False + + +def f(): + def any(exp): + pass + + for x in iterable: + if check(x): + return True + return False + + +def f(): + def all(exp): + pass + + for x in iterable: + if check(x): + return False + return True + + +def f(): + x = 1 + + # SIM110 + for x in iterable: + if check(x): + return True + return False + + +def f(): + x = 1 + + # SIM111 + for x in iterable: + if check(x): + return False + return True diff --git a/resources/test/fixtures/flake8_simplify/SIM210.py b/resources/test/fixtures/flake8_simplify/SIM210.py index e80972ffce..dafe5e24da 100644 --- a/resources/test/fixtures/flake8_simplify/SIM210.py +++ b/resources/test/fixtures/flake8_simplify/SIM210.py @@ -6,7 +6,9 @@ a = True if b + c else False # SIM210 a = False if b else True # OK -def bool(): - return False +def f(): + # OK + def bool(): + return False -a = True if b else False # OK + a = True if b else False diff --git a/src/rules/flake8_simplify/rules/ast_bool_op.rs b/src/rules/flake8_simplify/rules/ast_bool_op.rs index 3a21e8f2cd..a37d8b3ce3 100644 --- a/src/rules/flake8_simplify/rules/ast_bool_op.rs +++ b/src/rules/flake8_simplify/rules/ast_bool_op.rs @@ -47,6 +47,9 @@ pub fn duplicate_isinstance_call(checker: &mut Checker, expr: &Expr) { if func_name != "isinstance" { continue; } + if !checker.is_builtin("isinstance") { + continue; + } // Collect the name of the argument. let ExprKind::Name { id: arg_name, .. } = &args[0].node else { diff --git a/src/rules/flake8_simplify/rules/ast_for.rs b/src/rules/flake8_simplify/rules/ast_for.rs index cce4ca1ebe..6218b07b8a 100644 --- a/src/rules/flake8_simplify/rules/ast_for.rs +++ b/src/rules/flake8_simplify/rules/ast_for.rs @@ -202,7 +202,7 @@ pub fn convert_for_loop_to_any_all(checker: &mut Checker, stmt: &Stmt, sibling: }, Range::from_located(stmt), ); - if checker.patch(diagnostic.kind.rule()) { + if checker.patch(diagnostic.kind.rule()) && checker.is_builtin("any") { diagnostic.amend(Fix::replacement( contents, stmt.location, @@ -249,7 +249,7 @@ pub fn convert_for_loop_to_any_all(checker: &mut Checker, stmt: &Stmt, sibling: }, Range::from_located(stmt), ); - if checker.patch(diagnostic.kind.rule()) { + if checker.patch(diagnostic.kind.rule()) && checker.is_builtin("all") { diagnostic.amend(Fix::replacement( contents, stmt.location, diff --git a/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM110_SIM110.py.snap b/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM110_SIM110.py.snap index 9242e34718..71d25f431a 100644 --- a/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM110_SIM110.py.snap +++ b/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM110_SIM110.py.snap @@ -68,14 +68,25 @@ expression: diagnostics end_location: row: 126 column: 23 + fix: ~ + parent: ~ +- kind: + ConvertLoopToAny: + any: return any(check(x) for x in iterable) + location: + row: 144 + column: 4 + end_location: + row: 146 + column: 23 fix: content: - return any(check(x) for x in iterable) location: - row: 124 + row: 144 column: 4 end_location: - row: 127 + row: 147 column: 16 parent: ~ diff --git a/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM111_SIM111.py.snap b/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM111_SIM111.py.snap index ab38250525..f656ad9763 100644 --- a/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM111_SIM111.py.snap +++ b/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM111_SIM111.py.snap @@ -78,4 +78,34 @@ expression: diagnostics row: 87 column: 19 parent: ~ +- kind: + ConvertLoopToAll: + all: return all(not check(x) for x in iterable) + location: + row: 134 + column: 4 + end_location: + row: 136 + column: 24 + fix: ~ + parent: ~ +- kind: + ConvertLoopToAll: + all: return all(not check(x) for x in iterable) + location: + row: 154 + column: 4 + end_location: + row: 156 + column: 24 + fix: + content: + - return all(not check(x) for x in iterable) + location: + row: 154 + column: 4 + end_location: + row: 157 + column: 15 + parent: ~ diff --git a/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM210_SIM210.py.snap b/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM210_SIM210.py.snap index 4521af3433..b2609d824e 100644 --- a/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM210_SIM210.py.snap +++ b/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM210_SIM210.py.snap @@ -63,11 +63,11 @@ expression: diagnostics IfExprWithTrueFalse: expr: b location: - row: 12 - column: 4 + row: 14 + column: 8 end_location: - row: 12 - column: 24 + row: 14 + column: 28 fix: ~ parent: ~