Validate well-formedness of constraints, better constraint errors

src/cli/stv.rs

@@ -203,25 +203,25 @@ pub fn main(cmd_opts: SubcmdOptions) -> Result<(), i32> {
 	// Read and count election according to --numbers
 	if cmd_opts.numbers == "rational" {
 		let mut election = election_from_file(&cmd_opts.filename, cmd_opts.bin)?;
-		maybe_load_constraints(&mut election, &cmd_opts.constraints);
+		maybe_load_constraints(&mut election, &cmd_opts.constraints)?;
 		
 		// Must specify ::<N> here and in a few other places because ndarray causes E0275 otherwise
 		count_election::<Rational>(election, cmd_opts)?;
 	} else if cmd_opts.numbers == "float64" {
 		let mut election = election_from_file(&cmd_opts.filename, cmd_opts.bin)?;
-		maybe_load_constraints(&mut election, &cmd_opts.constraints);
+		maybe_load_constraints(&mut election, &cmd_opts.constraints)?;
 		count_election::<NativeFloat64>(election, cmd_opts)?;
 	} else if cmd_opts.numbers == "fixed" {
 		Fixed::set_dps(cmd_opts.decimals);
 		
 		let mut election = election_from_file(&cmd_opts.filename, cmd_opts.bin)?;
-		maybe_load_constraints(&mut election, &cmd_opts.constraints);
+		maybe_load_constraints(&mut election, &cmd_opts.constraints)?;
 		count_election::<Fixed>(election, cmd_opts)?;
 	} else if cmd_opts.numbers == "gfixed" {
 		GuardedFixed::set_dps(cmd_opts.decimals);
 		
 		let mut election = election_from_file(&cmd_opts.filename, cmd_opts.bin)?;
-		maybe_load_constraints(&mut election, &cmd_opts.constraints);
+		maybe_load_constraints(&mut election, &cmd_opts.constraints)?;
 		count_election::<GuardedFixed>(election, cmd_opts)?;
 	}
 	
@@ -244,12 +244,30 @@ fn election_from_file<N: Number>(path: &str, bin: bool) -> Result<Election<N>, i
 	}
 }
 
