Changed diagnostics to have full file paths

starkware-libs · Sep 7, 2023 · 00bedd0 · 00bedd0
1 parent 9b3381b
commit 00bedd0
Show file tree

Hide file tree

Showing 11 changed files with 131 additions and 21 deletions.
diff --git a/crates/cairo-lang-diagnostics/src/diagnostics.rs b/crates/cairo-lang-diagnostics/src/diagnostics.rs
@@ -41,13 +41,13 @@ impl DiagnosticLocation {
 
 impl DebugWithDb<dyn FilesGroup> for DiagnosticLocation {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &dyn FilesGroup) -> std::fmt::Result {
-        let file_name = self.file_id.file_name(db);
+        let file_path = self.file_id.full_path(db);
         let marks = get_location_marks(db, self);
         let pos = match self.span.start.position_in_file(db, self.file_id) {
             Some(pos) => format!("{}:{}", pos.line + 1, pos.col + 1),
             None => "?".into(),
         };
-        write!(f, "{file_name}:{pos}\n{marks}")
+        write!(f, "{file_path}:{pos}\n{marks}")
     }
 }
 

diff --git a/crates/cairo-lang-filesystem/src/ids.rs b/crates/cairo-lang-filesystem/src/ids.rs
@@ -109,6 +109,17 @@ pub struct VirtualFile {
     pub diagnostics_mappings: Arc<Vec<DiagnosticMapping>>,
     pub kind: FileKind,
 }
