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

Document comment policy around fix safety #14300

Merged
merged 1 commit into from
Nov 13, 2024
Merged

Document comment policy around fix safety #14300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions crates/ruff_diagnostics/src/fix.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,21 @@ use crate::edit::Edit;
#[cfg_attr(feature = "serde", serde(rename_all = "lowercase"))]
pub enum Applicability {
/// The fix is unsafe and should only be displayed for manual application by the user.
///
/// The fix is likely to be incorrect or the resulting code may have invalid syntax.
DisplayOnly,

/// The fix is unsafe and should only be applied with user opt-in.
/// The fix may be what the user intended, but it is uncertain; the resulting code will have valid syntax.
///
/// The fix may be what the user intended, but it is uncertain. The resulting code will have
/// valid syntax, but may lead to a change in runtime behavior, the removal of user comments,
/// or both.
Unsafe,

/// The fix is safe and can always be applied.
/// The fix is definitely what the user intended, or it maintains the exact meaning of the code.
///
/// The fix is definitely what the user intended, or maintains the exact meaning of the code.
/// User comments are preserved, unless the fix removes an entire statement or expression.
Safe,
}

Expand Down
10 changes: 7 additions & 3 deletions docs/linter.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,11 @@ whether a rule supports fixing, see [_Rules_](rules.md).
### Fix safety

Ruff labels fixes as "safe" and "unsafe". The meaning and intent of your code will be retained when
applying safe fixes, but the meaning could be changed when applying unsafe fixes.
applying safe fixes, but the meaning could change when applying unsafe fixes.

Specifically, an unsafe fix could lead to a change in runtime behavior, the removal of comments, or both,
while safe fixes are intended to preserve runtime behavior and will only remove comments when deleting
entire statements or expressions (e.g., removing unused imports).

For example, [`unnecessary-iterable-allocation-for-first-element`](rules/unnecessary-iterable-allocation-for-first-element.md)
(`RUF015`) is a rule which checks for potentially unperformant use of `list(...)[0]`. The fix
Expand All @@ -177,7 +181,7 @@ $ python -m timeit "head = next(iter(range(99999999)))"
5000000 loops, best of 5: 70.8 nsec per loop
```

However, when the collection is empty, this changes the raised exception from an `IndexError` to `StopIteration`:
However, when the collection is empty, this raised exception changes from an `IndexError` to `StopIteration`:

```console
$ python -c 'list(range(0))[0]'
Expand All @@ -193,7 +197,7 @@ Traceback (most recent call last):
StopIteration
```

Since this could break error handling, this fix is categorized as unsafe.
Since the change in exception type could break error handling upstream, this fix is categorized as unsafe.

Ruff only enables safe fixes by default. Unsafe fixes can be enabled by settings [`unsafe-fixes`](settings.md#unsafe-fixes) in your configuration file or passing the `--unsafe-fixes` flag to `ruff check`:

Expand Down
Loading