diff --git a/src/sema/diagnostics.rs b/src/sema/diagnostics.rs index b47cda270..f9dfeadfb 100644 --- a/src/sema/diagnostics.rs +++ b/src/sema/diagnostics.rs @@ -142,6 +142,7 @@ fn convert_diagnostic( .with_message(msg.message.to_owned()); let mut labels = Vec::new(); + let mut plain_notes = Vec::new(); if let Loc::File(file_no, start, end) = msg.loc { labels.push(diagnostic::Label::primary(file_id[&file_no], start..end)); @@ -154,14 +155,20 @@ fn convert_diagnostic( .with_message(note.message.to_owned()), ); } else { - unreachable!("note without file position"); + plain_notes.push(note.message.to_owned()); } } - if labels.is_empty() { + let diagnostic = if labels.is_empty() { diagnostic } else { diagnostic.with_labels(labels) + }; + + if plain_notes.is_empty() { + diagnostic + } else { + diagnostic.with_notes(plain_notes) } } diff --git a/src/sema/mod.rs b/src/sema/mod.rs index d6a33c678..328a42c0e 100644 --- a/src/sema/mod.rs +++ b/src/sema/mod.rs @@ -403,6 +403,24 @@ fn resolve_import( name: name.clone(), loc: filename.loc, }; + let symbol = match symbol { + ast::Symbol::Function(funcs) => ast::Symbol::Function( + funcs + .into_iter() + .map(|(loc, func_no)| { + ( + if matches!(loc, pt::Loc::File(..)) { + loc + } else { + filename.loc + }, + func_no, + ) + }) + .collect(), + ), + other => other, + }; // Only add symbol if it does not already exist with same definition if let Some(existing) = ns.function_symbols.get(&(file_no, None, name.clone())) { diff --git a/src/sema/namespace.rs b/src/sema/namespace.rs index 8915935b9..2a9239623 100644 --- a/src/sema/namespace.rs +++ b/src/sema/namespace.rs @@ -98,6 +98,9 @@ impl Namespace { self.function_symbols .get(&(file_no, contract_no, id.name.to_owned())) { + let already_defined_as_builtin = v + .iter() + .any(|(_, func_no)| self.functions[*func_no].loc == pt::Loc::Builtin); let notes = v .iter() .map(|(pos, _)| Note { @@ -108,7 +111,11 @@ impl Namespace { self.diagnostics.push(Diagnostic::error_with_notes( id.loc, - format!("{} is already defined as a function", id.name), + if already_defined_as_builtin { + format!("{} is already defined as a builtin function", id.name) + } else { + format!("{} is already defined as a function", id.name) + }, notes, )); diff --git a/tests/polkadot_tests/imports.rs b/tests/polkadot_tests/imports.rs index 5ef12ad50..79aee4b0b 100644 --- a/tests/polkadot_tests/imports.rs +++ b/tests/polkadot_tests/imports.rs @@ -2,6 +2,7 @@ use solang::file_resolver::FileResolver; use solang::Target; +use solang_parser::pt::Loc; use std::ffi::OsStr; #[test] @@ -145,6 +146,39 @@ fn enum_import() { ); } +#[test] +fn builtin_import_definition_note_uses_import_location() { + let mut cache = FileResolver::default(); + + cache.set_file_contents( + "a.sol", + r#" + import "polkadot"; + + contract Foo { + function chain_extension() public pure {} + } + "# + .to_string(), + ); + + let ns = solang::parse_and_resolve(OsStr::new("a.sol"), &mut cache, Target::default_polkadot()); + + assert_eq!( + ns.diagnostics.first_error(), + "chain_extension is already defined as a builtin function" + ); + + let diagnostics = ns.diagnostics.errors(); + assert_eq!(diagnostics.len(), 1); + assert_eq!(diagnostics[0].notes.len(), 1); + assert_eq!( + diagnostics[0].notes[0].message, + "location of previous definition" + ); + assert!(matches!(diagnostics[0].notes[0].loc, Loc::File(..))); +} + #[test] fn struct_import() { let mut cache = FileResolver::default();