Fixed as.Date() with invalid date string.

Miloslav Metelka · Miloslav Metelka · commit 8f8e74f8baf2 · 2019-03-10T15:20:07.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,7 @@ Bug fixes:
 * `length<-` would remove attributes from the target even if it was a shared value
 * `length(x) <- N` should not strip attributes if `length(x) == N`, which is not in line with GNU-R documentation,
 but relied upon in the `methods` package #55
+* `as.Date` with invalid date string #56
 
 # 1.0 RC 13
 
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/DatePOSIXFunctions.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/DatePOSIXFunctions.java
@@ -30,22 +30,37 @@
 import static com.oracle.truffle.r.runtime.builtins.RBuiltinKind.INTERNAL;
 
 import java.text.ParsePosition;
+import java.time.Clock;
 import java.time.DateTimeException;
 import java.time.Instant;
 import java.time.LocalDate;
 import java.time.LocalDateTime;
+import java.time.LocalTime;
 import java.time.ZoneId;
 import java.time.ZoneOffset;
 import java.time.ZonedDateTime;
+import java.time.chrono.Chronology;
+import java.time.chrono.ChronoLocalDate;
+import java.time.chrono.ChronoLocalDateTime;
+import java.time.chrono.ChronoPeriod;
+import java.time.chrono.ChronoZonedDateTime;
+import java.time.chrono.Era;
+import java.time.chrono.IsoChronology;
 import java.time.format.DateTimeFormatter;
 import java.time.format.DateTimeFormatterBuilder;
 import java.time.format.DateTimeParseException;
 import java.time.format.FormatStyle;
+import java.time.format.ResolverStyle;
+import java.time.format.SignStyle;
 import java.time.format.TextStyle;
 import java.time.temporal.ChronoField;
 import java.time.temporal.TemporalAccessor;
+import java.time.temporal.TemporalField;
+import java.time.temporal.ValueRange;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.TimeZone;
 
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
@@ -70,7 +85,7 @@
 import com.oracle.truffle.r.runtime.data.model.RAbstractListVector;
 import com.oracle.truffle.r.runtime.data.model.RAbstractStringVector;
 import com.oracle.truffle.r.runtime.data.model.RAbstractVector;
-import java.time.LocalTime;
+import java.time.Month;
 
 // from GnuR datatime.c
 
