Parsing updates, tests, documentation

- Added many parsing tests.
- Simplified parsing s.t. 'PDouble a' parsing now re-uses the 'a'
  parser.
- Documented parsing semantics.
- Modified Pace parsing so that units must be singular.

src/Running.hs

@@ -1,5 +1,3 @@
-{-# OPTIONS_GHC -Wwarn #-}
-
 module Running
   ( -- * To Pace
     displaySomePace,
@@ -56,8 +54,7 @@ calculatePace distance duration = mkPace $ duration .% distance.unDistance
 
 -- | Given a pace and distance, calculates the duration.
 calculateDuration ::
-  (MSemigroup a) =>
-  Distance d a ->
-  Pace d a ->
-  Duration Second a
+  Distance d PDouble ->
+  Pace d PDouble ->
+  Duration Second PDouble
 calculateDuration distance pace = pace.unPace .* distance.unDistance

src/Running/Data/Distance.hs

@@ -99,7 +99,12 @@ instance (FromInteger a) => FromInteger (Distance d a) where
 instance (ToInteger a) => ToInteger (Distance d a) where
   toZ (MkDistance x) = toZ x
 
--- relies on instances defined in Parser.hs
+-- NOTE: [Distance Parsing]
+--
+-- No units, simply reuses Double's instances in Parser.hs.
+--
+-- See NOTE: [SomeDistance Parsing]
+
 instance (Parser a) => Parser (Distance d a) where
   parser = MkDistance <$> parser
 
@@ -212,6 +217,10 @@ instance (FromInteger a) => FromInteger (SomeDistance a) where
 instance (ToInteger a) => ToInteger (SomeDistance a) where
   toZ (MkSomeDistance _ x) = toZ x
 
+-- NOTE: [SomeDistance Parsing]
+--
+-- Uses Distance's instance and adds units e.g. '4 km'.
+-- See NOTE: [Distance Parsing]
 instance (Parser a) => Parser (SomeDistance a) where
   parser = do
     x <- parser

src/Running/Data/Duration.hs

@@ -116,6 +116,14 @@ instance (FromInteger a) => FromInteger (Duration t a) where
 instance (ToInteger a) => ToInteger (Duration t a) where
   toZ (MkDuration x) = toZ x
 
+-- NOTE: [Duration Parsing]
+--
+-- Parses a time string and converts to the requested units. We have separate
+-- (overlapping) instances for 'a' and 'Positive a', since the latter needs
+-- to perform a positive check.
+--
+-- See NOTE: [SomeDuration Parsing]
+
 instance
   {-# OVERLAPPABLE #-}
   ( Fractional a,
@@ -143,11 +151,10 @@ instance
   Parser (Duration t (Positive a))
   where
   parser = do
-    seconds <- P.parseTimeString
-    -- Coverting before saves us a Division constraint.
-    let converted = seconds .%. singFactor @_ @t
-    posConverted <- mkPositiveFailZ converted
-    pure $ MkDuration posConverted
+    -- reuse non-positive parser
+    MkDuration x <- parser @(Duration t a)
+    y <- mkPositiveFailZ x
+    pure $ MkDuration y
 
 liftDuration2 ::
   (a -> a -> a) ->
@@ -329,43 +336,55 @@ instance (FromInteger a) => FromInteger (SomeDuration a) where
 instance (ToInteger a) => ToInteger (SomeDuration a) where
   toZ (MkSomeDuration _ x) = toZ x
 
-instance (Fractional a, FromInteger a, MGroup a) => Parser (SomeDuration a) where
-  -- This might seem odd since the Duration/SomeDuration parsing is different
-  -- from Distance/SomeDistance. To recap, the latter is:
-  --
-  -- Distance: "2.45" :: Distance t
-  --
-  --   That is, distance is just some numeric w/o units, and the user chooses
-  --   the type after the fact.
-  --
-  -- SomeDistance: "2.45 meters" :: SomeDistance
-  --
-  --   By contrast, the some distance string includes the units, hence we need
-  --   to existentially quantify the units, and the user does __not__ choose
-  --   the type directly (albeit indirectly via the string).
-  --
-  -- Duration: "1d2h3m4s" :: Duration t a
-  --
-  --   The string is a "time string" and __does__ include the units. This is
-  --   for better usability, as strings like "4m15s" are easier than
-  --   "4.25" :: Duration Second a. The requested unit type, t, only affects
-  --   how the type is stored after conversion e.g. "4m15s" becomes
-  --   255 :: Duration Second a.
-  --
-  -- SomeDuration: "1d2h3m4s" :: SomeDuration
-  --
-  --   Because Duration already handled unit parsing, we can simply reuse its
-  --   parser. There is no need to add "extra" parsing functionality, like
-  --   with SomeDistance.
-  --
-  --   Furthermore, notice that with Distance/SomeDistance we had the notion
-  --   of a "requested unit" i.e. either the type or string units. Here, though
-  --   we have no such notion, since we consume all possible unit types
-  --   (days, hours, minutes, seconds) every time, hence there is no requested
-  --   unit for SomeDuration. Thus we can choose whatever we want, so we go
-  --   with seconds.
+-- NOTE: [SomeDuration Parsing]
+--
+-- Reuses the Duration parser, and converts to Second. Interestingly, this is
+-- one of the few cases where the SomeX parser is simpler than the X parser.
+-- Why?
+--
+-- Most of the time, the SomeX parser adds some notion of units e.g.
+-- Distance and SomeDistance are '"2.45" :: Distance t a' and
+-- '"2.45 meters" :: SomeDistance a', respectively. Letting the user choose
+-- the units via the text means the type needs to be existentially quantified,
+-- hence the SomeX type.
+--
+-- In contrast, the time-strings parsed by Duration and SomeDuration allow
+-- the user to include multiple units e.g. "1h2d3m4s". This is for
+-- convenience, as supplying "2h4m15s" is easier than "2.071 hours" or
+-- "7455 seconds". Notice this means there is no notion of a "requested unit",
+-- like with SomeDistance.
+--
+-- Hence the SomeDuration has the same parsing as Duration. We merely choose
+-- the final unit ourselves (Seconds), since it is existentially quantified.
+--
+-- See NOTE: [Duration Parsing]
+
+instance
+  {-# OVERLAPPABLE #-}
+  ( Fractional a,
+    FromInteger a,
+    MGroup a
+  ) =>
+  Parser (SomeDuration a)
+  where
   parser = MkSomeDuration SSecond <$> parser
 
+instance
+  {-# OVERLAPPING #-}
+  ( Fractional a,
+    FromInteger a,
+    MGroup a,
+    Ord a,
+    Show a
+  ) =>
+  Parser (SomeDuration (Positive a))
+  where
+  parser = do
+    -- reuse non-positive parser
+    MkSomeDuration s (MkDuration x) <- parser @(SomeDuration a)
+    y <- mkPositiveFailZ x
+    pure $ MkSomeDuration s (MkDuration y)
+
 liftSomeDuration2 ::
   (FromInteger a, MSemigroup a) =>
   (forall t. Duration t a -> Duration t a -> Duration t a) ->

src/Running/Data/Pace.hs

@@ -141,7 +141,17 @@ instance (FromInteger a, PaceDistF d) => FromInteger (Pace d a) where
 instance (ToInteger a) => ToInteger (Pace d a) where
   toZ = toZ . (.unPace)
 
+-- NOTE: [Pace Parsing]
+--
+-- Parses a pace like "1h 4'30\"", where each h/m/s component is optional,
+-- but we must have at least one.
+--
+-- Like Duration, we have separate instances for 'a' and 'Positive a'.
+--
+-- See NOTE: [SomePace Parsing]
+
 instance
+  {-# OVERLAPPABLE #-}
   ( FromInteger a,
     PaceDistF d,
     Read a,
@@ -152,6 +162,7 @@ instance
   parser = MkPace <$> parsePaceDuration
 
 instance
+  {-# OVERLAPPING #-}
   ( FromInteger a,
     Ord a,
     PaceDistF d,
@@ -162,9 +173,10 @@ instance
   Parser (Pace d (Positive a))
   where
   parser = do
-    MkPace
-      . MkDuration
-      <$> (mkPositiveFailZ . (.unDuration) =<< parsePaceDuration)
+    -- reuse non-positive parser
+    MkPace (MkDuration x) <- parser @(Pace d a)
+    y <- mkPositiveFailZ x
+    pure (MkPace (MkDuration y))
 
 -- | Creates a pace from a duration.
 mkPace ::
@@ -279,6 +291,12 @@ instance (FromInteger a) => FromInteger (SomePace a) where
 instance (ToInteger a) => ToInteger (SomePace a) where
   toZ = toZ . (.unSomePace)
 
+-- NOTE: [SomePace Parsing]
+--
+-- Adds units to pace e.g. "1h 4'30\" /km".
+--
+-- See NOTE: [Pace Parsing]
+
 instance
   {-# OVERLAPPABLE #-}
   ( FromInteger a,
@@ -291,14 +309,30 @@ instance
     x <- parsePaceDuration
     MPC.space
     MPC.char '/'
-    parser >>= \case
-      Meter ->
-        fail "Meters are disallowed in Pace; use km or mi."
-      Kilometer -> pure $ MkSomePace SKilometer $ MkPace x
-      Mile -> pure $ MkSomePace SMile $ MkPace x
+
+    -- We do not use DistanceUnit's built-in parsing because we want the
+    -- long units here (kilometer and mile) to be __singular__, not plural.
+    eDUnit <-
+      MP.choice
+        [ MPC.string "meters" $> Left "meters",
+          MPC.string "km" $> Right Kilometer,
+          MPC.string "kilometers" $> Left "kilometers",
+          MPC.string "kilometer" $> Right Kilometer,
+          MPC.string "miles" $> Left "miles",
+          MPC.string "mile" $> Right Mile,
+          MPC.string "mi" $> Right Mile,
+          MPC.char 'm' $> Right Meter
+        ]
+
+    case eDUnit of
+      Left "meters" -> fail "Meters are disallowed in Pace; use km or mi."
+      Left d -> fail $ "Pace unit " ++ d ++ " should be singular"
+      Right Meter -> fail "Meters are disallowed in Pace; use km or mi."
+      Right Kilometer -> pure $ MkSomePace SKilometer $ MkPace x
+      Right Mile -> pure $ MkSomePace SMile $ MkPace x
 
 instance
-  {-# OVERLAPPABLE #-}
+  {-# OVERLAPPING #-}
   ( FromInteger a,
     Ord a,
     Read a,
@@ -308,14 +342,10 @@ instance
   Parser (SomePace (Positive a))
   where
   parser = do
-    x <- parsePaceDuration
-    y <- MkDuration <$> mkPositiveFailZ x.unDuration
-    MPC.string " /"
-    parser >>= \case
-      Meter ->
-        fail "Meters are disallowed in Pace; use km or mi."
-      Kilometer -> pure $ MkSomePace SKilometer $ MkPace y
-      Mile -> pure $ MkSomePace SMile $ MkPace y
+    -- reuse non-positive parser
+    MkSomePace s (MkPace (MkDuration x)) <- parser @(SomePace a)
+    y <- mkPositiveFailZ x
+    pure $ MkSomePace s (MkPace (MkDuration y))
 
 -- | Exposes the underlying duration.
 unSomePace :: SomePace a -> Duration Second a

test/unit/Unit/Prelude.hs

@@ -1,3 +1,5 @@
+{-# LANGUAGE AllowAmbiguousTypes #-}
+
 module Unit.Prelude
   ( module X,
 
@@ -7,12 +9,26 @@ module Unit.Prelude
 
     -- * HUnit
     (@/=?),
+    assertLeft,
 
-    -- * Misc
-    mkDistanceD,
-    mkDurationD,
+    -- * Parsing
     parseOrDie,
     parseOrDieM,
+    parseOrDieM_,
+
+    -- * Constructors
+    mkDistanceD,
+    mkDistancePD,
+    mkSomeDistanceD,
+    mkSomeDistancePD,
+    mkDurationD,
+    mkDurationPD,
+    mkSomeDurationD,
+    mkSomeDurationPD,
+    mkPaceD,
+    mkPacePD,
+    mkSomePaceD,
+    mkSomePacePD,
   )
 where
 
@@ -35,8 +51,17 @@ import Hedgehog as X
 import Hedgehog qualified as H
 import Running.Class.Parser (Parser)
 import Running.Class.Parser qualified as Parser
-import Running.Data.Distance (Distance (MkDistance))
-import Running.Data.Duration (Duration (MkDuration))
+import Running.Data.Distance
+  ( Distance (MkDistance),
+    SomeDistance (MkSomeDistance),
+  )
+import Running.Data.Distance.Units (SDistanceUnit)
+import Running.Data.Duration
+  ( Duration (MkDuration),
+    SomeDuration (MkSomeDuration),
+  )
+import Running.Data.Duration.Units (STimeUnit)
+import Running.Data.Pace (Pace (MkPace), PaceDistF, SomePace (MkSomePace))
 import Running.Prelude as X hiding (IO)
 import System.IO as X (IO)
 import Test.Tasty as X (TestName, TestTree, testGroup)
@@ -64,6 +89,18 @@ x @/=? y = assertBool msg (x /= y)
 
 infix 1 @/=?
 
+assertLeft :: (Show b) => List Char -> Either a b -> Assertion
+assertLeft _ (Left _) = pure ()
+assertLeft s (Right x) = assertFailure err
+  where
+    err =
+      mconcat
+        [ s,
+          ": Expected Left, received Right '",
+          show x,
+          "'"
+        ]
+
 parseOrDie :: (HasCallStack, Parser a) => Text -> a
 parseOrDie = errorMapLeft unpackText . Parser.parse
 
@@ -73,11 +110,44 @@ parseOrDieM t = do
     Right r -> pure r
     Left err -> fail (unpackText err)
 
+parseOrDieM_ :: forall a m. (MonadFail m, Parser a) => Text -> m ()
+parseOrDieM_ = void . parseOrDieM @a
+
 hdiff :: (Show a, Show b) => a -> (a -> b -> Bool) -> b -> PropertyT IO ()
 hdiff = H.diff
 
 mkDistanceD :: forall t. Double -> Distance t Double
 mkDistanceD = MkDistance
 
+mkDistancePD :: forall t. Double -> Distance t PDouble
+mkDistancePD = MkDistance . unsafePositive
+
+mkSomeDistanceD :: SDistanceUnit d -> Double -> SomeDistance Double
+mkSomeDistanceD s = MkSomeDistance s . mkDistanceD
+
+mkSomeDistancePD :: SDistanceUnit d -> Double -> SomeDistance PDouble
+mkSomeDistancePD s = MkSomeDistance s . mkDistancePD
+
 mkDurationD :: forall t. Double -> Duration t Double
 mkDurationD = MkDuration
+
+mkDurationPD :: forall t. Double -> Duration t PDouble
+mkDurationPD = MkDuration . unsafePositive
+
+mkSomeDurationD :: STimeUnit s -> Double -> SomeDuration Double
+mkSomeDurationD s = MkSomeDuration s . mkDurationD
+
+mkSomeDurationPD :: STimeUnit s -> Double -> SomeDuration PDouble
+mkSomeDurationPD s = MkSomeDuration s . mkDurationPD
+
+mkPaceD :: forall d. (PaceDistF d) => Double -> Pace d Double
+mkPaceD = MkPace . MkDuration
+
+mkPacePD :: forall d. (PaceDistF d) => Double -> Pace d PDouble
+mkPacePD = MkPace . MkDuration . unsafePositive
+
+mkSomePaceD :: forall d. (PaceDistF d) => SDistanceUnit d -> Double -> SomePace Double
+mkSomePaceD s = MkSomePace s . mkPaceD
+
+mkSomePacePD :: forall d. (PaceDistF d) => SDistanceUnit d -> Double -> SomePace PDouble
+mkSomePacePD s = MkSomePace s . mkPacePD