Pretty print `Diagnostic`s in snapshot tests #3906

MichaReiser · 2023-04-07T17:34:30Z

This PR extends the TextEmitter to print diffs for fixes (explicit opt-in) and changes the snapshot tests to use the TextEmitter instead of assert_yaml_snapshot.

Pretty printing the diagnostics makes our linter tests more resilient to changes. For example, changing the field ordering of Diagnostic or its representation no longer breaks the snapshot tests because it doesn't change the pretty printed output.

The pretty printed output further has the advantage that it is easier to spot errors. No more offset counting to verify that the diagnostic or edit points to the right locations.

MichaReiser · 2023-04-07T17:34:50Z

Current dependencies on/for this PR:

main
- PR Extract message emitters from Printer #3895
  - PR Cheap cloneable LineIndex #3896
    - PR Store source code on message #3897
      - PR Render code frame with context #3901
        
        PR Introduce SourceFile to avoid cloning the message filename #3904
        
        PR Order Edits by Locations #3905
        
        PR Pretty print Diagnostics in snapshot tests #3906 👈

This comment was auto-generated by Graphite.

MichaReiser · 2023-04-07T17:45:17Z

...es/flake8_blind_except/snapshots/ruff__rules__flake8_blind_except__tests__BLE001_BLE.py.snap

+57 |     pass
+   |
+
+./resources/test/fixtures/flake8_blind_except/BLE.py:60:8: BLE001 Do not catch blind exception: `Exception`


a blind exception... interesting 🙈

charliermarsh · 2023-04-07T18:57:18Z

crates/ruff/src/message/diff.rs

+
+/// Renders a diff that shows the code fixes.
+///
+/// The implementation isn't fully fledged out and only used by tests. Before using in production, try


What do we do in production? Continue to show just the changed line?

Oh right, sorry, in production we don't show suggested fixes at all, we only show the suggestion message, and highlight the relevant code.

I do wonder if there's anything we can leverage from Clippy or rustc to facilitate printing diffs.

We can look into it when redoing Diagnostics. I would like to replace annotate snippet and colored. Ideally, the code frame and diff is something we build ourselves as it is an important core component of all our tools. This is something that we could inherit from Rome

charliermarsh · 2023-04-07T18:57:45Z

crates/ruff/src/message/diff.rs

