Fix Panic When Cleaning Sum of Best (#871)

When cleaning the sum of best, the message tries to be as helpful as
possible by showing as much information about the problematic segments
as possible. Part of that is information about the date of the attempt.
However, as it turns out, the attempt may not actually exist. This
shouldn't generally happen, but we can't trust the correctness of the
splits files, so for whatever reason, the attempt may be missing. In
that case, we simply skip that information.

Author: CryZe

src/run/editor/cleaning.rs

@@ -9,10 +9,10 @@
 use time::UtcOffset;
 
 use crate::{
-    analysis::sum_of_segments::{best, track_branch, Prediction},
+    Attempt, Run, Segment, TimeSpan, TimingMethod,
+    analysis::sum_of_segments::{Prediction, best, track_branch},
     platform::{prelude::*, to_local},
     timing::formatter::{SegmentTime, TimeFormatter},
-    Attempt, Run, Segment, TimeSpan, TimingMethod,
 };
 use core::{fmt, mem::replace};
 
@@ -56,7 +56,7 @@ pub struct PotentialCleanUp<'r> {
     ending_segment: &'r Segment,
     time_between: TimeSpan,
     combined_sum_of_best: Option<TimeSpan>,
-    attempt: &'r Attempt,
+    attempt: Option<&'r Attempt>,
     method: TimingMethod,
     clean_up: CleanUp,
 }
@@ -98,7 +98,7 @@ impl fmt::Display for PotentialCleanUp<'_> {
             )?;
         }
 
-        if let Some(started) = self.attempt.started() {
+        if let Some(started) = self.attempt.and_then(|a| a.started()) {
             let local = to_local(started.time);
             // We want to show the time zone in case it can't be resolved and
             // defaults to UTC.
@@ -260,11 +260,12 @@ fn check_prediction<'a>(
                                 .expect("Start time must not be empty")
                                 .time
                     }),
+                    // The attempt may not exist, as we have all sorts of weird
+                    // malformed files.
                     attempt: run
                         .attempt_history()
                         .iter()
-                        .find(|attempt| attempt.index() == run_index)
-                        .expect("The attempt has to exist"),
+                        .find(|attempt| attempt.index() == run_index),
                     method,
                     clean_up: CleanUp {
                         ending_index,

tests/clean_sum_of_best.rs

@@ -0,0 +1,14 @@
+use livesplit_core::run::{editor::SumOfBestCleaner, parser::livesplit};
+
+mod run_files;
+
+#[test]
+fn dont_panic_if_attempt_doesnt_exist() {
+    // There's an attempt that does not exist that it wants to clean up. We
+    // shouldn't panic then and it should just ignore it.
+    let mut run = livesplit::parse(run_files::CLEAN_SUM_OF_BEST).unwrap();
+    let mut cleaner = SumOfBestCleaner::new(&mut run);
+    let cleanup = cleaner.next_potential_clean_up().unwrap().into();
+    cleaner.apply(cleanup);
+    assert!(cleaner.next_potential_clean_up().is_none());
+}

tests/run_files/clean_sum_of_best.lss

@@ -25,3 +25,4 @@ pub const TIME_SPLIT_TRACKER_WITHOUT_ATTEMPT_COUNT: &str = include_str!("1734.ti
 pub const TIME_SPLIT_TRACKER: &str = include_str!("timesplittracker.txt");
 pub const URN: &str = include_str!("urn.json");
 pub const WSPLIT: &str = include_str!("wsplit");
+pub const CLEAN_SUM_OF_BEST: &str = include_str!("clean_sum_of_best.lss");