Render code frame with context #3901
Conversation
|
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
PR Check ResultsEcosystem✅ ecosystem check detected no changes. BenchmarkLinuxWindows |
There was a problem hiding this comment.
This file nicely shows the change. You can see that the emitter now shows the additional lines 5-6 and 7-8 to give the user more context about where in the code the violation is. Mainly because x = 1 doesn't give you much context.
There was a problem hiding this comment.
Yeah this looks great. Do you know what (e.g.) Rust's own rules are for this?
There was a problem hiding this comment.
Rust seems to render the code span of the diagnostic without showing any context lines
warning: unused variable: `a`
--> crates/ruff_cli/src/printer.rs:160:25
|
160 | let a = 0usize as u32;
| ^ help: if this is intentional, prefix it with an underscore: `_a`
|
= note: `#[warn(unused_variables)]` on by default
The same is true for clippy.
warning: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers
--> crates/ruff_cli/src/printer.rs:160:29
|
160 | let a = 0usize as u32;
| ^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation
= note: `-W clippy::cast-possible-truncation` implied by `-W clippy::pedantic`
I guess there are reasons for doing either. Not showing additional context lines is faster and leads to smaller output. Showing the additional lines can help users to understand the error better. E.g that the variable is unused: What's the context? What if I use the same variable name multiple times.
MichaReiser
force-pushed
the
source-frame-context
branch
from
cd76098 to
177cb9b
Compare
| pub(super) fn create_messages() -> Vec<Message> { | ||
| let fib = r#"import os | ||
There was a problem hiding this comment.
I added an extra empty line here to demonstrate that it truncates trailing empty lines. I had to change the line number of the x diagnostic because of that and it propagated to all snapshots.
| .chars() | ||
| .count(); |
There was a problem hiding this comment.
This is an odd formatting... 🐘
MichaReiser
force-pushed
the
store-content-on-message
branch
from
46ea1ec to
db04291
Compare
MichaReiser
force-pushed
the
source-frame-context
branch
from
177cb9b to
cee4474
Compare
MichaReiser
force-pushed
the
store-content-on-message
branch
from
db04291 to
5e03d44
Compare
MichaReiser
force-pushed
the
source-frame-context
branch
from
cee4474 to
a6b47b6
Compare
MichaReiser
force-pushed
the
source-frame-context
branch
from
a6b47b6 to
f07154c
Compare
This PR changes the
textandgroupedemitter to show up to two lines of context before and after the offending code.Performance
This change significantly impacts performance for projects that have many diagnostics (500k). Running ruff on cpython with all rules and
--show-sourceregresses by about 50% when using a cache. I think this is fine. We can decide to limit the diagnostics to show at max 500 (or another arbitrary) number by default that users can override if they want to see more. It's also worth pointing out that the slowest part is still my terminal trying to render all diagnostics 😄./target/release/ruff: This version./ruff-source-code: base reference (main)./target/release/ruff ./crates/ruff/resources/test/cpython/ -e./ruff-source-code ./crates/ruff/resources/test/cpython/ -e./target/release/ruff ./crates/ruff/resources/test/cpython/ -e --no-cache./ruff-source-code ./crates/ruff/resources/test/cpython/ -e --no-cache./target/release/ruff ./crates/ruff/resources/test/cpython/ -e --select=ALL./ruff-source-code ./crates/ruff/resources/test/cpython/ -e --select=ALL./target/release/ruff ./crates/ruff/resources/test/cpython/ -e --no-cache --select=ALL./ruff-source-code ./crates/ruff/resources/test/cpython/ -e --no-cache --select=ALL./target/release/ruff ./crates/ruff/resources/test/cpython/ -e --show-source./ruff-source-code ./crates/ruff/resources/test/cpython/ -e --show-source./target/release/ruff ./crates/ruff/resources/test/cpython/ -e --no-cache --show-source./ruff-source-code ./crates/ruff/resources/test/cpython/ -e --no-cache --show-source./target/release/ruff ./crates/ruff/resources/test/cpython/ -e --select=ALL --show-source./ruff-source-code ./crates/ruff/resources/test/cpython/ -e --select=ALL --show-source./target/release/ruff ./crates/ruff/resources/test/cpython/ -e --no-cache --select=ALL --show-source./ruff-source-code ./crates/ruff/resources/test/cpython/ -e --no-cache --select=ALL --show-source