Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add padding to prevent some autofix errors #7461

Merged
merged 1 commit into from
Sep 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,6 @@
setattr(foo, "abc123", None)
setattr(foo, r"abc123", None)
setattr(foo.bar, r"baz", None)

# Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722458885
assert getattr(func, '_rpc')is True
1 change: 1 addition & 0 deletions crates/ruff/resources/test/fixtures/pycodestyle/E713.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,4 @@
assert {"x": not foo} in bar
assert [42, not foo] in bar
assert not (re.search(r"^.:\\Users\\[^\\]*\\Downloads\\.*") is None)
assert not('name' in request)or not request['name']
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,8 @@ def isinstances():
result = isinstance(var[6], int) or isinstance(var[7], int)
result = isinstance(var[6], (float, int)) or False
return result


# Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722460483
if(isinstance(self.k, int)) or (isinstance(self.k, float)):
...
5 changes: 2 additions & 3 deletions crates/ruff/resources/test/fixtures/pyupgrade/UP003.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,5 @@
# OK
y = x.dtype.type(0.0)

# OK
type = lambda *args, **kwargs: None
type("")
# Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722459841
assert isinstance(fullname, type("")is not True)
5 changes: 5 additions & 0 deletions crates/ruff/resources/test/fixtures/pyupgrade/UP012.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,8 @@
(f"foo{bar}").encode(encoding="utf-8")
("unicode text©").encode("utf-8")
("unicode text©").encode(encoding="utf-8")


# Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722459882
def _match_ignore(line):
input=stdin and'\n'.encode()or None
27 changes: 16 additions & 11 deletions crates/ruff/src/rules/flake8_bugbear/rules/getattr_with_constant.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::autofix::edits::pad;
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Constant, Expr};
Expand Down Expand Up @@ -80,17 +81,21 @@ pub(crate) fn getattr_with_constant(
let mut diagnostic = Diagnostic::new(GetAttrWithConstant, expr.range());
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
if matches!(
obj,
Expr::Name(_) | Expr::Attribute(_) | Expr::Subscript(_) | Expr::Call(_)
) {
format!("{}.{}", checker.locator().slice(obj), value)
} else {
// Defensively parenthesize any other expressions. For example, attribute accesses
// on `int` literals must be parenthesized, e.g., `getattr(1, "real")` becomes
// `(1).real`. The same is true for named expressions and others.
format!("({}).{}", checker.locator().slice(obj), value)
},
pad(
if matches!(
obj,
Expr::Name(_) | Expr::Attribute(_) | Expr::Subscript(_) | Expr::Call(_)
) {
format!("{}.{}", checker.locator().slice(obj), value)
} else {
// Defensively parenthesize any other expressions. For example, attribute accesses
// on `int` literals must be parenthesized, e.g., `getattr(1, "real")` becomes
// `(1).real`. The same is true for named expressions and others.
format!("({}).{}", checker.locator().slice(obj), value)
},
expr.range(),
checker.locator(),
),
expr.range(),
)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,4 +316,19 @@ B009_B010.py:34:1: B009 [*] Do not call `getattr` with a constant attribute valu
37 37 |
38 38 | # Valid setattr usage

B009_B010.py:58:8: B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access.
|
57 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722458885
58 | assert getattr(func, '_rpc')is True
| ^^^^^^^^^^^^^^^^^^^^^ B009
|
= help: Replace `getattr` with attribute access

ℹ Suggested fix
55 55 | setattr(foo.bar, r"baz", None)
56 56 |
57 57 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722458885
58 |-assert getattr(func, '_rpc')is True
58 |+assert func._rpc is True


Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ B009_B010.py:53:1: B010 [*] Do not call `setattr` with a constant attribute valu
53 |+foo.abc123 = None
54 54 | setattr(foo, r"abc123", None)
55 55 | setattr(foo.bar, r"baz", None)
56 56 |

B009_B010.py:54:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access.
|
Expand All @@ -100,13 +101,17 @@ B009_B010.py:54:1: B010 [*] Do not call `setattr` with a constant attribute valu
54 |-setattr(foo, r"abc123", None)
54 |+foo.abc123 = None
55 55 | setattr(foo.bar, r"baz", None)
56 56 |
57 57 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722458885

B009_B010.py:55:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access.
|
53 | setattr(foo, "abc123", None)
54 | setattr(foo, r"abc123", None)
55 | setattr(foo.bar, r"baz", None)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B010
56 |
57 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722458885
|
= help: Replace `setattr` with assignment

