diff --git a/Cargo.lock b/Cargo.lock index 18049fc76f8..aebcc0ca10b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -627,7 +627,10 @@ dependencies = [ "cairo-lang-debug", "cairo-lang-utils", "env_logger", + "indoc", + "itertools 0.14.0", "path-clean", + "pretty_assertions", "rust-analyzer-salsa", "semver", "serde", diff --git a/crates/cairo-lang-defs/src/cache/mod.rs b/crates/cairo-lang-defs/src/cache/mod.rs index d494f065b6f..2aeb1420dbf 100644 --- a/crates/cairo-lang-defs/src/cache/mod.rs +++ b/crates/cairo-lang-defs/src/cache/mod.rs @@ -3,10 +3,10 @@ use std::ops::{Deref, DerefMut}; use std::path::PathBuf; use std::sync::Arc; -use cairo_lang_diagnostics::{DiagnosticLocation, DiagnosticNote, Maybe, Severity}; +use cairo_lang_diagnostics::{DiagnosticNote, Maybe, Severity}; use cairo_lang_filesystem::flag::Flag; use cairo_lang_filesystem::ids::{ - CodeMapping, CrateId, CrateLongId, FileId, FileKind, FileLongId, VirtualFile, + CodeMapping, CrateId, CrateLongId, FileId, FileKind, FileLongId, SpanInFile, VirtualFile, }; use cairo_lang_filesystem::span::{TextOffset, TextSpan, TextWidth}; use cairo_lang_syntax::node::ast::{ @@ -1757,7 +1757,7 @@ impl FileIdCached { #[derive(Serialize, Deserialize, Clone, PartialEq, Eq)] struct VirtualFileCached { - parent: Option, + parent: Option, name: SmolStr, content: String, code_mappings: Vec, @@ -1768,7 +1768,7 @@ struct VirtualFileCached { impl VirtualFileCached { fn new(virtual_file: &VirtualFile, ctx: &mut DefCacheSavingContext<'_>) -> Self { Self { - parent: virtual_file.parent.map(|parent| FileIdCached::new(parent, ctx)), + parent: virtual_file.parent.map(|parent| SpanInFileCached::new(&parent, ctx)), name: virtual_file.name.clone(), content: String::from(&*(virtual_file.content)), code_mappings: virtual_file.code_mappings.to_vec(), @@ -1862,14 +1862,14 @@ type PluginFileDiagnosticNotesCached = OrderedHashMap, + location: Option, } impl DiagnosticNoteCached { fn new(note: DiagnosticNote, ctx: &mut DefCacheSavingContext<'_>) -> DiagnosticNoteCached { DiagnosticNoteCached { text: note.text.clone(), - location: note.location.map(|location| DiagnosticLocationCached::new(&location, ctx)), + location: note.location.map(|location| SpanInFileCached::new(&location, ctx)), } } fn embed(self, ctx: &mut DefCacheLoadingContext<'_>) -> DiagnosticNote { @@ -1880,23 +1880,17 @@ impl DiagnosticNoteCached { } } -#[derive(Serialize, Deserialize, Clone)] -struct DiagnosticLocationCached { +#[derive(Serialize, Deserialize, Clone, PartialEq, Eq)] +struct SpanInFileCached { file_id: FileIdCached, span: TextSpan, } -impl DiagnosticLocationCached { - fn new( - location: &DiagnosticLocation, - ctx: &mut DefCacheSavingContext<'_>, - ) -> DiagnosticLocationCached { - DiagnosticLocationCached { - file_id: FileIdCached::new(location.file_id, ctx), - span: location.span, - } +impl SpanInFileCached { + fn new(location: &SpanInFile, ctx: &mut DefCacheSavingContext<'_>) -> SpanInFileCached { + SpanInFileCached { file_id: FileIdCached::new(location.file_id, ctx), span: location.span } } - fn embed(self, ctx: &mut DefCacheLoadingContext<'_>) -> DiagnosticLocation { - DiagnosticLocation { file_id: self.file_id.embed(ctx), span: self.span } + fn embed(self, ctx: &mut DefCacheLoadingContext<'_>) -> SpanInFile { + SpanInFile { file_id: self.file_id.embed(ctx), span: self.span } } } diff --git a/crates/cairo-lang-defs/src/db.rs b/crates/cairo-lang-defs/src/db.rs index f08b55502e2..911e0aaf1c9 100644 --- a/crates/cairo-lang-defs/src/db.rs +++ b/crates/cairo-lang-defs/src/db.rs @@ -797,10 +797,11 @@ fn priv_module_sub_files( } if let Some(generated) = result.code { + let stable_ptr = item_ast.stable_ptr(db).untyped(); let generated_file_id = FileLongId::External( PluginGeneratedFileLongId { module_id, - stable_ptr: item_ast.stable_ptr(db).untyped(), + stable_ptr, name: generated.name.clone(), } .intern(db) @@ -814,7 +815,7 @@ fn priv_module_sub_files( files.insert( generated_file_id, VirtualFile { - parent: Some(file_id), + parent: Some(stable_ptr.span_in_file(db)), name: generated.name, content: generated.content.into(), code_mappings: generated.code_mappings.into(), diff --git a/crates/cairo-lang-defs/src/diagnostic_utils.rs b/crates/cairo-lang-defs/src/diagnostic_utils.rs index b6398638a04..abdf0735063 100644 --- a/crates/cairo-lang-defs/src/diagnostic_utils.rs +++ b/crates/cairo-lang-defs/src/diagnostic_utils.rs @@ -1,8 +1,7 @@ use std::fmt; use cairo_lang_debug::DebugWithDb; -use cairo_lang_diagnostics::DiagnosticLocation; -use cairo_lang_filesystem::ids::FileId; +use cairo_lang_filesystem::ids::{FileId, SpanInFile}; use cairo_lang_filesystem::span::{TextSpan, TextWidth}; use cairo_lang_syntax::node::db::SyntaxGroup; use cairo_lang_syntax::node::ids::SyntaxStablePtrId; @@ -50,33 +49,27 @@ impl StableLocation { self.stable_ptr } - /// Returns the [DiagnosticLocation] that corresponds to the [StableLocation]. - pub fn diagnostic_location(&self, db: &dyn DefsGroup) -> DiagnosticLocation { + /// Returns the [SpanInFile] that corresponds to the [StableLocation]. + pub fn diagnostic_location(&self, db: &dyn DefsGroup) -> SpanInFile { match self.inner_span { + None => self.stable_ptr.span_in_file(db), Some((start, width)) => { let start = self.syntax_node(db).offset(db).add_width(start); let end = start.add_width(width); - DiagnosticLocation { file_id: self.file_id(db), span: TextSpan { start, end } } - } - None => { - let syntax_node = self.syntax_node(db); - DiagnosticLocation { - file_id: self.file_id(db), - span: syntax_node.span_without_trivia(db), - } + SpanInFile { file_id: self.file_id(db), span: TextSpan { start, end } } } } } - /// Returns the [DiagnosticLocation] that corresponds to the [StableLocation]. + /// Returns the [SpanInFile] that corresponds to the [StableLocation]. pub fn diagnostic_location_until( &self, db: &dyn DefsGroup, until_stable_ptr: SyntaxStablePtrId, - ) -> DiagnosticLocation { + ) -> SpanInFile { let start = self.stable_ptr.lookup(db).span_start_without_trivia(db); let end = until_stable_ptr.lookup(db).span_end_without_trivia(db); - DiagnosticLocation { file_id: self.stable_ptr.file_id(db), span: TextSpan { start, end } } + SpanInFile { file_id: self.stable_ptr.file_id(db), span: TextSpan { start, end } } } } diff --git a/crates/cairo-lang-diagnostics/src/diagnostics.rs b/crates/cairo-lang-diagnostics/src/diagnostics.rs index 481fe01bf71..56d96c7cd6e 100644 --- a/crates/cairo-lang-diagnostics/src/diagnostics.rs +++ b/crates/cairo-lang-diagnostics/src/diagnostics.rs @@ -4,14 +4,12 @@ use std::sync::Arc; use cairo_lang_debug::debug::DebugWithDb; use cairo_lang_filesystem::db::{FilesGroup, get_originating_location}; -use cairo_lang_filesystem::ids::FileId; -use cairo_lang_filesystem::span::TextSpan; +use cairo_lang_filesystem::ids::{FileId, SpanInFile}; use cairo_lang_utils::Upcast; use cairo_lang_utils::ordered_hash_map::OrderedHashMap; use itertools::Itertools; use crate::error_code::{ErrorCode, OptionErrorCodeExt}; -use crate::location_marks::get_location_marks; #[cfg(test)] #[path = "diagnostics_test.rs"] @@ -37,7 +35,7 @@ impl fmt::Display for Severity { pub trait DiagnosticEntry: Clone + fmt::Debug + Eq + Hash { type DbType: Upcast + ?Sized; fn format(&self, db: &Self::DbType) -> String; - fn location(&self, db: &Self::DbType) -> DiagnosticLocation; + fn location(&self, db: &Self::DbType) -> SpanInFile; fn notes(&self, _db: &Self::DbType) -> &[DiagnosticNote] { &[] } @@ -58,98 +56,41 @@ pub trait DiagnosticEntry: Clone + fmt::Debug + Eq + Hash { /// [`FileId`]. pub type PluginFileDiagnosticNotes = OrderedHashMap; -// The representation of a source location inside a diagnostic. -#[derive(Clone, Debug, Eq, Hash, PartialEq)] -pub struct DiagnosticLocation { - pub file_id: FileId, - pub span: TextSpan, -} -impl DiagnosticLocation { - /// Get the location of right after this diagnostic's location (with width 0). - pub fn after(&self) -> Self { - Self { file_id: self.file_id, span: self.span.after() } - } - - /// Get the location of the originating user code. - pub fn user_location(&self, db: &dyn FilesGroup) -> Self { - let (file_id, span) = get_originating_location(db, self.file_id, self.span, None); - Self { file_id, span } - } - - /// Get the location of the originating user code, - /// along with [`DiagnosticNote`]s for this translation. - /// The notes are collected from the parent files of the originating location. - pub fn user_location_with_plugin_notes( - &self, - db: &dyn FilesGroup, - file_notes: &PluginFileDiagnosticNotes, - ) -> (Self, Vec) { - let mut parent_files = Vec::new(); - let (file_id, span) = - get_originating_location(db, self.file_id, self.span, Some(&mut parent_files)); - let diagnostic_notes = parent_files - .into_iter() - .rev() - .filter_map(|file_id| file_notes.get(&file_id).cloned()) - .collect_vec(); - (Self { file_id, span }, diagnostic_notes) - } - - /// Helper function to format the location of a diagnostic. - pub fn fmt_location(&self, f: &mut fmt::Formatter<'_>, db: &dyn FilesGroup) -> fmt::Result { - let file_path = self.file_id.full_path(db); - let start = match self.span.start.position_in_file(db, self.file_id) { - Some(pos) => format!("{}:{}", pos.line + 1, pos.col + 1), - None => "?".into(), - }; - - let end = match self.span.end.position_in_file(db, self.file_id) { - Some(pos) => format!("{}:{}", pos.line + 1, pos.col + 1), - None => "?".into(), - }; - write!(f, "{file_path}:{start}: {end}") - } -} - -impl DebugWithDb for DiagnosticLocation { - fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &dyn FilesGroup) -> fmt::Result { - let file_path = self.file_id.full_path(db); - let mut marks = String::new(); - let mut ending_pos = String::new(); - let starting_pos = match self.span.start.position_in_file(db, self.file_id) { - Some(starting_text_pos) => { - if let Some(ending_text_pos) = self.span.end.position_in_file(db, self.file_id) { - if starting_text_pos.line != ending_text_pos.line { - ending_pos = - format!("-{}:{}", ending_text_pos.line + 1, ending_text_pos.col); - } - } - marks = get_location_marks(db, self, true); - format!("{}:{}", starting_text_pos.line + 1, starting_text_pos.col + 1) - } - None => "?".into(), - }; - write!(f, "{file_path}:{starting_pos}{ending_pos}\n{marks}") - } -} - /// A note about a diagnostic. /// May include a relevant diagnostic location. #[derive(Clone, Debug, Eq, Hash, PartialEq)] pub struct DiagnosticNote { pub text: String, - pub location: Option, + pub location: Option, } impl DiagnosticNote { pub fn text_only(text: String) -> Self { Self { text, location: None } } - pub fn with_location(text: String, location: DiagnosticLocation) -> Self { + pub fn with_location(text: String, location: SpanInFile) -> Self { Self { text, location: Some(location) } } } +/// Get the location of the originating user code, +/// along with [`DiagnosticNote`]s for this translation. +/// The notes are collected from the parent files of the originating location. +pub fn user_location_with_plugin_notes( + db: &dyn FilesGroup, + location: SpanInFile, + file_notes: &PluginFileDiagnosticNotes, +) -> (SpanInFile, Vec) { + let mut parent_files = Vec::new(); + let origin = get_originating_location(db, location, Some(&mut parent_files)); + let diagnostic_notes = parent_files + .into_iter() + .rev() + .filter_map(|file_id| file_notes.get(&file_id).cloned()) + .collect(); + (origin, diagnostic_notes) +} + impl DebugWithDb for DiagnosticNote { fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &(dyn FilesGroup + 'static)) -> fmt::Result { write!(f, "{}", self.text)?; @@ -244,7 +185,7 @@ impl Default for DiagnosticsBuilder { pub fn format_diagnostics( db: &(dyn FilesGroup + 'static), message: &str, - location: DiagnosticLocation, + location: SpanInFile, ) -> String { format!("{message}\n --> {:?}\n", location.debug(db)) } @@ -321,7 +262,7 @@ impl Diagnostics { let mut msg = String::new(); let diag_location = entry.location(db); let (user_location, parent_file_notes) = - diag_location.user_location_with_plugin_notes(files_db, file_notes); + user_location_with_plugin_notes(files_db, diag_location, file_notes); let include_generated_location = diag_location != user_location && std::env::var("CAIRO_DEBUG_GENERATED_CODE").is_ok(); diff --git a/crates/cairo-lang-diagnostics/src/diagnostics_test.rs b/crates/cairo-lang-diagnostics/src/diagnostics_test.rs index 41b79ce916f..6cfe50659f0 100644 --- a/crates/cairo-lang-diagnostics/src/diagnostics_test.rs +++ b/crates/cairo-lang-diagnostics/src/diagnostics_test.rs @@ -1,12 +1,12 @@ use cairo_lang_filesystem::db::FilesGroup; -use cairo_lang_filesystem::ids::{FileId, FileKind, FileLongId, VirtualFile}; +use cairo_lang_filesystem::ids::{FileId, FileKind, FileLongId, SpanInFile, VirtualFile}; use cairo_lang_filesystem::span::{TextOffset, TextSpan, TextWidth}; use cairo_lang_filesystem::test_utils::FilesDatabaseForTesting; use cairo_lang_utils::Intern; use indoc::indoc; use test_log::test; -use super::{DiagnosticEntry, DiagnosticLocation, DiagnosticsBuilder}; +use super::{DiagnosticEntry, DiagnosticsBuilder}; // Test diagnostic. #[derive(Clone, Debug, Eq, Hash, PartialEq)] @@ -20,8 +20,8 @@ impl DiagnosticEntry for SimpleDiag { "Simple diagnostic.".into() } - fn location(&self, _db: &dyn cairo_lang_filesystem::db::FilesGroup) -> DiagnosticLocation { - DiagnosticLocation { + fn location(&self, _db: &dyn cairo_lang_filesystem::db::FilesGroup) -> SpanInFile { + SpanInFile { file_id: self.file_id, span: TextSpan { start: TextOffset::START, diff --git a/crates/cairo-lang-diagnostics/src/lib.rs b/crates/cairo-lang-diagnostics/src/lib.rs index 5e685587eab..2fa6d0ba2fe 100644 --- a/crates/cairo-lang-diagnostics/src/lib.rs +++ b/crates/cairo-lang-diagnostics/src/lib.rs @@ -2,13 +2,11 @@ //! source files. pub use diagnostics::{ - DiagnosticAdded, DiagnosticEntry, DiagnosticLocation, DiagnosticNote, Diagnostics, - DiagnosticsBuilder, FormattedDiagnosticEntry, Maybe, PluginFileDiagnosticNotes, Severity, - ToMaybe, ToOption, format_diagnostics, skip_diagnostic, + DiagnosticAdded, DiagnosticEntry, DiagnosticNote, Diagnostics, DiagnosticsBuilder, + FormattedDiagnosticEntry, Maybe, PluginFileDiagnosticNotes, Severity, ToMaybe, ToOption, + format_diagnostics, skip_diagnostic, }; pub use error_code::{ErrorCode, OptionErrorCodeExt}; -pub use location_marks::get_location_marks; mod diagnostics; mod error_code; -mod location_marks; diff --git a/crates/cairo-lang-filesystem/Cargo.toml b/crates/cairo-lang-filesystem/Cargo.toml index e064872ee74..37144dbb528 100644 --- a/crates/cairo-lang-filesystem/Cargo.toml +++ b/crates/cairo-lang-filesystem/Cargo.toml @@ -9,6 +9,7 @@ description = "Virtual filesystem for the compiler." [dependencies] cairo-lang-debug = { path = "../cairo-lang-debug", version = "~2.11.4" } cairo-lang-utils = { path = "../cairo-lang-utils", version = "~2.11.4", features = ["serde"] } +itertools = { workspace = true, default-features = true } path-clean.workspace = true salsa.workspace = true semver.workspace = true @@ -18,6 +19,8 @@ toml.workspace = true [dev-dependencies] env_logger.workspace = true +indoc.workspace = true +pretty_assertions.workspace = true serde_json.workspace = true test-log.workspace = true diff --git a/crates/cairo-lang-filesystem/src/db.rs b/crates/cairo-lang-filesystem/src/db.rs index 190f65db9d2..62347630df8 100644 --- a/crates/cairo-lang-filesystem/src/db.rs +++ b/crates/cairo-lang-filesystem/src/db.rs @@ -14,7 +14,7 @@ use crate::cfg::CfgSet; use crate::flag::Flag; use crate::ids::{ BlobId, BlobLongId, CodeMapping, CodeOrigin, CrateId, CrateLongId, Directory, FileId, - FileLongId, FlagId, FlagLongId, VirtualFile, + FileLongId, FlagId, FlagLongId, SpanInFile, VirtualFile, }; use crate::span::{FileSummary, TextOffset, TextSpan, TextWidth}; @@ -371,25 +371,20 @@ fn blob_content(db: &dyn FilesGroup, blob: BlobId) -> Option> { /// Returns the location of the originating user code. pub fn get_originating_location( db: &dyn FilesGroup, - mut file_id: FileId, - mut span: TextSpan, + SpanInFile { mut file_id, mut span }: SpanInFile, mut parent_files: Option<&mut Vec>, -) -> (FileId, TextSpan) { +) -> SpanInFile { if let Some(ref mut parent_files) = parent_files { parent_files.push(file_id); } while let Some((parent, code_mappings)) = get_parent_and_mapping(db, file_id) { - if let Some(origin) = translate_location(&code_mappings, span) { - span = origin; - file_id = parent; - if let Some(ref mut parent_files) = parent_files { - parent_files.push(file_id); - } - } else { - break; + file_id = parent.file_id; + if let Some(ref mut parent_files) = parent_files { + parent_files.push(file_id); } + span = translate_location(&code_mappings, span).unwrap_or(parent.span); } - (file_id, span) + SpanInFile { file_id, span } } /// This function finds a span in original code that corresponds to the provided span in the @@ -485,7 +480,7 @@ pub fn translate_location(code_mapping: &[CodeMapping], span: TextSpan) -> Optio pub fn get_parent_and_mapping( db: &dyn FilesGroup, file_id: FileId, -) -> Option<(FileId, Arc<[CodeMapping]>)> { +) -> Option<(SpanInFile, Arc<[CodeMapping]>)> { let vf = match file_id.lookup_intern(db) { FileLongId::OnDisk(_) => return None, FileLongId::Virtual(vf) => vf, diff --git a/crates/cairo-lang-filesystem/src/ids.rs b/crates/cairo-lang-filesystem/src/ids.rs index f5a7d6016f8..76b13df20fd 100644 --- a/crates/cairo-lang-filesystem/src/ids.rs +++ b/crates/cairo-lang-filesystem/src/ids.rs @@ -1,13 +1,16 @@ +use core::fmt; use std::collections::BTreeMap; use std::path::PathBuf; use std::sync::Arc; +use cairo_lang_debug::DebugWithDb; use cairo_lang_utils::{Intern, LookupIntern, define_short_id}; use path_clean::PathClean; use serde::{Deserialize, Serialize}; use smol_str::SmolStr; -use crate::db::{CORELIB_CRATE_NAME, FilesGroup}; +use crate::db::{CORELIB_CRATE_NAME, FilesGroup, get_originating_location}; +use crate::location_marks::get_location_marks; use crate::span::{TextOffset, TextSpan}; pub const CAIRO_FILE_EXTENSION: &str = "cairo"; @@ -141,7 +144,7 @@ impl CodeOrigin { #[derive(Clone, Debug, Hash, PartialEq, Eq)] pub struct VirtualFile { - pub parent: Option, + pub parent: Option, pub name: SmolStr, pub content: Arc, pub code_mappings: Arc<[CodeMapping]>, @@ -154,8 +157,12 @@ pub struct VirtualFile { impl VirtualFile { fn full_path(&self, db: &dyn FilesGroup) -> String { if let Some(parent) = self.parent { - // TODO(yuval): consider a different path format for virtual files. - format!("{}[{}]", parent.full_path(db), self.name) + use std::fmt::Write; + + let mut f = String::new(); + parent.fmt_location(&mut f, db).unwrap(); + write!(&mut f, "[{}]", self.name).unwrap(); + f } else { self.name.clone().into() } @@ -243,3 +250,58 @@ impl BlobId { BlobLongId::OnDisk(path.clean()).intern(db) } } + +/// A location within a file. +#[derive(Copy, Clone, Debug, Eq, Hash, PartialEq)] +pub struct SpanInFile { + pub file_id: FileId, + pub span: TextSpan, +} +impl SpanInFile { + /// Get the location of right after this diagnostic's location (with width 0). + pub fn after(&self) -> Self { + Self { file_id: self.file_id, span: self.span.after() } + } + + /// Get the location of the originating user code. + pub fn user_location(&self, db: &dyn FilesGroup) -> Self { + get_originating_location(db, *self, None) + } + + /// Helper function to format the location of a diagnostic. + pub fn fmt_location(&self, f: &mut impl fmt::Write, db: &dyn FilesGroup) -> fmt::Result { + let file_path = self.file_id.full_path(db); + let start = match self.span.start.position_in_file(db, self.file_id) { + Some(pos) => format!("{}:{}", pos.line + 1, pos.col + 1), + None => "?".into(), + }; + + let end = match self.span.end.position_in_file(db, self.file_id) { + Some(pos) => format!("{}:{}", pos.line + 1, pos.col + 1), + None => "?".into(), + }; + write!(f, "{file_path}:{start}: {end}") + } +} + +impl DebugWithDb for SpanInFile { + fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &dyn FilesGroup) -> fmt::Result { + let file_path = self.file_id.full_path(db); + let mut marks = String::new(); + let mut ending_pos = String::new(); + let starting_pos = match self.span.start.position_in_file(db, self.file_id) { + Some(starting_text_pos) => { + if let Some(ending_text_pos) = self.span.end.position_in_file(db, self.file_id) { + if starting_text_pos.line != ending_text_pos.line { + ending_pos = + format!("-{}:{}", ending_text_pos.line + 1, ending_text_pos.col); + } + } + marks = get_location_marks(db, *self, true); + format!("{}:{}", starting_text_pos.line + 1, starting_text_pos.col + 1) + } + None => "?".into(), + }; + write!(f, "{file_path}:{starting_pos}{ending_pos}\n{marks}") + } +} diff --git a/crates/cairo-lang-filesystem/src/lib.rs b/crates/cairo-lang-filesystem/src/lib.rs index b27bf5d04cd..4c800d162f4 100644 --- a/crates/cairo-lang-filesystem/src/lib.rs +++ b/crates/cairo-lang-filesystem/src/lib.rs @@ -5,5 +5,6 @@ pub mod db; pub mod detect; pub mod flag; pub mod ids; +pub mod location_marks; pub mod span; pub mod test_utils; diff --git a/crates/cairo-lang-diagnostics/src/location_marks.rs b/crates/cairo-lang-filesystem/src/location_marks.rs similarity index 90% rename from crates/cairo-lang-diagnostics/src/location_marks.rs rename to crates/cairo-lang-filesystem/src/location_marks.rs index a1ddd072a89..9e39b531f8c 100644 --- a/crates/cairo-lang-diagnostics/src/location_marks.rs +++ b/crates/cairo-lang-filesystem/src/location_marks.rs @@ -1,9 +1,10 @@ use std::sync::Arc; -use cairo_lang_filesystem::span::{FileSummary, TextPosition, TextSpan, TextWidth}; use itertools::repeat_n; -use crate::DiagnosticLocation; +use crate::db::FilesGroup; +use crate::ids::SpanInFile; +use crate::span::{FileSummary, TextPosition, TextSpan, TextWidth}; #[cfg(test)] #[path = "location_marks_test.rs"] @@ -11,8 +12,8 @@ mod test; /// Given a diagnostic location, returns a string with the location marks. pub fn get_location_marks( - db: &dyn cairo_lang_filesystem::db::FilesGroup, - location: &DiagnosticLocation, + db: &dyn FilesGroup, + location: SpanInFile, skip_middle_lines: bool, ) -> String { let span = &location.span; @@ -34,10 +35,7 @@ pub fn get_location_marks( } /// Given a single line diagnostic location, returns a string with the location marks. -fn get_single_line_location_marks( - db: &dyn cairo_lang_filesystem::db::FilesGroup, - location: &DiagnosticLocation, -) -> String { +fn get_single_line_location_marks(db: &dyn FilesGroup, location: SpanInFile) -> String { // TODO(ilya, 10/10/2023): Handle locations which spread over a few lines. let content = db.file_content(location.file_id).expect("File missing from DB."); let summary = db.file_summary(location.file_id).expect("File missing from DB."); @@ -67,8 +65,8 @@ fn get_single_line_location_marks( /// Given a multiple lines diagnostic location, returns a string with the location marks. fn get_multiple_lines_location_marks( - db: &dyn cairo_lang_filesystem::db::FilesGroup, - location: &DiagnosticLocation, + db: &dyn FilesGroup, + location: SpanInFile, skip_middle_lines: bool, ) -> String { let content = db.file_content(location.file_id).expect("File missing from DB."); diff --git a/crates/cairo-lang-diagnostics/src/location_marks_test.rs b/crates/cairo-lang-filesystem/src/location_marks_test.rs similarity index 76% rename from crates/cairo-lang-diagnostics/src/location_marks_test.rs rename to crates/cairo-lang-filesystem/src/location_marks_test.rs index 3f774c8135d..b6fc2f8e47a 100644 --- a/crates/cairo-lang-diagnostics/src/location_marks_test.rs +++ b/crates/cairo-lang-filesystem/src/location_marks_test.rs @@ -1,14 +1,13 @@ -use cairo_lang_filesystem::db::FilesGroup; -use cairo_lang_filesystem::ids::{FileKind, FileLongId, VirtualFile}; -use cairo_lang_filesystem::span::{TextSpan, TextWidth}; -use cairo_lang_filesystem::test_utils::FilesDatabaseForTesting; use cairo_lang_utils::Intern; use indoc::indoc; use pretty_assertions::assert_eq; use test_log::test; -use super::get_location_marks; -use crate::DiagnosticLocation; +use crate::db::FilesGroup; +use crate::ids::{FileKind, FileLongId, SpanInFile, VirtualFile}; +use crate::location_marks::get_location_marks; +use crate::span::{TextSpan, TextWidth}; +use crate::test_utils::FilesDatabaseForTesting; #[test] fn test_location_marks() { @@ -33,7 +32,7 @@ fn test_location_marks() { let third_line = summary.line_offsets[2]; // Empty span. - let location = DiagnosticLocation { + let location = SpanInFile { file_id: file, span: TextSpan { start: second_line.add_width(TextWidth::new_for_testing(13)), @@ -42,7 +41,7 @@ fn test_location_marks() { }; assert_eq!( - get_location_marks(&db, &location, true) + "\n", + get_location_marks(&db, location, true) + "\n", indoc! {" Second liné. ^ @@ -50,7 +49,7 @@ fn test_location_marks() { ); // Span of length 1. - let location = DiagnosticLocation { + let location = SpanInFile { file_id: file, span: TextSpan { start: third_line.add_width(TextWidth::new_for_testing(3)), @@ -59,7 +58,7 @@ fn test_location_marks() { }; assert_eq!( - get_location_marks(&db, &location, true) + "\n", + get_location_marks(&db, location, true) + "\n", indoc! {" Third liné. ^ @@ -67,7 +66,7 @@ fn test_location_marks() { ); // Span of length 2. - let location = DiagnosticLocation { + let location = SpanInFile { file_id: file, span: TextSpan { start: third_line.add_width(TextWidth::new_for_testing(3)), @@ -76,7 +75,7 @@ fn test_location_marks() { }; assert_eq!( - get_location_marks(&db, &location, true) + "\n", + get_location_marks(&db, location, true) + "\n", indoc! {" Third liné. ^^ @@ -84,7 +83,7 @@ fn test_location_marks() { ); // Span of length > 1. - let location = DiagnosticLocation { + let location = SpanInFile { file_id: file, span: TextSpan { start: second_line.add_width(TextWidth::new_for_testing(7)), @@ -93,7 +92,7 @@ fn test_location_marks() { }; assert_eq!( - get_location_marks(&db, &location, true) + "\n", + get_location_marks(&db, location, true) + "\n", indoc! {" Second liné. ^^^^ @@ -101,7 +100,7 @@ fn test_location_marks() { ); // Multiline span. - let location = DiagnosticLocation { + let location = SpanInFile { file_id: file, span: TextSpan { start: second_line.add_width(TextWidth::new_for_testing(7)), @@ -110,7 +109,7 @@ fn test_location_marks() { }; assert_eq!( - get_location_marks(&db, &location, true) + "\n", + get_location_marks(&db, location, true) + "\n", indoc! {" Second liné. ________^ @@ -120,7 +119,7 @@ fn test_location_marks() { ); // Span that ends past the end of the file. - let location = DiagnosticLocation { + let location = SpanInFile { file_id: file, span: TextSpan { start: third_line.add_width(TextWidth::new_for_testing(7)), @@ -129,7 +128,7 @@ fn test_location_marks() { }; assert_eq!( - get_location_marks(&db, &location, true) + "\n", + get_location_marks(&db, location, true) + "\n", indoc! {" Third liné. ^^^^ @@ -137,13 +136,13 @@ fn test_location_marks() { ); // Empty span past the end of the file. - let location = DiagnosticLocation { + let location = SpanInFile { file_id: file, span: TextSpan { start: summary.last_offset, end: summary.last_offset }, }; assert_eq!( - get_location_marks(&db, &location, true) + "\n", + get_location_marks(&db, location, true) + "\n", indoc! {" Third liné. ^ diff --git a/crates/cairo-lang-lowering/src/diagnostic.rs b/crates/cairo-lang-lowering/src/diagnostic.rs index a205ba82f3b..44d3ef7e21b 100644 --- a/crates/cairo-lang-lowering/src/diagnostic.rs +++ b/crates/cairo-lang-lowering/src/diagnostic.rs @@ -1,8 +1,8 @@ use cairo_lang_defs::diagnostic_utils::StableLocation; use cairo_lang_diagnostics::{ - DiagnosticAdded, DiagnosticEntry, DiagnosticLocation, DiagnosticNote, DiagnosticsBuilder, - Severity, + DiagnosticAdded, DiagnosticEntry, DiagnosticNote, DiagnosticsBuilder, Severity, }; +use cairo_lang_filesystem::ids::SpanInFile; use cairo_lang_semantic as semantic; use cairo_lang_semantic::corelib::LiteralError; use cairo_lang_semantic::db::SemanticGroup; @@ -100,7 +100,7 @@ impl DiagnosticEntry for LoweringDiagnostic { &self.location.notes } - fn location(&self, db: &Self::DbType) -> DiagnosticLocation { + fn location(&self, db: &Self::DbType) -> SpanInFile { if let LoweringDiagnosticKind::Unreachable { block_end_ptr } = &self.kind { return self.location.stable_location.diagnostic_location_until(db, *block_end_ptr); } diff --git a/crates/cairo-lang-lowering/src/lower/generated_test.rs b/crates/cairo-lang-lowering/src/lower/generated_test.rs index 01ac7c04b9c..f720d058043 100644 --- a/crates/cairo-lang-lowering/src/lower/generated_test.rs +++ b/crates/cairo-lang-lowering/src/lower/generated_test.rs @@ -2,7 +2,7 @@ use std::fmt::Write; use cairo_lang_debug::DebugWithDb; use cairo_lang_defs::ids::TopLevelLanguageElementId; -use cairo_lang_diagnostics::get_location_marks; +use cairo_lang_filesystem::location_marks::get_location_marks; use cairo_lang_semantic::test_utils::setup_test_function; use cairo_lang_test_utils::parse_test_file::TestRunnerResult; use cairo_lang_utils::Intern; @@ -83,7 +83,7 @@ fn test_generated_function( func_description, get_location_marks( db, - &generated_id.stable_location(db).unwrap().diagnostic_location(db), + generated_id.stable_location(db).unwrap().diagnostic_location(db), true ) ) diff --git a/crates/cairo-lang-parser/src/diagnostic.rs b/crates/cairo-lang-parser/src/diagnostic.rs index 13ea83a0426..7b462be7454 100644 --- a/crates/cairo-lang-parser/src/diagnostic.rs +++ b/crates/cairo-lang-parser/src/diagnostic.rs @@ -1,6 +1,6 @@ use cairo_lang_diagnostics::DiagnosticEntry; use cairo_lang_filesystem::db::FilesGroup; -use cairo_lang_filesystem::ids::FileId; +use cairo_lang_filesystem::ids::{FileId, SpanInFile}; use cairo_lang_filesystem::span::TextSpan; use cairo_lang_syntax::node::kind::SyntaxKind; use smol_str::SmolStr; @@ -244,8 +244,8 @@ Did you mean to write `{identifier}!{left}...{right}'?", } } - fn location(&self, _db: &dyn FilesGroup) -> cairo_lang_diagnostics::DiagnosticLocation { - cairo_lang_diagnostics::DiagnosticLocation { file_id: self.file_id, span: self.span } + fn location(&self, _db: &dyn FilesGroup) -> SpanInFile { + SpanInFile { file_id: self.file_id, span: self.span } } fn is_same_kind(&self, other: &Self) -> bool { diff --git a/crates/cairo-lang-plugins/src/test_utils.rs b/crates/cairo-lang-plugins/src/test_utils.rs index dfde7ff2a24..e04bad616c6 100644 --- a/crates/cairo-lang-plugins/src/test_utils.rs +++ b/crates/cairo-lang-plugins/src/test_utils.rs @@ -2,9 +2,8 @@ use cairo_lang_defs::db::DefsGroup; use cairo_lang_defs::diagnostic_utils::StableLocation; use cairo_lang_defs::ids::{LanguageElementId, ModuleId, ModuleItemId}; use cairo_lang_defs::plugin::PluginDiagnostic; -use cairo_lang_diagnostics::{ - DiagnosticEntry, DiagnosticLocation, DiagnosticsBuilder, ErrorCode, Severity, -}; +use cairo_lang_diagnostics::{DiagnosticEntry, DiagnosticsBuilder, ErrorCode, Severity}; +use cairo_lang_filesystem::ids::SpanInFile; use cairo_lang_syntax::node::kind::SyntaxKind; use cairo_lang_syntax::node::{TypedStablePtr, TypedSyntaxNode, ast}; use cairo_lang_utils::unordered_hash_set::UnorderedHashSet; @@ -77,7 +76,7 @@ impl DiagnosticEntry for TestDiagnosticEntry { fn format(&self, _db: &Self::DbType) -> String { self.0.message.to_string() } - fn location(&self, db: &Self::DbType) -> DiagnosticLocation { + fn location(&self, db: &Self::DbType) -> SpanInFile { match self.0.inner_span { Some(inner_span) => StableLocation::with_inner_span(self.0.stable_ptr, inner_span) .diagnostic_location(db), diff --git a/crates/cairo-lang-semantic/src/diagnostic.rs b/crates/cairo-lang-semantic/src/diagnostic.rs index 9fe6ebf3d7f..f7b4a4b7704 100644 --- a/crates/cairo-lang-semantic/src/diagnostic.rs +++ b/crates/cairo-lang-semantic/src/diagnostic.rs @@ -9,10 +9,11 @@ use cairo_lang_defs::ids::{ }; use cairo_lang_defs::plugin::PluginDiagnostic; use cairo_lang_diagnostics::{ - DiagnosticAdded, DiagnosticEntry, DiagnosticLocation, DiagnosticNote, DiagnosticsBuilder, - ErrorCode, Severity, error_code, + DiagnosticAdded, DiagnosticEntry, DiagnosticNote, DiagnosticsBuilder, ErrorCode, Severity, + error_code, }; use cairo_lang_filesystem::db::Edition; +use cairo_lang_filesystem::ids::SpanInFile; use cairo_lang_filesystem::span::TextWidth; use cairo_lang_parser::ParserDiagnostic; use cairo_lang_syntax as syntax; @@ -1077,14 +1078,11 @@ impl DiagnosticEntry for SemanticDiagnostic { .into(), } } - fn location(&self, db: &Self::DbType) -> DiagnosticLocation { + fn location(&self, db: &Self::DbType) -> SpanInFile { if let SemanticDiagnosticKind::MacroGeneratedCodeParserDiagnostic(parser_diagnostic) = &self.kind { - return DiagnosticLocation { - file_id: parser_diagnostic.file_id, - span: parser_diagnostic.span, - }; + return SpanInFile { file_id: parser_diagnostic.file_id, span: parser_diagnostic.span }; }; let mut location = self.stable_location.diagnostic_location(db); diff --git a/crates/cairo-lang-semantic/src/expr/compute.rs b/crates/cairo-lang-semantic/src/expr/compute.rs index 80db6779fd6..d4d8d6d768e 100644 --- a/crates/cairo-lang-semantic/src/expr/compute.rs +++ b/crates/cairo-lang-semantic/src/expr/compute.rs @@ -694,8 +694,8 @@ fn compute_expr_inline_macro_semantic( let prev_macro_call_data = ctx.resolver.macro_call_data.clone(); let InlineMacroExpansion { content, name, info } = expand_inline_macro(ctx, syntax)?; let new_file_id = FileLongId::Virtual(VirtualFile { - parent: Some(syntax.stable_ptr(ctx.db).untyped().file_id(ctx.db)), - name: name.into(), + parent: Some(syntax.stable_ptr(ctx.db).untyped().span_in_file(ctx.db)), + name: name.clone().into(), content, code_mappings: info.mappings.clone(), kind: FileKind::Expr, @@ -754,7 +754,7 @@ fn expand_macro_for_statement( let prev_macro_call_data = ctx.resolver.macro_call_data.clone(); let InlineMacroExpansion { content, name, info } = expand_inline_macro(ctx, syntax)?; let new_file_id = FileLongId::Virtual(VirtualFile { - parent: Some(syntax.stable_ptr(ctx.db).untyped().file_id(ctx.db)), + parent: Some(syntax.stable_ptr(ctx.db).untyped().span_in_file(ctx.db)), name: name.into(), content, code_mappings: info.mappings.clone(), diff --git a/crates/cairo-lang-semantic/src/expr/test_data/inline_macros b/crates/cairo-lang-semantic/src/expr/test_data/inline_macros index d9e624b845a..bc5b70b4322 100644 --- a/crates/cairo-lang-semantic/src/expr/test_data/inline_macros +++ b/crates/cairo-lang-semantic/src/expr/test_data/inline_macros @@ -1059,9 +1059,9 @@ macro a_macro { //! > expected_diagnostics error: Expected path after modifier. - --> lib.cairo[a_macro]:1:17 - $defsite - ^ + --> lib.cairo:7:5 + a_macro!(x); + ^^^^^^^^^^^ //! > ========================================================================== @@ -1147,14 +1147,14 @@ fn consume_z(_z: felt252) {} //! > expected_diagnostics warning: Path in a macro without a resolver modifier ($callsite or $defsite) - currently resolving as $callsite but this will not be supported in future versions. - --> lib.cairo[bas_use_z_from_callsite]:1:9 - z - ^ + --> lib.cairo:10:5 + bas_use_z_from_callsite!(); + ^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0006]: Identifier not found. - --> lib.cairo[bas_use_z_from_callsite]:1:9 - z - ^ + --> lib.cairo:10:5 + bas_use_z_from_callsite!(); + ^^^^^^^^^^^^^^^^^^^^^^^^^^ //! > ========================================================================== @@ -1191,9 +1191,9 @@ macro wrap_bas_use_z_from_callsite { //! > expected_diagnostics error[E0006]: Identifier not found. - --> lib.cairo[wrap_bas_use_z_from_callsite][bas_use_z_from_callsite]:1:20 - $callsite::z - ^ + --> lib.cairo:17:5 + wrap_bas_use_z_from_callsite!(); + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ //! > ========================================================================== @@ -1221,9 +1221,9 @@ fn bar() {} //! > expected_diagnostics warning: Path in a macro without a resolver modifier ($callsite or $defsite) - currently resolving as $callsite but this will not be supported in future versions. - --> lib.cairo[call_bar]:1:9 - bar() - ^^^ + --> lib.cairo:9:5 + call_bar!(); + ^^^^^^^^^^^ //! > ========================================================================== @@ -1357,9 +1357,9 @@ macro test_macro { //! > expected_diagnostics error: Parser error in macro-expanded code: Missing tokens. Expected an expression. - --> lib.cairo[test_macro]:1:12 - 1 + - ^ + --> lib.cairo:7:5 + test_macro!(); + ^^^^^^^^^^^^^ //! > ========================================================================== @@ -1455,16 +1455,14 @@ macro statement_list_returning_macro { //! > expected_diagnostics error: Parser error in macro-expanded code: Missing tokens. Expected an expression. - --> lib.cairo[statement_list_returning_macro]:1:1 - let a = 1; -^ + --> lib.cairo:8:14 + let _x = statement_list_returning_macro!(); + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Parser error in macro-expanded code: Skipped tokens. Expected: end of expr. - --> lib.cairo[statement_list_returning_macro]:1:1-2:18 - let a = 1; - _^ -| let b = 2; -|__________________^ + --> lib.cairo:8:14 + let _x = statement_list_returning_macro!(); + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ //! > ========================================================================== @@ -1543,14 +1541,9 @@ macro empty_path { //! > expected_diagnostics error: Expected path after modifier. - --> lib.cairo[empty_path]:1:17 - $defsite; - ^ - -error: Expected path after modifier. - --> lib.cairo[empty_path]:2:18 - $callsite; - ^ + --> lib.cairo:8:5 + empty_path!(); + ^^^^^^^^^^^^^ //! > ========================================================================== diff --git a/crates/cairo-lang-sierra-generator/src/program_generator.rs b/crates/cairo-lang-sierra-generator/src/program_generator.rs index 66dd67332f9..76935dc37bf 100644 --- a/crates/cairo-lang-sierra-generator/src/program_generator.rs +++ b/crates/cairo-lang-sierra-generator/src/program_generator.rs @@ -3,8 +3,9 @@ use std::sync::Arc; use cairo_lang_debug::DebugWithDb; use cairo_lang_defs::diagnostic_utils::StableLocation; -use cairo_lang_diagnostics::{Maybe, get_location_marks}; +use cairo_lang_diagnostics::Maybe; use cairo_lang_filesystem::ids::CrateId; +use cairo_lang_filesystem::location_marks::get_location_marks; use cairo_lang_lowering::ids::ConcreteFunctionWithBodyId; use cairo_lang_sierra::extensions::GenericLibfuncEx; use cairo_lang_sierra::extensions::core::CoreLibfunc; @@ -205,7 +206,7 @@ impl DebugWithDb for SierraProgramWithDebug { &self.debug_info.statements_locations.locations.get(&StatementIdx(i)) { let loc = - get_location_marks(db, &loc.first().unwrap().diagnostic_location(db), true); + get_location_marks(db, loc.first().unwrap().diagnostic_location(db), true); println!("{}", loc.split('\n').map(|l| format!("// {l}")).join("\n")); } } diff --git a/crates/cairo-lang-sierra-generator/src/statements_locations.rs b/crates/cairo-lang-sierra-generator/src/statements_locations.rs index ae48245412a..7116153397e 100644 --- a/crates/cairo-lang-sierra-generator/src/statements_locations.rs +++ b/crates/cairo-lang-sierra-generator/src/statements_locations.rs @@ -167,7 +167,7 @@ pub fn file_module_absolute_identifier(db: &dyn DefsGroup, mut file_id: FileId) while let FileLongId::Virtual(VirtualFile { parent: Some(parent), .. }) = file_id.lookup_intern(db) { - file_id = parent; + file_id = parent.file_id; } let file_modules = db.file_modules(file_id).to_option()?; diff --git a/crates/cairo-lang-sierra-generator/src/statements_locations_test.rs b/crates/cairo-lang-sierra-generator/src/statements_locations_test.rs index d36b07ff171..5da5133e956 100644 --- a/crates/cairo-lang-sierra-generator/src/statements_locations_test.rs +++ b/crates/cairo-lang-sierra-generator/src/statements_locations_test.rs @@ -1,4 +1,4 @@ -use cairo_lang_diagnostics::get_location_marks; +use cairo_lang_filesystem::location_marks::get_location_marks; use cairo_lang_lowering::db::LoweringGroup; use cairo_lang_lowering::ids::ConcreteFunctionWithBodyId; use cairo_lang_semantic::test_utils::setup_test_function; @@ -47,7 +47,7 @@ pub fn test_sierra_locations( } sierra_code.push_str(&get_location_marks( db, - &location.diagnostic_location(db), + location.diagnostic_location(db), true, )); sierra_code.push('\n'); diff --git a/crates/cairo-lang-starknet/src/plugin/test.rs b/crates/cairo-lang-starknet/src/plugin/test.rs index 62b3859f8fb..91a632a7932 100644 --- a/crates/cairo-lang-starknet/src/plugin/test.rs +++ b/crates/cairo-lang-starknet/src/plugin/test.rs @@ -4,9 +4,8 @@ use cairo_lang_compiler::diagnostics::get_diagnostics_as_string; use cairo_lang_debug::debug::DebugWithDb; use cairo_lang_defs::db::DefsGroup; use cairo_lang_defs::ids::ModuleId; -use cairo_lang_diagnostics::DiagnosticLocation; use cairo_lang_filesystem::db::FilesGroup; -use cairo_lang_filesystem::ids::FileLongId; +use cairo_lang_filesystem::ids::{FileLongId, SpanInFile}; use cairo_lang_filesystem::span::TextSpan; use cairo_lang_plugins::test_utils::expand_module_text; use cairo_lang_semantic::test_utils::setup_test_module; @@ -49,8 +48,7 @@ impl TestFileRunner for ExpandContractTestRunner { for file_id in files { let content = db.file_content(file_id).unwrap(); - let content_location = - DiagnosticLocation { file_id, span: TextSpan::from_str(&content) }; + let content_location = SpanInFile { file_id, span: TextSpan::from_str(&content) }; let original_location = content_location.user_location(db.upcast()); let origin = if content_location == original_location { "".to_string() diff --git a/crates/cairo-lang-syntax/src/node/ids.rs b/crates/cairo-lang-syntax/src/node/ids.rs index 540dea88ff6..a91442e09bd 100644 --- a/crates/cairo-lang-syntax/src/node/ids.rs +++ b/crates/cairo-lang-syntax/src/node/ids.rs @@ -1,6 +1,6 @@ use std::sync::Arc; -use cairo_lang_filesystem::ids::FileId; +use cairo_lang_filesystem::ids::{FileId, SpanInFile}; use cairo_lang_filesystem::span::TextWidth; use cairo_lang_utils::{LookupIntern, define_short_id}; @@ -59,6 +59,10 @@ impl SyntaxStablePtrId { let SyntaxStablePtr::Child { parent, .. } = self.lookup_intern(db) else { panic!() }; parent } + /// Returns the span in file of this stable pointer without trivia. + pub fn span_in_file(&self, db: &dyn SyntaxGroup) -> SpanInFile { + SpanInFile { file_id: self.file_id(db), span: self.lookup(db).span_without_trivia(db) } + } /// Returns the stable pointer of the `n`th parent of this stable pointer. /// n = 0: returns itself. /// n = 1: return the parent.