bpo-36959: Fix error messages for invalid ISO format string in _strptime() (GH-13408)

Previously some error messages complained about incompatible
combinations of directives that are not contained in the format string.

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

3 files changed, 61 insertions, 40 deletions

diff --git a/Lib/_strptime.py b/Lib/_strptime.py
index 77ccdc9..798cf9f 100644
--- a/Lib/_strptime.py
+++ b/Lib/_strptime.py
@@ -342,8 +342,6 @@ def _strptime(data_string, format="%a %b %d %H:%M:%S %Y"):
     tz = -1
     gmtoff = None
     gmtoff_fraction = 0
-    # Default to -1 to signify that values not known; not critical to have,
-    # though
     iso_week = week_of_year = None
     week_of_year_start = None
     # weekday and julian defaulted to None so as to signal need to calculate
@@ -470,17 +468,17 @@ def _strptime(data_string, format="%a %b %d %H:%M:%S %Y"):
 
     # Deal with the cases where ambiguities arise
     # don't assume default values for ISO week/year
-    if year is None and iso_year is not None:
-        if iso_week is None or weekday is None:
-            raise ValueError("ISO year directive '%G' must be used with "
-                             "the ISO week directive '%V' and a weekday "
-                             "directive ('%A', '%a', '%w', or '%u').")
+    if iso_year is not None:
         if julian is not None:
             raise ValueError("Day of the year directive '%j' is not "
                              "compatible with ISO year directive '%G'. "
                              "Use '%Y' instead.")
-    elif week_of_year is None and iso_week is not None:
-        if weekday is None:
+        elif iso_week is None or weekday is None:
+            raise ValueError("ISO year directive '%G' must be used with "
+                             "the ISO week directive '%V' and a weekday "
+                             "directive ('%A', '%a', '%w', or '%u').")
+    elif iso_week is not None:
+        if year is None or weekday is None:
             raise ValueError("ISO week directive '%V' must be used with "
                              "the ISO year directive '%G' and a weekday "
                              "directive ('%A', '%a', '%w', or '%u').")
@@ -490,11 +488,12 @@ def _strptime(data_string, format="%a %b %d %H:%M:%S %Y"):
                              "instead.")
 
     leap_year_fix = False
-    if year is None and month == 2 and day == 29:
-        year = 1904  # 1904 is first leap year of 20th century
-        leap_year_fix = True
-    elif year is None:
-        year = 1900
+    if year is None:
+        if month == 2 and day == 29:
+            year = 1904  # 1904 is first leap year of 20th century
+            leap_year_fix = True
+        else:
+            year = 1900
 
     # If we know the week of the year and what day of that week, we can figure
     # out the Julian day of the year.
diff --git a/Lib/test/test_strptime.py b/Lib/test/test_strptime.py
index 810c5a3..05c8afc 100644
--- a/Lib/test/test_strptime.py
+++ b/Lib/test/test_strptime.py
@@ -224,35 +224,55 @@ class StrptimeTests(unittest.TestCase):
             else:
                 self.fail("'%s' did not raise ValueError" % bad_format)
 
+        msg_week_no_year_or_weekday = r"ISO week directive '%V' must be used with " \
+            r"the ISO year directive '%G' and a weekday directive " \
+            r"\('%A', '%a', '%w', or '%u'\)."
+        msg_week_not_compatible = r"ISO week directive '%V' is incompatible with " \
+            r"the year directive '%Y'. Use the ISO year '%G' instead."
+        msg_julian_not_compatible = r"Day of the year directive '%j' is not " \
+            r"compatible with ISO year directive '%G'. Use '%Y' instead."
+        msg_year_no_week_or_weekday = r"ISO year directive '%G' must be used with " \
+            r"the ISO week directive '%V' and a weekday directive " \
+            r"\('%A', '%a', '%w', or '%u'\)."
+
+        locale_time = _strptime.LocaleTime()
+
         # Ambiguous or incomplete cases using ISO year/week/weekday directives
