diff --git a/src/schema.rs b/src/schema.rs index 7d7c2c910..df078e52d 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -186,10 +186,11 @@ impl Team { // Return's whether the provided team is a subteam of this team pub(crate) fn is_parent_of<'a>(&'a self, data: &'a Data, subteam: &Team) -> bool { + let mut visited = Vec::new(); let mut subteam = Some(subteam); - while let Some(s) = subteam { + while let Some(team) = subteam { // Get subteam's parent - let Some(parent) = s.subteam_of() else { + let Some(parent) = team.subteam_of() else { // The current subteam is a top level team. // Therefore this team cannot be its parent. return false; @@ -198,7 +199,16 @@ impl Team { if parent == self.name { return true; } + + visited.push(team.name.as_str()); + // Otherwise try the test again with the parent + // unless we have already visited it. + + if visited.contains(&parent) { + // We have found a cycle, give up. + return false; + } subteam = data.team(parent); } false diff --git a/src/validate.rs b/src/validate.rs index a95d434bc..0ef8cf4bf 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -161,25 +161,33 @@ fn validate_name_prefixes(data: &Data, errors: &mut Vec) { /// Ensure `subteam-of` points to an existing team fn validate_subteam_of(data: &Data, errors: &mut Vec) { - let teams: HashMap<_, _> = data - .teams() - .map(|t| (t.name(), t.subteam_of().is_some())) - .collect(); - wrapper(data.teams(), errors, |team, _| { - if let Some(subteam_of) = team.subteam_of() { - match teams.get(subteam_of) { - Some(false) => {} - Some(true) => bail!( - "team `{}` can't be a subteam of a subteam (`{}`)", - team.name(), - subteam_of - ), - None => bail!( + wrapper(data.teams(), errors, |mut team, _| { + let mut visited = Vec::new(); + while let Some(parent) = team.subteam_of() { + visited.push(team.name()); + + if visited.contains(&parent) { + bail!("team `{}` is part of a cycle", parent); + } + + let Some(parent) = data.team(parent) else { + bail!( "the parent of team `{}` doesn't exist: `{}`", team.name(), - subteam_of - ), + parent, + ); + }; + + if !matches!(team.kind(), TeamKind::Team) && parent.subteam_of().is_some() { + bail!( + "{} `{}` can't be a subteam of a subteam (`{}`)", + team.kind(), + team.name(), + parent.name(), + ); } + + team = parent; } Ok(()) });