Update fix summary message in `check --diff` to include unsafe fix hints #7790

zanieb · 2023-10-03T18:42:48Z

Requires #7769

Updates the CLI display for ruff check --fix to hint availability of unsafe fixes.

❯ ruff check example.py --select F601,T201 --diff --no-cache
No errors fixed (1 fix available with `--unsafe-fixes`).

❯ ruff check example.py --select F601,T201,W292 --diff --no-cache
--- 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 (1 additional fix available with `--unsafe-fixes`).

❯ ruff check example.py --select F601,T201,W292 --diff --no-cache --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.

codspeed-hq · 2023-10-03T19:09:06Z

CodSpeed Performance Report

Merging #7790 will not alter performance

_{Comparing zanie/applicability-summary (67e1782) with main (3c25d26)}

Summary

✅ 25 untouched benchmarks

github-actions · 2023-10-03T19:10:44Z

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

@evanrittenhouse

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 <[email protected]>

zanieb · 2023-10-06T15:20:08Z

crates/ruff_cli/src/printer.rs

-                if let Some(fixables) = FixableSummary::try_from(diagnostics, self.unsafe_fixes) {
-                    writeln!(writer, "{fixables}")?;
+                if let Some(fixables) = fixables {
+                    let fix_prefix = format!("[{}]", "*".cyan());


This is all unchanged and pulled out of FixableStatistics.fmt because I don't think it makes sense to have it on the struct.

zanieb · 2023-10-06T15:20:33Z

crates/ruff_cli/src/printer.rs

@@ -476,41 +525,3 @@ impl FixableSummary {
        self.applicable > 0
    }
 }
-
-impl Display for FixableSummary {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {


I don't think this belongs here, this is one specific message derived from this struct.

zanieb · 2023-10-06T15:24:58Z

I'd like integration test coverage for this but the messages are omitted there ~~for some reason~~; see #7838.

crates/ruff_cli/src/printer.rs

… notes

charliermarsh

I guess we should use the "Would be fixed" wording from below — just an oversight.

I think this still needs to be addressed but otherwise all good!

konstin · 2023-10-09T09:54:25Z

(1 additional fix available with --unsafe-fixes)

This to me sounds a lot like "please enable unsafe fixes to make this message go away". Maybe something like "(Excluding 1 unsafe fix)`?

zanieb · 2023-10-09T14:33:50Z

My qualm with "excluding" is it makes it sound like you've turned it off but it's just off by default. Hm..

One possibility is that we could change unsafe-fixes to be Optional throughout our settings and if it's not set we use these messages but if it's set to false explicitly we hide the suggestion?

@evanrittenhouse

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 <[email protected]>

…nts (#7790) Requires #7769 Updates the CLI display for `ruff check --fix` to hint availability of unsafe fixes. ``` ❯ ruff check example.py --select F601,T201 --diff --no-cache No errors fixed (1 fix available with `--unsafe-fixes`). ``` ``` ❯ ruff check example.py --select F601,T201,W292 --diff --no-cache --- 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 (1 additional fix available with `--unsafe-fixes`). ``` ``` ❯ ruff check example.py --select F601,T201,W292 --diff --no-cache --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. ```

zanieb marked this pull request as draft October 3, 2023 18:42

This comment was marked as resolved.

Sign in to view

zanieb force-pushed the zanie/applicability-summary branch from 951a207 to 67bfefc Compare October 3, 2023 18:55

zanieb mentioned this pull request Oct 3, 2023

Update CLI to respect fix applicability #7769

Merged

zanieb added the cli Related to the command-line interface label Oct 3, 2023

Base automatically changed from zanie/applicability to main October 6, 2023 03:41

zanieb force-pushed the zanie/applicability-summary branch from 67bfefc to 29e210b Compare October 6, 2023 15:18

zanieb commented Oct 6, 2023

View reviewed changes

zanieb changed the title ~~Update fix summary message in check --diff to include suggested fix notes~~ Update fix summary message in check --diff to include unsafe fix hints Oct 6, 2023

zanieb marked this pull request as ready for review October 6, 2023 15:25

zanieb mentioned this pull request Oct 6, 2023

Write summary messages to stderr when fixing via stdin (instead of omitting them) #7838

Merged

charliermarsh reviewed Oct 6, 2023

View reviewed changes

crates/ruff_cli/src/printer.rs Outdated Show resolved Hide resolved

zanieb added 3 commits October 6, 2023 12:11

Update fix summary message in check --diff to include suggested fix…

8b43b31

… notes

Remove "additional" when no errors are fixed

f3f7030

Update snapshots for #7838

b60cbab

zanieb force-pushed the zanie/applicability-summary branch from 29e210b to b60cbab Compare October 6, 2023 17:12

charliermarsh approved these changes Oct 8, 2023

View reviewed changes

Add "would be" handling

67e1782

dhruvmanila approved these changes Oct 10, 2023

View reviewed changes

zanieb merged commit 2b95d38 into main Oct 10, 2023
16 checks passed

zanieb deleted the zanie/applicability-summary branch October 10, 2023 15:50

p-linnane mentioned this pull request Oct 16, 2023

ruff 0.1.0 Homebrew/homebrew-core#151291

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update fix summary message in `check --diff` to include unsafe fix hints #7790

Update fix summary message in `check --diff` to include unsafe fix hints #7790

zanieb commented Oct 3, 2023 •

edited

Loading

This comment was marked as resolved.

codspeed-hq bot commented Oct 3, 2023 •

edited

Loading

github-actions bot commented Oct 3, 2023 •

edited

Loading

zanieb Oct 6, 2023

zanieb Oct 6, 2023

zanieb commented Oct 6, 2023 •

edited

Loading

charliermarsh left a comment

konstin commented Oct 9, 2023

zanieb commented Oct 9, 2023

Update fix summary message in check --diff to include unsafe fix hints #7790

Update fix summary message in check --diff to include unsafe fix hints #7790

Conversation

zanieb commented Oct 3, 2023 • edited Loading

This comment was marked as resolved.

codspeed-hq bot commented Oct 3, 2023 • edited Loading

CodSpeed Performance Report

Merging #7790 will not alter performance

Summary

github-actions bot commented Oct 3, 2023 • edited Loading

PR Check Results

Ecosystem

zanieb Oct 6, 2023

Choose a reason for hiding this comment

zanieb Oct 6, 2023

Choose a reason for hiding this comment

zanieb commented Oct 6, 2023 • edited Loading

charliermarsh left a comment

Choose a reason for hiding this comment

konstin commented Oct 9, 2023

zanieb commented Oct 9, 2023

Update fix summary message in `check --diff` to include unsafe fix hints #7790

Update fix summary message in `check --diff` to include unsafe fix hints #7790

zanieb commented Oct 3, 2023 •

edited

Loading

codspeed-hq bot commented Oct 3, 2023 •

edited

Loading

github-actions bot commented Oct 3, 2023 •

edited

Loading

zanieb commented Oct 6, 2023 •

edited

Loading