mirror of https://github.com/astral-sh/ruff
[`flake8-pie`] Mark autofix for `PIE804` as unsafe if the dictionary contains comments (#18046)
## Summary Fixes #18036
This commit is contained in:
parent
6f8f7506b4
commit
6b3ff6f5b8
|
|
@ -28,3 +28,14 @@ abc(a=1, **{'a': c}, **{'b': c}) # PIE804
|
||||||
# Some values need to be parenthesized.
|
# Some values need to be parenthesized.
|
||||||
abc(foo=1, **{'bar': (bar := 1)}) # PIE804
|
abc(foo=1, **{'bar': (bar := 1)}) # PIE804
|
||||||
abc(foo=1, **{'bar': (yield 1)}) # PIE804
|
abc(foo=1, **{'bar': (yield 1)}) # PIE804
|
||||||
|
|
||||||
|
# https://github.com/astral-sh/ruff/issues/18036
|
||||||
|
# The autofix for this is unsafe due to the comments inside the dictionary.
|
||||||
|
foo(
|
||||||
|
**{
|
||||||
|
# Comment 1
|
||||||
|
"x": 1.0,
|
||||||
|
# Comment 2
|
||||||
|
"y": 2.0,
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,7 @@
|
||||||
use itertools::Itertools;
|
use itertools::Itertools;
|
||||||
use rustc_hash::{FxBuildHasher, FxHashSet};
|
use rustc_hash::{FxBuildHasher, FxHashSet};
|
||||||
|
|
||||||
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
|
use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
|
||||||
use ruff_macros::{derive_message_formats, ViolationMetadata};
|
use ruff_macros::{derive_message_formats, ViolationMetadata};
|
||||||
use ruff_python_ast::parenthesize::parenthesized_range;
|
use ruff_python_ast::parenthesize::parenthesized_range;
|
||||||
use ruff_python_ast::{self as ast, Expr};
|
use ruff_python_ast::{self as ast, Expr};
|
||||||
|
|
@ -19,6 +19,7 @@ use crate::fix::edits::{remove_argument, Parentheses};
|
||||||
/// arguments directly.
|
/// arguments directly.
|
||||||
///
|
///
|
||||||
/// ## Example
|
/// ## Example
|
||||||
|
///
|
||||||
/// ```python
|
/// ```python
|
||||||
/// def foo(bar):
|
/// def foo(bar):
|
||||||
/// return bar + 1
|
/// return bar + 1
|
||||||
|
|
@ -28,6 +29,7 @@ use crate::fix::edits::{remove_argument, Parentheses};
|
||||||
/// ```
|
/// ```
|
||||||
///
|
///
|
||||||
/// Use instead:
|
/// Use instead:
|
||||||
|
///
|
||||||
/// ```python
|
/// ```python
|
||||||
/// def foo(bar):
|
/// def foo(bar):
|
||||||
/// return bar + 1
|
/// return bar + 1
|
||||||
|
|
@ -36,6 +38,26 @@ use crate::fix::edits::{remove_argument, Parentheses};
|
||||||
/// print(foo(bar=2)) # prints 3
|
/// print(foo(bar=2)) # prints 3
|
||||||
/// ```
|
/// ```
|
||||||
///
|
///
|
||||||
|
/// ## Fix safety
|
||||||
|
///
|
||||||
|
/// This rule's fix is marked as unsafe for dictionaries with comments interleaved between
|
||||||
|
/// the items, as comments may be removed.
|
||||||
|
///
|
||||||
|
/// For example, the fix would be marked as unsafe in the following case:
|
||||||
|
///
|
||||||
|
/// ```python
|
||||||
|
/// foo(
|
||||||
|
/// **{
|
||||||
|
/// # comment
|
||||||
|
/// "x": 1.0,
|
||||||
|
/// # comment
|
||||||
|
/// "y": 2.0,
|
||||||
|
/// }
|
||||||
|
/// )
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// as this is converted to `foo(x=1.0, y=2.0)` without any of the comments.
|
||||||
|
///
|
||||||
/// ## References
|
/// ## References
|
||||||
/// - [Python documentation: Dictionary displays](https://docs.python.org/3/reference/expressions.html#dictionary-displays)
|
/// - [Python documentation: Dictionary displays](https://docs.python.org/3/reference/expressions.html#dictionary-displays)
|
||||||
/// - [Python documentation: Calls](https://docs.python.org/3/reference/expressions.html#calls)
|
/// - [Python documentation: Calls](https://docs.python.org/3/reference/expressions.html#calls)
|
||||||
|
|
@ -113,7 +135,7 @@ pub(crate) fn unnecessary_dict_kwargs(checker: &Checker, call: &ast::ExprCall) {
|
||||||
.iter()
|
.iter()
|
||||||
.all(|kwarg| !duplicate_keywords.contains(kwarg))
|
.all(|kwarg| !duplicate_keywords.contains(kwarg))
|
||||||
{
|
{
|
||||||
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
|
let edit = Edit::range_replacement(
|
||||||
kwargs
|
kwargs
|
||||||
.iter()
|
.iter()
|
||||||
.zip(dict.iter_values())
|
.zip(dict.iter_values())
|
||||||
|
|
@ -134,7 +156,15 @@ pub(crate) fn unnecessary_dict_kwargs(checker: &Checker, call: &ast::ExprCall) {
|
||||||
})
|
})
|
||||||
.join(", "),
|
.join(", "),
|
||||||
keyword.range(),
|
keyword.range(),
|
||||||
)));
|
);
|
||||||
|
diagnostic.set_fix(Fix::applicable_edit(
|
||||||
|
edit,
|
||||||
|
if checker.comment_ranges().intersects(dict.range()) {
|
||||||
|
Applicability::Unsafe
|
||||||
|
} else {
|
||||||
|
Applicability::Safe
|
||||||
|
},
|
||||||
|
));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -208,6 +208,8 @@ PIE804.py:29:12: PIE804 [*] Unnecessary `dict` kwargs
|
||||||
29 |-abc(foo=1, **{'bar': (bar := 1)}) # PIE804
|
29 |-abc(foo=1, **{'bar': (bar := 1)}) # PIE804
|
||||||
29 |+abc(foo=1, bar=(bar := 1)) # PIE804
|
29 |+abc(foo=1, bar=(bar := 1)) # PIE804
|
||||||
30 30 | abc(foo=1, **{'bar': (yield 1)}) # PIE804
|
30 30 | abc(foo=1, **{'bar': (yield 1)}) # PIE804
|
||||||
|
31 31 |
|
||||||
|
32 32 | # https://github.com/astral-sh/ruff/issues/18036
|
||||||
|
|
||||||
PIE804.py:30:12: PIE804 [*] Unnecessary `dict` kwargs
|
PIE804.py:30:12: PIE804 [*] Unnecessary `dict` kwargs
|
||||||
|
|
|
|
||||||
|
|
@ -215,6 +217,8 @@ PIE804.py:30:12: PIE804 [*] Unnecessary `dict` kwargs
|
||||||
29 | abc(foo=1, **{'bar': (bar := 1)}) # PIE804
|
29 | abc(foo=1, **{'bar': (bar := 1)}) # PIE804
|
||||||
30 | abc(foo=1, **{'bar': (yield 1)}) # PIE804
|
30 | abc(foo=1, **{'bar': (yield 1)}) # PIE804
|
||||||
| ^^^^^^^^^^^^^^^^^^^^ PIE804
|
| ^^^^^^^^^^^^^^^^^^^^ PIE804
|
||||||
|
31 |
|
||||||
|
32 | # https://github.com/astral-sh/ruff/issues/18036
|
||||||
|
|
|
|
||||||
= help: Remove unnecessary kwargs
|
= help: Remove unnecessary kwargs
|
||||||
|
|
||||||
|
|
@ -224,3 +228,34 @@ PIE804.py:30:12: PIE804 [*] Unnecessary `dict` kwargs
|
||||||
29 29 | abc(foo=1, **{'bar': (bar := 1)}) # PIE804
|
29 29 | abc(foo=1, **{'bar': (bar := 1)}) # PIE804
|
||||||
30 |-abc(foo=1, **{'bar': (yield 1)}) # PIE804
|
30 |-abc(foo=1, **{'bar': (yield 1)}) # PIE804
|
||||||
30 |+abc(foo=1, bar=(yield 1)) # PIE804
|
30 |+abc(foo=1, bar=(yield 1)) # PIE804
|
||||||
|
31 31 |
|
||||||
|
32 32 | # https://github.com/astral-sh/ruff/issues/18036
|
||||||
|
33 33 | # The autofix for this is unsafe due to the comments inside the dictionary.
|
||||||
|
|
||||||
|
PIE804.py:35:5: PIE804 [*] Unnecessary `dict` kwargs
|
||||||
|
|
|
||||||
|
33 | # The autofix for this is unsafe due to the comments inside the dictionary.
|
||||||
|
34 | foo(
|
||||||
|
35 | / **{
|
||||||
|
36 | | # Comment 1
|
||||||
|
37 | | "x": 1.0,
|
||||||
|
38 | | # Comment 2
|
||||||
|
39 | | "y": 2.0,
|
||||||
|
40 | | }
|
||||||
|
| |_____^ PIE804
|
||||||
|
41 | )
|
||||||
|
|
|
||||||
|
= help: Remove unnecessary kwargs
|
||||||
|
|
||||||
|
ℹ Unsafe fix
|
||||||
|
32 32 | # https://github.com/astral-sh/ruff/issues/18036
|
||||||
|
33 33 | # The autofix for this is unsafe due to the comments inside the dictionary.
|
||||||
|
34 34 | foo(
|
||||||
|
35 |- **{
|
||||||
|
36 |- # Comment 1
|
||||||
|
37 |- "x": 1.0,
|
||||||
|
38 |- # Comment 2
|
||||||
|
39 |- "y": 2.0,
|
||||||
|
40 |- }
|
||||||
|
35 |+ x=1.0, y=2.0
|
||||||
|
41 36 | )
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue