language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-07-24 13:34:40 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

add diagnostic Span (couples File and TextRange) (#16101)

Browse source 
This essentially makes it impossible to construct a `Diagnostic`
that has a `TextRange` but no `File`.

This is meant to be a precursor to multi-span support.

(Note that I consider this more of a prototyping-change and not
necessarily what this is going to look like longer term.)

Reviewers can probably review this PR as one big diff instead of
commit-by-commit.
This commit is contained in:
Andrew Gallant 2025-02-11 14:55:12 -05:00 • committed by GitHub
parent 9c179314ed
commit 6e34f74c16
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 115 additions and 114 deletions
Download patch file Download diff file Expand all files Collapse all files

19
crates/red_knot_project/src/lib.rs
View file

@ -8,14 +8,13 @@ pub use metadata::{ProjectDiscoveryError, ProjectMetadata};
use red_knot_python_semantic::lint::{LintRegistry, LintRegistryBuilder, RuleSelection};
use red_knot_python_semantic::register_lints;
use red_knot_python_semantic::types::check_types;
use ruff_db::diagnostic::{Diagnostic, DiagnosticId, ParseDiagnostic, Severity};
use ruff_db::diagnostic::{Diagnostic, DiagnosticId, ParseDiagnostic, Severity, Span};
use ruff_db::files::{system_path_to_file, File};
use ruff_db::parsed::parsed_module;
use ruff_db::source::{source_text, SourceTextError};
use ruff_db::system::walk_directory::WalkState;
use ruff_db::system::{FileType, SystemPath};
use ruff_python_ast::PySourceType;
use ruff_text_size::TextRange;
use rustc_hash::{FxBuildHasher, FxHashSet};
use salsa::Durability;
use salsa::Setter;
@ -345,7 +344,13 @@ fn check_file_impl(db: &dyn Db, file: File) -> Vec<Box<dyn Diagnostic>> {
boxed
}));
diagnostics.sort_unstable_by_key(|diagnostic| diagnostic.range().unwrap_or_default().start());
diagnostics.sort_unstable_by_key(|diagnostic| {
diagnostic
.span()
.and_then(|span| span.range())
.unwrap_or_default()
.start()
});
diagnostics
}
@ -458,12 +463,8 @@ impl Diagnostic for IOErrorDiagnostic {
self.error.to_string().into()
}
fn file(&self) -> Option<File> {
Some(self.file)
}
fn range(&self) -> Option<TextRange> {
None
fn span(&self) -> Option<Span> {
Some(Span::from(self.file))
}
fn severity(&self) -> Severity {

39
crates/red_knot_project/src/metadata/options.rs
View file

@ -4,11 +4,10 @@ use red_knot_python_semantic::lint::{GetLintError, Level, LintSource, RuleSelect
use red_knot_python_semantic::{
ProgramSettings, PythonPlatform, PythonVersion, SearchPathSettings, SitePackages,
};
use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity};
use ruff_db::files::{system_path_to_file, File};
use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity, Span};
use ruff_db::files::system_path_to_file;
use ruff_db::system::{System, SystemPath};
use ruff_macros::Combine;
use ruff_text_size::TextRange;
use rustc_hash::FxHashMap;
use serde::{Deserialize, Serialize};
use std::borrow::Cow;
@ -189,7 +188,14 @@ impl Options {
),
};
diagnostics.push(diagnostic.with_file(file).with_range(rule_name.range()));
let span = file.map(Span::from).map(|span| {
if let Some(range) = rule_name.range() {
span.with_range(range)
} else {
span
}
});
diagnostics.push(diagnostic.with_span(span));
}
}
}
@ -348,8 +354,7 @@ pub struct OptionDiagnostic {
id: DiagnosticId,
message: String,
severity: Severity,
file: Option<File>,
range: Option<TextRange>,
span: Option<Span>,
}
impl OptionDiagnostic {
@ -358,21 +363,13 @@ impl OptionDiagnostic {
id,
message,
severity,
file: None,
range: None,
span: None,
}
}
#[must_use]
fn with_file(mut self, file: Option<File>) -> Self {
self.file = file;
self
}
#[must_use]
fn with_range(mut self, range: Option<TextRange>) -> Self {
self.range = range;
self
fn with_span(self, span: Option<Span>) -> Self {
OptionDiagnostic { span, ..self }
}
}
@ -385,12 +382,8 @@ impl Diagnostic for OptionDiagnostic {
Cow::Borrowed(&self.message)
}
fn file(&self) -> Option<File> {
self.file
}
fn range(&self) -> Option<TextRange> {
self.range
fn span(&self) -> Option<Span> {
self.span.clone()
}
fn severity(&self) -> Severity {

10
crates/red_knot_python_semantic/src/types/diagnostic.rs
View file

@ -8,7 +8,7 @@ use crate::types::string_annotation::{
};
use crate::types::{ClassLiteralType, KnownInstanceType, Type};
use crate::{declare_lint, Db};
use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity};
use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity, Span};
use ruff_db::files::File;
use ruff_python_ast::{self as ast, AnyNodeRef};
use ruff_text_size::{Ranged, TextRange};
@ -802,12 +802,8 @@ impl Diagnostic for TypeCheckDiagnostic {
TypeCheckDiagnostic::message(self).into()
}
fn file(&self) -> Option<File> {
Some(TypeCheckDiagnostic::file(self))
}
fn range(&self) -> Option<TextRange> {
Some(Ranged::range(self))
fn span(&self) -> Option<Span> {
Some(Span::from(self.file).with_range(self.range))
}
fn severity(&self) -> Severity {

10
crates/red_knot_server/src/server/api/requests/diagnostic.rs
View file

@ -75,11 +75,13 @@ fn to_lsp_diagnostic(
diagnostic: &dyn ruff_db::diagnostic::Diagnostic,
encoding: crate::PositionEncoding,
) -> Diagnostic {
let range = if let (Some(file), Some(range)) = (diagnostic.file(), diagnostic.range()) {
let index = line_index(db.upcast(), file);
let source = source_text(db.upcast(), file);
let range = if let Some(span) = diagnostic.span() {
let index = line_index(db.upcast(), span.file());
let source = source_text(db.upcast(), span.file());
range.to_range(&source, &index, encoding)
span.range()
.map(|range| range.to_range(&source, &index, encoding))
.unwrap_or_default()
} else {
Range::default()
};

13
crates/red_knot_test/src/diagnostic.rs
View file

@ -26,7 +26,8 @@ where
.into_iter()
.map(|diagnostic| DiagnosticWithLine {
line_number: diagnostic
.range()
.span()
.and_then(|span| span.range())
.map_or(OneIndexed::from_zero_indexed(0), |range| {
line_index.line_index(range.start())
}),
@ -144,7 +145,7 @@ struct DiagnosticWithLine<T> {
mod tests {
use crate::db::Db;
use crate::diagnostic::Diagnostic;
use ruff_db::diagnostic::{DiagnosticId, LintName, Severity};
use ruff_db::diagnostic::{DiagnosticId, LintName, Severity, Span};
use ruff_db::files::{system_path_to_file, File};
use ruff_db::source::line_index;
use ruff_db::system::{DbWithTestSystem, SystemPathBuf};
@ -198,12 +199,8 @@ mod tests {
"dummy".into()
}
fn file(&self) -> Option<File> {
Some(self.file)
}
fn range(&self) -> Option<TextRange> {
Some(self.range)
fn span(&self) -> Option<Span> {
Some(Span::from(self.file).with_range(self.range))
}
fn severity(&self) -> Severity {

13
crates/red_knot_test/src/matcher.rs
View file

@ -257,7 +257,8 @@ impl Matcher {
fn column<T: Diagnostic>(&self, diagnostic: &T) -> OneIndexed {
diagnostic
.range()
.span()
.and_then(|span| span.range())
.map(|range| {
self.line_index
.source_location(range.start(), &self.source)
@ -334,7 +335,7 @@ impl Matcher {
#[cfg(test)]
mod tests {
use super::FailuresByLine;
use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity};
use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity, Span};
use ruff_db::files::{system_path_to_file, File};
use ruff_db::system::{DbWithTestSystem, SystemPathBuf};
use ruff_python_trivia::textwrap::dedent;
@ -385,12 +386,8 @@ mod tests {
self.message.into()
}
fn file(&self) -> Option<File> {
Some(self.file)
}
fn range(&self) -> Option<TextRange> {
Some(self.range)
fn span(&self) -> Option<Span> {
Some(Span::from(self.file).with_range(self.range))
}
fn severity(&self) -> Severity {

10
crates/ruff_benchmark/benches/red_knot.rs
View file

@ -229,8 +229,14 @@ fn assert_diagnostics(db: &dyn Db, diagnostics: &[Box<dyn Diagnostic>]) {
.map(|diagnostic| {
(
diagnostic.id(),
diagnostic.file().map(|file| file.path(db).as_str()),
diagnostic.range().map(Range::<usize>::from),
diagnostic
.span()
.map(|span| span.file())
.map(|file| file.path(db).as_str()),
diagnostic
.span()
.and_then(|span| span.range())
.map(Range::<usize>::from),
diagnostic.message(),
diagnostic.severity(),
)

115
crates/ruff_db/src/diagnostic.rs
View file

@ -164,19 +164,11 @@ pub trait Diagnostic: Send + Sync + std::fmt::Debug {
fn message(&self) -> Cow<str>;
/// The file this diagnostic is associated with.
///
/// File can be `None` for diagnostics that don't originate from a file.
/// For example:
/// * A diagnostic indicating that a directory couldn't be read.
/// * A diagnostic related to a CLI argument
fn file(&self) -> Option<File>;
/// The primary range of the diagnostic in `file`.
/// The primary span of the diagnostic.
///
/// The range can be `None` if the diagnostic doesn't have a file
/// or it applies to the entire file (e.g. the file should be executable but isn't).
fn range(&self) -> Option<TextRange>;
fn span(&self) -> Option<Span>;
fn severity(&self) -> Severity;
@ -191,6 +183,47 @@ pub trait Diagnostic: Send + Sync + std::fmt::Debug {
}
}
/// A span represents the source of a diagnostic.
///
/// It consists of a `File` and an optional range into that file. When the
/// range isn't present, it semantically implies that the diagnostic refers to
/// the entire file. For example, when the file should be executable but isn't.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Span {
file: File,
range: Option<TextRange>,
}
impl Span {
/// Returns the `File` attached to this `Span`.
pub fn file(&self) -> File {
self.file
}
/// Returns the range, if available, attached to this `Span`.
///
/// When there is no range, it is convention to assume that this `Span`
/// refers to the corresponding `File` as a whole. In some cases, consumers
/// of this API may use the range `0..0` to represent this case.
pub fn range(&self) -> Option<TextRange> {
self.range
}
/// Returns a new `Span` with the given `range` attached to it.
pub fn with_range(self, range: TextRange) -> Span {
Span {
range: Some(range),
..self
}
}
}
impl From<File> for Span {
fn from(file: File) -> Span {
Span { file, range: None }
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Ord, PartialOrd)]
pub enum Severity {
Info,
@ -236,8 +269,8 @@ impl std::fmt::Display for DisplayDiagnostic<'_> {
let rendered = renderer.render(message);
writeln!(f, "{rendered}")
};
match (self.diagnostic.file(), self.diagnostic.range()) {
(None, _) => {
match self.diagnostic.span() {
None => {
// NOTE: This is pretty sub-optimal. It doesn't render well. We
// really want a snippet, but without a `File`, we can't really
// render anything. It looks like this case currently happens
@ -248,20 +281,20 @@ impl std::fmt::Display for DisplayDiagnostic<'_> {
let msg = format!("{}: {}", self.diagnostic.id(), self.diagnostic.message());
render(f, level.title(&msg))
}
(Some(file), range) => {
let path = file.path(self.db).to_string();
let source = source_text(self.db, file);
Some(span) => {
let path = span.file.path(self.db).to_string();
let source = source_text(self.db, span.file);
let title = self.diagnostic.id().to_string();
let message = self.diagnostic.message();
let Some(range) = range else {
let Some(range) = span.range else {
let snippet = Snippet::source(source.as_str()).origin(&path).line_start(1);
return render(f, level.title(&title).snippet(snippet));
};
// The bits below are a simplified copy from
// `crates/ruff_linter/src/message/text.rs`.
let index = line_index(self.db, file);
let index = line_index(self.db, span.file);
let source_code = SourceCode::new(source.as_str(), &index);
let content_start_index = source_code.line_index(range.start());
@ -315,12 +348,8 @@ where
(**self).message()
}
fn file(&self) -> Option<File> {
(**self).file()
}
fn range(&self) -> Option<TextRange> {
(**self).range()
fn span(&self) -> Option<Span> {
(**self).span()
}
fn severity(&self) -> Severity {
@ -340,12 +369,8 @@ where
(**self).message()
}
fn file(&self) -> Option<File> {
(**self).file()
}
fn range(&self) -> Option<TextRange> {
(**self).range()
fn span(&self) -> Option<Span> {
(**self).span()
}
fn severity(&self) -> Severity {
@ -362,12 +387,8 @@ impl Diagnostic for Box<dyn Diagnostic> {
(**self).message()
}
fn file(&self) -> Option<File> {
(**self).file()
}
fn range(&self) -> Option<TextRange> {
(**self).range()
fn span(&self) -> Option<Span> {
(**self).span()
}
fn severity(&self) -> Severity {
@ -384,12 +405,8 @@ impl Diagnostic for &'_ dyn Diagnostic {
(**self).message()
}
fn file(&self) -> Option<File> {
(**self).file()
}
fn range(&self) -> Option<TextRange> {
(**self).range()
fn span(&self) -> Option<Span> {
(**self).span()
}
fn severity(&self) -> Severity {
@ -406,12 +423,8 @@ impl Diagnostic for std::sync::Arc<dyn Diagnostic> {
(**self).message()
}
fn file(&self) -> Option<File> {
(**self).file()
}
fn range(&self) -> Option<TextRange> {
(**self).range()
fn span(&self) -> Option<Span> {
(**self).span()
}
fn severity(&self) -> Severity {
@ -440,12 +453,8 @@ impl Diagnostic for ParseDiagnostic {
self.error.error.to_string().into()
}
fn file(&self) -> Option<File> {
Some(self.file)
}
fn range(&self) -> Option<TextRange> {
Some(self.error.location)
fn span(&self) -> Option<Span> {
Some(Span::from(self.file).with_range(self.error.location))
}
fn severity(&self) -> Severity {