Expand All @@ -116,5 +121,8 @@ B009_B010.py:55:1: B010 [*] Do not call `setattr` with a constant attribute valu
54 54 | setattr(foo, r"abc123", None)
55 |-setattr(foo.bar, r"baz", None)
55 |+foo.bar.baz = None
56 56 |
57 57 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722458885
58 58 | assert getattr(func, '_rpc')is True


33 changes: 21 additions & 12 deletions crates/ruff/src/rules/pycodestyle/rules/not_tests.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::autofix::edits::pad;
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, CmpOp, Expr};
Expand Down Expand Up @@ -95,12 +96,16 @@ pub(crate) fn not_tests(checker: &mut Checker, unary_op: &ast::ExprUnaryOp) {
let mut diagnostic = Diagnostic::new(NotInTest, unary_op.operand.range());
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
generate_comparison(
left,
&[CmpOp::NotIn],
comparators,
unary_op.into(),
checker.indexer().comment_ranges(),
pad(
generate_comparison(
left,
&[CmpOp::NotIn],
comparators,
unary_op.into(),
checker.indexer().comment_ranges(),
checker.locator(),
),
unary_op.range(),
checker.locator(),
),
unary_op.range(),
Expand All @@ -114,12 +119,16 @@ pub(crate) fn not_tests(checker: &mut Checker, unary_op: &ast::ExprUnaryOp) {
let mut diagnostic = Diagnostic::new(NotIsTest, unary_op.operand.range());
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
generate_comparison(
left,
&[CmpOp::IsNot],
comparators,
unary_op.into(),
checker.indexer().comment_ranges(),
pad(
generate_comparison(
left,
&[CmpOp::IsNot],
comparators,
unary_op.into(),
checker.indexer().comment_ranges(),
checker.locator(),
),
unary_op.range(),
checker.locator(),
),
unary_op.range(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,20 @@ E713.py:14:9: E713 [*] Test for membership should be `not in`
16 16 |
17 17 | #: Okay

E713.py:40:12: E713 [*] Test for membership should be `not in`
|
38 | assert [42, not foo] in bar
39 | assert not (re.search(r"^.:\\Users\\[^\\]*\\Downloads\\.*") is None)
40 | assert not('name' in request)or not request['name']
| ^^^^^^^^^^^^^^^^^ E713
|
= help: Convert to `not in`

ℹ Fix
37 37 | assert {"x": not foo} in bar
38 38 | assert [42, not foo] in bar
39 39 | assert not (re.search(r"^.:\\Users\\[^\\]*\\Downloads\\.*") is None)
40 |-assert not('name' in request)or not request['name']
40 |+assert 'name' not in request or not request['name']


Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use itertools::Itertools;
use ruff_python_ast::{self as ast, Arguments, BoolOp, Expr};
use rustc_hash::{FxHashMap, FxHashSet};

use crate::autofix::edits::pad;
use crate::autofix::snippet::SourceCodeSnippet;
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
Expand Down Expand Up @@ -133,7 +134,10 @@ pub(crate) fn repeated_isinstance_calls(
expr.range(),
);
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Fix::automatic(Edit::range_replacement(call, expr.range())));
diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
pad(call, expr.range(), checker.locator()),
expr.range(),
)));
}
checker.diagnostics.push(diagnostic);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,4 +124,21 @@ repeated_isinstance_calls.py:30:14: PLR1701 [*] Merge `isinstance` calls: `isins
32 32 | # not merged but valid
33 33 | result = isinstance(var[5], int) and var[5] * 14 or isinstance(var[5], float) and var[5] * 14.4

repeated_isinstance_calls.py:42:3: PLR1701 [*] Merge `isinstance` calls: `isinstance(self.k, float | int)`
|
41 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722460483
42 | if(isinstance(self.k, int)) or (isinstance(self.k, float)):
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1701
43 | ...
|
= help: Replace with `isinstance(self.k, float | int)`

ℹ Fix
39 39 |
40 40 |
41 41 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722460483
42 |-if(isinstance(self.k, int)) or (isinstance(self.k, float)):
42 |+if isinstance(self.k, float | int):
43 43 | ...


Original file line number Diff line number Diff line change
Expand Up @@ -124,4 +124,21 @@ repeated_isinstance_calls.py:30:14: PLR1701 [*] Merge `isinstance` calls: `isins
32 32 | # not merged but valid
33 33 | result = isinstance(var[5], int) and var[5] * 14 or isinstance(var[5], float) and var[5] * 14.4