@@ -435,7 +450,7 @@ protected RList strptime(RAbstractStringVector x, RAbstractStringVector format,
             DateTimeFormatterBuilder[] builders = createFormatters(format, true);
             DateTimeFormatter[] formatters = new DateTimeFormatter[builders.length];
             for (int i = 0; i < builders.length; i++) {
-                formatters[i] = builders[i].toFormatter();
+                formatters[i] = builders[i].toFormatter().withChronology(LeapYearChronology.INSTANCE);
             }
 
             for (int i = 0; i < length; i++) {
@@ -485,7 +500,7 @@ private static DateTimeFormatterBuilder[] createFormatters(RAbstractStringVector
     private static DateTimeFormatterBuilder createFormatter(String format, boolean forInput) {
         DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
         if (forInput) {
-            // Opposite of STRICT mode; allows to parse single-digit hour and minute like '0:3:22'.
+            // Lenient parsing required to parse datetimes like "2002-6-24-0-0-10"
             builder.parseLenient();
         }
         boolean escaped = false;
@@ -564,7 +579,11 @@ private static DateTimeFormatterBuilder createFormatter(String format, boolean f
                          * 24:00:00 are accepted for input, since ISO 8601 allows these. For output
                          * 00-23 is required thus using HOUR_OF_DAY.
                          */
-                        builder.appendValue(forInput ? ChronoField.CLOCK_HOUR_OF_DAY : ChronoField.HOUR_OF_DAY, 2);
+                        if (forInput) {
+                            builder.appendValue(ChronoField.CLOCK_HOUR_OF_DAY, 1, 2, SignStyle.NOT_NEGATIVE);
+                        } else {
+                            builder.appendValue(ChronoField.HOUR_OF_DAY, 2);
+                        }
                         break;
                     case 'I':
                         // Hours as decimal number (01–12).
@@ -580,7 +599,11 @@ private static DateTimeFormatterBuilder createFormatter(String format, boolean f
                         break;
                     case 'M':
                         // Minute as decimal number (00–59).
-                        builder.appendValue(ChronoField.MINUTE_OF_HOUR, 2);
+                        if (forInput) {
+                            builder.appendValue(ChronoField.MINUTE_OF_HOUR, 1, 2, SignStyle.NOT_NEGATIVE);
+                        } else {
+                            builder.appendValue(ChronoField.MINUTE_OF_HOUR, 2);
+                        }
                         break;
                     case 'n':
                         // Newline on output, arbitrary whitespace on input.
@@ -625,16 +648,21 @@ private static DateTimeFormatterBuilder createFormatter(String format, boolean f
                          * Second as decimal number (00–61), allowing for up to two leap-seconds
                          * (but POSIX-compliant implementations will ignore leap seconds).
                          */
-                        builder.appendValue(ChronoField.SECOND_OF_MINUTE, 2);
+                        if (forInput) {
+                            builder.appendValue(ChronoField.SECOND_OF_MINUTE, 1, 2, SignStyle.NOT_NEGATIVE);
+                        } else {
+                            builder.appendValue(ChronoField.SECOND_OF_MINUTE, 2);
+                        }
                         break;
                     case 't':
                         // Tab on output, arbitrary whitespace on input.
                         builder.appendLiteral('\t');
                         break;
                     case 'T':
                         // Equivalent to %H:%M:%S.
-                        builder.appendValue(ChronoField.CLOCK_HOUR_OF_DAY, 2).appendLiteral(':').appendValue(ChronoField.MINUTE_OF_HOUR, 2).appendLiteral(':').appendValue(
-                                        ChronoField.SECOND_OF_MINUTE, 2);
+                        builder.appendValue(ChronoField.CLOCK_HOUR_OF_DAY, forInput ? 1 : 2).appendLiteral(':').appendValue(ChronoField.MINUTE_OF_HOUR, forInput ? 1 : 2).appendLiteral(
+                                        ':').appendValue(
+                                                        ChronoField.SECOND_OF_MINUTE, forInput ? 1 : 2);
                         break;
                     case 'u':
                         // Weekday as a decimal number (1–7, Monday is 1).
@@ -786,4 +814,165 @@ private static String getTimeZomeFromAttribute(RAbstractListVector x) {
         }
         return zone;
     }
+
+    private static final class LeapYearChronology implements Chronology {
+
+        static LeapYearChronology INSTANCE = new LeapYearChronology(IsoChronology.INSTANCE);
+
+        private static final int[] maxDayOfMonths = new int[]{31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
+
+        private final Chronology delegate;
+
+        LeapYearChronology(Chronology delegate) {
+            this.delegate = delegate;
+        }
+
+        @Override
+        public String getId() {
+            return delegate.getId();
+        }
+
+        @Override
+        public String getCalendarType() {
+            return delegate.getCalendarType();
+        }
+
+        @Override
+        public ChronoLocalDate date(Era era, int yearOfEra, int month, int dayOfMonth) {
+            return delegate.date(era, yearOfEra, month, dayOfMonth);
+        }
+
+        @Override
+        public ChronoLocalDate date(int prolepticYear, int month, int dayOfMonth) {
+            return delegate.date(prolepticYear, month, dayOfMonth);
+        }
+
+        @Override
+        public ChronoLocalDate dateYearDay(Era era, int yearOfEra, int dayOfYear) {
+            return delegate.dateYearDay(era, yearOfEra, dayOfYear);
+        }
+
+        @Override
+        public ChronoLocalDate dateYearDay(int prolepticYear, int dayOfYear) {
+            return delegate.dateYearDay(prolepticYear, dayOfYear);
+        }
+
+        @Override
+        public ChronoLocalDate dateEpochDay(long epochDay) {
+            return delegate.dateEpochDay(epochDay);
+        }
+
+        @Override
+        public ChronoLocalDate dateNow() {
+            return delegate.dateNow();
+        }
+
+        @Override
+        public ChronoLocalDate dateNow(ZoneId zone) {
+            return delegate.dateNow(zone);
+        }
+
+        @Override
+        public ChronoLocalDate dateNow(Clock clock) {
+            return delegate.dateNow(clock);
+        }
+
+        @Override
+        public ChronoLocalDate date(TemporalAccessor temporal) {
+            return delegate.date(temporal);
+        }
+
+        @Override
+        public ChronoLocalDateTime<? extends ChronoLocalDate> localDateTime(TemporalAccessor temporal) {
+            return delegate.localDateTime(temporal);
+        }
+
+        @Override
+        public ChronoZonedDateTime<? extends ChronoLocalDate> zonedDateTime(TemporalAccessor temporal) {
+            return delegate.zonedDateTime(temporal);
+        }
+
+        @Override
+        public ChronoZonedDateTime<? extends ChronoLocalDate> zonedDateTime(Instant instant, ZoneId zone) {
+            return delegate.zonedDateTime(instant, zone);
+        }
+
+        @Override
+        public boolean isLeapYear(long prolepticYear) {
+            return delegate.isLeapYear(prolepticYear);
+        }
+
+        @Override
+        public int prolepticYear(Era era, int yearOfEra) {
+            return delegate.prolepticYear(era, yearOfEra);
+        }
+
+        @Override
+        public Era eraOf(int eraValue) {
+            return delegate.eraOf(eraValue);
+        }
+
+        @Override
+        public List<Era> eras() {
+            return delegate.eras();
+        }
+
+        @Override
+        public ValueRange range(ChronoField field) {
+            return delegate.range(field);
+        }
+
+        @Override
+        public String getDisplayName(TextStyle style, Locale locale) {
+            return delegate.getDisplayName(style, locale);
+        }
+
+        @Override
+        public ChronoLocalDate resolveDate(Map<TemporalField, Long> fieldValues, ResolverStyle resolverStyle) {
+            // date(int prolepticYear, int month, int dayOfMonth) not called -> handle here
+            Long day = fieldValues.get(ChronoField.DAY_OF_MONTH);
+            if (day != null && day >= 29) {
+                Long month = fieldValues.get(ChronoField.MONTH_OF_YEAR);
+                if (month != null && month <= 12) {
+                    if (month == 2 && day == 29) {
+                        Long year = fieldValues.get(ChronoField.YEAR);
+                        if (year != null && !isLeapYear(year)) {
+                            throw new DateTimeException("Invalid date 'February 29' as '" + year + "' is not a leap year");
+                        }
+                    } else {
+                        int monthInt = (int) (long) month;
+                        if (day > maxDayOfMonths[(monthInt) - 1]) {
+                            throw new DateTimeException("Invalid date '" + Month.of(monthInt).name() + " " + day + "'");
+                        }
+                    }
+                }
+            }
+            return delegate.resolveDate(fieldValues, resolverStyle);
+        }
+
+        @Override
+        public ChronoPeriod period(int years, int months, int days) {
+            return delegate.period(years, months, days);
+        }
+
+        @Override
+        public int compareTo(Chronology other) {
+            return delegate.compareTo(other);
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            return delegate.equals(obj);
+        }
+
+        @Override
+        public int hashCode() {
+            return delegate.hashCode();
+        }
+
+        @Override
+        public String toString() {
+            return delegate.toString();
+        }
+    }
 }
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
@@ -5778,6 +5778,75 @@ actual    34    35    36    36    38    39
 #argv <- structure(list(x = c('2007-11-06', NA)), .Names = 'x');do.call('as.Date.character', argv)
 [1] "2007-11-06" NA
 
+##com.oracle.truffle.r.test.builtins.TestBuiltin_asDatecharacter.testasDatecharacter1#
+#{ as.Date('2016-02-29') }
+[1] "2016-02-29"
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_asDatecharacter.testasDatecharacter1#
+#{ as.Date('2017-00-10') }
+Error in charToDate(x) :
+  character string is not in a standard unambiguous format
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_asDatecharacter.testasDatecharacter1#
+#{ as.Date('2017-01-31') }
+[1] "2017-01-31"
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_asDatecharacter.testasDatecharacter1#
+#{ as.Date('2017-01-32') }
+Error in charToDate(x) :
+  character string is not in a standard unambiguous format
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_asDatecharacter.testasDatecharacter1#
+#{ as.Date('2017-02-28') }
+[1] "2017-02-28"
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_asDatecharacter.testasDatecharacter1#
+#{ as.Date('2017-02-29') }
+Error in charToDate(x) :
+  character string is not in a standard unambiguous format
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_asDatecharacter.testasDatecharacter1#
+#{ as.Date('2017-02-30') }
+Error in charToDate(x) :
+  character string is not in a standard unambiguous format
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_asDatecharacter.testasDatecharacter1#
+#{ as.Date('2017-03-31') }
+[1] "2017-03-31"
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_asDatecharacter.testasDatecharacter1#
+#{ as.Date('2017-04-31') }
+Error in charToDate(x) :
+  character string is not in a standard unambiguous format
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_asDatecharacter.testasDatecharacter1#
+#{ as.Date('2017-04-32') }
+Error in charToDate(x) :
+  character string is not in a standard unambiguous format
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_asDatecharacter.testasDatecharacter1#
+#{ as.Date('2017-05-31') }
+[1] "2017-05-31"
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_asDatecharacter.testasDatecharacter1#
+#{ as.Date('2017-10-00') }
+Error in charToDate(x) :
+  character string is not in a standard unambiguous format
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_asDatecharacter.testasDatecharacter1#
+#{ as.Date('2017-12-31') }
+[1] "2017-12-31"
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_asDatecharacter.testasDatecharacter1#
+#{ as.Date('2017-12-32') }
+Error in charToDate(x) :
+  character string is not in a standard unambiguous format
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_asDatecharacter.testasDatecharacter1#
+#{ as.Date('2017-13-01') }
+Error in charToDate(x) :
+  character string is not in a standard unambiguous format
+
 ##com.oracle.truffle.r.test.builtins.TestBuiltin_asDatedefault.testasDatedefault1#
 #argv <- structure(list(x = logical(0)), .Names = 'x');do.call('as.Date.default', argv)
 Date of length 0
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_asDatecharacter.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_asDatecharacter.java
@@ -14,7 +14,7 @@
  * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
  *
  * Copyright (c) 2014, Purdue University
- * Copyright (c) 2014, 2018, Oracle and/or its affiliates
+ * Copyright (c) 2014, 2019, Oracle and/or its affiliates
  *
  * All rights reserved.
  */
@@ -31,5 +31,23 @@ public class TestBuiltin_asDatecharacter extends TestBase {
     @Test
     public void testasDatecharacter1() {
         assertEval("argv <- structure(list(x = c('2007-11-06', NA)), .Names = 'x');do.call('as.Date.character', argv)");
+
+        assertEval("{ as.Date('2017-02-29') }");
+        assertEval("{ as.Date('2017-01-31') }");
+        assertEval("{ as.Date('2017-01-32') }");
+        assertEval("{ as.Date('2017-02-28') }");
+        assertEval("{ as.Date('2017-02-29') }");
+        assertEval("{ as.Date('2016-02-29') }");
+        assertEval("{ as.Date('2017-02-30') }");
+        assertEval("{ as.Date('2017-03-31') }");
+        assertEval("{ as.Date('2017-04-31') }");
+        assertEval("{ as.Date('2017-05-31') }");
+        assertEval("{ as.Date('2017-04-32') }");
+        assertEval("{ as.Date('2017-00-10') }");
+        assertEval("{ as.Date('2017-10-00') }");
+        assertEval("{ as.Date('2017-12-31') }");
+        assertEval("{ as.Date('2017-12-32') }");
+        assertEval("{ as.Date('2017-13-01') }");
+
     }
 }
