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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

(馃悶) blank-line-with-whitespace (W293) fix is highly unsafe #10038

Closed
KotlinIsland opened this issue Feb 19, 2024 · 2 comments 路 Fixed by #10049
Closed

(馃悶) blank-line-with-whitespace (W293) fix is highly unsafe #10038

KotlinIsland opened this issue Feb 19, 2024 · 2 comments 路 Fixed by #10049
Labels
bug Something isn't working fixes Related to suggested fixes for violations

Comments

@KotlinIsland
Copy link
Contributor

KotlinIsland commented Feb 19, 2024

def test_something():
    assert foo() == """\
assert [1, 2, 3] == [1, 4, 3]
  
  At index 1 diff: 2 != 4
  Use -v to get more diff"""

Here, the white space on line 2 of the expected value is intentional, yet ruff check --fix deletes it without question.

Or is this actually a bug, and it shouldn't be applying to strings at all?

Workaround

Disable this rule and use ruff format instead, as it doesn't modify the content of strings.

@MichaReiser
Copy link
Member

I think that rule shouldn't apply to strings, or if it does, it should be marked as unsafe.

We generally recommend the use of the formatter over style related lint rules if you use the formatter anyway (avoids running unnecessary rules )

@MichaReiser MichaReiser added bug Something isn't working fixes Related to suggested fixes for violations labels Feb 19, 2024
@dhruvmanila
Copy link
Member

It might be an actual bug because we do mark it as unsafe inside a multiline string:

diagnostic.set_fix(Fix::applicable_edit(
Edit::range_deletion(range),
// Removing trailing whitespace is not safe inside multiline strings.
if indexer.multiline_ranges().contains_range(range) {
Applicability::Unsafe
} else {
Applicability::Safe
},
));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixes Related to suggested fixes for violations
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants