Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parseTimeM succeeds on hours/minutes greater than specified in documentation #70

Closed
epsilonhalbe opened this issue Mar 31, 2017 · 5 comments
Milestone

Comments

@epsilonhalbe
Copy link

The documentation for the format strings says

%H
hour of day (24-hour), 0-padded to two chars, 00 - 23
...
%M
minute of hour, 0-padded to two chars, 00 - 59

but while

parseTimeM True defaultTimeLocale "%Y-%m-%d %H:%M" "2017-03-31 23:59" :: Either String UTCTime
parseTimeM True defaultTimeLocale "%Y-%m-%d %H:%M" "2017-02-29 23:59" :: Either String UTCTime

Succeeds/Fails correctly, the following

parseTimeM True defaultTimeLocale "%Y-%m-%d %H:%M" "2017-03-31 23:99" :: Either String UTCTime
parseTimeM True defaultTimeLocale "%Y-%m-%d %H:%M" "2017-03-31 99:59" :: Either String UTCTime
parseTimeM True defaultTimeLocale "%Y-%m-%d %H:%M" "2017-03-31 99:99" :: Either String UTCTime

Succeed - with hours/minutes I would not consider correct.

I would expect parseTimeM to have the property "if it is parseable then the formatted string is equal to the initial string".

import Data.Time
import Test.QuickCheck
import Text.Printf

newtype YmdHMString = YmdHM String deriving Eq

instance Show YmdHMString where
  show (YmdHM s) = s

instance Arbitrary YmdHMString where
  arbitrary = do year   <- getPositive <$> (arbitrary :: Gen (Positive Integer))
                 month  <- choose (00,99) :: Gen Int
                 day    <- choose (00,99) :: Gen Int
                 hour   <- choose (00,99) :: Gen Int
                 minute <- choose (00,99) :: Gen Int
                 return $ YmdHM $ printf "%d-%02d-%02d %02d:%02d"
                                         year month day hour minute

prop_YmdHM (YmdHM s) = let fmt = "%Y-%m-%d %H:%M"
                           time = parseTimeM True defaultTimeLocale fmt s :: Maybe UTCTime
                        in maybe True ((s==).formatTime defaultTimeLocale fmt) time
@epsilonhalbe
Copy link
Author

@AshleyYakeley do you think this is an issue or are there cases where the time parser should accept "99:99" as input?

@ghost
Copy link

ghost commented Apr 14, 2017

Starting to look briefly at this. I will report back any progress I make.

@epsilonhalbe
Copy link
Author

I have a fix for it n my forked repo, but I don't know enough about the quirks of time, and by this i don't mean the library, but what time values are reasonable or not, to file a pull request without any feedback. This is one of the most depended on libraries so i don't want to be hasty with a change, as it possibly affects a lot of people

@ghost
Copy link

ghost commented Apr 14, 2017

Okay, I will not look further into this. I am happy to comment on any PRs your make (maybe from a branch to master within your fork as a first pass review?).

@AshleyYakeley
Copy link
Member

fix merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants