fix(sema): keep builtin import notes on source locations

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)
     }
 }
 

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())) {

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,
             ));
 

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();