From 15614a4e8f9b7915470a876d338868fd0dc8ca0b Mon Sep 17 00:00:00 2001 From: RunasSudo Date: Wed, 27 Oct 2021 19:52:51 +1100 Subject: [PATCH] rust-clippy linting --- src/cli/stv.rs | 6 +- src/constraints.rs | 143 ++++++++++++++-------------- src/election.rs | 4 +- src/lib.rs | 1 + src/logger.rs | 13 +-- src/main.rs | 7 +- src/numbers/mod.rs | 2 +- src/parser/bin.rs | 2 +- src/parser/blt.rs | 14 ++- src/parser/csp.rs | 10 +- src/sharandom.rs | 2 +- src/stv/gregory/mod.rs | 13 +-- src/stv/gregory/prettytable_html.rs | 14 +-- src/stv/gregory/transfers.rs | 27 +++--- src/stv/meek.rs | 14 +-- src/stv/mod.rs | 60 +++++------- src/stv/sample.rs | 26 ++--- src/stv/wasm.rs | 15 ++- src/ties.rs | 22 ++--- src/writer/bin.rs | 2 +- src/writer/blt.rs | 8 +- 21 files changed, 192 insertions(+), 213 deletions(-) diff --git a/src/cli/stv.rs b/src/cli/stv.rs index 4f5ea7d..27614c3 100644 --- a/src/cli/stv.rs +++ b/src/cli/stv.rs @@ -248,7 +248,7 @@ fn maybe_load_constraints(election: &mut Election, constraints: &O 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").to_string()).into_iter())); + election.constraints = Some(Constraints::from_con(lines.map(|r| r.expect("IO Error")))); } } @@ -267,7 +267,7 @@ where cmd_opts.round_votes, cmd_opts.round_quota, cmd_opts.sum_surplus_transfers.into(), - cmd_opts.meek_surplus_tolerance.into(), + cmd_opts.meek_surplus_tolerance, cmd_opts.normalise_ballots, cmd_opts.quota.into(), cmd_opts.quota_criterion.into(), @@ -324,7 +324,7 @@ where let total_ballots = election.ballots.iter().fold(N::zero(), |acc, b| { acc + &b.orig_value }); print!("Count computed by OpenTally (revision {}). Read {:.0} ballots from \"{}\" for election \"{}\". There are {} candidates for {} vacancies. ", crate::VERSION, total_ballots, filename, election.name, election.candidates.len(), election.seats); let opts_str = opts.describe::(); - if opts_str.len() > 0 { + if !opts_str.is_empty() { println!("Counting using options \"{}\".", opts_str); } else { println!("Counting using default options."); diff --git a/src/constraints.rs b/src/constraints.rs index edf9080..e1219e8 100644 --- a/src/constraints.rs +++ b/src/constraints.rs @@ -40,47 +40,11 @@ impl Constraints { let mut constraints = Constraints(Vec::new()); for line in lines { - let mut bits = line.split(" ").peekable(); + let mut bits = line.split(' ').peekable(); - // Read constraint category - let mut constraint_name = String::new(); - let x = bits.next().expect("Syntax Error"); - if x.starts_with('"') { - if x.ends_with('"') { - constraint_name.push_str(&x[1..x.len()-1]); - } else { - constraint_name.push_str(&x[1..]); - while !bits.peek().expect("Syntax Error").ends_with('"') { - constraint_name.push_str(" "); - constraint_name.push_str(bits.next().unwrap()); - } - let x = bits.next().unwrap(); - constraint_name.push_str(" "); - constraint_name.push_str(&x[..x.len()-1]); - } - } else { - constraint_name.push_str(x); - } - - // Read constraint group - let mut group_name = String::new(); - let x = bits.next().expect("Syntax Error"); - if x.starts_with('"') { - if x.ends_with('"') { - group_name.push_str(&x[1..x.len()-1]); - } else { - group_name.push_str(&x[1..]); - while !bits.peek().expect("Syntax Error").ends_with('"') { - group_name.push_str(" "); - group_name.push_str(bits.next().unwrap()); - } - let x = bits.next().unwrap(); - group_name.push_str(" "); - group_name.push_str(&x[..x.len()-1]); - } - } else { - group_name.push_str(x); - } + // Read constraint category and group + let constraint_name = read_quoted_string(&mut bits); + let group_name = read_quoted_string(&mut bits); // Read min, max let min: usize = bits.next().expect("Syntax Error").parse().expect("Syntax Error"); @@ -93,7 +57,7 @@ impl Constraints { } // Insert constraint/group - let constraint = match constraints.0.iter_mut().filter(|c| c.name == constraint_name).next() { + let constraint = match constraints.0.iter_mut().find(|c| c.name == constraint_name) { Some(c) => { c } None => { let c = Constraint { @@ -111,9 +75,9 @@ impl Constraints { constraint.groups.push(ConstrainedGroup { name: group_name, - candidates: candidates, - min: min, - max: max, + candidates, + min, + max, }); } @@ -123,6 +87,39 @@ impl Constraints { } } +/// Read an optionally quoted string, returning the string without quotes +fn read_quoted_string<'a, I: Iterator>(bits: &mut I) -> String { + let x = bits.next().expect("Syntax Error"); + if let Some(x1) = x.strip_prefix('"') { + if let Some(x2) = x.strip_suffix('"') { + // Complete string + return String::from(x2); + } else { + // Incomplete string + let mut result = String::from(x1); + + // Read until matching " + loop { + let x = bits.next().expect("Syntax Error"); + result.push(' '); + if let Some(x1) = x.strip_suffix('"') { + // End of string + result.push_str(x1); + break; + } else { + // Middle of string + result.push_str(x); + } + } + + return result; + } + } else { + // Unquoted string + return String::from(x); + } +} + /// A single dimension of constraint #[derive(Clone, Debug)] #[cfg_attr(not(target_arch = "wasm32"), derive(Archive, Deserialize, Serialize))] @@ -264,7 +261,7 @@ impl ConstraintMatrix { self.0[&idx].elected = 0; // The axis along which to sum - if multiple, just pick the first, as these should agree - let zero_axis = (0..idx.ndim()).filter(|d| idx[*d] == 0).next().unwrap(); + let zero_axis = (0..idx.ndim()).find(|d| idx[*d] == 0).unwrap(); // Traverse along the axis and sum the candidates let mut idx2 = idx.clone(); @@ -374,87 +371,87 @@ impl fmt::Display for ConstraintMatrix { // TODO: >2 dimensions if shape.len() == 1 { - result.push_str("+"); + result.push('+'); for _ in 0..shape[0] { result.push_str("-------------+"); } - result.push_str("\n"); + result.push('\n'); - result.push_str("|"); + result.push('|'); for x in 0..shape[0] { result.push_str(&format!(" Elected: {:2}", self[&[x]].elected)); result.push_str(if x == 0 { " ‖" } else { " |" }); } - result.push_str("\n"); + result.push('\n'); - result.push_str("|"); + result.push('|'); for x in 0..shape[0] { result.push_str(&format!(" Min: {:2}", self[&[x]].min)); result.push_str(if x == 0 { " ‖" } else { " |" }); } - result.push_str("\n"); + result.push('\n'); - result.push_str("|"); + result.push('|'); for x in 0..shape[0] { result.push_str(&format!(" Max: {:2}", self[&[x]].max)); result.push_str(if x == 0 { " ‖" } else { " |" }); } - result.push_str("\n"); + result.push('\n'); - result.push_str("|"); + result.push('|'); for x in 0..shape[0] { result.push_str(&format!(" Cands: {:2}", self[&[x]].cands)); result.push_str(if x == 0 { " ‖" } else { " |" }); } - result.push_str("\n"); + result.push('\n'); - result.push_str("+"); + result.push('+'); for _ in 0..shape[0] { result.push_str("-------------+"); } - result.push_str("\n"); + result.push('\n'); } else if shape.len() == 2 { for y in 0..shape[1] { - result.push_str("+"); + result.push('+'); for _ in 0..shape[0] { result.push_str(if y == 1 { "=============+" } else { "-------------+" }); } - result.push_str("\n"); + result.push('\n'); - result.push_str("|"); + result.push('|'); for x in 0..shape[0] { result.push_str(&format!(" Elected: {:2}", self[&[x, y]].elected)); result.push_str(if x == 0 { " ‖" } else { " |" }); } - result.push_str("\n"); + result.push('\n'); - result.push_str("|"); + result.push('|'); for x in 0..shape[0] { result.push_str(&format!(" Min: {:2}", self[&[x, y]].min)); result.push_str(if x == 0 { " ‖" } else { " |" }); } - result.push_str("\n"); + result.push('\n'); - result.push_str("|"); + result.push('|'); for x in 0..shape[0] { result.push_str(&format!(" Max: {:2}", self[&[x, y]].max)); result.push_str(if x == 0 { " ‖" } else { " |" }); } - result.push_str("\n"); + result.push('\n'); - result.push_str("|"); + result.push('|'); for x in 0..shape[0] { result.push_str(&format!(" Cands: {:2}", self[&[x, y]].cands)); result.push_str(if x == 0 { " ‖" } else { " |" }); } - result.push_str("\n"); + result.push('\n'); } - result.push_str("+"); + result.push('+'); for _ in 0..shape[0] { result.push_str("-------------+"); } - result.push_str("\n"); + result.push('\n'); } else { todo!(); } @@ -499,7 +496,7 @@ fn candidates_in_constraint_cell<'a, N: Number>(election: &'a Election, candi } /// Clone and update the constraints matrix, with the state of the given candidates set to candidate_state -pub fn try_constraints(state: &CountState, candidates: &Vec<&Candidate>, candidate_state: CandidateState) -> Result<(), ConstraintError> { +pub fn try_constraints(state: &CountState, candidates: &[&Candidate], candidate_state: CandidateState) -> Result<(), ConstraintError> { if state.constraint_matrix.is_none() { return Ok(()); } @@ -507,11 +504,11 @@ pub fn try_constraints(state: &CountState, candidates: &Vec<&Candi let mut trial_candidates = state.candidates.clone(); // TODO: Can probably be optimised by not cloning CountCard::parcels for candidate in candidates { - trial_candidates.get_mut(candidate).unwrap().state = candidate_state.clone(); + trial_candidates.get_mut(candidate).unwrap().state = candidate_state; } // Update cands/elected - cm.update_from_state(&state.election, &trial_candidates); + cm.update_from_state(state.election, &trial_candidates); cm.recount_cands(); // Iterate for stable state @@ -528,7 +525,7 @@ pub fn update_constraints(state: &mut CountState, opts: &STVOption let cm = state.constraint_matrix.as_mut().unwrap(); // Update cands/elected - cm.update_from_state(&state.election, &state.candidates); + cm.update_from_state(state.election, &state.candidates); cm.recount_cands(); // Iterate for stable state diff --git a/src/election.rs b/src/election.rs index 53badf0..73cc170 100644 --- a/src/election.rs +++ b/src/election.rs @@ -149,7 +149,7 @@ impl<'a, N: Number> CountState<'a, N> { /// Construct a new blank [CountState] for the given [Election] pub fn new(election: &'a Election) -> Self { let mut state = CountState { - election: &election, + election, candidates: HashMap::new(), exhausted: CountCard::new(), loss_fraction: CountCard::new(), @@ -194,7 +194,7 @@ impl<'a, N: Number> CountState<'a, N> { } // Fill in grand total, etc. - cm.update_from_state(&state.election, &state.candidates); + cm.update_from_state(state.election, &state.candidates); cm.init(); //println!("{}", cm); diff --git a/src/lib.rs b/src/lib.rs index 91a21ac..405c7b5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,6 +16,7 @@ */ #![warn(missing_docs)] +#![allow(clippy::collapsible_else_if, clippy::collapsible_if, clippy::comparison_chain, clippy::derive_ord_xor_partial_ord, clippy::needless_bool, clippy::needless_return, clippy::new_without_default, clippy::too_many_arguments)] //! Open source counting software for various preferential voting election systems diff --git a/src/logger.rs b/src/logger.rs index 4af14e4..07a19cb 100644 --- a/src/logger.rs +++ b/src/logger.rs @@ -27,10 +27,10 @@ impl<'a> Logger<'a> { /// If consecutive smart log entries have the same templates, they will be merged pub fn log(&mut self, entry: LogEntry<'a>) { if let LogEntry::Smart(mut smart) = entry { - if self.entries.len() > 0 { + if !self.entries.is_empty() { if let LogEntry::Smart(last_smart) = self.entries.last_mut().unwrap() { if last_smart.template1 == smart.template1 && last_smart.template2 == smart.template2 { - &last_smart.data.append(&mut smart.data); + last_smart.data.append(&mut smart.data); } else { self.entries.push(LogEntry::Smart(smart)); } @@ -55,9 +55,9 @@ impl<'a> Logger<'a> { /// If consecutive smart log entries have the same templates, they will be merged pub fn log_smart(&mut self, template1: &'a str, template2: &'a str, data: Vec<&'a str>) { self.log(LogEntry::Smart(SmartLogEntry { - template1: template1, - template2: template2, - data: data, + template1, + template2, + data, })); } @@ -88,7 +88,7 @@ pub struct SmartLogEntry<'a> { impl<'a> SmartLogEntry<'a> { /// Render the [SmartLogEntry] to a [String] pub fn render(&self) -> String { - if self.data.len() == 0 { + if self.data.is_empty() { panic!("Attempted to format smart log entry with no data"); } else if self.data.len() == 1 { return String::from(self.template1).replace("{}", self.data.first().unwrap()); @@ -99,6 +99,7 @@ impl<'a> SmartLogEntry<'a> { } /// Join the given strings, with commas and terminal "and" +#[allow(clippy::ptr_arg)] pub fn smart_join(data: &Vec<&str>) -> String { return format!("{} and {}", data[0..data.len()-1].join(", "), data.last().unwrap()); } diff --git a/src/main.rs b/src/main.rs index 9408571..3be616e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -15,6 +15,8 @@ * along with this program. If not, see . */ +#![allow(clippy::needless_return)] + use opentally::cli; use clap::Clap; @@ -27,10 +29,11 @@ struct Opts { command: Command, } +#[allow(clippy::large_enum_variant)] #[derive(Clap)] enum Command { Convert(cli::convert::SubcmdOptions), - STV(cli::stv::SubcmdOptions), + Stv(cli::stv::SubcmdOptions), } fn main() { @@ -48,6 +51,6 @@ fn main_() -> Result<(), i32> { return match opts.command { Command::Convert(cmd_opts) => cli::convert::main(cmd_opts), - Command::STV(cmd_opts) => cli::stv::main(cmd_opts), + Command::Stv(cmd_opts) => cli::stv::main(cmd_opts), }; } diff --git a/src/numbers/mod.rs b/src/numbers/mod.rs index acd2d68..f34f9c6 100644 --- a/src/numbers/mod.rs +++ b/src/numbers/mod.rs @@ -149,7 +149,7 @@ impl DeserializeWith, N, D> fo impl DeserializeWith, Option, D> for SerializedOptionNumber where Archived: Deserialize { fn deserialize_with(field: &Archived, deserializer: &mut D) -> Result, D::Error> { let s = field.deserialize(deserializer)?; - if s.len() == 0 { + if s.is_empty() { return Ok(None); } else { return Ok(Some(N::parse(&s))); diff --git a/src/parser/bin.rs b/src/parser/bin.rs index 15f3be5..7d205aa 100644 --- a/src/parser/bin.rs +++ b/src/parser/bin.rs @@ -32,7 +32,7 @@ pub fn parse_path, N: Number>(path: P) -> Election { /// Parse the given BIN file pub fn parse_bytes(content: &[u8]) -> Election { let archived = unsafe { - archived_root::>(&content) + archived_root::>(content) }; return archived.deserialize(&mut Infallible).unwrap(); } diff --git a/src/parser/blt.rs b/src/parser/blt.rs index 8cb1308..ef74245 100644 --- a/src/parser/blt.rs +++ b/src/parser/blt.rs @@ -166,7 +166,7 @@ impl> BLTParser { let ballot = Ballot { orig_value: N::parse(&self.ballot_value_buf), - preferences: preferences, + preferences, }; self.election.ballots.push(ballot); @@ -218,13 +218,11 @@ impl> BLTParser { /// Parse a quoted or raw string fn string(&mut self) -> Result { - match self.quoted_string() { - Ok(s) => { return Ok(s); } - Err(_) => {} + if let Ok(s) = self.quoted_string() { + return Ok(s); } - match self.raw_string() { - Ok(s) => { return Ok(s); } - Err(_) => {} + if let Ok(s) = self.raw_string() { + return Ok(s); } return Err(ParseError::Expected(self.line_no, self.col_no, self.lookahead(), "string")); } @@ -396,5 +394,5 @@ pub fn parse_iterator, N: Number>(input: Peekable) -> pub fn parse_path, N: Number>(path: P) -> Result, ParseError> { let mut reader = BufReader::new(File::open(path).expect("IO Error")); let chars = reader.chars().map(|r| r.expect("IO Error")).peekable(); - return Ok(parse_iterator(chars)?); + return parse_iterator(chars); } diff --git a/src/parser/csp.rs b/src/parser/csp.rs index 4451c21..3d6136b 100644 --- a/src/parser/csp.rs +++ b/src/parser/csp.rs @@ -61,7 +61,7 @@ pub fn parse_reader(reader: R, require_1: bool, require_sequ match col_map.get(&csv_col) { Some(cand_index) => { // Preference - if preference.len() == 0 || preference == "-" { + if preference.is_empty() || preference == "-" { continue; } @@ -85,10 +85,10 @@ pub fn parse_reader(reader: R, require_1: bool, require_sequ // Sort by ranking let mut unique_rankings: Vec = preferences.iter().map(|(r, _)| *r).unique().collect(); - unique_rankings.sort(); + unique_rankings.sort_unstable(); if require_1 { - if unique_rankings.first().map(|r| *r == 1).unwrap_or(false) == false { + if !unique_rankings.first().map(|r| *r == 1).unwrap_or(false) { // No #1 preference ballots.push(Ballot { orig_value: value, @@ -135,9 +135,9 @@ pub fn parse_reader(reader: R, require_1: bool, require_sequ return Ok(Election { name: String::new(), seats: 0, - candidates: candidates, + candidates, withdrawn_candidates: Vec::new(), - ballots: ballots, + ballots, total_votes: None, constraints: None, }); diff --git a/src/sharandom.rs b/src/sharandom.rs index 9af16e6..48dffe0 100644 --- a/src/sharandom.rs +++ b/src/sharandom.rs @@ -28,7 +28,7 @@ impl<'r> SHARandom<'r> { /// Return a new [SHARandom] with the given seed pub fn new(seed: &'r str) -> Self { Self { - seed: seed, + seed, counter: 0, } } diff --git a/src/stv/gregory/mod.rs b/src/stv/gregory/mod.rs index 77eb782..517b0ad 100644 --- a/src/stv/gregory/mod.rs +++ b/src/stv/gregory/mod.rs @@ -157,8 +157,8 @@ where } match opts.surplus { - SurplusMethod::WIG | SurplusMethod::UIG | SurplusMethod::EG => { distribute_surplus(state, &opts, elected_candidate); } - SurplusMethod::IHare | SurplusMethod::Hare => { sample::distribute_surplus(state, &opts, elected_candidate)?; } + SurplusMethod::WIG | SurplusMethod::UIG | SurplusMethod::EG => { distribute_surplus(state, opts, elected_candidate); } + SurplusMethod::IHare | SurplusMethod::Hare => { sample::distribute_surplus(state, opts, elected_candidate)?; } _ => unreachable!() } @@ -209,7 +209,7 @@ where for<'r> &'r N: ops::Div<&'r N, Output=N>, for<'r> &'r N: ops::Neg { - state.title = StageKind::SurplusOf(&elected_candidate); + state.title = StageKind::SurplusOf(elected_candidate); state.logger.log_literal(format!("Surplus of {} distributed.", elected_candidate.name)); let count_card = &state.candidates[elected_candidate]; @@ -356,7 +356,7 @@ where // Transfer exhausted votes let parcel = Parcel { votes: result.exhausted.votes, - value_fraction: value_fraction, // TODO: Reweight exhausted votes + value_fraction, // TODO: Reweight exhausted votes source_order: state.num_elected + state.num_excluded, }; state.exhausted.parcels.push(parcel); @@ -382,6 +382,7 @@ where } /// Perform one stage of a candidate exclusion according to the Gregory method, based on [STVOptions::exclusion] +#[allow(clippy::branches_sharing_code)] pub fn exclude_candidates<'a, N: Number>(state: &mut CountState<'a, N>, opts: &STVOptions, excluded_candidates: Vec<&'a Candidate>) where for<'r> &'r N: ops::Mul<&'r N, Output=N>, @@ -479,7 +480,7 @@ where // Group all votes of one value in single parcel parcels.push(Parcel { - votes: votes, + votes, value_fraction: max_value, source_order: 0, // source_order is unused in this mode }); @@ -564,7 +565,7 @@ where let mut total_ballots = N::new(); let mut total_votes = N::new(); - let value = match parcels.first() { Some(p) => Some(p.value_fraction.clone()), _ => None }; + let value = parcels.first().map(|p| p.value_fraction.clone()); let mut transfer_table = TransferTable::new_exclusion( state.election.candidates.iter().filter(|c| state.candidates[c].state == CandidateState::Hopeful || state.candidates[c].state == CandidateState::Guarded).collect(), diff --git a/src/stv/gregory/prettytable_html.rs b/src/stv/gregory/prettytable_html.rs index 14271e4..9cfcf46 100644 --- a/src/stv/gregory/prettytable_html.rs +++ b/src/stv/gregory/prettytable_html.rs @@ -42,8 +42,8 @@ impl Table { } /// Render the table as HTML - pub fn to_string(&self) -> String { - return format!(r#"{}
"#, self.rows.iter().map(|r| r.to_string()).join("")); + pub fn to_html(&self) -> String { + return format!(r#"{}
"#, self.rows.iter().map(|r| r.to_html()).join("")); } } @@ -62,8 +62,8 @@ impl Row { } /// Render the row as HTML - fn to_string(&self) -> String { - return format!(r#"{}"#, self.cells.iter().map(|c| c.to_string()).join("")); + fn to_html(&self) -> String { + return format!(r#"{}"#, self.cells.iter().map(|c| c.to_html()).join("")); } } @@ -90,10 +90,10 @@ impl Cell { if spec.contains("H2") { self.attrs.push(r#"colspan="2""#); } - if spec.contains("c") { + if spec.contains('c') { self.attrs.push(r#"style="text-align:center""#); } - if spec.contains("r") { + if spec.contains('r') { self.attrs.push(r#"style="text-align:right""#); } @@ -101,7 +101,7 @@ impl Cell { } /// Render the cell as HTML - fn to_string(&self) -> String { + fn to_html(&self) -> String { return format!(r#"<{}>{}"#, self.attrs.join(" "), html_escape::encode_text(&self.content)); } } diff --git a/src/stv/gregory/transfers.rs b/src/stv/gregory/transfers.rs index 1da8653..88338af 100644 --- a/src/stv/gregory/transfers.rs +++ b/src/stv/gregory/transfers.rs @@ -136,13 +136,6 @@ impl<'e, N: Number> TransferTable<'e, N> { if let Some(n) = &self.surpfrac_numer { new_value_fraction *= n; } - if let Some(n) = &self.surpfrac_denom { - new_value_fraction /= n; - } - // Round if required - if let Some(dps) = opts.round_values { - new_value_fraction.floor_mut(dps); - } } else { if let Some(n) = &self.surpfrac_numer { new_value_fraction = n.clone(); @@ -150,13 +143,15 @@ impl<'e, N: Number> TransferTable<'e, N> { // Transferred at original value new_value_fraction = column.value_fraction.clone(); } - if let Some(n) = &self.surpfrac_denom { - new_value_fraction /= n; - } - // Round if required - if let Some(dps) = opts.round_values { - new_value_fraction.floor_mut(dps); - } + } + + if let Some(n) = &self.surpfrac_denom { + new_value_fraction /= n; + } + + // Round if required + if let Some(dps) = opts.round_values { + new_value_fraction.floor_mut(dps); } } @@ -300,7 +295,7 @@ impl<'e, N: Number> TransferTable<'e, N> { let mut table = Table::new(); set_table_format(&mut table); - let show_transfers_per_ballot = !self.surpfrac.is_none() && opts.sum_surplus_transfers == SumSurplusTransfersMode::PerBallot; + let show_transfers_per_ballot = self.surpfrac.is_some() && opts.sum_surplus_transfers == SumSurplusTransfersMode::PerBallot; let num_cols; if show_transfers_per_ballot { @@ -462,7 +457,7 @@ impl<'e, N: Number> TransferTable<'e, N> { /// Render table as plain text //#[cfg(not(target_arch = "wasm32"))] pub fn render_text(&self, opts: &STVOptions) -> String { - return self.render(opts).to_string(); + return self.render(opts).to_html(); } // Render table as HTML diff --git a/src/stv/meek.rs b/src/stv/meek.rs index aaf3f34..b21a62a 100644 --- a/src/stv/meek.rs +++ b/src/stv/meek.rs @@ -54,7 +54,7 @@ impl<'t, N: Number> BallotTree<'t, N> { } /// Descend one level of the [BallotTree] - fn descend_tree(&mut self, candidates: &'t Vec) { + fn descend_tree(&mut self, candidates: &'t [Candidate]) { let mut next_preferences: HashMap<&Candidate, BallotTree> = HashMap::new(); let mut next_exhausted = BallotTree::new(); @@ -111,7 +111,7 @@ where let mut ballot_tree = BallotTree::new(); for ballot in state.election.ballots.iter() { ballot_tree.ballots.push(BallotInTree { - ballot: ballot, + ballot, up_to_pref: 0, }); ballot_tree.num_ballots += &ballot.orig_value; @@ -155,7 +155,7 @@ where } state.exhausted.votes = N::new(); - distribute_recursively(&mut state.candidates, &mut state.exhausted, state.ballot_tree.as_mut().unwrap(), N::one(), &state.election, opts); + distribute_recursively(&mut state.candidates, &mut state.exhausted, state.ballot_tree.as_mut().unwrap(), N::one(), state.election, opts); } /// Distribute preferences recursively @@ -166,7 +166,7 @@ where for<'r> &'r N: ops::Mul<&'r N, Output=N>, { // Descend tree if required - if let None = tree.next_exhausted { + if tree.next_exhausted.is_none() { tree.descend_tree(&election.candidates); } @@ -211,8 +211,8 @@ where exhausted.votes += &remaining_multiplier * &tree.next_exhausted.as_ref().unwrap().as_ref().num_ballots; } -fn recompute_keep_values<'s, N: Number>(state: &mut CountState<'s, N>, opts: &STVOptions, has_surplus: &Vec<&'s Candidate>) { - for candidate in has_surplus.into_iter() { +fn recompute_keep_values<'s, N: Number>(state: &mut CountState<'s, N>, opts: &STVOptions, has_surplus: &[&'s Candidate]) { + for candidate in has_surplus { let count_card = state.candidates.get_mut(candidate).unwrap(); count_card.keep_value = Some(count_card.keep_value.take().unwrap() * state.quota.as_ref().unwrap() / &count_card.votes); @@ -224,7 +224,7 @@ fn recompute_keep_values<'s, N: Number>(state: &mut CountState<'s, N>, opts: &ST } /// Determine if the specified surpluses should be distributed, according to [STVOptions::meek_surplus_tolerance] -fn should_distribute_surpluses<'a, N: Number>(state: &CountState<'a, N>, has_surplus: &Vec<&'a Candidate>, opts: &STVOptions) -> bool +fn should_distribute_surpluses<'a, N: Number>(state: &CountState<'a, N>, has_surplus: &[&'a Candidate], opts: &STVOptions) -> bool where for<'r> &'r N: ops::Sub<&'r N, Output=N>, for<'r> &'r N: ops::Div<&'r N, Output=N>, diff --git a/src/stv/mod.rs b/src/stv/mod.rs index a73324c..e336762 100644 --- a/src/stv/mod.rs +++ b/src/stv/mod.rs @@ -203,9 +203,9 @@ impl STVOptions { flags.push(format!("--constraints {}", path)); if self.constraint_mode != ConstraintMode::GuardDoom { flags.push(self.constraint_mode.describe()); } } - if self.hide_excluded { flags.push(format!("--hide-excluded")); } - if self.sort_votes { flags.push(format!("--sort-votes")); } - if self.transfers_detail { flags.push(format!("--transfers-detail")); } + if self.hide_excluded { flags.push("--hide-excluded".to_string()); } + if self.sort_votes { flags.push("--sort-votes".to_string()); } + if self.transfers_detail { flags.push("--transfers-detail".to_string()); } if self.pp_decimals != 2 { flags.push(format!("--pp-decimals {}", self.pp_decimals)); } return flags.join(" "); } @@ -651,19 +651,19 @@ where state.step_all(); // Finish count - if finished_before_stage(&state) { + if finished_before_stage(state) { return Ok(true); } // Attempt early bulk election if opts.early_bulk_elect { - if bulk_elect(state, &opts)? { + if bulk_elect(state, opts)? { return Ok(false); } } // Continue exclusions - if continue_exclusion(state, &opts)? { + if continue_exclusion(state, opts)? { calculate_quota(state, opts); elect_hopefuls(state, opts, true)?; update_tiebreaks(state, opts); @@ -671,7 +671,7 @@ where } // Exclude doomed candidates - if exclude_doomed(state, &opts)? { + if exclude_doomed(state, opts)? { calculate_quota(state, opts); elect_hopefuls(state, opts, true)?; update_tiebreaks(state, opts); @@ -679,7 +679,7 @@ where } // Distribute surpluses - if distribute_surpluses(state, &opts)? { + if distribute_surpluses(state, opts)? { calculate_quota(state, opts); elect_hopefuls(state, opts, true)?; update_tiebreaks(state, opts); @@ -687,7 +687,7 @@ where } // Attempt late bulk election - if bulk_elect(state, &opts)? { + if bulk_elect(state, opts)? { return Ok(false); } @@ -700,7 +700,7 @@ where } // Exclude lowest hopeful - exclude_hopefuls(state, &opts)?; // Cannot fail + exclude_hopefuls(state, opts)?; // Cannot fail calculate_quota(state, opts); elect_hopefuls(state, opts, true)?; update_tiebreaks(state, opts); @@ -735,19 +735,13 @@ fn next_preferences<'a, N: Number>(state: &CountState<'a, N>, votes: Vec { - let candidate = &state.election.candidates[preference]; - let count_card = &state.candidates[candidate]; - - if let CandidateState::Hopeful | CandidateState::Guarded = count_card.state { - next_candidate = Some(candidate); - break; - } - } - None => { break; } + while let Some(preference) = vote.next_preference() { + let candidate = &state.election.candidates[preference]; + let count_card = &state.candidates[candidate]; + + if let CandidateState::Hopeful | CandidateState::Guarded = count_card.state { + next_candidate = Some(candidate); + break; } } @@ -1417,12 +1411,12 @@ fn hopefuls_to_bulk_exclude<'a, N: Number>(state: &CountState<'a, N>, _opts: &ST } // Do not exclude if this could change the order of exclusion - let total_votes = try_exclude.into_iter().fold(N::new(), |agg, (_, cc)| agg + &cc.votes); + let total_votes = try_exclude.iter().fold(N::new(), |agg, (_, cc)| agg + &cc.votes); if i != 0 && total_votes + &total_surpluses >= hopefuls[hopefuls.len()-i].1.votes { continue; } - let try_exclude = try_exclude.into_iter().map(|(c, _)| **c).collect(); + let try_exclude: Vec<&Candidate> = try_exclude.iter().map(|(c, _)| **c).collect(); // Do not exclude if this violates constraints match constraints::try_constraints(state, &try_exclude, CandidateState::Excluded) { @@ -1586,9 +1580,9 @@ fn finished_before_stage(state: &CountState) -> bool { /// Break a tie between the given candidates according to [STVOptions::ties], selecting the highest candidate /// /// The given candidates are assumed to be tied in this round. -pub fn choose_highest<'c, N: Number>(state: &mut CountState, opts: &STVOptions, candidates: &Vec<&'c Candidate>, prompt_text: &str) -> Result<&'c Candidate, STVError> { +pub fn choose_highest<'c, N: Number>(state: &mut CountState, opts: &STVOptions, candidates: &[&'c Candidate], prompt_text: &str) -> Result<&'c Candidate, STVError> { for strategy in opts.ties.iter() { - match strategy.choose_highest(state, opts, &candidates, prompt_text) { + match strategy.choose_highest(state, opts, candidates, prompt_text) { Ok(c) => { return Ok(c); } @@ -1607,9 +1601,9 @@ pub fn choose_highest<'c, N: Number>(state: &mut CountState, opts: &STVOption /// Break a tie between the given candidates according to [STVOptions::ties], selecting the lowest candidate /// /// The given candidates are assumed to be tied in this round. -pub fn choose_lowest<'c, N: Number>(state: &mut CountState, opts: &STVOptions, candidates: &Vec<&'c Candidate>, prompt_text: &str) -> Result<&'c Candidate, STVError> { +pub fn choose_lowest<'c, N: Number>(state: &mut CountState, opts: &STVOptions, candidates: &[&'c Candidate], prompt_text: &str) -> Result<&'c Candidate, STVError> { for strategy in opts.ties.iter() { - match strategy.choose_lowest(state, opts, &candidates, prompt_text) { + match strategy.choose_lowest(state, opts, candidates, prompt_text) { Ok(c) => { return Ok(c); } @@ -1665,10 +1659,8 @@ fn init_tiebreaks(state: &mut CountState, opts: &STVOptions) { /// If required, update the state of the forwards or backwards tie-breaking strategies, according to [STVOptions::ties] fn update_tiebreaks(state: &mut CountState, _opts: &STVOptions) { - if let None = state.forwards_tiebreak { - if let None = state.backwards_tiebreak { - return; - } + if state.forwards_tiebreak.is_none() && state.backwards_tiebreak.is_none() { + return; } // Sort candidates in this stage by votes, grouping by ties @@ -1727,7 +1719,7 @@ fn update_tiebreaks(state: &mut CountState, _opts: &STVOptions) { continue; } else { // Tied in this round - refer to last round - let mut tied_this_round: Vec<&Candidate> = group.into_iter().map(|c| *c).collect(); + let mut tied_this_round: Vec<&Candidate> = group.iter().copied().collect(); tied_this_round.sort_unstable_by(|a, b| hm_orig[a].cmp(&hm_orig[b])); let tied_this_round = tied_this_round.into_iter() .group_by(|c| hm_orig[c]); diff --git a/src/stv/sample.rs b/src/stv/sample.rs index 3abdaad..68db912 100644 --- a/src/stv/sample.rs +++ b/src/stv/sample.rs @@ -52,7 +52,7 @@ where for<'r> &'r N: ops::Div<&'r N, Output=N>, for<'r> &'r N: ops::Neg { - state.title = StageKind::SurplusOf(&elected_candidate); + state.title = StageKind::SurplusOf(elected_candidate); state.logger.log_literal(format!("Surplus of {} distributed.", elected_candidate.name)); let count_card = state.candidates.get_mut(elected_candidate).unwrap(); @@ -204,7 +204,7 @@ where let mut top_remainders = cands_by_remainder.iter().take(n_to_round).collect::>(); // Check for tied remainders - if candidate_transfers_remainders[top_remainders.last().unwrap()] == candidate_transfers_remainders[cands_by_remainder.iter().nth(n_to_round).unwrap()] { + if candidate_transfers_remainders[top_remainders.last().unwrap()] == candidate_transfers_remainders[&cands_by_remainder[n_to_round]] { // Get the top entry let top_entry = &candidate_transfers_remainders[top_remainders.last().unwrap()]; @@ -237,7 +237,7 @@ where let candidate_transfers_usize: usize = format!("{:.0}", candidate_transfers).parse().expect("Transfer overflows usize"); let count_card = state.candidates.get_mut(candidate).unwrap(); - count_card.transfer(&candidate_transfers); + count_card.transfer(candidate_transfers); checksum += candidate_transfers; let parcel = Parcel { @@ -375,19 +375,13 @@ where fn transfer_ballot<'a, N: Number>(state: &mut CountState<'a, N>, opts: &STVOptions, source_candidate: &Candidate, mut vote: Vote<'a, N>, ignore_nontransferable: bool) -> Result<(), STVError> { // Get next preference let mut next_candidate = None; - - loop { - match vote.next_preference() { - Some(preference) => { - let candidate = &state.election.candidates[preference]; - let count_card = &state.candidates[candidate]; - - if let CandidateState::Hopeful | CandidateState::Guarded = count_card.state { - next_candidate = Some(candidate); - break; - } - } - None => { break; } + while let Some(preference) = vote.next_preference() { + let candidate = &state.election.candidates[preference]; + let count_card = &state.candidates[candidate]; + + if let CandidateState::Hopeful | CandidateState::Guarded = count_card.state { + next_candidate = Some(candidate); + break; } } diff --git a/src/stv/wasm.rs b/src/stv/wasm.rs index d910037..399bb54 100644 --- a/src/stv/wasm.rs +++ b/src/stv/wasm.rs @@ -183,10 +183,7 @@ macro_rules! impl_type { /// Call [render_html](crate::stv::transfers::TransferTable::render_html) on [CountState::transfer_table] pub fn transfer_table_render_html(&self, opts: &STVOptions) -> Option { - return match &self.0.transfer_table { - Some(tt) => Some(tt.render_text(&opts.0)), // TODO - None => None, - }; + return self.0.transfer_table.as_ref().map(|tt| tt.render_text(&opts.0)); } } @@ -324,7 +321,7 @@ pub fn describe_count(filename: String, election: &Election, opts: result.push_str(&format!(r#"). Read {:.0} ballots from ‘{}’ for election ‘{}’. There are {} candidates for {} vacancies. "#, total_ballots, filename, election.name, election.candidates.len(), election.seats)); let opts_str = opts.describe::(); - if opts_str.len() > 0 { + if !opts_str.is_empty() { result.push_str(&format!(r#"Counting using options {}.

"#, opts_str)) } else { result.push_str(r#"Counting using default options.

"#); @@ -338,7 +335,7 @@ pub fn init_results_table(election: &Election, opts: &stv::STVOpti let mut result = String::from(r#""#); if report_style == "ballots_votes" { - result.push_str(&r#""#); + result.push_str(r#""#); } for candidate in election.candidates.iter() { @@ -375,7 +372,7 @@ pub fn update_results_table(stage_num: usize, state: &CountState, // Insert borders to left of new exclusions in Wright STV let classes_o; // Outer version let classes_i; // Inner version - if opts.exclusion == stv::ExclusionMethod::Wright && (if let StageKind::ExclusionOf(_) = state.title { true } else { false }) { + if opts.exclusion == stv::ExclusionMethod::Wright && matches!(state.title, StageKind::ExclusionOf(_)) { classes_o = r#" class="blw""#; classes_i = r#"blw "#; } else { @@ -387,7 +384,7 @@ pub fn update_results_table(stage_num: usize, state: &CountState, let hide_xfers_trsp; if let StageKind::FirstPreferences = state.title { hide_xfers_trsp = true; - } else if opts.exclusion == stv::ExclusionMethod::Wright && (if let StageKind::ExclusionOf(_) = state.title { true } else { false }) { + } else if opts.exclusion == stv::ExclusionMethod::Wright && matches!(state.title, StageKind::ExclusionOf(_)) { hide_xfers_trsp = true; } else { hide_xfers_trsp = false; @@ -631,7 +628,7 @@ pub fn update_results_table(stage_num: usize, state: &CountState, /// Get the comment for the current stage pub fn update_stage_comments(state: &CountState, stage_num: usize) -> String { let mut comments = state.logger.render().join(" "); - if let Some(_) = state.transfer_table { + if state.transfer_table.is_some() { comments.push_str(&format!(r##" [View detailed transfers]"##, stage_num)); } return comments; diff --git a/src/ties.rs b/src/ties.rs index ced7b9d..701b70c 100644 --- a/src/ties.rs +++ b/src/ties.rs @@ -64,12 +64,12 @@ impl TieStrategy { /// Break a tie between the given candidates, selecting the highest candidate /// /// The given candidates are assumed to be tied in this round - pub fn choose_highest<'c, N: Number>(&self, state: &mut CountState, opts: &STVOptions, candidates: &Vec<&'c Candidate>, prompt_text: &str) -> Result<&'c Candidate, STVError> { + pub fn choose_highest<'c, N: Number>(&self, state: &mut CountState, opts: &STVOptions, candidates: &[&'c Candidate], prompt_text: &str) -> Result<&'c Candidate, STVError> { match self { Self::Forwards => { match &state.forwards_tiebreak { Some(tb) => { - let mut candidates = candidates.clone(); + let mut candidates: Vec<&Candidate> = candidates.iter().copied().collect(); // Compare b to a to sort high-to-low candidates.sort_unstable_by(|a, b| tb[b].cmp(&tb[a])); if tb[candidates[0]] == tb[candidates[1]] { @@ -88,7 +88,7 @@ impl TieStrategy { Self::Backwards => { match &state.backwards_tiebreak { Some(tb) => { - let mut candidates = candidates.clone(); + let mut candidates: Vec<&Candidate> = candidates.iter().copied().collect(); candidates.sort_unstable_by(|a, b| tb[b].cmp(&tb[a])); if tb[candidates[0]] == tb[candidates[1]] { return Err(STVError::UnresolvedTie); @@ -122,10 +122,10 @@ impl TieStrategy { /// Break a tie between the given candidates, selecting the lowest candidate /// /// The given candidates are assumed to be tied in this round - pub fn choose_lowest<'c, N: Number>(&self, state: &mut CountState, opts: &STVOptions, candidates: &Vec<&'c Candidate>, prompt_text: &str) -> Result<&'c Candidate, STVError> { + pub fn choose_lowest<'c, N: Number>(&self, state: &mut CountState, opts: &STVOptions, candidates: &[&'c Candidate], prompt_text: &str) -> Result<&'c Candidate, STVError> { match self { Self::Forwards => { - let mut candidates = candidates.clone(); + let mut candidates: Vec<&Candidate> = candidates.iter().copied().collect(); candidates.sort_unstable_by(|a, b| state.forwards_tiebreak.as_ref().unwrap()[a] .cmp(&state.forwards_tiebreak.as_ref().unwrap()[b]) @@ -138,7 +138,7 @@ impl TieStrategy { } } Self::Backwards => { - let mut candidates = candidates.clone(); + let mut candidates: Vec<&Candidate> = candidates.iter().copied().collect(); candidates.sort_unstable_by(|a, b| state.backwards_tiebreak.as_ref().unwrap()[a] .cmp(&state.backwards_tiebreak.as_ref().unwrap()[b]) @@ -161,7 +161,7 @@ impl TieStrategy { } /// Return all maximal items according to the given key -pub fn multiple_max_by(items: &Vec, key: K) -> Vec +pub fn multiple_max_by(items: &[E], key: K) -> Vec where K: Fn(&E) -> C { @@ -190,7 +190,7 @@ where } /// Return all minimal items according to the given key -pub fn multiple_min_by(items: &Vec, key: K) -> Vec +pub fn multiple_min_by(items: &[E], key: K) -> Vec where K: Fn(&E) -> C { @@ -220,7 +220,7 @@ where /// Prompt the candidate for input, depending on CLI or WebAssembly target #[cfg(not(target_arch = "wasm32"))] -fn prompt<'c, N: Number>(state: &CountState, opts: &STVOptions, candidates: &Vec<&'c Candidate>, prompt_text: &str) -> Result<&'c Candidate, STVError> { +fn prompt<'c, N: Number>(state: &CountState, opts: &STVOptions, candidates: &[&'c Candidate], prompt_text: &str) -> Result<&'c Candidate, STVError> { // Show intrastage progress if required if !state.logger.entries.is_empty() { // Print stage details @@ -233,7 +233,7 @@ fn prompt<'c, N: Number>(state: &CountState, opts: &STVOptions, candidates: & // Print summary rows print!("{}", state.describe_summary(opts)); - println!(""); + println!(); } println!("Multiple tied candidates:"); @@ -270,7 +270,7 @@ extern "C" { } #[cfg(target_arch = "wasm32")] -fn prompt<'c, N: Number>(state: &CountState, opts: &STVOptions, candidates: &Vec<&'c Candidate>, prompt_text: &str) -> Result<&'c Candidate, STVError> { +fn prompt<'c, N: Number>(state: &CountState, opts: &STVOptions, candidates: &[&'c Candidate], prompt_text: &str) -> Result<&'c Candidate, STVError> { let mut message = String::new(); // Show intrastage progress if required diff --git a/src/writer/bin.rs b/src/writer/bin.rs index cc54bfc..fd59af7 100644 --- a/src/writer/bin.rs +++ b/src/writer/bin.rs @@ -31,5 +31,5 @@ pub fn write(election: Election, output: W) { // Write output let mut output = BufWriter::new(output); - output.write(&buffer).expect("IO Error"); + output.write_all(&buffer).expect("IO Error"); } diff --git a/src/writer/blt.rs b/src/writer/blt.rs index a41370a..10ada82 100644 --- a/src/writer/blt.rs +++ b/src/writer/blt.rs @@ -31,8 +31,8 @@ pub fn write(election: Election, output: W) { // Write withdrawn candidates if !election.withdrawn_candidates.is_empty() { - output.write(election.withdrawn_candidates.into_iter().map(|idx| format!("-{}", idx + 1)).join(" ").as_bytes()).expect("IO Error"); - output.write(b"\n").expect("IO Error"); + output.write_all(election.withdrawn_candidates.into_iter().map(|idx| format!("-{}", idx + 1)).join(" ").as_bytes()).expect("IO Error"); + output.write_all(b"\n").expect("IO Error"); } // Write ballots @@ -43,10 +43,10 @@ pub fn write(election: Election, output: W) { output.write_fmt(format_args!(" {}", preference.into_iter().map(|p| p + 1).join("="))).expect("IO Error"); } - output.write(b" 0\n").expect("IO Error"); + output.write_all(b" 0\n").expect("IO Error"); } - output.write(b"0\n").expect("IO Error"); + output.write_all(b"0\n").expect("IO Error"); // Write candidate names for candidate in election.candidates {