+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)
+        } else {
+            self.name.clone().into()
+        }
+    }
+}
+
 define_short_id!(FileId, FileLongId, FilesGroup, lookup_intern_file);
 impl FileId {
     pub fn new(db: &dyn FilesGroup, path: PathBuf) -> FileId {
@@ -122,6 +133,12 @@ impl FileId {
             FileLongId::Virtual(vf) => vf.name.to_string(),
         }
     }
+    pub fn full_path(self, db: &dyn FilesGroup) -> String {
+        match db.lookup_intern_file(self) {
+            FileLongId::OnDisk(path) => path.to_str().unwrap_or("<unknown>").to_string(),
+            FileLongId::Virtual(vf) => vf.full_path(db),
+        }
+    }
     pub fn kind(self, db: &dyn FilesGroup) -> FileKind {
         match db.lookup_intern_file(self) {
             FileLongId::OnDisk(_) => FileKind::Module,

diff --git a/crates/cairo-lang-plugins/src/test.rs b/crates/cairo-lang-plugins/src/test.rs
@@ -2,6 +2,7 @@ use std::sync::Arc;
 
 use cairo_lang_defs::db::{DefsDatabase, DefsGroup};
 use cairo_lang_defs::ids::{LanguageElementId, ModuleId, ModuleItemId};
+use cairo_lang_defs::plugin::{MacroPlugin, PluginDiagnostic, PluginGeneratedFile, PluginResult};
 use cairo_lang_diagnostics::{format_diagnostics, DiagnosticLocation};
 use cairo_lang_filesystem::cfg::CfgSet;
 use cairo_lang_filesystem::db::{
@@ -10,6 +11,7 @@ use cairo_lang_filesystem::db::{
 use cairo_lang_filesystem::ids::{CrateLongId, Directory, FileLongId};
 use cairo_lang_parser::db::ParserDatabase;
 use cairo_lang_syntax::node::db::{SyntaxDatabase, SyntaxGroup};
+use cairo_lang_syntax::node::helpers::QueryAttrs;
 use cairo_lang_syntax::node::{ast, TypedSyntaxNode};
 use cairo_lang_test_utils::has_disallowed_diagnostics;
 use cairo_lang_utils::ordered_hash_map::OrderedHashMap;
@@ -29,6 +31,15 @@ cairo_lang_test_utils::test_file_test!(
     test_expand_plugin
 );
 
+cairo_lang_test_utils::test_file_test!(
+    expand_general_plugin,
+    "src/test_data",
+    {
+        general: "general",
+    },
+    test_general_plugin
+);
+
 #[salsa::database(DefsDatabase, ParserDatabase, SyntaxDatabase, FilesDatabase)]
 pub struct DatabaseForTesting {
     storage: salsa::Storage<DatabaseForTesting>,
@@ -104,11 +115,32 @@ fn expand_module_text(
     output
 }
 
+/// Tests expansion of given code, with only the default plugins.
 pub fn test_expand_plugin(
     inputs: &OrderedHashMap<String, String>,
     args: &OrderedHashMap<String, String>,
+) -> Result<OrderedHashMap<String, String>, String> {
+    test_expand_plugin_inner(inputs, args, &vec![])
+}
+
+/// Tests expansion of given code, with the default plugins plus a dedicated plugin.
+fn test_general_plugin(
+    inputs: &OrderedHashMap<String, String>,
+    args: &OrderedHashMap<String, String>,
+) -> Result<OrderedHashMap<String, String>, String> {
+    test_expand_plugin_inner(inputs, args, &[Arc::new(DoubleIndirectionPlugin)])
+}
+
+/// Tests expansion of given code, with the default plugins plus the given extra plugins.
+pub fn test_expand_plugin_inner(
+    inputs: &OrderedHashMap<String, String>,
+    args: &OrderedHashMap<String, String>,
+    extra_plugins: &[Arc<dyn MacroPlugin>],
 ) -> Result<OrderedHashMap<String, String>, String> {
     let db = &mut DatabaseForTesting::default();
+    let mut plugins = db.macro_plugins();
+    plugins.extend_from_slice(extra_plugins);
+    db.set_macro_plugins(plugins);
 
     let cfg_set: Option<CfgSet> =
         inputs.get("cfg").map(|s| serde_json::from_str(s.as_str()).unwrap());
@@ -138,3 +170,44 @@ pub fn test_expand_plugin(
         ("expected_diagnostics".into(), joined_diagnostics),
     ]))
 }
+
+#[derive(Debug)]
+struct DoubleIndirectionPlugin;
+impl MacroPlugin for DoubleIndirectionPlugin {
+    fn generate_code(&self, db: &dyn SyntaxGroup, item_ast: ast::Item) -> PluginResult {
+        match item_ast {
+            ast::Item::Struct(struct_ast) => {
+                if struct_ast.has_attr(db, "first") {
+                    PluginResult {
+                        code: Some(PluginGeneratedFile {
+                            name: "virt1".into(),
+                            content: "#[second] struct A {}\n".to_string(),
+                            diagnostics_mappings: Default::default(),
+                            aux_data: None,
+                        }),
+                        ..PluginResult::default()
+                    }
+                } else if struct_ast.has_attr(db, "second") {
+                    PluginResult {
+                        code: Some(PluginGeneratedFile {
+                            name: "virt2".into(),
+                            content: "struct B {}\n".to_string(),
+                            diagnostics_mappings: Default::default(),
+                            aux_data: None,
+                        }),
+                        ..PluginResult::default()
+                    }
+                } else {
+                    PluginResult {
+                        diagnostics: vec![PluginDiagnostic {
+                            stable_ptr: struct_ast.stable_ptr().untyped(),
+                            message: "Double indirection diagnostic".to_string(),
+                        }],
+                        ..PluginResult::default()
+                    }
+                }
+            }
+            _ => PluginResult::default(),
+        }
+    }
+}
diff --git a/crates/cairo-lang-plugins/src/test_data/derive b/crates/cairo-lang-plugins/src/test_data/derive
@@ -415,26 +415,26 @@ impl TooManyDefaultValuesDefault of Default::<TooManyDefaultValues> {
 
 //! > expected_diagnostics
 error: Expected args.
- --> lib.cairo:1:9
+ --> test_src/lib.cairo:1:9
 #[derive()]
         ^^
 
 error: Expected path.
- --> lib.cairo:4:10
+ --> test_src/lib.cairo:4:10
 #[derive(1)]
          ^
 
 error: Unsupported trait for derive for extern types.
- --> lib.cairo:13:10
+ --> test_src/lib.cairo:13:10
 #[derive(Clone)]
          ^***^
 
 error: derive `Default` for enum only supported with a default variant.
- --> lib.cairo:16:10
+ --> test_src/lib.cairo:16:10
 #[derive(Default)]
          ^*****^
 
 error: Multiple variants annotated with `#[default]`
- --> lib.cairo:26:5
+ --> test_src/lib.cairo:26:5
     #[default]
     ^********^
diff --git a/crates/cairo-lang-plugins/src/test_data/general b/crates/cairo-lang-plugins/src/test_data/general
@@ -0,0 +1,20 @@
+//! > Test expansion of double indirection plugin.
+
+//! > test_runner_name
+test_general_plugin(allow_diagnostics: true)
+
+//! > cairo_code
+#[first]
+struct X {}
+
+//! > expanded_cairo_code
+#[first]
+struct X {}
+#[second] struct A {}
+struct B {}
+
+//! > expected_diagnostics
+error: Double indirection diagnostic
+ --> test_src/lib.cairo[virt1][virt2]:1:1
+struct B {}
+^*********^
diff --git a/crates/cairo-lang-plugins/src/test_data/generate_trait b/crates/cairo-lang-plugins/src/test_data/generate_trait
@@ -151,12 +151,12 @@ impl ImplWithUnsupportedAttributeArg<T> of OtherTrait<T> {
 
 //! > expected_diagnostics
 error: Generated trait must have a single element path.
- --> lib.cairo:2:29
+ --> test_src/lib.cairo:2:29
 impl ImplWithBadTrait<T> of Some::Path::To::Trait<T> {
                             ^**********************^
 
 error: Generated trait must have generic args matching the impl's generic params.
- --> lib.cairo:6:36
+ --> test_src/lib.cairo:6:36
 impl ImplNotMatchingGenerics<T> of TraitNotMatchingGenerics<S> {
                                    ^*************************^
 

diff --git a/crates/cairo-lang-plugins/src/test_data/panicable b/crates/cairo-lang-plugins/src/test_data/panicable
@@ -138,21 +138,21 @@ extern fn bar() -> Result::<felt252, Err> nopanic;
 
 //! > expected_diagnostics
 error: Failed to extract panic data attribute
- --> lib.cairo:1:1
+ --> test_src/lib.cairo:1:1
 #[panic_with(123, foo_bad_err_code)]
 ^**********************************^
 
 error: Failed to extract panic data attribute
- --> lib.cairo:4:1
+ --> test_src/lib.cairo:4:1
 #[panic_with(missing_args)]
 ^*************************^
 
 error: Currently only wrapping functions returning an Option<T> or Result<T, E>
- --> lib.cairo:8:39
+ --> test_src/lib.cairo:8:39
 extern fn bad_ret_type(_a: some_type) -> felt252 nopanic;
                                       ^********^
 
 error: `#[panic_with]` cannot be applied multiple times to the same item.
- --> lib.cairo:11:1
+ --> test_src/lib.cairo:11:1
 #[panic_with('3', bar_changed)]
 ^*****************************^
diff --git a/crates/cairo-lang-semantic/src/expr/semantic_test_data/inline_macros b/crates/cairo-lang-semantic/src/expr/semantic_test_data/inline_macros
@@ -205,7 +205,7 @@ Block(
 
 //! > semantic_diagnostics
 error: Type annotations needed. Failed to infer ?0
- --> array_inline_macro:2:54
+ --> lib.cairo[array_inline_macro]:2:54
             let mut __array_builder_macro_result__ = ArrayTrait::new();
                                                      ^********^
 

diff --git a/crates/cairo-lang-starknet/src/abi_test.rs b/crates/cairo-lang-starknet/src/abi_test.rs
@@ -234,12 +234,12 @@ fn test_abi_failure() {
         diagnostics,
         indoc! {"
         error: Trait has no implementation in context: core::starknet::event::Event::<test::A>
-         --> event_impl:8:34
+         --> lib.cairo/event_impl:8:34
                         starknet::Event::append_keys_and_data(
                                          ^******************^
 
         error: Trait has no implementation in context: core::starknet::event::Event::<test::A>
-         --> event_impl:20:44
+         --> lib.cairo/event_impl:20:44
                         let val = starknet::Event::deserialize(
                                                    ^*********^
 

diff --git a/crates/cairo-lang-starknet/src/plugin/plugin_test_data/contracts/diagnostics b/crates/cairo-lang-starknet/src/plugin/plugin_test_data/contracts/diagnostics
@@ -371,7 +371,7 @@ error: Plugin diagnostic: Identifier not found.
         ^*****^
 
 error: Invalid drop trait implementation, Candidate impl core::traits::SnapshotDrop::<?0> has an unused generic parameter..
- --> contract:9:5
+ --> lib.cairo[contract]:9:5
     impl ContractStateDrop of Drop<ContractState> {}
     ^**********************************************^
 
@@ -716,7 +716,7 @@ impl EventIsEvent of starknet::Event<Event> {
 
 //! > expected_diagnostics
 error: Trait has no implementation in context: core::serde::Serde::<test::MyType>
- --> contract:29:40
+ --> lib.cairo[contract]:29:40
         serde::Serde::<super::MyType>::deserialize(ref data),
                                        ^*********^
 
@@ -989,7 +989,7 @@ error: Plugin diagnostic: Contract entry points cannot have generic arguments
           ^*^
 
 error: Type not found.
- --> contract:29:24
+ --> lib.cairo[contract]:29:24
         serde::Serde::<T>::deserialize(ref data),
                        ^
 

diff --git a/crates/cairo-lang-starknet/src/plugin/plugin_test_data/contracts/with_component_diagnostics b/crates/cairo-lang-starknet/src/plugin/plugin_test_data/contracts/with_component_diagnostics
@@ -1680,7 +1680,7 @@ error: Plugin diagnostic: Type not found.
                                                        ^*****^
 
 error: Cannot infer trait core::starknet::storage_access::StorePacking::<?0, ?1>. First generic argument must be known.
- --> contract:38:79
+ --> lib.cairo[contract]:38:79
                     starknet::Store::<super::super::test_component::Storage>::read(
                                                                               ^**^
 
@@ -1695,7 +1695,7 @@ error: Plugin diagnostic: Type not found.
                                                        ^*****^
 
 error: Cannot infer trait core::starknet::storage_access::StorePacking::<?0, ?1>. First generic argument must be known.
- --> contract:48:79
+ --> lib.cairo[contract]:48:79
                     starknet::Store::<super::super::test_component::Storage>::write(
                                                                               ^***^