Merge pull request #100 from github/thyeggman/cronstrue-hover

Produce hover sentence for cron expressions

actions-languageservice/src/context-providers/github.ts

@@ -99,7 +99,6 @@ function getEventContext(workflowContext: WorkflowContext): ExpressionData {
 function merge(d: data.Dictionary, toAdd: Object): data.Dictionary {
   for (const [key, value] of Object.entries(toAdd)) {
     if (value && typeof value === "object" && !d.get(key)) {
-
       if (!Array.isArray(value) && Object.entries(value).length === 0) {
         // Allow an empty object to be any value
         d.add(key, new data.Null());

actions-languageservice/src/hover.test.ts

@@ -96,6 +96,39 @@ jobs:
     expect(result?.contents).toEqual("Runs your workflow when you push a commit or tag.");
   });
 
+  it("on a cron schedule", async () => {
+    const input = `on:
+  schedule:
+    - cron: '0,30 0|,12 * * *'
+`;
+    const result = await hover(...getPositionFromCursor(input));
+    expect(result).not.toBeUndefined();
+    expect(result?.contents).toEqual(
+      "Runs at 0 and 30 minutes past the hour, at 00:00 and 12:00\n\n" +
+        "Actions schedules run at most every 5 minutes. " +
+        "[Learn more](https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#onschedule)"
+    );
+  });
+
+  it("on a cron mapping key", async () => {
+    const input = `on:
+  schedule:
+    - c|ron: '0 0 * * *'
+`;
+    const result = await hover(...getPositionFromCursor(input));
+    expect(result).not.toBeUndefined();
+    expect(result?.contents).toEqual("");
+  });
+
+  it("on an invalid cron schedule", async () => {
+    const input = `on:
+  schedule:
+    - cron: '0 0 |* * * * *'
+`;
+    const result = await hover(...getPositionFromCursor(input));
+    expect(result).not.toBeUndefined();
+    expect(result?.contents).toEqual("");
+  });
 
   it("shows context inherited from parent nodes", async () => {
     const input = `

actions-languageservice/src/hover.ts

@@ -1,7 +1,10 @@
 import {convertWorkflowTemplate, parseWorkflow, ParseWorkflowResult} from "@github/actions-workflow-parser";
 import {ErrorPolicy} from "@github/actions-workflow-parser/model/convert";
 import {TemplateToken} from "@github/actions-workflow-parser/templates/tokens/template-token";
+import {TokenResult} from "./utils/find-token";
 import {File} from "@github/actions-workflow-parser/workflows/file";
+import {isString} from "@github/actions-workflow-parser/templates/tokens/type-guards";
+import {StringToken} from "@github/actions-workflow-parser/templates/tokens/string-token";
 import {Position, TextDocument} from "vscode-languageserver-textdocument";
 import {Hover} from "vscode-languageserver-types";
 import {getWorkflowContext, WorkflowContext} from "./context/workflow-context";
@@ -9,6 +12,7 @@ import {info} from "./log";
 import {nullTrace} from "./nulltrace";
 import {findToken} from "./utils/find-token";
 import {mapRange} from "./utils/range";
+import {getCronDescription} from "@github/actions-workflow-parser/model/converter/cron";
 
 export type DescriptionProvider = {
   getDescription(context: WorkflowContext, token: TemplateToken, path: TemplateToken[]): Promise<string | undefined>;
@@ -26,14 +30,26 @@ export async function hover(document: TextDocument, position: Position, config?:
   };
   const result = parseWorkflow(file.name, [file], nullTrace);
 
-  const {token, path} = findToken(position, result.value);
+  const tokenResult = findToken(position, result.value);
+  const token = tokenResult.token;
   if (!token?.definition) {
     return null;
   }
 
   info(`Calculating hover for token with definition ${token.definition.key}`);
 
-  let description = await getDescription(document, config, result, token, path);
+  if (tokenResult.parent && isCronMappingValue(tokenResult)) {
+    const tokenValue = (token as StringToken).value
+    let description = getCronDescription(tokenValue);
+    if (description) {
+      return {
+        contents: description,
+        range: mapRange(token.range)
+      } as Hover;
+    }
+  }
+
+  let description = await getDescription(document, config, result, token, tokenResult.path);
 
   const allowedContext = token.definitionInfo?.allowedContext;
   if (allowedContext && allowedContext?.length > 0) {
@@ -64,3 +80,9 @@ async function getDescription(
   const description = await config.descriptionProvider.getDescription(workflowContext, token, path);
   return description || defaultDescription;
 }
+
+function isCronMappingValue(tokenResult: TokenResult): boolean {
+  return tokenResult.parent?.definition?.key === "cron-mapping" &&
+    isString(tokenResult.token!) &&
+    tokenResult.token.value !== "cron"
+}

actions-languageservice/src/validate.expressions.test.ts

@@ -1248,7 +1248,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - run: echo \${{ github.event.client_payload.anything }}
-      - run: echo \${{ github.event.client_payload.branch }}`
+      - run: echo \${{ github.event.client_payload.branch }}`;
 
       const result = await validate(createDocument("wf.yaml", input));
 

actions-workflow-parser/src/model/converter/cron-constants.ts

@@ -0,0 +1,32 @@
+// Constants for parsing and validating cron expressions
+
+const MONTHS = {
+  jan: 1,
+  feb: 2,
+  mar: 3,
+  apr: 4,
+  may: 5,
+  jun: 6,
+  jul: 7,
+  aug: 8,
+  sep: 9,
+  oct: 10,
+  nov: 11,
+  dec: 12
+};
+
+const DAYS = {
+  sun: 0,
+  mon: 1,
+  tue: 2,
+  wed: 3,
+  thu: 4,
+  fri: 5,
+  sat: 6
+};
+
+export const MINUTE_RANGE = {min: 0, max: 59};
+export const HOUR_RANGE = {min: 0, max: 23};
+export const DOM_RANGE = {min: 1, max: 31};
+export const MONTH_RANGE = {min: 1, max: 12, names: MONTHS};
+export const DOW_RANGE = {min: 0, max: 6, names: DAYS};

actions-workflow-parser/src/model/converter/cron.test.ts

@@ -1,63 +1,79 @@
-import { isValidCron } from "./cron"
+import {isValidCron, getCronDescription} from "./cron";
 
-describe("isValidCron", () => {
+describe("cron", () => {
-  const valid = [
+  describe("valid cron", () => {
-    ["0 0 * * *", "every day at midnight"],
+    const valid = [
-    ["0 000 001 * *", "accepts leading zeros"],
+      ["0 0 * * *", "every day at midnight"],
-    ["15 * * * *", "accepts numbers in range"],
+      ["0 000 001 * *", "accepts leading zeros"],
-    ["2,10 4,5 * * *", "accepts comma separated values"],
+      ["15 * * * *", "accepts numbers in range"],
-    ["30 4-6 * * *", "accepts range"],
+      ["2,10 4,5 * * *", "accepts comma separated values"],
-    ["0 4-4 * * *", "accepts range with two equal values"],
+      ["30 4-6 * * *", "accepts range"],
-    ["20/15 * * * *", "accepts step with numerical values"],
+      ["0 4-4 * * *", "accepts range with two equal values"],
-    ["30 5,17 * * *", "accepts numbers and ranges"],
+      ["20/15 * * * *", "accepts step with numerical values"],
-    ["28 */4 * * *", "accepts step with * and numerical value"],
+      ["30 5,17 * * *", "accepts numbers and ranges"],
-    ["28 5,*/4 * * *", "accepts comma separated value with step"],
+      ["28 */4 * * *", "accepts step with * and numerical value"],
-    ["28 5,*/4,6-8 * * *", "accepts comma separated value with step and range"],
+      ["28 5,*/4 * * *", "accepts comma separated value with step"],
-    ["0 0 * * SUN", "accepts day of week short name"],
+      ["28 5,*/4,6-8 * * *", "accepts comma separated value with step and range"],
-    ["0 0 * * SUN-TUE", "accepts day of week short name range"],
+      ["0 0 * * SUN", "accepts day of week short name"],
-    ["0 0 * * SUN-2", "accepts day of week range combined with number"],
+      ["0 0 * * SUN-TUE", "accepts day of week short name range"],
-    ["0 2-4/5 * * *", "accepts range with step"],
+      ["0 0 * * SUN-2", "accepts day of week range combined with number"],
-    ["0   0  *  *               *", "accepts multiple spaces"],
+      ["0 2-4/5 * * *", "accepts range with step"],
-  ]
+      ["0   0  *  *               *", "accepts multiple spaces"]
+    ];
 
-  for (const [cron, reason] of valid) {
+    for (const [cron, reason] of valid) {
-    it(`${cron} should be valid: ${reason}`, () => {
+      it(`${cron} should be valid: ${reason}`, () => {
-      expect(isValidCron(cron)).toBe(true)
+        expect(isValidCron(cron)).toBe(true);
-    })
+      });
-  }
+    }
+  });
 
-  const invalid = [
+  describe("invalid cron", () => {
-    ["0 0 * *", "too few parts"],
+    const invalid = [
-    ["0 0 * * * * *", "too many parts"],
+      ["0 0 * *", "too few parts"],
-    ["0 -1 * * *", "should not accept negative numbers"],
+      ["0 0 * * * * *", "too many parts"],
-    ["0 1- * * *", "should not accept trailing -"],
+      ["0 -1 * * *", "should not accept negative numbers"],
-    ["0 /1 * * *", "should not accept leading / (empty value)"],
+      ["0 1- * * *", "should not accept trailing -"],
-    ["0 1/ * * *", "should not accept trailing / (empty value)"],
+      ["0 /1 * * *", "should not accept leading / (empty value)"],
-    ["0 ,1 * * *", "should not accept leading , (empty value)"],
+      ["0 1/ * * *", "should not accept trailing / (empty value)"],
-    ["0 1, * * *", "should not accept trailing , (empty value)"],
+      ["0 ,1 * * *", "should not accept leading , (empty value)"],
-    ["0 5--5 * * *", "should not accept multiple -"],
+      ["0 1, * * *", "should not accept trailing , (empty value)"],
-    ["0 *//5 * * *", "should not accept multiple /"],
+      ["0 5--5 * * *", "should not accept multiple -"],
-    ["0 */* * * *", "step start and size may not both be *"],
+      ["0 *//5 * * *", "should not accept multiple /"],
-    ["0 5/* * * *", "steps size may not be *"],
+      ["0 */* * * *", "step start and size may not both be *"],
-    ["0 *-4 5-* *-* *", "range may not contain *"],
+      ["0 5/* * * *", "steps size may not be *"],
-    ["0 *//5 * * *", "should not accept multiple /"],
+      ["0 *-4 5-* *-* *", "range may not contain *"],
-    ["0 ,, * * *", "should not accept multiple ,"],
+      ["0 ,, * * *", "should not accept multiple ,"],
-    [", , , , ,", "comma is not a valid part"],
+      [", , , , ,", "comma is not a valid part"],
-    ["0 ** * * *", "should not accept multiple *"],
+      ["0 ** * * *", "should not accept multiple *"],
-    ["0 0 * * BUN", "invalid short name"],
+      ["0 0 * * BUN", "invalid short name"],
-    ["0 0 * SUN JAN", "short name in incorrect position"],
+      ["0 0 * SUN JAN", "short name in incorrect position"],
-    ["0 0 * * FRI-TUE", "should not accept short name range with start > end"],
+      ["0 0 * * FRI-TUE", "should not accept short name range with start > end"],
-    ["0 12-4 * * *", "should not accept nuerical range with start > end"],
+      ["0 12-4 * * *", "should not accept nuerical range with start > end"],
-    ["0 */0 * * *", "step size may not be 0"],
+      ["0 */0 * * *", "step size may not be 0"],
-    ["0 2/4-5 * * *", "step size may not be a range"],
+      ["0 2/4-5 * * *", "step size may not be a range"],
-    ["0 2-4-6 * * *", "range may not contain multiple -"],
+      ["0 2-4-6 * * *", "range may not contain multiple -"],
-    ["0 2/4/6 * * *", "step may not contain multiple /"],
+      ["0 2/4/6 * * *", "step may not contain multiple /"],
-    ["0 * * */FEB */TUE", "step size may not be a short name"],
+      ["0 * * */FEB */TUE", "step size may not be a short name"]
-  ]
+    ];
 
-  for (const [cron, reason] of invalid) {
+    for (const [cron, reason] of invalid) {
-    it(`${cron} should be invalid: ${reason}`, () => {
+      it(`${cron} should be invalid: ${reason}`, () => {
-      expect(isValidCron(cron)).toBe(false)
+        expect(isValidCron(cron)).toBe(false);
-    })
+      });
-  }
+    }
-})
+  });
+
+  describe("getCronDescription", () => {
+    it(`Produces a sentence for valid cron`, () => {
+      expect(getCronDescription("0 * * * *")).toEqual(
+        "Runs every hour\n\n" +
+          "Actions schedules run at most every 5 minutes. [Learn more](https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#onschedule)"
+      );
+    });
+
+    it(`Returns nothing for invalid cron`, () => {
+      expect(getCronDescription("* * * * * *")).toBeUndefined();
+    });
+  });
+});

actions-workflow-parser/src/model/converter/cron.ts

@@ -1,118 +1,115 @@
-const MONTHS = {
+import cronstrue from "cronstrue";
-  jan: 1,
-  feb: 2,
-  mar: 3,
-  apr: 4,
-  may: 5,
-  jun: 6,
-  jul: 7,
-  aug: 8,
-  sep: 9,
-  oct: 10,
-  nov: 11,
-  dec: 12,
-}
 
-const DAYS = {
+import {MONTH_RANGE, HOUR_RANGE, MINUTE_RANGE, DOM_RANGE, DOW_RANGE} from "./cron-constants";
-  sun: 0,
+
-  mon: 1,
+type Range = {
-  tue: 2,
+  min: number;
-  wed: 3,
+  max: number;
-  thu: 4,
+  names?: Record<string, number>;
-  fri: 5,
+};
-  sat: 6,
-}
 
-// TODO: make this parseCron
 export function isValidCron(cron: string): boolean {
   // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule
 
-  const parts = cron.split(/ +/)
+  const parts = cron.split(/ +/);
   if (parts.length != 5) {
-    return false
+    return false;
   }
-
+  const [minutes, hours, dom, months, dow] = parts;
-  const [minutes, hours, dom, months, dow] = parts
 
   return (
-    validateRange(minutes, { min: 0, max: 59 }) &&
+    validateCronPart(minutes, MINUTE_RANGE) &&
-    validateRange(hours, { min: 0, max: 23 }) &&
+    validateCronPart(hours, HOUR_RANGE) &&
-    validateRange(dom, { min: 1, max: 31 }) &&
+    validateCronPart(dom, DOM_RANGE) &&
-    validateRange(months, { min: 1, max: 12, names: MONTHS }) &&
+    validateCronPart(months, MONTH_RANGE) &&
-    validateRange(dow, { min: 0, max: 6, names: DAYS })
+    validateCronPart(dow, DOW_RANGE)
-  )
+  );
 }
 
-type Range = {
+export function getCronDescription(cronspec: string): string | undefined {
-  min: number
+  if (!isValidCron(cronspec)) {
-  max: number
+    return;
-  names?: Record<string, number>
+  }
+
+  let desc = "";
+  try {
+    desc = cronstrue.toString(cronspec, {
+      dayOfWeekStartIndexZero: true,
+      monthStartIndexZero: false,
+      use24HourTimeFormat: true,
+      // cronstrue sets the description as the error if throwExceptionOnParseError is false
+      // so we need to distinguish between an error and a valid description
+      throwExceptionOnParseError: true
+    });
+  } catch (err) {
+    return;
+  }
+
+  // Make first character lowercase
+  let result = "Runs " + desc.charAt(0).toLowerCase() + desc.slice(1);
+  result +=
+    "\n\nActions schedules run at most every 5 minutes." +
+    " [Learn more](https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#onschedule)";
+  return result;
 }
 
-function validateRange(
+function validateCronPart(value: string, range: Range, allowSeparators = true): boolean {
-  value: string,
-  range: Range,
-  allowSeparators = true
-): boolean {
   if (range.names && range.names[value.toLowerCase()] !== undefined) {
-    return true
+    return true;
   }
 
   if (value === "*") {
-    return true
+    return true;
   }
 
   // Operator precedence: , > / > -
   if (value.includes(",")) {
     if (!allowSeparators) {
-      return false
+      return false;
     }
-    return value.split(",").every((v) => v && validateRange(v, range))
+    return value.split(",").every(v => v && validateCronPart(v, range));
   }
 
   if (value.includes("/")) {
     if (!allowSeparators) {
-      return false
+      return false;
     }
 
-    const [start, step, ...rest] = value.split("/")
+    const [start, step, ...rest] = value.split("/");
-    const stepNumber = +step
+    const stepNumber = +step;
     if (rest.length > 0 || isNaN(stepNumber) || stepNumber <= 0 || !start || !step) {
-      return false
+      return false;
     }
 
     // Separators are only allowed in the part before the `/`, e.g. `1-5/2`
-    return (
+    return validateCronPart(start, range) && validateCronPart(step, range, false);
-      validateRange(start, range) && validateRange(step, range, false)
-    )
   }
 
   if (value.includes("-")) {
     if (!allowSeparators) {
-      return false
+      return false;
     }
-    const [start, end, ...rest] = value.split("-")
+
+    const [start, end, ...rest] = value.split("-");
     if (rest.length > 0 || !start || !end) {
-      return false
+      return false;
     }
 
     // Convert name to integers so we can make sure end >= start
-    const startNumber = convertToNumber(range, start)
+    const startNumber = convertToNumber(start, range.names);
-    const endNumber = convertToNumber(range, end)
+    const endNumber = convertToNumber(end, range.names);
-    return (
+    return validateCronPart(start, range, false) && validateCronPart(end, range, false) && endNumber >= startNumber;
-      validateRange(start, range, false) && validateRange(end, range, false) && endNumber >= startNumber
-    )
   }
 
-  const number = +value
+  const number = +value;
-  return !isNaN(number) && number >= range.min && number <= range.max
+  return !isNaN(number) && number >= range.min && number <= range.max;
 }
 
-function convertToNumber(range: Range, value: string): number {
+// Converts a string integer or a short name to a number
-  if (range.names && range.names[value.toLowerCase()] !== undefined) {
+function convertToNumber(value: string, names?: Record<string, number>): number {
-    return +range.names[value.toLowerCase()]
+  if (names && names[value.toLowerCase()] !== undefined) {
+    return +names[value.toLowerCase()];
+  } else {
+    return +value;
   }
-  else {
+}
-    return +value
-  }
-}

actions-workflow-parser/src/model/converter/events.ts

@@ -13,7 +13,7 @@ import {
   TypesFilterConfig,
   WorkflowFilterConfig
 } from "../workflow-template";
-import {isValidCron} from "./cron"
+import {isValidCron} from "./cron";
 import {convertStringList} from "./string-list";
 import {convertEventWorkflowDispatchInputs} from "./workflow-dispatch";
 
@@ -146,7 +146,7 @@ function convertSchedule(context: TemplateContext, token: SequenceToken): Schedu
         const cron = schedule.value.assertString(`schedule cron`);
         // Validate the cron string
         if (!isValidCron(cron.value)) {
-          context.error(cron, "Invalid cron string")
+          context.error(cron, "Invalid cron string");
         }
         result.push({cron: cron.value});
       } else {

actions-workflow-parser/src/templates/template-reader.ts

@@ -226,7 +226,6 @@ class TemplateReader {
         }
 
         const nextValue = this.readValue(nextDefinition);
-
         mapping.add(nextKey, nextValue);
         continue;
       }

package-lock.json

@@ -12,6 +12,9 @@
         "./actions-languageserver",
         "./browser-playground"
       ],
+      "dependencies": {
+        "cronstrue": "^2.21.0"
+      },
       "devDependencies": {
         "lerna": "^6.0.3"
       }
@@ -5296,6 +5299,14 @@
         "node": ">= 6"
       }
     },
+    "node_modules/cronstrue": {
+      "version": "2.21.0",
+      "resolved": "https://registry.npmjs.org/cronstrue/-/cronstrue-2.21.0.tgz",
+      "integrity": "sha512-YxabE1ZSHA1zJZMPCTSEbc0u4cRRenjqqTgCwJT7OvkspPSvfYFITuPFtsT+VkBuavJtFv2kJXT+mKSnlUJxfg==",
+      "bin": {
+        "cronstrue": "bin/cli.js"
+      }
+    },
     "node_modules/cross-spawn": {
       "version": "7.0.3",
       "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.3.tgz",
@@ -18165,6 +18176,11 @@
         }
       }
     },
+    "cronstrue": {
+      "version": "2.21.0",
+      "resolved": "https://registry.npmjs.org/cronstrue/-/cronstrue-2.21.0.tgz",
+      "integrity": "sha512-YxabE1ZSHA1zJZMPCTSEbc0u4cRRenjqqTgCwJT7OvkspPSvfYFITuPFtsT+VkBuavJtFv2kJXT+mKSnlUJxfg=="
+    },
     "cross-spawn": {
       "version": "7.0.3",
       "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.3.tgz",

package.json

@@ -10,5 +10,8 @@
   "devDependencies": {
     "lerna": "^6.0.3"
   },
-  "name": "actions-languageservices"
+  "name": "actions-languageservices",
+  "dependencies": {
+    "cronstrue": "^2.21.0"
+  }
 }