diff --git a/src/error.rs b/src/error.rs index a41517d..a195a0e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -37,6 +37,20 @@ pub enum ReportalError { alias: String, }, + /// A repository alias collides with another repository's + /// canonical key or declared alias. + #[error( + "Repo alias conflict: '{conflicting_value}' is declared by repo '{repository_name}' but also by {conflicting_entity_description}" + )] + RepoAliasConflict { + /// The repository declaring the conflicting alias. + repository_name: String, + /// The alias or canonical key claimed more than once. + conflicting_value: String, + /// Human-readable description of the other repository claim. + conflicting_entity_description: String, + }, + /// The editor process failed to launch. #[error("Failed to open editor: {reason}")] EditorLaunchFailure { diff --git a/src/reportal_config/reportal_config_root.rs b/src/reportal_config/reportal_config_root.rs index b506121..2d2140e 100644 --- a/src/reportal_config/reportal_config_root.rs +++ b/src/reportal_config/reportal_config_root.rs @@ -79,6 +79,7 @@ impl ReportalConfig { /// default config cannot be written, `ConfigParseFailure` if /// the TOML is malformed, `WorkspaceHasDanglingRepo` if a /// workspace references an unknown repo alias, or + /// `RepoAliasConflict` if repository aliases are ambiguous, or /// `WorkspaceAliasConflict` if any workspace's name or alias /// collides with another entry. pub fn load_or_initialize() -> Result { @@ -103,8 +104,8 @@ impl ReportalConfig { Ok(parsed_config) } - /// Validates that every workspace alias is globally unambiguous - /// across both the workspace and repo namespaces. + /// Validates that repository and workspace aliases are globally + /// unambiguous across their supported namespaces. /// /// The resolver in `resolve_canonical_key` returns the first /// match it finds, so a config where two entries declare the @@ -115,9 +116,11 @@ impl ReportalConfig { /// /// # Errors /// - /// Returns [`ReportalError::WorkspaceAliasConflict`] at the - /// first collision encountered. + /// Returns [`ReportalError::RepoAliasConflict`] or + /// [`ReportalError::WorkspaceAliasConflict`] at the first + /// collision encountered. pub fn validate_alias_collisions(&self) -> Result<(), ReportalError> { + self.validate_repo_alias_collisions()?; for (workspace_name, workspace_entry) in &self.workspaces { self.check_workspace_canonical_name_repo_collision(workspace_name)?; for declared_alias in workspace_entry.aliases() { @@ -130,6 +133,58 @@ impl ReportalConfig { Ok(()) } + /// Rejects repository aliases that are repeated by another repo + /// or collide with a peer repository's canonical key. A repo may + /// list its own canonical key as a redundant alias because that + /// does not make lookup ambiguous; every other collision is + /// rejected before the resolver can depend on map ordering. + /// + /// # Errors + /// + /// Returns [`ReportalError::RepoAliasConflict`] when a declared + /// repository alias has more than one owner. + fn validate_repo_alias_collisions(&self) -> Result<(), ReportalError> { + let mut declared_alias_owners: BTreeMap = BTreeMap::new(); + for (repository_name, repository_entry) in &self.repos { + for declared_alias in repository_entry.aliases() { + self.validate_repository_alias_ownership( + repository_name, + declared_alias, + &mut declared_alias_owners, + )?; + } + } + Ok(()) + } + + /// Checks one repository alias against peer canonical keys and + /// aliases, keeping the outer validation loop shallow. + fn validate_repository_alias_ownership( + &self, + repository_name: &str, + declared_alias: &str, + declared_alias_owners: &mut BTreeMap, + ) -> Result<(), ReportalError> { + if self.repos.contains_key(declared_alias) && declared_alias != repository_name { + return Err(ReportalError::RepoAliasConflict { + repository_name: repository_name.to_owned(), + conflicting_value: declared_alias.to_owned(), + conflicting_entity_description: format!( + "repo '{declared_alias}' as its canonical key" + ), + }); + } + if let Some(previous_owner) = declared_alias_owners.get(declared_alias) { + return Err(ReportalError::RepoAliasConflict { + repository_name: repository_name.to_owned(), + conflicting_value: declared_alias.to_owned(), + conflicting_entity_description: format!("repo '{previous_owner}' as an alias"), + }); + } + declared_alias_owners.insert(declared_alias.to_owned(), repository_name.to_owned()); + Ok(()) + } + /// Rejects a workspace whose canonical name collides with a /// repo's canonical key or declared alias. /// @@ -777,6 +832,50 @@ mod tests { Ok(()) } + #[test] + fn duplicate_repository_aliases_are_rejected() { + let mut config = ReportalConfig::create_default(); + config + .repos + .insert("alpha".to_owned(), make_repo(vec!["shared".to_owned()])); + config + .repos + .insert("bravo".to_owned(), make_repo(vec!["shared".to_owned()])); + + let outcome = config.validate_alias_collisions(); + assert!( + outcome.is_err(), + "two repositories declaring the same alias must be rejected, got {outcome:?}", + ); + } + + #[test] + fn repository_alias_matching_peer_canonical_key_is_rejected() { + let mut config = ReportalConfig::create_default(); + config + .repos + .insert("alpha".to_owned(), make_repo(vec!["bravo".to_owned()])); + config.repos.insert("bravo".to_owned(), make_repo(vec![])); + + let outcome = config.validate_alias_collisions(); + assert!( + outcome.is_err(), + "a repository alias matching a peer canonical key must be rejected, got {outcome:?}", + ); + } + + #[test] + fn unique_repository_alias_still_resolves() -> Result<(), ReportalError> { + let mut config = ReportalConfig::create_default(); + config + .repos + .insert("alpha".to_owned(), make_repo(vec!["a".to_owned()])); + + config.validate_alias_collisions()?; + assert_eq!(config.get_repo("a")?.raw_path(), "C:/fake"); + Ok(()) + } + fn make_inline_only_workspace() -> WorkspaceEntry { WorkspaceEntry { repos: vec![WorkspaceMember::InlinePath {