Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed diagnostics to have full file paths #4024

Merged
merged 1 commit into from
Sep 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/cairo-lang-diagnostics/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
}
}

Expand Down
17 changes: 17 additions & 0 deletions crates/cairo-lang-filesystem/src/ids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
Expand Down
73 changes: 73 additions & 0 deletions crates/cairo-lang-plugins/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand All @@ -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;
Expand All @@ -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>,
Expand Down Expand Up @@ -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, &[])
}

/// 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());
Expand Down Expand Up @@ -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(),
}
}
}
10 changes: 5 additions & 5 deletions crates/cairo-lang-plugins/src/test_data/derive
Original file line number Diff line number Diff line change
Expand Up @@ -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]
^********^
20 changes: 20 additions & 0 deletions crates/cairo-lang-plugins/src/test_data/general
Original file line number Diff line number Diff line change
@@ -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 {}
^*********^
4 changes: 2 additions & 2 deletions crates/cairo-lang-plugins/src/test_data/generate_trait
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,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> {
^*************************^

Expand Down
8 changes: 4 additions & 4 deletions crates/cairo-lang-plugins/src/test_data/panicable
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
^*****************************^
Original file line number Diff line number Diff line change
Expand Up @@ -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();
^********^

Expand Down
4 changes: 2 additions & 2 deletions crates/cairo-lang-starknet/src/abi_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
^*********^

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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> {}
^**********************************************^

Expand Down Expand Up @@ -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),
^*********^

Expand Down Expand Up @@ -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),
^

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
^**^

Expand All @@ -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(
^***^

Expand Down