Invalid moment rework

[icambron]

Apologies for the number of commits here; it was hard to squash them because I merged in develop instead of rebasing (which I had to do because part of the branch was already merged. Here's a summary of the changes:

Parsing flags

We generate a set of parsing flags when parsing with a format. They are:

{
  empty: false,                             //did we find anything to parse at all
  unusedTokens: ['MM', 'DD'],    //tokens we didn't use in the parse
  unusedInput: ['stuff', 'stuff'],     //other input not parsed
  charsLeftOver: 15,                   //formally ._il, the total number of unused input characters. Sum of the strlens for unusedInput
  overflowMonthOk: false,          //whether or not we consider an overflowed date OK, as in "DDD" parsing
  overflow: 3,                              //array index of where overflow occurred (i.e. invalidAt())
  nullInput: false,                         //was the input null?
  invalidMonth: false,                  //month name that didn't exist, moment('splurk', 'MMMM')
  userInvalidated: false               //user used moment.invalid() without specifying why
}

Access with moment().parsingFlags(). Note that the flags do not include dstInvalid.

Redefined isValid

isValid() is now a function of three things:

• Is date.valueOf() just NaN?
• What parsing flags are set?
• Are we parsing as strict?

Which parsing flags matter is determined by strict. This makes it a lot easier to understand and diagnose validity. There are some behavioral changes. In general, isValid() is more lenient. Some examples:

moment('xx-xx-2001', 'DD-MM-YY') was invalid and now isn't. That's because it parses as Jan 20. I think this is correct. Use strict if you don't want that.

`moment('123456790.12', 'X.SS') was invalid and now isn't.

Open questions:

• isValid() completely ignores whether the date parsed corresponds to a time that doesn't exist because of DST. This is a significant change breaking change. Should it check that?

Improved format scoring

As of #1094, we do a much better job of picking a parsing format out of a list by checking how many unparsed characters there are. We can use the parsing flags to improve that:

1. Prefer valid moments to all invalid moments, as measured by isValid()
2. Prefer formats with fewer unused tokens
3. Prefer formats that result in fewer unparsed input characters

Reworked strict parsing

Strict parsing doesn't need its own separate function to check things. We just have strict behave differently in these ways:

1. Prepend ^ to the token regexes. This works because we slice() the string progressively
2. Place non-format characters in the unsedTokens parsing flag
3. Make isValid() consider additional parsing flags

Open stuff:

• I need additional tests for that third item. This is covered by the create() tests

isDSTShifted

We want to know if you asked for 1:30 AM on March 3 (assuming you're in the US). isDSTShifted() answers that. There are a few questions/issues there:

• I'm not sure how to write tests for it that work across time zones. We could have it skip the test if you're not in a US timezone with DST. Anyway, there are currently no tests for this.
• is isDSTShifted the right term for this? isDSTInvalid()? isUnexpectedTime? Don't know what to call it.
• Does it work reliably? It works by assuming that if the date is valid && the overflowMonthOk flag is off, that the only reason for _a != moment(_a).toArray() is that DST messed with the date. I'm not 100% sure that's true.

format() for invalid moments

format() always returns "Invalid date" for !m.isValid(). The string can be localized.

Open issues:

• Is this what we want to do?
• Could use more tests in general done
• Need to test localization done

Little stuff

moment.invalid() now takes an optional object of parsing flags. So if you're making a third-party plugin, you can generate moments with the appropriate flags, even made-up ones. If you don't specify flags, it sets the userInvalidated flag for you.

Other internal changes

        YEAR = 0,
        MONTH = 1,
        DATE = 2,
        HOUR = 3,
        MINUTE = 4,
        SECOND = 5,
        MILLISECOND = 6,

[ichernev]

OK, this is a long PR!

The main test file: test/moment.parsing_flags.js should be test/moment/parsing_flags.js.

About the DST -- to be honest I don't like the monthOverflowOk. It would be better to just implement proper day of year (by subtracting number of days in each month) and remove that flag altogether.

Now the week tokens (#1141) would be hard to incorporate. If we can convert a week date to a regular year, month, day then we can just use a complete date-time array to initialize Date, and then compare arrays for DST. If we first set the date components only (year, month, day OR iso week year, iso week, iso weekday OR local variant) then its possible for the date part to fall into DST, for those DST that start at 00:00 at some day, which would render a day that's off by one.

The rest seems fine.

[icambron]

The main test file: test/moment.parsing_flags.js should be test/moment/parsing_flags.js.

Haha, oops. :e Vim fail.

It would be better to just implement proper day of year (by subtracting number of days in each month) and remove that flag altogether.

Sure. That's a bit more code, but it does fix some edge cases with the DST check. Will fix.

On the week tokens, good catch; for some reason I never thought of that issue, when writing #1141. So I propose we merge this first and then I'll rework #1141 as appropriate (see my notes there - it's a bit of work).

I'll patch up the items above in the next day or two, and also get it up to date with the develop branch.

[icambron]

@ichernev made a few changes:

• Renamed the file
• Got rid of overflowMonthOK and replaced it with something more explicit. Should avoid the DST edge case.
• Rebased.

[ichernev]

2 should be DATE. Here and on next line, right?

[icambron]

yup, fixed

[icambron]

JS has some really weird corners...

[icambron]

There are probably some edge cases caused here. If the post 1970 date JS uses is on a DST, then this'll get shifted, even if the pre-1970 date wouldn't have been. But since a lot of the browsers (all of them on Windows, for example), don't support different historic rules anyway, I'm not sure this is the biggest deal. I'm open to suggestions, though.

[ichernev]

Oh, wow. I did not foresee that Javascript bump :) Dates before 1970 are used much less anyway, and I really don't see something we can do about them. The current implementation is much worse in regard to DST shifting issues, so I guess we can stick with this one for a while.

[ichernev]

Ok, that looks great! We can create a date object from date and time, yeey :) I think this PR turned out great, I just hope momen't users won't be disappointed by the change in isValid().

Thank you for the hard work!

[icambron]

👍