Reject duplicate top-level symbols

Author: olwang

README.md

@@ -24,6 +24,7 @@ It currently implements:
 Implemented diagnostic classes include:
 
 - file mode violations
+- duplicate top-level declarations that would make symbol resolution ambiguous
 - missing named arguments
 - unknown, missing, and duplicate call arguments for known signatures
 - unknown callees outside known functions, constructors, enum variants, and builtin signatures

src/analyzer.rs

@@ -1,6 +1,6 @@
 use crate::checks;
 use crate::diagnostic::{Diagnostic, code};
-use crate::hir::{FunctionSig, Hir, HirTypeKind};
+use crate::hir::{DuplicateSymbolKind, FunctionSig, Hir, HirTypeKind};
 use crate::lexer::{Token, lex};
 use crate::syntax::ast::{Callee, EffectDecl, Item};
 use crate::syntax::parse_source;
@@ -29,6 +29,7 @@ pub(crate) struct Analyzer<'a> {
 impl Analyzer<'_> {
     fn run(&mut self) {
         self.check_file_mode_present();
+        self.check_duplicate_declarations();
         self.check_signature_explicitness();
         self.check_resource_fields();
         checks::mode::check(self);
@@ -116,6 +117,32 @@ impl Analyzer<'_> {
         }
     }
 