-fn maybe_load_constraints<N: Number>(election: &mut Election<N>, constraints: &Option<String>) {
+fn maybe_load_constraints<N: Number>(election: &mut Election<N>, constraints: &Option<String>) -> Result<(), i32> {
 	if let Some(c) = constraints {
 		let file = File::open(c).expect("IO Error");
 		let lines = io::BufReader::new(file).lines();
-		election.constraints = Some(Constraints::from_con(lines.map(|r| r.expect("IO Error"))));
+		let lines: Vec<_> = lines.map(|r| r.expect("IO Error")).collect();
+		
+		match Constraints::from_con(lines.into_iter()) {
+			Ok(c) => {
+				election.constraints = Some(c);
+			}
+			Err(err) => {
+				println!("Constraint Syntax Error: {}", err);
+				return Err(1);
+			}
+		}
+		
+		// Validate constraints
+		if let Err(err) = election.constraints.as_ref().unwrap().validate_constraints(election.candidates.len()) {
+			println!("Constraint Validation Error: {}", err);
+			return Err(1);
+		}
 	}
+	
+	Ok(())
 }
 
 fn count_election<N: Number>(election: Election<N>, cmd_opts: SubcmdOptions) -> Result<(), i32>

src/constraints.rs

@@ -27,6 +27,7 @@ use rkyv::{Archive, Deserialize, Serialize};
 
 use std::collections::HashMap;
 use std::fmt;
+use std::num::ParseIntError;
 use std::ops;
 
 /// Constraints for an [crate::election::Election]
@@ -36,24 +37,28 @@ pub struct Constraints(pub Vec<Constraint>);
 
 impl Constraints {
 	/// Parse the given CON file and return a [Constraints]
-	pub fn from_con<I: Iterator<Item=String>>(lines: I) -> Self {
+	pub fn from_con<S: AsRef<str>, I: Iterator<Item=S>>(lines: I) -> Result<Self, ParseError> {
 		let mut constraints = Constraints(Vec::new());
 		
-		for line in lines {
+		for (line_no, line) in lines.enumerate() {
-			let mut bits = line.split(' ').peekable();
+			let mut bits = line.as_ref().split(' ').peekable();
 			
 			// Read constraint category and group
-			let constraint_name = read_quoted_string(&mut bits);
+			let constraint_name = read_quoted_string(line_no, &mut bits)?;
-			let group_name = read_quoted_string(&mut bits);
+			let group_name = read_quoted_string(line_no, &mut bits)?;
 			
 			// Read min, max
-			let min: usize = bits.next().expect("Syntax Error").parse().expect("Syntax Error");
+			let min: usize = bits
-			let max: usize = bits.next().expect("Syntax Error").parse().expect("Syntax Error");
+				.next().ok_or(ParseError::UnexpectedEOL(line_no, "minimum number"))?
+				.parse().map_err(|e| ParseError::InvalidNumber(line_no, e))?;
+			let max: usize = bits
+				.next().ok_or(ParseError::UnexpectedEOL(line_no, "maximum number"))?
+				.parse().map_err(|e| ParseError::InvalidNumber(line_no, e))?;
 			
 			// Read candidates
 			let mut candidates: Vec<usize> = Vec::new();
 			for x in bits {
-				candidates.push(x.parse::<usize>().expect("Syntax Error") - 1);
+				candidates.push(x.parse::<usize>().map_err(|e| ParseError::InvalidNumber(line_no, e))? - 1);
 			}
 			
 			// Insert constraint/group
@@ -70,7 +75,7 @@ impl Constraints {
 			};
 			
 			if constraint.groups.iter().any(|g| g.name == group_name) {
-				panic!("Duplicate group \"{}\" in constraint \"{}\"", group_name, constraint.name);
+				return Err(ParseError::DuplicateGroup(line_no, group_name, constraint.name.clone()));
 			}
 			
 			constraint.groups.push(ConstrainedGroup {
@@ -81,26 +86,132 @@ impl Constraints {
 			});
 		}
 		
-		// TODO: Validate constraints
+		return Ok(constraints);
+	}
+	
+	/// Validate that each candidate is specified exactly once in each constraint
+	pub fn validate_constraints(&self, num_candidates: usize) -> Result<(), ValidationError> {
+		for constraint in &self.0 {
+			let mut remaining_candidates: Vec<usize> = (0..num_candidates).collect();
+			
+			for group in &constraint.groups {
+				for candidate in &group.candidates {
+					match remaining_candidates.iter().position(|c| c == candidate) {
+						Some(idx) => {
+							remaining_candidates.remove(idx);
+						}
+						None => {
+							return Err(ValidationError::DuplicateCandidate(*candidate, constraint.name.clone()));
+						}
+					}
+				}
+			}
+			
+			if !remaining_candidates.is_empty() {
+				return Err(ValidationError::UnassignedCandidate(*remaining_candidates.first().unwrap(), constraint.name.clone()));
+			}
+		}
 		
-		return constraints;
+		Ok(())
 	}
 }
 
+/// Error parsing constraints
+pub enum ParseError {
+	/// Duplicate group in a constraint
+	DuplicateGroup(usize, String, String),
+	/// Unexpected EOL, expected ...
+	UnexpectedEOL(usize, &'static str),
+	/// Invalid number
+	InvalidNumber(usize, ParseIntError),
+}
+
+impl fmt::Display for ParseError {
+	fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+		match self {
+			ParseError::DuplicateGroup(line_no, group_name, constraint_name) => {
+				f.write_fmt(format_args!(r#"Line {}, duplicate group "{}" in constraint "{}""#, line_no, group_name, constraint_name))
+			}
+			ParseError::UnexpectedEOL(line_no, expected) => {
+				f.write_fmt(format_args!(r#"Line {}, unexpected end-of-line, expected {}"#, line_no, expected))
+			}
+			ParseError::InvalidNumber(line_no, err) => {
+				f.write_fmt(format_args!(r#"Line {}, invalid number: {}"#, line_no, err))
+			}
+		}
+	}
+}
+
+impl fmt::Debug for ParseError {
+	fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+		return fmt::Display::fmt(self, f);
+	}
+}
+
+/// Error validating constraints
+pub enum ValidationError {
+	/// Duplicate candidate in a constraint
+	DuplicateCandidate(usize, String),
+	/// Unassigned candidate in a constraint
+	UnassignedCandidate(usize, String),
+}
+
+impl fmt::Display for ValidationError {
+	fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+		match self {
+			ValidationError::DuplicateCandidate(candidate, constraint_name) => {
+				f.write_fmt(format_args!(r#"Duplicate candidate {} in constraint "{}""#, candidate + 1, constraint_name))
+			}
+			ValidationError::UnassignedCandidate(candidate, constraint_name) => {
+				f.write_fmt(format_args!(r#"Unassigned candidate {} in constraint "{}""#, candidate + 1, constraint_name))
+			}
+		}
+	}
+}
+
+impl fmt::Debug for ValidationError {
+	fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+		return fmt::Display::fmt(self, f);
+	}
+}
+
+#[test]
+fn duplicate_contraint_group() {
+	let input = r#""Constraint 1" "Group 1" 0 3 1 2 3
+"Constraint 1" "Group 1" 0 3 4 5 6"#;
+	Constraints::from_con(input.lines()).unwrap_err();
+}
+
+#[test]
+fn duplicate_candidate() {
+	let input = r#""Constraint 1" "Group 1" 0 3 1 2 3 4
+"Constraint 1" "Group 2" 0 3 4 5 6"#;
+	let constraints = Constraints::from_con(input.lines()).unwrap();
+	constraints.validate_constraints(6).unwrap_err();
+}
+
+#[test]
+fn unassigned_candidate() {
+	let input = r#""Constraint 1" "Group 1" 0 3 1 2 3
+"Constraint 1" "Group 2" 0 3 4 5 6"#;
+	let constraints = Constraints::from_con(input.lines()).unwrap();
+	constraints.validate_constraints(7).unwrap_err();
+}
+
 /// Read an optionally quoted string, returning the string without quotes
-fn read_quoted_string<'a, I: Iterator<Item=&'a str>>(bits: &mut I) -> String {
+fn read_quoted_string<'a, I: Iterator<Item=&'a str>>(line_no: usize, bits: &mut I) -> Result<String, ParseError> {
-	let x = bits.next().expect("Syntax Error");
+	let x = bits.next().ok_or(ParseError::UnexpectedEOL(line_no, "string continuation"))?;
 	if let Some(x1) = x.strip_prefix('"') {
 		if let Some(x2) = x.strip_suffix('"') {
 			// Complete string
-			return String::from(x2);
+			return Ok(String::from(x2));
 		} else {
 			// Incomplete string
 			let mut result = String::from(x1);
 			
 			// Read until matching "
 			loop {
-				let x = bits.next().expect("Syntax Error");
+				let x = bits.next().ok_or(ParseError::UnexpectedEOL(line_no, "string continuation"))?;
 				result.push(' ');
 				if let Some(x1) = x.strip_suffix('"') {
 					// End of string
@@ -112,11 +223,11 @@ fn read_quoted_string<'a, I: Iterator<Item=&'a str>>(bits: &mut I) -> String {
 				}
 			}
 			
-			return result;
+			return Ok(result);
 		}
 	} else {
 		// Unquoted string
-		return String::from(x);
+		return Ok(String::from(x));
 	}
 }
 
@@ -227,7 +338,8 @@ impl ConstraintMatrix {
 						return j + 1;
 					}
 				}
-				panic!("Candidate \"{}\" not represented in constraint \"{}\"", candidate.name, c.name);
+				// Should be caught by validate_constraints
+				unreachable!("Candidate \"{}\" not represented in constraint \"{}\"", candidate.name, c.name);
 			}).collect();
 			let cell = &mut self[&idx[..]];
 			

src/election.rs

@@ -179,6 +179,7 @@ impl<'a, N: Number> CountState<'a, N> {
 		
 		// Init constraints
 		if let Some(constraints) = &election.constraints {
+			// Init constraint matrix
 			let mut num_groups: Vec<usize> = constraints.0.iter().map(|c| c.groups.len()).collect();
 			let mut cm = ConstraintMatrix::new(&mut num_groups[..]);
 			

src/stv/wasm.rs

@@ -92,7 +92,15 @@ macro_rules! impl_type {
 		#[cfg_attr(feature = "wasm", wasm_bindgen)]
 		#[allow(non_snake_case)]
 		pub fn [<election_load_constraints_$type>](election: &mut [<Election$type>], text: String) {
-			election.0.constraints = Some(Constraints::from_con(text.lines().map(|s| s.to_string()).into_iter()));
+			election.0.constraints = match Constraints::from_con(text.lines()) {
+				Ok(c) => Some(c),
+				Err(err) => wasm_error!("Constraint Syntax Error", err),
+			};
+			
+			// Validate constraints
+			if let Err(err) = election.0.constraints.as_ref().unwrap().validate_constraints(election.0.candidates.len()) {
+				wasm_error!("Constraint Validation Error", err);
+			}
 		}
 		
 		/// Wrapper for [stv::preprocess_election]

tests/tests_impl/constraints.rs

@@ -37,7 +37,7 @@ fn prsa1_constr1_rational() {
 	let file = File::open("tests/data/prsa1_constr1.con").expect("IO Error");
 	let file_reader = io::BufReader::new(file);
 	let lines = file_reader.lines();
-	election.constraints = Some(Constraints::from_con(lines.map(|r| r.expect("IO Error").to_string()).into_iter()));
+	election.constraints = Some(Constraints::from_con(lines.map(|r| r.expect("IO Error").to_string()).into_iter()).unwrap());
 	
 	let stv_opts = stv::STVOptionsBuilder::default()
 		.round_surplus_fractions(Some(3))
@@ -86,7 +86,7 @@ fn prsa1_constr2_rational() {
 	let file = File::open("tests/data/prsa1_constr2.con").expect("IO Error");
 	let file_reader = io::BufReader::new(file);
 	let lines = file_reader.lines();
-	election.constraints = Some(Constraints::from_con(lines.map(|r| r.expect("IO Error").to_string()).into_iter()));
+	election.constraints = Some(Constraints::from_con(lines.map(|r| r.expect("IO Error").to_string()).into_iter()).unwrap());
 	
 	let stv_opts = stv::STVOptionsBuilder::default()
 		.round_surplus_fractions(Some(3))
@@ -135,7 +135,7 @@ fn prsa1_constr3_rational() {
 	let file = File::open("tests/data/prsa1_constr3.con").expect("IO Error");
 	let file_reader = io::BufReader::new(file);
 	let lines = file_reader.lines();
-	election.constraints = Some(Constraints::from_con(lines.map(|r| r.expect("IO Error").to_string()).into_iter()));
+	election.constraints = Some(Constraints::from_con(lines.map(|r| r.expect("IO Error").to_string()).into_iter()).unwrap());
 	
 	let stv_opts = stv::STVOptionsBuilder::default()
 		.round_surplus_fractions(Some(3))
@@ -196,7 +196,7 @@ fn ers97old_cantbulkexclude_rational() {
 	let file = File::open("tests/data/ers97old_cantbulkexclude.con").expect("IO Error");
 	let file_reader = io::BufReader::new(file);
 	let lines = file_reader.lines();
-	election.constraints = Some(Constraints::from_con(lines.map(|r| r.expect("IO Error").to_string()).into_iter()));
+	election.constraints = Some(Constraints::from_con(lines.map(|r| r.expect("IO Error").to_string()).into_iter()).unwrap());
 	
 	let stv_opts = stv::STVOptionsBuilder::default()
 		.round_surplus_fractions(Some(2))