Don't misinterpret colors as dates (fixes tidyverse#388)

NEWS.md

@@ -1,5 +1,7 @@
 # readxl (development version)
 
+* Colors in number formats are no longer misinterpreted as dates (#388)
+
 # readxl 1.3.0
 
 ## Dependency changes

src/ColSpec.h

@@ -141,36 +141,47 @@ bool inline isDateTime(int id, const std::set<int> custom) {
   return custom.count(id) > 0;
 }
 
+// Adapted from @reviewher https://github.com/tidyverse/readxl/issues/388
+#define CASEI(c) case c: case c | 0x20
+#define CMPLC(i,n) if(x[i+i] | 0x20 == n)
+
 inline bool isDateFormat(std::string x) {
-  // TO FIX? So far no bug reports due to this.
-  // Logic below is too simple. For example, it deems this format string a date:
-  // "$"#,##0_);[Red]\("$"#,##0\)
-  // because of the `d` in `[Red]`
-  //
-  // Ideally this can wait until we are using something like
+  // Ideally this would use something like
   // https://github.com/WizardMac/TimeFormatStrings
   // which presumably offers fancier ways to analyze format codes.
-  for (size_t i = 0; i < x.size(); ++i) {
-    switch (x[i]) {
-    case 'd':
-    case 'D':
-    case 'm': // 'mm' for minutes
-    case 'M':
-    case 'y':
-    case 'Y':
-    case 'h': // 'hh'
-    case 'H':
-    case 's': // 'ss'
-    case 'S':
-      return true;
-    default:
+  char escaped = 0;
+  char bracket = 0;
+  for (size_t i = 0; i < x.size(); ++i) switch (x[i]) {
+    CASEI('D'):
+    CASEI('E'):
+    CASEI('H'):
+    CASEI('M'):
+    CASEI('S'):
+    CASEI('Y'):
+      if(!escaped && !bracket) return true;
       break;
-    }
+    case '"':
+      escaped = 1 - escaped; break;
+    case '\\': ++i; break;
+    case '[': if(!escaped) bracket = 1; break;
+    case ']': if(!escaped) bracket = 0; break;
+    CASEI('G'):
+      if(i + 6 < x.size())
+      CMPLC(1,'e')
+      CMPLC(2,'n')
+      CMPLC(3,'e')
+      CMPLC(4,'r')
+      CMPLC(5,'a')
+      CMPLC(6,'l')
+        return false;
   }
 
   return false;
 }
 
+#undef CMPLC
+#undef CASEI
+
 inline std::vector<ColType> recycleTypes(std::vector<ColType> types,
                                          int ncol) {
   if (types.size() == 1) {

tests/testthat/test-dates.R

@@ -51,3 +51,31 @@ test_that("we get correct dates prior to March 1, 1900, in 1900 date system", {
   expect_identical(df$dttm[!leap_day], dttms[!leap_day])
   expect_true(is.na(df$dttm[leap_day]))
 })
+
+test_that("colors in number formats aren't misinterpreted as dates", {
+  expect_warning(
+    read_xlsx(
+      test_sheet("color-date.xlsx"),
+      col_names = "X1",
+      col_types = "numeric"
+    ),
+    NA # rather than "Expecting numeric in A1 / R1C1: got a date" etc.
+  )
+  expect_warning(
+    read_xlsx(
+      test_sheet("color-date-lowercase.xlsx"),
+      col_names = "X1",
+      col_types = "numeric"
+    ),
+    NA # rather than "Expecting numeric in A1 / R1C1: got a date"
+  )
+  expect_warning(
+    read_xls(
+      test_sheet("color-date.xls"),
+      col_names = "X1",
+      col_types = "numeric"
+    ),
+    NA # rather than "Expecting numeric in A1 / R1C1: got a date"
+  )
+})
+