Fix JavaScript setMonth() behavior

[jwarkentin]

JavaScript does some weird stuff with setMonth(). This change makes its behavior more consistent.

If you have a moment object with a date around the end of the month and you try to change the month to a month that doesn't have that many days you end up with an unexpected result. You can see this exhibited using moment like this:

>>> var m = moment('20130531', 'YYYYMMDD');
>>> m.month();
4
>>> m.month(3);
>>> m.month();
4

In JS the setMonth() method sets the month and then applies the date and rolls over into the next month if the date (e.g. 31) is higher than the last day of the month.

While this is the normal behavior of the JS Date object, I would argue that it is the wrong behavior. It causes bugs that users unfamiliar with the oddities of the language won't expect. In addition the date you end up with is somewhat arbitrary and not helpful in any case I can imagine because it's not consistent or predictable without extra logic. If I'm using an object and I try to explicitly set the month I want the object to go to that month, not occasionally end up on both a different month AND date from what I expect.

If the month doesn't have enough days it's much less buggy or unexpected for it to consistently go to the last day of the month than to end up on some day toward the start of the month depending on what day of the month it is and what month I'm switching to.

In my particular case it caused a bug that could only be observed on certain days of certain months, like today. In my code it creates a new moment object and then tries to set the month based on a value returned from the server. But since the object was created today it causes it to set the month wrong. If I tried the exact same thing yesterday it would have worked fine. At least with this fix it creates consistent and predictable behavior and always sets the month to the one requested.

[timrwood]

We actually do something similar in moment#add and moment#subtract.

https://github.com/timrwood/moment/blob/31fb450d23fc4b35a5c7159d0aed8af431ecf11d/moment.js#L353-L358

This is also related to #754 and #602.

I think this is probably a safe change to make, and is definitely the more expected result. Would you mind adding some unit tests for this as well? Maybe in the test/moment/getters_setters.js file?

[jwarkentin]

I just modified the pull request with a test. Let me know what you think. BTW, you may want to update your CONTRIBUTING.md file. I actually had to also run npm install grunt-cli -g, npm install within the project folder, and grunt default to run the tests, not grunt test. Just thought you might like to know :)

[ichernev]

The pull request looks good. I'm just wondering if its not better to use the add month method instead, so we won't have logic duplication of fix-the-day-first-when-changing-month.

[jwarkentin]

Good point. I just modified my pull request. It is easier and more proper to implement the add month behavior via the set month method than the other way around. I also modified the set month code a bit. The implementation in the add month method was shorter and better looking.

[ichernev]

That look good! Thank you for contributing.