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.

Sign up for GitHub

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

Jump to bottom

Auto fix for RUF015 is not safe #7019

Closed
jaap3 opened this issue Aug 31, 2023 · 3 comments
Closed

Auto fix for RUF015 is not safe #7019

jaap3 opened this issue Aug 31, 2023 · 3 comments

Comments

@jaap3
Copy link
Contributor

jaap3 commented Aug 31, 2023 •
edited
Loading

Using ruff 0.0.286 the following:

bar = (n for n in ())
try:
    foo = list(bar)[0]
except IndexError:
    pass
else:
    print(foo)

is rewritten to:

bar = (n for n in ())
try:
    foo = next(iter(bar))
except IndexError:
    pass
else:
    print(foo)

The rewritten code is now prone to raising an uncaught StopIteration error.

Same thing goes for the rewrite of:

import contextlib

bar = (n for n in ())

with contextlib.suppress(IndexError):
    foo = list(bar)[0]
    print(foo)
@charliermarsh
Copy link
Member

Thanks — this is mentioned in the documentation (https://beta.ruff.rs/docs/rules/unnecessary-iterable-allocation-for-first-element/) and the rule is marked internally as a suggested fix. In the future, suggested fixes will require a dedicated flag to run rather than running automatically, which should help with this.

@jaap3
Copy link
Contributor Author

jaap3 commented Aug 31, 2023

👍 thanks, didn't think to check the docs for this. I will mark it as unfixable in our config for now (with a reference to this ticket). Looking forward to a version that doesn't apply this fix by default, any idea how far in the future that might be? Can I watch an issue/PR to stay updated?

@MichaReiser
Copy link
Member

You can track the applicability work here #4185 (or the parent issue)

@zanieb zanieb mentioned this issue Oct 3, 2023
Merged
zanieb added a commit that referenced this issue Oct 6, 2023
@zanieb @evanrittenhouse
Update CLI to respect fix applicability (#7769)
22e1874 
Rebase of #5119 authored by
@evanrittenhouse with additional refinements.

## Changes

- Adds `--unsafe-fixes` / `--no-unsafe-fixes` flags to `ruff check`
- Violations with unsafe fixes are not shown as fixable unless opted-in
- Fix applicability is respected now
    - `Applicability::Never` fixes are no longer applied
    - `Applicability::Sometimes` fixes require opt-in
    - `Applicability::Always` fixes are unchanged
- Hints for availability of `--unsafe-fixes` added to `ruff check`
output

## Examples

Check hints at hidden unsafe fixes
```
❯ ruff check example.py --no-cache --select F601,W292
example.py:1:14: F601 Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).
```

We could add an indicator for which violations have hidden fixes in the
future.

Check treats unsafe fixes as applicable with opt-in
```
❯ ruff check example.py --no-cache --select F601,W292 --unsafe-fixes
example.py:1:14: F601 [*] Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 2 fixable with the --fix option.
```

Also can be enabled in the config file

```
❯ cat ruff.toml
unsafe-fixes = true
```

And opted-out per invocation

```
❯ ruff check example.py --no-cache --select F601,W292 --no-unsafe-fixes
example.py:1:14: F601 Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).
```

Diff does not include unsafe fixes
```
❯ ruff check example.py --no-cache --select F601,W292 --diff
--- example.py
+++ example.py
@@ -1,2 +1,2 @@
 x = {'a': 1, 'a': 1}
-print(('foo'))
+print(('foo'))
\ No newline at end of file

Would fix 1 error.
```

Unless there is opt-in
```
❯ ruff check example.py --no-cache --select F601,W292 --diff --unsafe-fixes
--- example.py
+++ example.py
@@ -1,2 +1,2 @@
-x = {'a': 1}
-print(('foo'))
+x = {'a': 1, 'a': 1}
+print(('foo'))
\ No newline at end of file

Would fix 2 errors.
```

#7790 will improve the diff
messages following this pull request

Similarly, `--fix` and `--fix-only` require the `--unsafe-fixes` flag to
apply unsafe fixes.

## Related

Replaces #5119
Closes #4185
Closes #7214
Closes #4845
Closes #3863
Addresses #6835
Addresses #7019
Needs follow-up #6962
Needs follow-up #4845
Needs follow-up #7436
Needs follow-up #7025
Needs follow-up #6434
Follow-up #7790 
Follow-up #7792

---------

Co-authored-by: Evan Rittenhouse <evanrittenhouse@gmail.com>
konstin pushed a commit that referenced this issue Oct 11, 2023
@zanieb @evanrittenhouse @konstin
Update CLI to respect fix applicability (#7769)
c4cd510 
Rebase of #5119 authored by
@evanrittenhouse with additional refinements.

## Changes

- Adds `--unsafe-fixes` / `--no-unsafe-fixes` flags to `ruff check`
- Violations with unsafe fixes are not shown as fixable unless opted-in
- Fix applicability is respected now
    - `Applicability::Never` fixes are no longer applied
    - `Applicability::Sometimes` fixes require opt-in
    - `Applicability::Always` fixes are unchanged
- Hints for availability of `--unsafe-fixes` added to `ruff check`
output

## Examples

Check hints at hidden unsafe fixes
```
❯ ruff check example.py --no-cache --select F601,W292
example.py:1:14: F601 Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).
```

We could add an indicator for which violations have hidden fixes in the
future.

Check treats unsafe fixes as applicable with opt-in
```
❯ ruff check example.py --no-cache --select F601,W292 --unsafe-fixes
example.py:1:14: F601 [*] Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 2 fixable with the --fix option.
```

Also can be enabled in the config file

```
❯ cat ruff.toml
unsafe-fixes = true
```

And opted-out per invocation

```
❯ ruff check example.py --no-cache --select F601,W292 --no-unsafe-fixes
example.py:1:14: F601 Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).
```

Diff does not include unsafe fixes
```
❯ ruff check example.py --no-cache --select F601,W292 --diff
--- example.py
+++ example.py
@@ -1,2 +1,2 @@
 x = {'a': 1, 'a': 1}
-print(('foo'))
+print(('foo'))
\ No newline at end of file

Would fix 1 error.
```

Unless there is opt-in
```
❯ ruff check example.py --no-cache --select F601,W292 --diff --unsafe-fixes
--- example.py
+++ example.py
@@ -1,2 +1,2 @@
-x = {'a': 1}
-print(('foo'))
+x = {'a': 1, 'a': 1}
+print(('foo'))
\ No newline at end of file

Would fix 2 errors.
```

#7790 will improve the diff
messages following this pull request

Similarly, `--fix` and `--fix-only` require the `--unsafe-fixes` flag to
apply unsafe fixes.

## Related

Replaces #5119
Closes #4185
Closes #7214
Closes #4845
Closes #3863
Addresses #6835
Addresses #7019
Needs follow-up #6962
Needs follow-up #4845
Needs follow-up #7436
Needs follow-up #7025
Needs follow-up #6434
Follow-up #7790 
Follow-up #7792

---------

Co-authored-by: Evan Rittenhouse <evanrittenhouse@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jaap3 @MichaReiser @charliermarsh