mirror of
https://github.com/astral-sh/ruff
synced 2026-01-09 23:54:36 -05:00
44755e6e86
## Summary This PR switches the `full` output format in Ruff over to use the rendering code in `ruff_db`. As proposed in the design doc, this involves a lot of changes to the snapshot output. I also had to comment out this assertion with a TODO to replace it after https://github.com/astral-sh/ruff/issues/19688 because many of Ruff's "file-level" annotations aren't actually file-level. They just happen to occur at the start of the file, especially in tests with very short snippets.529d81daca/crates/ruff_annotate_snippets/src/renderer/display_list.rs (L1204-L1208)I broke up the snapshot commits at the end into several blocks, but I don't think it's enough to help with review. The first few (notebooks, syntax errors, and test rules) are small enough to look at, but I couldn't really think of other categories beyond that. I'm happy to break those up or pick out specific examples beyond what I have below, if that would help. The minimal code changes are in this [range](abd28f1e77), with the snapshot commits following. Moving the `FullRenderer` and updating the `EmitterFlags` aren't strictly necessary either. I even dropped the renderer commit this morning but figured it made sense to keep it since we have the `full` module for tests. I don't feel strongly either way. ## Test Plan I did actually click through all 1700 snapshots individually instead of accepting them all at once, although I moved through them quickly. There are a few main categories: ### Lint diagnostics ```diff -unused.py:8:19: F401 [*] `pathlib` imported but unused +F401 [*] `pathlib` imported but unused + --> unused.py:8:19 | 7 | # Unused, _not_ marked as required (due to the alias). 8 | import pathlib as non_alias - | ^^^^^^^^^ F401 + | ^^^^^^^^^ 9 | 10 | # Unused, marked as required. | - = help: Remove unused import: `pathlib` +help: Remove unused import: `pathlib` ``` - The filename and line numbers are moved to the second line - The second noqa code next to the underline is removed ### Syntax errors These are much like the above. ```diff - -:1:16: invalid-syntax: Expected one or more symbol names after import + invalid-syntax: Expected one or more symbol names after import + --> -:1:16 | 1 | from foo import | ^ ``` One thing I noticed while reviewing some of these, but I don't think is strictly syntax-error-related, is that some of the new diagnostics have a little less context after the error. I don't think this is a problem, but it's one small discrepancy I hadn't noticed before. Here's a minor example: ```diff -syntax_errors.py:1:15: invalid-syntax: Expected one or more symbol names after import +invalid-syntax: Expected one or more symbol names after import + --> syntax_errors.py:1:15 | 1 | from os import | ^ 2 | 3 | if call(foo -4 | def bar(): | ``` And one of the biggest examples: ```diff -E30_syntax_error.py:18:11: invalid-syntax: Expected ')', found newline +invalid-syntax: Expected ')', found newline + --> E30_syntax_error.py:18:11 | 16 | pass 17 | 18 | foo = Foo( | ^ -19 | -20 | -21 | def top( | ``` Similarly, a few of the lint diagnostics showed that the cut indicator calculation for overly long lines is also slightly different, but I think that's okay too. ### Full-file diagnostics ```diff -comment.py:1:1: I002 [*] Missing required import: `from __future__ import annotations` +I002 [*] Missing required import: `from __future__ import annotations` +--> comment.py:1:1 +help: Insert required import: `from __future__ import annotations` + ``` As noted above, these will be much more rare after #19688 too. This case isn't a true full-file diagnostic and will render a snippet in the future, but you can see that we're now rendering the help message that would have been discarded before. In contrast, this is a true full-file diagnostic and should still look like this after #19688: ```diff -__init__.py:1:1: A005 Module `logging` shadows a Python standard-library module +A005 Module `logging` shadows a Python standard-library module +--> __init__.py:1:1 ``` ### Jupyter notebooks There's nothing particularly different about these, just showing off the cell index again. ```diff - Jupyter.ipynb:cell 3:1:7: F821 Undefined name `x` + F821 Undefined name `x` + --> Jupyter.ipynb:cell 3:1:7 | 1 | print(x) - | ^ F821 + | ^ | ```
This is a fork of the annotate-snippets crate. The principle motivation for
this fork, at the time of writing, is issue #167. Specifically, we wanted to
upgrade our version of annotate-snippets, but do so without changing our
diagnostic message format.
This copy of annotate-snippets is basically identical to upstream, but with
an extra Level::None variant that permits skipping over a new non-optional
header emitted by annotate-snippets.
More generally, it seems plausible that we may want to tweak other aspects of the output format in the future, so it might make sense to stick with our own copy so that we can be masters of our own destiny.