Return an error when an alias matches another alias

data/error_policies/duplicate_alias.cas

@@ -0,0 +1,7 @@
+@alias(some_alias)
+domain foo {
+	allow(this, resource, file, read);
+}
+
+@alias(some_alias)
+domain bar {}

src/alias_map.rs

@@ -12,7 +12,6 @@ use crate::util::append_set_map;
 #[derive(Clone, Debug)]
 pub struct AliasMap<T> {
     declarations: BTreeMap<String, T>,
-    #[allow(dead_code)]
     aliases: BTreeMap<CascadeString, String>,
     // Secondary_indices allow efficient lookups based on some other key.  This is useful to
     // efficiently get a subset of the map based on a prepopulated key, such as all functions for a
@@ -156,8 +155,6 @@ impl<T: Declared> AliasMap<T> {
         errors.into_result(())
     }
 
-    // The need for alias_files is a little awkward. Without it, we can't report errors on the
-    // alias declarations.  Post-0.1 we'll include file info in CascadeStrings and this can go away
     pub fn set_aliases(&mut self, aliases: BTreeMap<CascadeString, String>) {
         self.update_alias_secondary_indices(&aliases);
         self.aliases = aliases;

src/compile.rs

@@ -1259,7 +1259,7 @@ pub fn get_reduced_infos(
     mark_non_virtual_children(&mut new_type_map);
 
     // Generate type aliases for the new reduced type map
-    let (new_t_aliases, alias_files) = collect_aliases(new_type_map.iter());
+    let (new_t_aliases, alias_files) = collect_aliases(new_type_map.iter())?;
     new_type_map.validate_aliases(&new_t_aliases, &alias_files)?;
     new_type_map.set_aliases(new_t_aliases);
 
@@ -1403,7 +1403,7 @@ pub fn get_funcs<'a>(
     // Stops if something went wrong for this major step.
     ret = ret.into_result_self()?;
     // Get function aliases
-    let (f_aliases, alias_files) = collect_aliases(reduced_func_map.iter());
+    let (f_aliases, alias_files) = collect_aliases(reduced_func_map.iter())?;
     reduced_func_map.validate_aliases(&f_aliases, &alias_files)?;
     reduced_func_map.set_aliases(f_aliases);
     ret.into_result(reduced_func_map)
@@ -2084,31 +2084,58 @@ fn organize_type_map(types: &TypeMap) -> Result<Vec<&TypeInfo>, CascadeErrors> {
 // in the maps
 // We gather files, because we aren't storing file info in CascadeStrings yet.  That can go away
 // once we store file info in CascadeStrings
+// Silence the clippy warning about the return type.  It's not *that* complex, and it will get
+// simpler naturally once we do the above
+#[allow(clippy::type_complexity)]
 pub fn collect_aliases<'a, I, T>(
     aliasable_map: I,
-) -> (
-    BTreeMap<CascadeString, String>,
-    BTreeMap<CascadeString, SimpleFile<String, String>>,
-)
+) -> Result<
+    (
+        BTreeMap<CascadeString, String>,
+        BTreeMap<CascadeString, SimpleFile<String, String>>,
+    ),
+    CascadeErrors,
+>
 where
     I: Iterator<Item = (&'a String, &'a T)>,
     T: Declared + 'a,
     &'a T: Annotated,
 {
     let mut aliases = BTreeMap::new();
     let mut alias_files = BTreeMap::new();
+    let mut errors = CascadeErrors::new();
     for (k, v) in aliasable_map {
         for a in v.get_annotations() {
             if let AnnotationInfo::Alias(a) = a {
-                aliases.insert(a.clone(), k.clone());
+                if aliases.insert(a.clone(), k.clone()).is_some() {
+                    errors.append(
+                        ErrorItem::make_compile_or_internal_error(
+                            "Alias name conflicts with an existing alias",
+                            v.get_file().as_ref(),
+                            a.get_range(),
+                            "",
+                        )
+                        .maybe_add_additional_message(
+                            alias_files.get(a),
+                            // This is the range of the *existing* key.  Insert updates the
+                            // value, but not the key when we overwrite.  Since our PartialEq
+                            // isn't identical (it just compares the strings, not the ranges),
+                            // we still have the old range in the key
+                            // https://doc.rust-lang.org/std/collections/struct.BTreeMap.html#method.insert
+                            aliases.get_key_value(a).and_then(|(a, _)| a.get_range()),
+                            "Existing alias found here",
+                        )
+                        .into(),
+                    );
+                }
                 if let Some(file) = v.get_file().clone() {
                     alias_files.insert(a.clone(), file);
                 }
             }
         }
     }
 
-    (aliases, alias_files)
+    errors.into_result((aliases, alias_files))
 }
 
 pub fn call_associated_calls<'a>(

src/lib.rs

@@ -146,7 +146,7 @@ fn compile_machine_policies_internal(
     }
 
     // Generate type aliases
-    let (t_aliases, alias_files) = compile::collect_aliases(type_map.iter());
+    let (t_aliases, alias_files) = compile::collect_aliases(type_map.iter())?;
     type_map.validate_aliases(&t_aliases, &alias_files)?;
     type_map.set_aliases(t_aliases);
 
@@ -233,7 +233,7 @@ fn compile_machine_policies_internal(
     compile::validate_modules(&policies, &type_map, &mut module_map)?;
 
     // Generate module aliases
-    let (m_aliases, alias_files) = compile::collect_aliases(module_map.iter());
+    let (m_aliases, alias_files) = compile::collect_aliases(module_map.iter())?;
     module_map.validate_aliases(&m_aliases, &alias_files)?;
     module_map.set_aliases(m_aliases);
 
@@ -1672,6 +1672,11 @@ mod tests {
         error_policy_test!("derive_noderive.cas", 1, ErrorItem::Compile(_));
     }
 
+    #[test]
+    fn duplicate_alias_test() {
+        error_policy_test!("duplicate_alias.cas", 1, ErrorItem::Compile(_));
+    }
+
     #[test]
     fn valid_nested_alias() {
         valid_policy_test(