mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-29 05:14:52 +00:00
Hide empty snippets for full-file diagnostics (#19653)
Summary
--
This is the other commit I wanted to spin off from #19415, currently
stacked on #19644.
This PR suppresses blank snippets for empty ranges at the very beginning
of a file, and for empty ranges in non-existent files. Ruff includes
empty ranges for IO errors, for example.
f4e93b6335/crates/ruff_linter/src/message/text.rs (L100-L110)
The diagnostics now look like this (new snapshot test):
```
error[test-diagnostic]: main diagnostic message
--> example.py:1:1
```
Instead of [^*]
```
error[test-diagnostic]: main diagnostic message
--> example.py:1:1
|
|
```
Test Plan
--
A new `ruff_db` test showing the expected output format
[^*]: This doesn't correspond precisely to the example in the PR because
of some details of the diagnostic builder helper methods in `ruff_db`,
but you can see another example in the current version of the summary in
#19415.
This commit is contained in:
parent
2db4e5dbea
commit
b324ae1be3
6 changed files with 88 additions and 5 deletions
|
@ -1183,6 +1183,21 @@ fn format_snippet<'m>(
|
||||||
let main_range = snippet.annotations.first().map(|x| x.range.start);
|
let main_range = snippet.annotations.first().map(|x| x.range.start);
|
||||||
let origin = snippet.origin;
|
let origin = snippet.origin;
|
||||||
let need_empty_header = origin.is_some() || is_first;
|
let need_empty_header = origin.is_some() || is_first;
|
||||||
|
|
||||||
|
let is_file_level = snippet.annotations.iter().any(|ann| ann.is_file_level);
|
||||||
|
if is_file_level {
|
||||||
|
assert!(
|
||||||
|
snippet.source.is_empty(),
|
||||||
|
"Non-empty file-level snippet that won't be rendered: {:?}",
|
||||||
|
snippet.source
|
||||||
|
);
|
||||||
|
let header = format_header(origin, main_range, &[], is_first);
|
||||||
|
return DisplaySet {
|
||||||
|
display_lines: header.map_or_else(Vec::new, |header| vec![header]),
|
||||||
|
margin: Margin::new(0, 0, 0, 0, term_width, 0),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
let mut body = format_body(
|
let mut body = format_body(
|
||||||
snippet,
|
snippet,
|
||||||
need_empty_header,
|
need_empty_header,
|
||||||
|
|
|
@ -124,6 +124,7 @@ pub struct Annotation<'a> {
|
||||||
pub(crate) range: Range<usize>,
|
pub(crate) range: Range<usize>,
|
||||||
pub(crate) label: Option<&'a str>,
|
pub(crate) label: Option<&'a str>,
|
||||||
pub(crate) level: Level,
|
pub(crate) level: Level,
|
||||||
|
pub(crate) is_file_level: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'a> Annotation<'a> {
|
impl<'a> Annotation<'a> {
|
||||||
|
@ -131,6 +132,11 @@ impl<'a> Annotation<'a> {
|
||||||
self.label = Some(label);
|
self.label = Some(label);
|
||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub fn is_file_level(mut self, yes: bool) -> Self {
|
||||||
|
self.is_file_level = yes;
|
||||||
|
self
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Types of annotations.
|
/// Types of annotations.
|
||||||
|
@ -165,6 +171,7 @@ impl Level {
|
||||||
range: span,
|
range: span,
|
||||||
label: None,
|
label: None,
|
||||||
level: self,
|
level: self,
|
||||||
|
is_file_level: false,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -712,6 +712,11 @@ pub struct Annotation {
|
||||||
is_primary: bool,
|
is_primary: bool,
|
||||||
/// The diagnostic tags associated with this annotation.
|
/// The diagnostic tags associated with this annotation.
|
||||||
tags: Vec<DiagnosticTag>,
|
tags: Vec<DiagnosticTag>,
|
||||||
|
/// Whether this annotation is a file-level or full-file annotation.
|
||||||
|
///
|
||||||
|
/// When set, rendering will only include the file's name and (optional) range. Everything else
|
||||||
|
/// is omitted, including any file snippet or message.
|
||||||
|
is_file_level: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Annotation {
|
impl Annotation {
|
||||||
|
@ -730,6 +735,7 @@ impl Annotation {
|
||||||
message: None,
|
message: None,
|
||||||
is_primary: true,
|
is_primary: true,
|
||||||
tags: Vec::new(),
|
tags: Vec::new(),
|
||||||
|
is_file_level: false,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -746,6 +752,7 @@ impl Annotation {
|
||||||
message: None,
|
message: None,
|
||||||
is_primary: false,
|
is_primary: false,
|
||||||
tags: Vec::new(),
|
tags: Vec::new(),
|
||||||
|
is_file_level: false,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -811,6 +818,21 @@ impl Annotation {
|
||||||
pub fn push_tag(&mut self, tag: DiagnosticTag) {
|
pub fn push_tag(&mut self, tag: DiagnosticTag) {
|
||||||
self.tags.push(tag);
|
self.tags.push(tag);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Set whether or not this annotation is file-level.
|
||||||
|
///
|
||||||
|
/// File-level annotations are only rendered with their file name and range, if available. This
|
||||||
|
/// is intended for backwards compatibility with Ruff diagnostics, which historically used
|
||||||
|
/// `TextRange::default` to indicate a file-level diagnostic. In the new diagnostic model, a
|
||||||
|
/// [`Span`] with a range of `None` should be used instead, as mentioned in the `Span`
|
||||||
|
/// documentation.
|
||||||
|
///
|
||||||
|
/// TODO(brent) update this usage in Ruff and remove `is_file_level` entirely. See
|
||||||
|
/// <https://github.com/astral-sh/ruff/issues/19688>, especially my first comment, for more
|
||||||
|
/// details.
|
||||||
|
pub fn set_file_level(&mut self, yes: bool) {
|
||||||
|
self.is_file_level = yes;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Tags that can be associated with an annotation.
|
/// Tags that can be associated with an annotation.
|
||||||
|
|
|
@ -387,6 +387,7 @@ struct ResolvedAnnotation<'a> {
|
||||||
line_end: OneIndexed,
|
line_end: OneIndexed,
|
||||||
message: Option<&'a str>,
|
message: Option<&'a str>,
|
||||||
is_primary: bool,
|
is_primary: bool,
|
||||||
|
is_file_level: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'a> ResolvedAnnotation<'a> {
|
impl<'a> ResolvedAnnotation<'a> {
|
||||||
|
@ -432,6 +433,7 @@ impl<'a> ResolvedAnnotation<'a> {
|
||||||
line_end,
|
line_end,
|
||||||
message: ann.get_message(),
|
message: ann.get_message(),
|
||||||
is_primary: ann.is_primary,
|
is_primary: ann.is_primary,
|
||||||
|
is_file_level: ann.is_file_level,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -653,6 +655,8 @@ struct RenderableAnnotation<'r> {
|
||||||
message: Option<&'r str>,
|
message: Option<&'r str>,
|
||||||
/// Whether this annotation is considered "primary" or not.
|
/// Whether this annotation is considered "primary" or not.
|
||||||
is_primary: bool,
|
is_primary: bool,
|
||||||
|
/// Whether this annotation applies to an entire file, rather than a snippet within it.
|
||||||
|
is_file_level: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'r> RenderableAnnotation<'r> {
|
impl<'r> RenderableAnnotation<'r> {
|
||||||
|
@ -670,6 +674,7 @@ impl<'r> RenderableAnnotation<'r> {
|
||||||
range,
|
range,
|
||||||
message: ann.message,
|
message: ann.message,
|
||||||
is_primary: ann.is_primary,
|
is_primary: ann.is_primary,
|
||||||
|
is_file_level: ann.is_file_level,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -695,7 +700,7 @@ impl<'r> RenderableAnnotation<'r> {
|
||||||
if let Some(message) = self.message {
|
if let Some(message) = self.message {
|
||||||
ann = ann.label(message);
|
ann = ann.label(message);
|
||||||
}
|
}
|
||||||
ann
|
ann.is_file_level(self.is_file_level)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2551,7 +2556,12 @@ watermelon
|
||||||
/// of the corresponding line minus one. (The "minus one" is because
|
/// of the corresponding line minus one. (The "minus one" is because
|
||||||
/// otherwise, the span will end where the next line begins, and this
|
/// otherwise, the span will end where the next line begins, and this
|
||||||
/// confuses `ruff_annotate_snippets` as of 2025-03-13.)
|
/// confuses `ruff_annotate_snippets` as of 2025-03-13.)
|
||||||
fn span(&self, path: &str, line_offset_start: &str, line_offset_end: &str) -> Span {
|
pub(super) fn span(
|
||||||
|
&self,
|
||||||
|
path: &str,
|
||||||
|
line_offset_start: &str,
|
||||||
|
line_offset_end: &str,
|
||||||
|
) -> Span {
|
||||||
let span = self.path(path);
|
let span = self.path(path);
|
||||||
|
|
||||||
let file = span.expect_ty_file();
|
let file = span.expect_ty_file();
|
||||||
|
@ -2574,7 +2584,7 @@ watermelon
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Like `span`, but only attaches a file path.
|
/// Like `span`, but only attaches a file path.
|
||||||
fn path(&self, path: &str) -> Span {
|
pub(super) fn path(&self, path: &str) -> Span {
|
||||||
let file = system_path_to_file(&self.db, path).unwrap();
|
let file = system_path_to_file(&self.db, path).unwrap();
|
||||||
Span::from(file)
|
Span::from(file)
|
||||||
}
|
}
|
||||||
|
|
|
@ -1,9 +1,10 @@
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use ruff_diagnostics::Applicability;
|
use ruff_diagnostics::Applicability;
|
||||||
|
use ruff_text_size::TextRange;
|
||||||
|
|
||||||
use crate::diagnostic::{
|
use crate::diagnostic::{
|
||||||
DiagnosticFormat, Severity,
|
Annotation, DiagnosticFormat, Severity,
|
||||||
render::tests::{TestEnvironment, create_diagnostics, create_syntax_error_diagnostics},
|
render::tests::{TestEnvironment, create_diagnostics, create_syntax_error_diagnostics},
|
||||||
};
|
};
|
||||||
|
|
||||||
|
@ -264,4 +265,24 @@ print()
|
||||||
|
|
|
|
||||||
");
|
");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// For file-level diagnostics, we expect to see the header line with the diagnostic information
|
||||||
|
/// and the `-->` line with the file information but no lines of source code.
|
||||||
|
#[test]
|
||||||
|
fn file_level() {
|
||||||
|
let mut env = TestEnvironment::new();
|
||||||
|
env.add("example.py", "");
|
||||||
|
env.format(DiagnosticFormat::Full);
|
||||||
|
|
||||||
|
let mut diagnostic = env.err().build();
|
||||||
|
let span = env.path("example.py").with_range(TextRange::default());
|
||||||
|
let mut annotation = Annotation::primary(span);
|
||||||
|
annotation.set_file_level(true);
|
||||||
|
diagnostic.annotate(annotation);
|
||||||
|
|
||||||
|
insta::assert_snapshot!(env.render(&diagnostic), @r"
|
||||||
|
error[test-diagnostic]: main diagnostic message
|
||||||
|
--> example.py:1:1
|
||||||
|
");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -70,7 +70,15 @@ where
|
||||||
);
|
);
|
||||||
|
|
||||||
let span = Span::from(file).with_range(range);
|
let span = Span::from(file).with_range(range);
|
||||||
let annotation = Annotation::primary(span);
|
let mut annotation = Annotation::primary(span);
|
||||||
|
// The `0..0` range is used to highlight file-level diagnostics.
|
||||||
|
//
|
||||||
|
// TODO(brent) We should instead set this flag on annotations for individual lint rules that
|
||||||
|
// actually need it, but we need to be able to cache the new diagnostic model first. See
|
||||||
|
// https://github.com/astral-sh/ruff/issues/19688.
|
||||||
|
if range == TextRange::default() {
|
||||||
|
annotation.set_file_level(true);
|
||||||
|
}
|
||||||
diagnostic.annotate(annotation);
|
diagnostic.annotate(annotation);
|
||||||
|
|
||||||
if let Some(suggestion) = suggestion {
|
if let Some(suggestion) = suggestion {
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue