Address feedback

actions-languageservice/src/hover.test.ts

@@ -110,7 +110,7 @@ jobs:
     );
   });
 
-  it("on a cron identifier", async () => {
+  it("on a cron mapping key", async () => {
     const input = `on:
   schedule:
     - c|ron: '0 0 * * *'
@@ -127,7 +127,7 @@ jobs:
 `;
     const result = await hover(...getPositionFromCursor(input));
     expect(result).not.toBeUndefined();
-    expect(result?.contents).toEqual("Invalid cron expression");
+    expect(result?.contents).toEqual("");
   });
 });
 

actions-languageservice/src/hover.ts

@@ -12,7 +12,7 @@ import {info} from "./log";
 import {nullTrace} from "./nulltrace";
 import {findToken} from "./utils/find-token";
 import {mapRange} from "./utils/range";
-import {getSentence} from "@github/actions-workflow-parser/model/converter/cron";
+import {getCronDescription} from "@github/actions-workflow-parser/model/converter/cron";
 
 export type DescriptionProvider = {
   getDescription(context: WorkflowContext, token: TemplateToken, path: TemplateToken[]): Promise<string | undefined>;
@@ -40,20 +40,13 @@ export async function hover(document: TextDocument, position: Position, config?:
 
   if (tokenResult.parent && isCronMappingValue(tokenResult)) {
     const tokenValue = (token as StringToken).value
-    let description = getSentence(tokenValue);
+    let description = getCronDescription(tokenValue);
     if (description) {
-      description +=
-        "\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 {
         contents: description,
         range: mapRange(token.range)
       } as Hover;
     }
-    return {
-      contents: "Invalid cron expression",
-      range: mapRange(token.range)
-    } as Hover;
   }
 
   let description = await getDescription(document, config, result, token, tokenResult.path);
@@ -90,7 +83,7 @@ async function getDescription(
 }
 
 function isCronMappingValue(tokenResult: TokenResult): boolean {
-  return tokenResult.parent?.definition?.key === "cron-mapping" &&
-    (tokenResult.token as StringToken).value !== "cron" &&
-    isString(tokenResult.token!);
+  //return tokenResult.parent?.definition?.key === "cron-mapping" &&
+  return isString(tokenResult.token!) &&
+    (tokenResult.token as StringToken).value !== "cron"
 }

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

@@ -1,4 +1,4 @@
-import { isValidCron, getSchedule, getSentence } from "./cron"
+import { isValidCron, getSchedule, getCronDescription } from "./cron"
 
 describe("cron", () => {
   describe("valid cron", () => {
@@ -145,13 +145,15 @@ describe("cron", () => {
     }
   })
 
-  describe("getSentence", () => {
+  describe("getCronDescription", () => {
     it(`Produces a sentence for valid cron`, () => {
-      expect(getSentence("0 * * * *")).toEqual("Every hour")
+      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(getSentence("* * * * * *")).toBeUndefined()
+      expect(getCronDescription("* * * * * *")).toBeUndefined()
     })
   })
 })

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

@@ -40,7 +40,7 @@ export function isValidCron(cron: string): boolean {
   )
 }
 
-export function getSentence(cronspec: string): string | undefined {
+export function getCronDescription(cronspec: string): string | undefined {
   const schedule = getSchedule(cronspec)
   if (!schedule) {
     return
@@ -61,7 +61,10 @@ export function getSentence(cronspec: string): string | undefined {
   }
 
   // Make first character lowercase
-  return "Runs " + desc.charAt(0).toLowerCase() + desc.slice(1)
+  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 parseCronPart(part: string, range: Range): number[] | undefined {
@@ -195,8 +198,6 @@ export function getSchedule(cron: string): Schedule | undefined {
   }
 }
 
-// Helpers
-
 // Converts a string integer or a short name to a number
 function convertToNumber(value: string, names?: Record<string, number>): number {
   if (names && names[value.toLowerCase()] !== undefined) {

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

@@ -215,7 +215,6 @@ class TemplateReader {
       const nextPropertyDef = this._schema.matchPropertyAndFilter(mappingDefinitions, nextKey.value);
       if (nextPropertyDef) {
         const nextDefinition = new DefinitionInfo(definition, nextPropertyDef.type);
-        const nextValue = this.readValue(nextDefinition);
 
         // Store the definition on the key, the value may have its own definition
         nextKey.definitionInfo = nextDefinition;
@@ -226,6 +225,7 @@ class TemplateReader {
           nextKey.description = nextPropertyDef.description;
         }
 
+        const nextValue = this.readValue(nextDefinition);
         mapping.add(nextKey, nextValue);
         continue;
       }