-        # 1. ISO week (%V) is specified, but the year is specified with %Y
-        # instead of %G
-        with self.assertRaises(ValueError):
-            _strptime._strptime("1999 50", "%Y %V")
-        # 2. ISO year (%G) and ISO week (%V) are specified, but weekday is not
-        with self.assertRaises(ValueError):
-            _strptime._strptime("1999 51", "%G %V")
-        # 3. ISO year (%G) and weekday are specified, but ISO week (%V) is not
-        for w in ('A', 'a', 'w', 'u'):
-            with self.assertRaises(ValueError):
-                _strptime._strptime("1999 51","%G %{}".format(w))
-        # 4. ISO year is specified alone (e.g. time.strptime('2015', '%G'))
-        with self.assertRaises(ValueError):
-            _strptime._strptime("2015", "%G")
-        # 5. Julian/ordinal day (%j) is specified with %G, but not %Y
-        with self.assertRaises(ValueError):
-            _strptime._strptime("1999 256", "%G %j")
-        # 6. Invalid ISO weeks
-        invalid_iso_weeks = [
-            "2019-00-1",
-            "2019-54-1",
-            "2021-53-1",
+        subtests = [
+            # 1. ISO week (%V) is specified, but the year is specified with %Y
+            # instead of %G
+            ("1999 50", "%Y %V", msg_week_no_year_or_weekday),
+            ("1999 50 5", "%Y %V %u", msg_week_not_compatible),
+            # 2. ISO year (%G) and ISO week (%V) are specified, but weekday is not
+            ("1999 51", "%G %V", msg_year_no_week_or_weekday),
+            # 3. ISO year (%G) and weekday are specified, but ISO week (%V) is not
+            ("1999 {}".format(locale_time.f_weekday[5]), "%G %A",
+                msg_year_no_week_or_weekday),
+            ("1999 {}".format(locale_time.a_weekday[5]), "%G %a",
+                msg_year_no_week_or_weekday),
+            ("1999 5", "%G %w", msg_year_no_week_or_weekday),
+            ("1999 5", "%G %u", msg_year_no_week_or_weekday),
+            # 4. ISO year is specified alone (e.g. time.strptime('2015', '%G'))
+            ("2015", "%G", msg_year_no_week_or_weekday),
+            # 5. Julian/ordinal day (%j) is specified with %G, but not %Y
+            ("1999 256", "%G %j", msg_julian_not_compatible),
+            ("1999 50 5 256", "%G %V %u %j", msg_julian_not_compatible),
+            # ISO week specified alone
+            ("50", "%V", msg_week_no_year_or_weekday),
+            # ISO year is unspecified, falling back to year
+            ("50 5", "%V %u", msg_week_no_year_or_weekday),
+            # 6. Invalid ISO weeks
+            ("2019-00-1", "%G-%V-%u",
+             "time data '2019-00-1' does not match format '%G-%V-%u'"),
+            ("2019-54-1", "%G-%V-%u",
+             "time data '2019-54-1' does not match format '%G-%V-%u'"),
+            ("2021-53-1", "%G-%V-%u", "Invalid week: 53"),
         ]
-        for invalid_iso_dtstr in invalid_iso_weeks:
-            with self.subTest(invalid_iso_dtstr):
-                with self.assertRaises(ValueError):
-                    _strptime._strptime(invalid_iso_dtstr, "%G-%V-%u")
 
+        for (data_string, format, message) in subtests:
+            with self.subTest(data_string=data_string, format=format):
+                with self.assertRaisesRegex(ValueError, message):
+                    _strptime._strptime(data_string, format)
 
     def test_strptime_exception_context(self):
         # check that this doesn't chain exceptions needlessly (see #17572)
diff --git a/Misc/NEWS.d/next/Library/2019-05-18-15-50-14.bpo-36959.ew6WZ4.rst b/Misc/NEWS.d/next/Library/2019-05-18-15-50-14.bpo-36959.ew6WZ4.rst
new file mode 100644
index 0000000..1ac05a7
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2019-05-18-15-50-14.bpo-36959.ew6WZ4.rst
@@ -0,0 +1,2 @@
+Fix some error messages for invalid ISO format string combinations in ``strptime()`` that referred to directives not contained in the format string.
+Patch by Gordon P. Hemsley.