Repository: harrisiirak/cron-parser (ts)

Issue:
parseExpression() loses ? information
Thank you for producing this very useful library. I think this is a bug, but I am happy to be corrected.

I have a cron expression with a question mark in it:
`0 0 17 * * ?`

No problems parsing it, but if I stringify it without making any changes, I expect to get the same cron. But instead the `?` is converted to a `*` and it becomes:
`0 0 17 * * *`

````js
const result = parseExpression("0 0 17 * * ?");
console.log(result.stringify(true));  // prints "0 0 17 * * *"
````

How can I parse such an expression, and stringify it later to get back the original specification?

Pull request description:
Extended serialization for cron fields
This PR improves several aspects of expression serialization:

* Supports `nth-day` (`#`) serialization
* Retains `?` in serialized cron expressions

It also includes some minor **BREAKING CHANGES**:

* The `CronField` constructor signature has changed. Instead of the `wildcard` boolean argument, it now takes a full-fledged `CronFieldOptions` object.
* The `nthDayOfWeek` option (which was undocumented or inconsistently used) has been removed from `CronExpressionOptions`.

These changes shouldn't affect most users, but caution is advised when working directly with `CronField`-related APIs.

Fixes #358 and #349


Patch:
diff --git a/src/CronExpression.ts b/src/CronExpression.ts
index 9bafc34..6335d6c 100644
--- a/src/CronExpression.ts
+++ b/src/CronExpression.ts
@@ -7,7 +7,6 @@ export type CronExpressionOptions = {
   endDate?: Date | string | number | CronDate;
   startDate?: Date | string | number | CronDate;
   tz?: string;
-  nthDayOfWeek?: number;
   expression?: string;
   hashSeed?: string;
   strict?: boolean;
@@ -27,7 +26,6 @@ export class CronExpression {
   #currentDate: CronDate;
   readonly #startDate: CronDate | null;
   readonly #endDate: CronDate | null;
-  readonly #nthDayOfWeek: number;
   readonly #fields: CronFieldCollection;
 
   /**
@@ -42,7 +40,6 @@ export class CronExpression {
     this.#currentDate = new CronDate(options.currentDate, this.#tz);
     this.#startDate = options.startDate ? new CronDate(options.startDate, this.#tz) : null;
     this.#endDate = options.endDate ? new CronDate(options.endDate, this.#tz) : null;
-    this.#nthDayOfWeek = options.nthDayOfWeek || 0;
     this.#fields = fields;
   }
 
@@ -236,9 +233,9 @@ export class CronExpression {
     }
 
     // Check nth day of week if specified
-    if (this.#nthDayOfWeek > 0) {
+    if (this.#fields.dayOfWeek.nthDay > 0) {
       const weekInMonth = Math.ceil(dt.getDate() / 7);
-      if (weekInMonth !== this.#nthDayOfWeek) {
+      if (weekInMonth !== this.#fields.dayOfWeek.nthDay) {
         return false;
       }
     }
@@ -367,7 +364,9 @@ export class CronExpression {
         currentDate.applyDateOperation(dateMathVerb, TimeUnit.Day, this.#fields.hour.values.length);
         continue;
       }
-      if (!(this.#nthDayOfWeek <= 0 || Math.ceil(currentDate.getDate() / 7) === this.#nthDayOfWeek)) {
+      if (
+        !(this.#fields.dayOfWeek.nthDay <= 0 || Math.ceil(currentDate.getDate() / 7) === this.#fields.dayOfWeek.nthDay)
+      ) {
         currentDate.applyDateOperation(dateMathVerb, TimeUnit.Day, this.#fields.hour.values.length);
         continue;
       }
diff --git a/src/CronExpressionParser.ts b/src/CronExpressionParser.ts
index 59ed100..19535d2 100644
--- a/src/CronExpressionParser.ts
+++ b/src/CronExpressionParser.ts
@@ -142,14 +142,14 @@ export class CronExpressionParser {
     ) as DayOfWeekRange[];
 
     const fields = new CronFieldCollection({
-      second: new CronSecond(second, ['*', '?'].includes(rawFields.second)),
-      minute: new CronMinute(minute, ['*', '?'].includes(rawFields.minute)),
-      hour: new CronHour(hour, ['*', '?'].includes(rawFields.hour)),
-      dayOfMonth: new CronDayOfMonth(dayOfMonth, ['*', '?'].includes(rawFields.dayOfMonth)),
-      month: new CronMonth(month, ['*', '?'].includes(rawFields.month)),
-      dayOfWeek: new CronDayOfWeek(dayOfWeek, ['*', '?'].includes(rawFields.dayOfWeek)),
+      second: new CronSecond(second, { rawValue: rawFields.second }),
+      minute: new CronMinute(minute, { rawValue: rawFields.minute }),
+      hour: new CronHour(hour, { rawValue: rawFields.hour }),
+      dayOfMonth: new CronDayOfMonth(dayOfMonth, { rawValue: rawFields.dayOfMonth }),
+      month: new CronMonth(month, { rawValue: rawFields.month }),
+      dayOfWeek: new CronDayOfWeek(dayOfWeek, { rawValue: rawFields.dayOfWeek, nthDayOfWeek }),
     });
-    return new CronExpression(fields, { ...options, expression, currentDate, nthDayOfWeek });
+    return new CronExpression(fields, { ...options, expression, currentDate });
   }
 
   /**
diff --git a/src/CronFieldCollection.ts b/src/CronFieldCollection.ts
index 92d40e7..3d62ecc 100644
--- a/src/CronFieldCollection.ts
+++ b/src/CronFieldCollection.ts
@@ -308,21 +308,21 @@ export class CronFieldCollection {
 
   /**
    * Handles a single range.
+   * @param {CronField} field - The cron field to stringify
    * @param {FieldRange} range {start: number, end: number, step: number, count: number} The range to handle.
-   * @param {number} min The minimum value for the field.
    * @param {number} max The maximum value for the field.
    * @returns {string | null} The stringified range or null if it cannot be stringified.
    * @private
    */
-  static #handleSingleRange(range: FieldRange, min: number, max: number): string | null {
+  static #handleSingleRange(field: CronField, range: FieldRange, max: number): string | null {
     const step = range.step;
     if (!step) {
       return null;
     }
-    if (step === 1 && range.start === min && range.end && range.end >= max) {
-      return '*';
+    if (step === 1 && range.start === field.min && range.end && range.end >= max) {
+      return field.hasQuestionMarkChar ? '?' : '*';
     }
-    if (step !== 1 && range.start === min && range.end && range.end >= max - step + 1) {
+    if (step !== 1 && range.start === field.min && range.end && range.end >= max - step + 1) {
       return `*/${step}`;
     }
     return null;
@@ -388,18 +388,23 @@ export class CronFieldCollection {
     if (field instanceof CronDayOfMonth) {
       max = this.#month.values.length === 1 ? CronMonth.daysInMonth[this.#month.values[0] - 1] : field.max;
     }
-    const ranges = CronFieldCollection.compactField(values);
 
+    const ranges = CronFieldCollection.compactField(values);
     if (ranges.length === 1) {
-      const singleRangeResult = CronFieldCollection.#handleSingleRange(ranges[0], field.min, max);
+      const singleRangeResult = CronFieldCollection.#handleSingleRange(field, ranges[0], max);
       if (singleRangeResult) {
         return singleRangeResult;
       }
     }
     return ranges
-      .map((range) =>
-        range.count === 1 ? range.start.toString() : CronFieldCollection.#handleMultipleRanges(range, max),
-      )
+      .map((range) => {
+        const value =
+          range.count === 1 ? range.start.toString() : CronFieldCollection.#handleMultipleRanges(range, max);
+        if (field instanceof CronDayOfWeek && field.nthDay > 0) {
+          return `${value}#${field.nthDay}`;
+        }
+        return value;
+      })
       .join(',');
   }
 
diff --git a/src/fields/CronDayOfMonth.ts b/src/fields/CronDayOfMonth.ts
index c827c7b..fe0c9b6 100644
--- a/src/fields/CronDayOfMonth.ts
+++ b/src/fields/CronDayOfMonth.ts
@@ -1,4 +1,4 @@
-import { CronField } from './CronField';
+import { CronField, CronFieldOptions } from './CronField';
 import { CronChars, CronMax, CronMin, DayOfMonthRange } from './types';
 
 const MIN_DAY = 1;
@@ -28,11 +28,11 @@ export class CronDayOfMonth extends CronField {
   /**
    * CronDayOfMonth constructor. Initializes the "day of the month" field with the provided values.
    * @param {DayOfMonthRange[]} values - Values for the "day of the month" field
-   * @param {boolean} [wildcard=false] - Whether this field is a wildcard
+   * @param {CronFieldOptions} [options] - Options provided by the parser
    * @throws {Error} if validation fails
    */
-  constructor(values: DayOfMonthRange[], wildcard = false) {
-    super(values, wildcard);
+  constructor(values: DayOfMonthRange[], options?: CronFieldOptions) {
+    super(values, options);
     this.validate();
   }
 
diff --git a/src/fields/CronDayOfWeek.ts b/src/fields/CronDayOfWeek.ts
index 7117d2f..ebd1b75 100644
--- a/src/fields/CronDayOfWeek.ts
+++ b/src/fields/CronDayOfWeek.ts
@@ -1,4 +1,4 @@
-import { CronField } from './CronField';
+import { CronField, CronFieldOptions } from './CronField';
 import { CronChars, CronMax, CronMin, DayOfWeekRange } from './types';
 
 const MIN_DAY = 0;
@@ -30,10 +30,10 @@ export class CronDayOfWeek extends CronField {
   /**
    * CronDayOfTheWeek constructor. Initializes the "day of the week" field with the provided values.
    * @param {DayOfWeekRange[]} values - Values for the "day of the week" field
-   * @param {boolean} [wildcard=false] - Whether this field is a wildcard
+   * @param {CronFieldOptions} [options] - Options provided by the parser
    */
-  constructor(values: DayOfWeekRange[], wildcard = false) {
-    super(values, wildcard);
+  constructor(values: DayOfWeekRange[], options?: CronFieldOptions) {
+    super(values, options);
     this.validate();
   }
 
@@ -44,4 +44,13 @@ export class CronDayOfWeek extends CronField {
   get values(): DayOfWeekRange[] {
     return super.values as DayOfWeekRange[];
   }
+
+  /**
+   * Returns the nth day of the week if specified in the cron expression.
+   * This is used for the '#' character in the cron expression.
+   * @returns {number} The nth day of the week (1-5) or 0 if not specified.
+   */
+  get nthDay(): number {
+    return this.options.nthDayOfWeek ?? 0;
+  }
 }
diff --git a/src/fields/CronField.ts b/src/fields/CronField.ts
index d3f167a..bda2d92 100644
--- a/src/fields/CronField.ts
+++ b/src/fields/CronField.ts
@@ -1,10 +1,29 @@
 import { CronChars, CronConstraints, CronFieldType, CronMax, CronMin } from './types';
 
+/**
+ * Represents the serialized form of a cron field.
+ * @typedef {Object} SerializedCronField
+ * @property {boolean} wildcard - Indicates if the field is a wildcard.
+ * @property {(number|string)[]} values - The values of the field.
+ */
 export type SerializedCronField = {
   wildcard: boolean;
   values: (number | string)[];
 };
 
+/**
+ * Represents the options for a cron field.
+ * @typedef {Object} CronFieldOptions
+ * @property {string} rawValue - The raw value of the field.
+ * @property {boolean} [wildcard] - Indicates if the field is a wildcard.
+ * @property {number} [nthDayOfWeek] - The nth day of the week.
+ */
+export type CronFieldOptions = {
+  rawValue?: string;
+  wildcard?: boolean;
+  nthDayOfWeek?: number;
+};
+
 /**
  * Represents a field within a cron expression.
  * This is a base class and should not be instantiated directly.
@@ -12,9 +31,13 @@ export type SerializedCronField = {
  */
 export abstract class CronField {
   readonly #hasLastChar: boolean = false;
+  readonly #hasQuestionMarkChar: boolean = false;
+
   readonly #wildcard: boolean = false;
   readonly #values: (number | string)[] = [];
 
+  protected readonly options: CronFieldOptions & { rawValue: string } = { rawValue: '' };
+
   /**
    * Returns the minimum value allowed for this field.
    */
@@ -43,7 +66,7 @@ export abstract class CronField {
    * Returns the regular expression used to validate this field.
    */
   static get validChars(): RegExp {
-    return /^[,*\dH/-]+$/;
+    return /^[?,*\dH/-]+$/;
   }
 
   /**
@@ -56,25 +79,27 @@ export abstract class CronField {
   /**
    * CronField constructor. Initializes the field with the provided values.
    * @param {number[] | string[]} values - Values for this field
-   * @param {boolean} [wildcard=false] - Whether this field is a wildcard
+   * @param {CronFieldOptions} [options] - Options provided by the parser
    * @throws {TypeError} if the constructor is called directly
    * @throws {Error} if validation fails
    */
-  protected constructor(
-    values: (number | string)[],
-    /* istanbul ignore next - we always pass a value */ wildcard = false,
-  ) {
+  protected constructor(values: (number | string)[], options: CronFieldOptions = { rawValue: '' }) {
     if (!Array.isArray(values)) {
       throw new Error(`${this.constructor.name} Validation error, values is not an array`);
     }
     if (!(values.length > 0)) {
       throw new Error(`${this.constructor.name} Validation error, values contains no values`);
     }
+
+    /* istanbul ignore next */
+    this.options = {
+      ...options,
+      rawValue: options.rawValue ?? '',
+    };
     this.#values = values.sort(CronField.sorter);
-    this.#wildcard = wildcard;
-    this.#hasLastChar = values.some((expression: number | string) => {
-      return typeof expression === 'string' && expression.indexOf('L') >= 0;
-    });
+    this.#wildcard = this.options.wildcard !== undefined ? this.options.wildcard : this.#isWildcardValue();
+    this.#hasLastChar = this.options.rawValue.includes('L');
+    this.#hasQuestionMarkChar = this.options.rawValue.includes('?');
   }
 
   /**
@@ -112,6 +137,14 @@ export abstract class CronField {
     return this.#hasLastChar;
   }
 
+  /**
+   * Indicates whether this field has a "question mark" character.
+   * @returns {boolean}
+   */
+  get hasQuestionMarkChar(): boolean {
+    return this.#hasQuestionMarkChar;
+  }
+
   /**
    * Indicates whether this field is a wildcard.
    * @returns {boolean}
@@ -144,7 +177,6 @@ export abstract class CronField {
 
   /**
    * Serializes the field to an object.
-   * @todo This is really only for debugging, should it be removed?
    * @returns {SerializedCronField}
    */
   serialize(): SerializedCronField {
@@ -179,4 +211,19 @@ export abstract class CronField {
       throw new Error(`${this.constructor.name} Validation error, duplicate values found: ${duplicate}`);
     }
   }
+
+  /**
+   * Determines if the field is a wildcard based on the values.
+   * When options.rawValue is not empty, it checks if the raw value is a wildcard, otherwise it checks if all values in the range are included.
+   * @returns {boolean}
+   */
+  #isWildcardValue(): boolean {
+    if (this.options.rawValue.length > 0) {
+      return ['*', '?'].includes(this.options.rawValue);
+    }
+
+    return Array.from({ length: this.max - this.min + 1 }, (_, i) => i + this.min).every((value) =>
+      this.#values.includes(value),
+    );
+  }
 }
diff --git a/src/fields/CronHour.ts b/src/fields/CronHour.ts
index 9d73128..b1719ec 100644
--- a/src/fields/CronHour.ts
+++ b/src/fields/CronHour.ts
@@ -1,4 +1,4 @@
-import { CronField } from './CronField';
+import { CronField, CronFieldOptions } from './CronField';
 import { CronChars, CronMax, CronMin, HourRange } from './types';
 
 const MIN_HOUR = 0;
@@ -26,10 +26,10 @@ export class CronHour extends CronField {
   /**
    * CronHour constructor. Initializes the "hour" field with the provided values.
    * @param {HourRange[]} values - Values for the "hour" field
-   * @param {boolean} [wildcard=false] - Whether this field is a wildcard
+   * @param {CronFieldOptions} [options] - Options provided by the parser
    */
-  constructor(values: HourRange[], wildcard = false) {
-    super(values, wildcard);
+  constructor(values: HourRange[], options?: CronFieldOptions) {
+    super(values, options);
     this.validate();
   }
 
diff --git a/src/fields/CronMinute.ts b/src/fields/CronMinute.ts
index ef293aa..bfe53bc 100644
--- a/src/fields/CronMinute.ts
+++ b/src/fields/CronMinute.ts
@@ -1,4 +1,4 @@
-import { CronField } from './CronField';
+import { CronField, CronFieldOptions } from './CronField';
 import { CronChars, CronMax, CronMin, SixtyRange } from './types';
 
 const MIN_MINUTE = 0;
@@ -26,10 +26,10 @@ export class CronMinute extends CronField {
   /**
    * CronSecond constructor. Initializes the "second" field with the provided values.
    * @param {SixtyRange[]} values - Values for the "second" field
-   * @param {boolean} [wildcard=false] - Whether this field is a wildcard
+   * @param {CronFieldOptions} [options] - Options provided by the parser
    */
-  constructor(values: SixtyRange[], wildcard = false) {
-    super(values, wildcard);
+  constructor(values: SixtyRange[], options?: CronFieldOptions) {
+    super(values, options);
     this.validate();
   }
 
diff --git a/src/fields/CronMonth.ts b/src/fields/CronMonth.ts
index 75c2460..0476a85 100644
--- a/src/fields/CronMonth.ts
+++ b/src/fields/CronMonth.ts
@@ -1,5 +1,5 @@
 import { DAYS_IN_MONTH } from '../CronDate';
-import { CronField } from './CronField';
+import { CronField, CronFieldOptions } from './CronField';
 import { CronChars, CronMax, CronMin, MonthRange } from './types';
 
 const MIN_MONTH = 1;
@@ -31,10 +31,10 @@ export class CronMonth extends CronField {
   /**
    * CronDayOfMonth constructor. Initializes the "day of the month" field with the provided values.
    * @param {MonthRange[]} values - Values for the "day of the month" field
-   * @param {boolean} [wildcard=false] - Whether this field is a wildcard
+   * @param {CronFieldOptions} [options] - Options provided by the parser
    */
-  constructor(values: MonthRange[], wildcard = false) {
-    super(values, wildcard);
+  constructor(values: MonthRange[], options?: CronFieldOptions) {
+    super(values, options);
     this.validate();
   }
 
diff --git a/src/fields/CronSecond.ts b/src/fields/CronSecond.ts
index 81b9c59..e97102b 100644
--- a/src/fields/CronSecond.ts
+++ b/src/fields/CronSecond.ts
@@ -1,5 +1,5 @@
 import { CronChars, CronMax, CronMin, SixtyRange } from './types';
-import { CronField } from './CronField';
+import { CronField, CronFieldOptions } from './CronField';
 
 const MIN_SECOND = 0;
 const MAX_SECOND = 59;
@@ -24,10 +24,10 @@ export class CronSecond extends CronField {
   /**
    * CronSecond constructor. Initializes the "second" field with the provided values.
    * @param {SixtyRange[]} values - Values for the "second" field
-   * @param {boolean} [wildcard=false] - Whether this field is a wildcard
+   * @param {CronFieldOptions} [options] - Options provided by the parser
    */
-  constructor(values: SixtyRange[], wildcard = false) {
-    super(values, wildcard);
+  constructor(values: SixtyRange[], options?: CronFieldOptions) {
+    super(values, options);
     this.validate();
   }
 


Test patch:
diff --git a/tests/CronExpression.test.ts b/tests/CronExpression.test.ts
index 1e6165e..0f4be23 100644
--- a/tests/CronExpression.test.ts
+++ b/tests/CronExpression.test.ts
@@ -266,6 +266,28 @@ describe('CronExpression', () => {
       expect(str).toEqual(expected);
     });
 
+    test('stringify cron expression with day of week #', () => {
+      const expressions: string[] = [];
+      for (let week = 1; week <= 5; week++) {
+        for (let day = 0; day <= 6; day++) {
+          expressions.push(`* * * * * ${day}#${week}`);
+        }
+      }
+
+      for (const expression of expressions) {
+        const interval = CronExpressionParser.parse(expression, {});
+        const str = interval.stringify(true);
+        expect(str).toEqual(expression);
+      }
+    });
+
+    test('stringify cron expression with ?', () => {
+      const expected = '? ? * * ?';
+      const interval = CronExpressionParser.parse(expected, {});
+      const str = interval.stringify();
+      expect(str).toEqual(expected);
+    });
+
     test('stringify cron expression with wildcard day of month and single month value', () => {
       const expected = '* * * 4 *';
       const interval = CronExpressionParser.parse(expected, {});
diff --git a/tests/CronExpressionParser.test.ts b/tests/CronExpressionParser.test.ts
index 857c9ab..4adf05b 100644
--- a/tests/CronExpressionParser.test.ts
+++ b/tests/CronExpressionParser.test.ts
@@ -1755,7 +1755,7 @@ describe('CronExpressionParser', () => {
           { expression: '* * * * H H', expected: '* * * * 8 0' },
           { expression: 'H H H H H H', expected: '5 34 15 12 8 0' },
           { expression: 'H/5 * * * * *', expected: '5/5 * * * * *' },
-          { expression: '* * * * * H#1', expected: '* * * * * 0' },
+          { expression: '* * * * * H#1', expected: '* * * * * 0#1' },
         ];
 
         for (const { expression, expected } of expressions) {
diff --git a/tests/CronField.test.ts b/tests/CronField.test.ts
index 0593930..948bd72 100644
--- a/tests/CronField.test.ts
+++ b/tests/CronField.test.ts
@@ -1,19 +1,21 @@
-import {
-  CronDayOfMonth,
-  CronDayOfWeek,
-  CronFieldCollection,
-  CronHour,
-  CronMinute,
-  CronMonth,
-  CronSecond,
-  DayOfMonthRange,
-  DayOfWeekRange,
-  HourRange,
-  MonthRange,
-  SixtyRange,
-} from '../src';
+import { CronDayOfWeek, CronSecond, SixtyRange } from '../src';
 
 describe('CronField', () => {
+  describe('wildcard detection', () => {
+    test('should detect wildcard based on input values', () => {
+      const secondField = new CronSecond([0, 1, 2, 3, 4, 5, 6, 7]);
+      expect(secondField.isWildcard).toBeFalsy();
+
+      const dayOfWeekField = new CronDayOfWeek([0, 1, 2, 3, 4, 5, 6, 7]);
+      expect(dayOfWeekField.isWildcard).toBeTruthy();
+    });
+
+    test('should detect wildcard based options override', () => {
+      const dayOfWeekField = new CronDayOfWeek([0, 1, 2, 3, 4, 5, 6], { rawValue: '', wildcard: true });
+      expect(dayOfWeekField.isWildcard).toBeTruthy();
+    });
+  });
+
   describe('validate', () => {
     test('should throw error when values is not an array', () => {
       expect(() => new CronSecond(0 as any)).toThrow('CronSecond Validation error, values is not an array');
diff --git a/tests/CronFieldCollection.test.ts b/tests/CronFieldCollection.test.ts
index cca6cba..68c0e18 100644
--- a/tests/CronFieldCollection.test.ts
+++ b/tests/CronFieldCollection.test.ts
@@ -38,7 +38,7 @@ describe('CronFieldCollection', () => {
     test('should serialize collection', () => {
       const expected = {
         second: {
-          wildcard: false,
+          wildcard: true,
           values: Array.from(Array(60).keys()),
         },
         minute: {