+}
+
+impl<'a> Diff<'a> {
+    pub fn from_message(message: &'a Message) -> Option<Diff> {


Should this be try_from_message? Or is try_from_* only used for Result?

Not sure. NonZeroUsize returns Option from new. My reasoning is that it isn't failing, it's just that not all Messages can print a diff.

crates/ruff/src/message/diff.rs

github-actions · 2023-04-07T20:30:29Z

PR Check Results

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.01     14.3±0.02ms     2.9 MB/sec    1.00     14.1±0.03ms     2.9 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.6±0.01ms     4.7 MB/sec    1.00      3.6±0.00ms     4.7 MB/sec
linter/all-rules/numpy/globals.py          1.00    458.9±1.56µs     6.4 MB/sec    1.00    457.8±0.81µs     6.4 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.1±0.03ms     4.2 MB/sec    1.00      6.0±0.05ms     4.2 MB/sec
linter/default-rules/large/dataset.py      1.02      7.3±0.01ms     5.5 MB/sec    1.00      7.2±0.01ms     5.6 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01   1637.4±8.55µs    10.2 MB/sec    1.00   1623.1±3.21µs    10.3 MB/sec
linter/default-rules/numpy/globals.py      1.00    180.4±0.34µs    16.4 MB/sec    1.00    180.4±1.29µs    16.4 MB/sec
linter/default-rules/pydantic/types.py     1.01      3.4±0.00ms     7.6 MB/sec    1.00      3.3±0.01ms     7.7 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.01     16.1±0.31ms     2.5 MB/sec    1.00     16.0±0.27ms     2.5 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      4.2±0.15ms     4.0 MB/sec    1.00      4.1±0.10ms     4.0 MB/sec
linter/all-rules/numpy/globals.py          1.00    498.7±8.04µs     5.9 MB/sec    1.01   503.7±13.83µs     5.9 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.8±0.16ms     3.7 MB/sec    1.00      6.8±0.15ms     3.7 MB/sec
linter/default-rules/large/dataset.py      1.00      8.2±0.13ms     5.0 MB/sec    1.00      8.2±0.12ms     4.9 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1789.4±34.77µs     9.3 MB/sec    1.01  1815.9±37.32µs     9.2 MB/sec
linter/default-rules/numpy/globals.py      1.00    196.3±5.87µs    15.0 MB/sec    1.01    197.3±6.50µs    15.0 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.7±0.06ms     6.8 MB/sec    1.01      3.8±0.07ms     6.8 MB/sec

MichaReiser force-pushed the edit-operation branch from 9521ae1 to 0d46f76 Compare April 7, 2023 17:39

MichaReiser force-pushed the test-snapshots branch 2 times, most recently from 1042404 to bc9565b Compare April 7, 2023 17:41

MichaReiser commented Apr 7, 2023

View reviewed changes

MichaReiser force-pushed the test-snapshots branch 3 times, most recently from 625430d to c61412e Compare April 7, 2023 18:21

charliermarsh reviewed Apr 7, 2023

View reviewed changes

charliermarsh approved these changes Apr 7, 2023

View reviewed changes

charliermarsh reviewed Apr 7, 2023

View reviewed changes

crates/ruff/src/message/diff.rs Show resolved Hide resolved

MichaReiser force-pushed the test-snapshots branch from c61412e to 117cace Compare April 7, 2023 20:16

MichaReiser force-pushed the edit-operation branch from 0d46f76 to cff3d22 Compare April 11, 2023 07:26

MichaReiser force-pushed the test-snapshots branch from 117cace to 0bab2f4 Compare April 11, 2023 07:27

MichaReiser added the internal An internal refactor or improvement label Apr 11, 2023

MichaReiser force-pushed the edit-operation branch 2 times, most recently from c7b5b1d to 2323617 Compare April 11, 2023 08:49

Base automatically changed from edit-operation to main April 11, 2023 08:56

Snapshot pretty printed Message for linter tests

04e924f

MichaReiser force-pushed the test-snapshots branch from 0bab2f4 to 04e924f Compare April 11, 2023 08:57

MichaReiser enabled auto-merge (squash) April 11, 2023 08:59

MichaReiser merged commit e8aebee into main Apr 11, 2023

MichaReiser deleted the test-snapshots branch April 11, 2023 09:03

This was referenced Apr 11, 2023

Implement flake8_todos #3921

Merged

Implement isort custom sections and ordering (#2419) #3900

Merged

[flake8-import-conventions] Add a rule for BannedImportAlias #3926

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pretty print `Diagnostic`s in snapshot tests #3906

Pretty print `Diagnostic`s in snapshot tests #3906

MichaReiser commented Apr 7, 2023 •

edited

Loading

MichaReiser commented Apr 7, 2023

MichaReiser Apr 7, 2023 •

edited

Loading

charliermarsh Apr 7, 2023

charliermarsh Apr 7, 2023

charliermarsh Apr 7, 2023

MichaReiser Apr 7, 2023

charliermarsh Apr 7, 2023

MichaReiser Apr 7, 2023

github-actions bot commented Apr 7, 2023 •

edited

Loading

Pretty print Diagnostics in snapshot tests #3906

Pretty print Diagnostics in snapshot tests #3906

Conversation

MichaReiser commented Apr 7, 2023 • edited Loading

MichaReiser commented Apr 7, 2023

MichaReiser Apr 7, 2023 • edited Loading

Choose a reason for hiding this comment

charliermarsh Apr 7, 2023

Choose a reason for hiding this comment

charliermarsh Apr 7, 2023

Choose a reason for hiding this comment

charliermarsh Apr 7, 2023

Choose a reason for hiding this comment

MichaReiser Apr 7, 2023

Choose a reason for hiding this comment

charliermarsh Apr 7, 2023

Choose a reason for hiding this comment

MichaReiser Apr 7, 2023

Choose a reason for hiding this comment

github-actions bot commented Apr 7, 2023 • edited Loading

PR Check Results

Benchmark

Linux

Windows

Pretty print `Diagnostic`s in snapshot tests #3906

Pretty print `Diagnostic`s in snapshot tests #3906

MichaReiser commented Apr 7, 2023 •

edited

Loading

MichaReiser Apr 7, 2023 •

edited

Loading

github-actions bot commented Apr 7, 2023 •

edited

Loading