From 74cceba5871e83e77a23536d8b64ff8888862dd3 Mon Sep 17 00:00:00 2001 From: Stephen Hicks Date: Wed, 29 Jan 2025 18:33:23 -0500 Subject: [PATCH] feat(common): throw error for suspicious date patterns (#59798) Adjusts the date pipe and formatDate function to detect suspicious usages of the week-numbering year formatter without including the week number, as this is often confused for the calendar year and likely to result in incorrect results near New Years, meaning that those issues aren't typically caught during development. This commit starts throwing a development-only error to reveal this issue right away. BREAKING CHANGE: Using the `Y` formatter (week-numbering year) without also including `w` (week number) is now detected as suspicious date pattern, as `y` is typically intended. PR Close #59798 --- goldens/public-api/common/errors.api.md | 2 + packages/common/src/errors.ts | 3 ++ packages/common/src/i18n/format_date.ts | 33 +++++++++++++++ packages/common/test/i18n/format_date_spec.ts | 40 +++++++++++++------ 4 files changed, 65 insertions(+), 13 deletions(-) diff --git a/goldens/public-api/common/errors.api.md b/goldens/public-api/common/errors.api.md index 5622f253600ab..b60df1fa20c54 100644 --- a/goldens/public-api/common/errors.api.md +++ b/goldens/public-api/common/errors.api.md @@ -37,6 +37,8 @@ export const enum RuntimeErrorCode { // (undocumented) REQUIRED_INPUT_MISSING = 2954, // (undocumented) + SUSPICIOUS_DATE_FORMAT = 2300, + // (undocumented) TOO_MANY_PRELOADED_IMAGES = 2961, // (undocumented) TOO_MANY_PRIORITY_ATTRIBUTES = 2966, diff --git a/packages/common/src/errors.ts b/packages/common/src/errors.ts index c5e1765ca250f..3f5bf97605909 100644 --- a/packages/common/src/errors.ts +++ b/packages/common/src/errors.ts @@ -21,6 +21,9 @@ export const enum RuntimeErrorCode { // NgForOf errors NG_FOR_MISSING_DIFFER = -2200, + // I18n errors + SUSPICIOUS_DATE_FORMAT = 2300, + // Keep 2800 - 2900 for Http Errors. // Image directive errors diff --git a/packages/common/src/i18n/format_date.ts b/packages/common/src/i18n/format_date.ts index 34a3e3bd2ed1c..10c8f9bbd8094 100644 --- a/packages/common/src/i18n/format_date.ts +++ b/packages/common/src/i18n/format_date.ts @@ -6,6 +6,11 @@ * found in the LICENSE file at https://angular.dev/license */ +import { + ɵRuntimeError as RuntimeError, + ɵformatRuntimeError as formatRuntimeError, +} from '@angular/core'; + import { FormatWidth, FormStyle, @@ -24,6 +29,7 @@ import { Time, TranslationWidth, } from './locale_data_api'; +import {RuntimeErrorCode} from '../errors'; export const ISO8601_DATE_REGEX = /^(\d{4,})-?(\d\d)-?(\d\d)(?:T(\d\d)(?::?(\d\d)(?::?(\d\d)(?:\.(\d+))?)?)?(Z|([+-])(\d\d):?(\d\d))?)?$/; @@ -104,6 +110,10 @@ export function formatDate( } } + if (typeof ngDevMode === 'undefined' || ngDevMode) { + assertValidDateFormat(parts); + } + let dateTimezoneOffset = date.getTimezoneOffset(); if (timezone) { dateTimezoneOffset = timezoneToOffset(timezone, dateTimezoneOffset); @@ -123,6 +133,29 @@ export function formatDate( return text; } +/** + * Asserts that the given date format is free from common mistakes. Throws an + * error if one is found (except for the case of all "Y", in which case we just + * log a warning). This should only be called in development mode. + */ +function assertValidDateFormat(parts: string[]) { + if (parts.some((part) => /^Y+$/.test(part)) && !parts.some((part) => /^w+$/.test(part))) { + // "Y" indicates "week-based year", which differs from the actual calendar + // year for a few days around Jan 1 most years. Unless "w" is also + // present (e.g. a date like "2024-W52") this is likely a mistake. Users + // probably meant "y" instead. + const message = `Suspicious use of week-based year "Y" in date pattern "${parts.join( + '', + )}". Did you mean to use calendar year "y" instead?`; + if (parts.length === 1) { + // NOTE: allow "YYYY" with just a warning, since it's used in tests. + console.error(formatRuntimeError(RuntimeErrorCode.SUSPICIOUS_DATE_FORMAT, message)); + } else { + throw new RuntimeError(RuntimeErrorCode.SUSPICIOUS_DATE_FORMAT, message); + } + } +} + /** * Create a new Date object with the given date value, and the time set to midnight. * diff --git a/packages/common/test/i18n/format_date_spec.ts b/packages/common/test/i18n/format_date_spec.ts index 293ccea097316..d5d35fc8dfb15 100644 --- a/packages/common/test/i18n/format_date_spec.ts +++ b/packages/common/test/i18n/format_date_spec.ts @@ -246,6 +246,14 @@ describe('Format date', () => { BBBBB: 'at night', }; + // Suppress console warnings for 'YYYY' patterns. + const consoleError = console.error; + spyOn(console, 'error').and.callFake((...args: unknown[]) => { + if (!/Suspicious use of week-based year/.test(String(args))) { + consoleError(...args); + } + }); + Object.keys(dateFixtures).forEach((pattern: string) => { expectDateFormatAs(date, pattern, dateFixtures[pattern]); }); @@ -450,17 +458,23 @@ describe('Format date', () => { // https://github.com/angular/angular/issues/38739 it('should return correct ISO 8601 week-numbering year for dates close to year end/beginning', () => { - expect(formatDate('2013-12-27', 'YYYY', 'en')).toEqual('2013'); - expect(formatDate('2013-12-29', 'YYYY', 'en')).toEqual('2013'); - expect(formatDate('2013-12-31', 'YYYY', 'en')).toEqual('2014'); + expect(formatDate('2013-12-27', `YYYY 'W'ww`, 'en')).toEqual('2013 W52'); + expect(formatDate('2013-12-29', `YYYY 'W'ww`, 'en')).toEqual('2013 W52'); + expect(formatDate('2013-12-31', `YYYY 'W'ww`, 'en')).toEqual('2014 W01'); // Dec. 31st is a Sunday, last day of the last week of 2023 - expect(formatDate('2023-12-31', 'YYYY', 'en')).toEqual('2023'); + expect(formatDate('2023-12-31', `YYYY 'W'ww`, 'en')).toEqual('2023 W52'); - expect(formatDate('2010-01-02', 'YYYY', 'en')).toEqual('2009'); - expect(formatDate('2010-01-04', 'YYYY', 'en')).toEqual('2010'); - expect(formatDate('0049-01-01', 'YYYY', 'en')).toEqual('0048'); - expect(formatDate('0049-01-04', 'YYYY', 'en')).toEqual('0049'); + expect(formatDate('2010-01-02', `YYYY 'W'ww`, 'en')).toEqual('2009 W53'); + expect(formatDate('2010-01-04', `YYYY 'W'ww`, 'en')).toEqual('2010 W01'); + expect(formatDate('0049-01-01', `YYYY 'W'ww`, 'en')).toEqual('0048 W53'); + expect(formatDate('0049-01-04', `YYYY 'W'ww`, 'en')).toEqual('0049 W01'); + }); + + it('should throw an error when using YYYY incorrectly', () => { + expect(() => formatDate('2013-12-31', `YYYY/MM/dd`, ɵDEFAULT_LOCALE_ID)).toThrowError( + /.*Suspicious use of week-based year "Y".*/, + ); }); // https://github.com/angular/angular/issues/53813 @@ -480,11 +494,11 @@ describe('Format date', () => { // https://github.com/angular/angular/issues/40377 it('should format date with year between 0 and 99 correctly', () => { - expect(formatDate('0098-01-11', 'YYYY', ɵDEFAULT_LOCALE_ID)).toEqual('0098'); - expect(formatDate('0099-01-11', 'YYYY', ɵDEFAULT_LOCALE_ID)).toEqual('0099'); - expect(formatDate('0100-01-11', 'YYYY', ɵDEFAULT_LOCALE_ID)).toEqual('0100'); - expect(formatDate('0001-01-11', 'YYYY', ɵDEFAULT_LOCALE_ID)).toEqual('0001'); - expect(formatDate('0000-01-11', 'YYYY', ɵDEFAULT_LOCALE_ID)).toEqual('0000'); + expect(formatDate('0098-01-11', `YYYY 'W'ww`, ɵDEFAULT_LOCALE_ID)).toEqual('0098 W02'); + expect(formatDate('0099-01-11', `YYYY 'W'ww`, ɵDEFAULT_LOCALE_ID)).toEqual('0099 W02'); + expect(formatDate('0100-01-11', `YYYY 'W'ww`, ɵDEFAULT_LOCALE_ID)).toEqual('0100 W02'); + expect(formatDate('0001-01-11', `YYYY 'W'ww`, ɵDEFAULT_LOCALE_ID)).toEqual('0001 W02'); + expect(formatDate('0000-01-11', `YYYY 'W'ww`, ɵDEFAULT_LOCALE_ID)).toEqual('0000 W02'); }); // https://github.com/angular/angular/issues/26922