+    fn check_duplicate_declarations(&mut self) {
+        for duplicate in self.hir.duplicate_symbols() {
+            self.diagnostics.push(
+                Diagnostic::error(
+                    code::DUPLICATE_DECLARATION,
+                    format!(
+                        "{} `{}` is declared more than once.",
+                        duplicate_symbol_label(duplicate.kind),
+                        duplicate.name
+                    ),
+                    duplicate.duplicate_span.clone(),
+                    "duplicate declaration",
+                )
+                .with_cause(format!(
+                    "The first declaration is at {}:{}.",
+                    duplicate.first_span.line, duplicate.first_span.column
+                ))
+                .with_fix(
+                    "rename_declaration",
+                    "Rename or remove one declaration so the symbol table is unambiguous.",
+                    "manual",
+                ),
+            );
+        }
+    }
+
     fn check_resource_fields(&mut self) {
         for item in &self.syntax_program.items {
             let Item::Type(decl) = item else {
@@ -156,3 +183,11 @@ fn effect_name(effect: &EffectDecl) -> &str {
         EffectDecl::Name(name) | EffectDecl::Retains(name) => name,
     }
 }
+
+fn duplicate_symbol_label(kind: DuplicateSymbolKind) -> &'static str {
+    match kind {
+        DuplicateSymbolKind::Function => "function",
+        DuplicateSymbolKind::Type => "type",
+        DuplicateSymbolKind::Constructor => "callable",
+    }
+}

src/diagnostic.rs

@@ -5,6 +5,7 @@ pub mod code {
     pub const MISSING_RETURN_TYPE: &str = "RS0002";
     pub const MISSING_PARAMETER_TYPE: &str = "RS0003";
     pub const UNKNOWN_EFFECT: &str = "RS0004";
+    pub const DUPLICATE_DECLARATION: &str = "RS0005";
     pub const FILE_MODE_VIOLATION: &str = "RS0101";
     pub const UNNAMED_ARGUMENT: &str = "RS0201";
     pub const MISSING_DATA_EFFECT: &str = "RS0202";
@@ -208,6 +209,11 @@ static DIAGNOSTIC_EXPLANATIONS: &[DiagnosticExplanation] = &[
         title: "unknown effect",
         explanation: "The effect list contains an effect name outside the currently recognized MVP surface.",
     },
+    DiagnosticExplanation {
+        code: code::DUPLICATE_DECLARATION,
+        title: "duplicate declaration",
+        explanation: "Top-level type, constructor, and function names must be unique so symbol resolution cannot silently overwrite an earlier declaration.",
+    },
     DiagnosticExplanation {
         code: code::FILE_MODE_VIOLATION,
         title: "file mode violation",

src/hir.rs

@@ -1,5 +1,6 @@
 use std::collections::{HashMap, HashSet};
 
+use crate::diagnostic::Span;
 use crate::syntax::ast::{
     DataEffect, EffectDecl, FieldDecl, FunctionDecl, Item, Param, Program as SyntaxProgram,
     TypeDecl, TypeKind,
@@ -61,21 +62,64 @@ pub struct TypeInfo {
     pub fields: HashMap<String, FieldInfo>,
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum DuplicateSymbolKind {
+    Function,
+    Type,
+    Constructor,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct DuplicateSymbol {
+    pub kind: DuplicateSymbolKind,
+    pub name: String,
+    pub first_span: Span,
+    pub duplicate_span: Span,
+}
+
 #[derive(Debug, Default)]
 pub struct Hir {
     signatures: HashMap<String, FunctionSig>,
     types: HashMap<String, TypeInfo>,
     fields_by_name: HashMap<String, Vec<FieldInfo>>,
+    duplicate_symbols: Vec<DuplicateSymbol>,
 }
 
 impl Hir {
     pub fn from_syntax(program: &SyntaxProgram) -> Self {
         let mut hir = Self::default();
         hir.insert_builtins();
+        let mut type_symbols = HashMap::new();
+        let mut callable_symbols = HashMap::new();
         for item in &program.items {
             match item {
-                Item::Function(function) => hir.insert_function(function_sig_from_decl(function)),
-                Item::Type(type_decl) => hir.insert_type(type_info_from_decl(type_decl)),
+                Item::Function(function) => {
+                    record_duplicate_symbol(
+                        &mut hir.duplicate_symbols,
+                        &mut callable_symbols,
+                        DuplicateSymbolKind::Function,
+                        &function.name,
+                        &function.span,
+                    );
+                    hir.insert_function(function_sig_from_decl(function));
+                }
+                Item::Type(type_decl) => {
+                    record_duplicate_symbol(
+                        &mut hir.duplicate_symbols,
+                        &mut type_symbols,
+                        DuplicateSymbolKind::Type,
+                        &type_decl.name,
+                        &type_decl.span,
+                    );
+                    record_duplicate_symbol(
+                        &mut hir.duplicate_symbols,
+                        &mut callable_symbols,
+                        DuplicateSymbolKind::Constructor,
+                        &type_decl.name,
+                        &type_decl.span,
+                    );
+                    hir.insert_type(type_info_from_decl(type_decl));
+                }
             }
         }
         hir
@@ -109,6 +153,10 @@ impl Hir {
         self.fields_named(field_name).any(|field| field.is_handle)
     }
 
+    pub fn duplicate_symbols(&self) -> &[DuplicateSymbol] {
+        &self.duplicate_symbols
+    }
+
     fn insert_function(&mut self, signature: FunctionSig) {
         let key = match &signature.namespace {
             Some(namespace) => qualified_key(namespace, &signature.name),
@@ -155,6 +203,39 @@ fn function_sig_from_decl(function: &FunctionDecl) -> FunctionSig {
     }
 }
 
+fn record_duplicate_symbol(
+    duplicates: &mut Vec<DuplicateSymbol>,
+    symbols: &mut HashMap<String, (DuplicateSymbolKind, Span)>,
+    kind: DuplicateSymbolKind,
+    name: &str,
+    span: &Span,
+) {
+    if let Some((first_kind, first_span)) = symbols.get(name) {
+        duplicates.push(DuplicateSymbol {
+            kind: duplicate_symbol_kind(*first_kind, kind),
+            name: name.to_string(),
+            first_span: first_span.clone(),
+            duplicate_span: span.clone(),
+        });
+        return;
+    }
+
+    symbols.insert(name.to_string(), (kind, span.clone()));
+}
+
+fn duplicate_symbol_kind(
+    first: DuplicateSymbolKind,
+    duplicate: DuplicateSymbolKind,
+) -> DuplicateSymbolKind {
+    match (first, duplicate) {
+        (DuplicateSymbolKind::Function, DuplicateSymbolKind::Function) => {
+            DuplicateSymbolKind::Function
+        }
+        (DuplicateSymbolKind::Type, DuplicateSymbolKind::Type) => DuplicateSymbolKind::Type,
+        _ => DuplicateSymbolKind::Constructor,
+    }
+}
+
 fn param_sig_from_decl(param: &Param) -> ParamSig {
     ParamSig {
         name: param.name.clone(),
@@ -679,4 +760,30 @@ fn cache_put(cache: mut Cache, value: read Image) -> Unit
             .expect("builtin signature exists");
         assert_eq!(load.return_type.as_deref(), Some("Image"));
     }
+
+    #[test]
+    fn records_duplicate_callable_symbols() {
+        let source = r#"
+mode: managed
+
+struct Image {
+    pixels: Buffer
+}
+
+fn Image(path: read Path) -> Image {
+}
+"#;
+
+        let program = parse_source("test.rss", source);
+        let hir = Hir::from_syntax(&program);
+        let duplicate = hir
+            .duplicate_symbols()
+            .first()
+            .expect("constructor/function duplicate is recorded");
+
+        assert_eq!(duplicate.kind, DuplicateSymbolKind::Constructor);
+        assert_eq!(duplicate.name, "Image");
+        assert_eq!(duplicate.first_span.line, 4);
+        assert_eq!(duplicate.duplicate_span.line, 8);
+    }
 }

tests/fixtures/fail/duplicate-declarations.rss

@@ -0,0 +1,14 @@
+// expect: RS0005
+mode: managed
+
+struct Image {
+    pixels: Buffer
+}
+
+fn Image(path: read Path) -> Image {
+    return Image(pixels: read path)
+}
+
+fn Image(path: read Path) -> Image {
+    return Image(pixels: read path)
+}