repeated_isinstance_calls.py:42:3: PLR1701 [*] Merge `isinstance` calls: `isinstance(self.k, (float, int))`
|
41 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722460483
42 | if(isinstance(self.k, int)) or (isinstance(self.k, float)):
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1701
43 | ...
|
= help: Replace with `isinstance(self.k, (float, int))`

ℹ Fix
39 39 |
40 40 |
41 41 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722460483
42 |-if(isinstance(self.k, int)) or (isinstance(self.k, float)):
42 |+if isinstance(self.k, (float, int)):
43 43 | ...


3 changes: 2 additions & 1 deletion crates/ruff/src/rules/pyupgrade/rules/type_of_primitive.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use ruff_python_ast::{self as ast, Expr};

use crate::autofix::edits::pad;
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_text_size::Ranged;
Expand Down Expand Up @@ -76,7 +77,7 @@ pub(crate) fn type_of_primitive(checker: &mut Checker, expr: &Expr, func: &Expr,
let builtin = primitive.builtin();
if checker.semantic().is_builtin(&builtin) {
diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
primitive.builtin(),
pad(primitive.builtin(), expr.range(), checker.locator()),
expr.range(),
)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use ruff_python_parser::{lexer, AsMode, Tok};
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextRange};

use crate::autofix::edits::{remove_argument, Parentheses};
use crate::autofix::edits::{pad, remove_argument, Parentheses};
use crate::checkers::ast::Checker;
use crate::registry::Rule;

Expand Down Expand Up @@ -151,7 +151,10 @@ fn replace_with_bytes_literal(
prev = range.end();
}

Fix::automatic(Edit::range_replacement(replacement, call.range()))
Fix::automatic(Edit::range_replacement(
pad(replacement, call.range(), locator),
call.range(),
))
}

/// UP012
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,19 @@ UP003.py:5:1: UP003 [*] Use `complex` instead of `type(...)`
7 7 | # OK
8 8 | type(arg)(" ")

UP003.py:14:29: UP003 [*] Use `str` instead of `type(...)`
|
13 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722459841
14 | assert isinstance(fullname, type("")is not True)
| ^^^^^^^^ UP003
|
= help: Replace `type(...)` with `str`

ℹ Fix
11 11 | y = x.dtype.type(0.0)
12 12 |
13 13 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722459841
14 |-assert isinstance(fullname, type("")is not True)
14 |+assert isinstance(fullname, str is not True)


Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ UP012.py:75:1: UP012 [*] Unnecessary UTF-8 `encoding` argument to `encode`
75 |+(f"foo{bar}").encode()
76 76 | ("unicode text©").encode("utf-8")
77 77 | ("unicode text©").encode(encoding="utf-8")
78 78 |

UP012.py:76:1: UP012 [*] Unnecessary UTF-8 `encoding` argument to `encode`
|
Expand All @@ -528,6 +529,8 @@ UP012.py:76:1: UP012 [*] Unnecessary UTF-8 `encoding` argument to `encode`
76 |-("unicode text©").encode("utf-8")
76 |+("unicode text©").encode()
77 77 | ("unicode text©").encode(encoding="utf-8")
78 78 |
79 79 |

UP012.py:77:1: UP012 [*] Unnecessary UTF-8 `encoding` argument to `encode`
|
Expand All @@ -544,5 +547,24 @@ UP012.py:77:1: UP012 [*] Unnecessary UTF-8 `encoding` argument to `encode`
76 76 | ("unicode text©").encode("utf-8")
77 |-("unicode text©").encode(encoding="utf-8")
77 |+("unicode text©").encode()
78 78 |
79 79 |
80 80 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722459882

UP012.py:82:17: UP012 [*] Unnecessary call to `encode` as UTF-8
|
80 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722459882
81 | def _match_ignore(line):
82 | input=stdin and'\n'.encode()or None
| ^^^^^^^^^^^^^ UP012
|
= help: Rewrite as bytes literal

ℹ Fix
79 79 |
80 80 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722459882
81 81 | def _match_ignore(line):
82 |- input=stdin and'\n'.encode()or None
82 |+ input=stdin and b'\n' or None