PR feedback

Author: rentziass

languageservice/src/code-actions/quickfix/add-missing-inputs.ts

@@ -29,17 +29,19 @@ export const addMissingInputsProvider: CodeActionProvider = {
   }
 };
 
-function createInputEdits(data: MissingInputsDiagnosticData): TextEdit[] | undefined {
+function createInputEdits(data: MissingInputsDiagnosticData): TextEdit[] {
   const edits: TextEdit[] = [];
 
+  const formatInputLines = (indent: string) =>
+    data.missingInputs.map(input => {
+      const value = input.default ?? '""';
+      return `${indent}${input.name}: ${value}`;
+    });
+
   if (data.hasWithKey && data.withIndent !== undefined) {
     // `with:` exists - use its indentation + 2 for inputs
-    const inputIndent = " ".repeat(data.withIndent + 2);
-
-    const inputLines = data.missingInputs.map(input => {
-      const value = input.default !== undefined ? input.default : '""';
-      return `${inputIndent}${input.name}: ${value}`;
-    });
+    const inputIndent = " ".repeat(data.withIndent + data.indentSize);
+    const inputLines = formatInputLines(inputIndent);
 
     edits.push({
       range: {start: data.insertPosition, end: data.insertPosition},
@@ -48,12 +50,8 @@ function createInputEdits(data: MissingInputsDiagnosticData): TextEdit[] | undef
   } else {
     // No `with:` key - `with:` at step indentation, inputs at step indentation + 2
     const withIndent = " ".repeat(data.stepIndent);
-    const inputIndent = " ".repeat(data.stepIndent + 2);
-
-    const inputLines = data.missingInputs.map(input => {
-      const value = input.default !== undefined ? input.default : '""';
-      return `${inputIndent}${input.name}: ${value}`;
-    });
+    const inputIndent = " ".repeat(data.stepIndent + data.indentSize);
+    const inputLines = formatInputLines(inputIndent);
 
     const newText = [`${withIndent}with:\n`, ...inputLines.map(line => `${line}\n`)].join("");
 

languageservice/src/code-actions/tests/testdata/quickfix/four-space-indent.golden.yml

@@ -0,0 +1,9 @@
+on: push
+jobs:
+    build:
+        runs-on: ubuntu-latest
+        steps:
+            - uses: actions/cache@v1
+              with:
+                  path: ""
+                  key: ""

languageservice/src/code-actions/tests/testdata/quickfix/four-space-indent.yml

@@ -0,0 +1,6 @@
+on: push
+jobs:
+    build:
+        runs-on: ubuntu-latest
+        steps:
+            - uses: actions/cache@v1 # want "Missing required inputs: `path`, `key`" fix="Add missing inputs: path, key"

languageservice/src/code-actions/types.ts

@@ -2,7 +2,6 @@ import {CodeAction, Diagnostic} from "vscode-languageserver-types";
 
 export interface CodeActionContext {
   uri: string;
-  // TODO: add things like workflow template, parsed content, etc.
 }
 
 /**

languageservice/src/validate-action.ts

@@ -22,6 +22,7 @@ export interface MissingInputsDiagnosticData {
   // Indentation of the `with:` key if present, or the step's base indentation
   withIndent?: number;
   stepIndent: number;
+  indentSize: number;
   // Position where new content should be inserted
   insertPosition: {line: number; character: number};
 }
@@ -30,7 +31,8 @@ export async function validateAction(
   diagnostics: Diagnostic[],
   stepToken: TemplateToken,
   step: Step | undefined,
-  config: ValidationConfig | undefined
+  config: ValidationConfig | undefined,
+  indentSize: number
 ): Promise<void> {
   if (!isMapping(stepToken) || !step || !isActionStep(step) || !config?.actionsMetadataProvider) {
     return;
@@ -135,6 +137,7 @@ export async function validateAction(
       hasWithKey: withKey !== undefined,
       withIndent,
       stepIndent,
+      indentSize,
       insertPosition
     };
 

languageservice/src/validate.actions.test.ts

@@ -258,6 +258,7 @@ jobs:
             ref: "v1"
           },
           hasWithKey: true,
+          indentSize: 2,
           insertPosition: {
             character: 0,
             line: 9
@@ -324,6 +325,7 @@ jobs:
             ref: "v1"
           },
           hasWithKey: true,
+          indentSize: 2,
           insertPosition: {
             character: 0,
             line: 9
@@ -378,6 +380,7 @@ jobs:
             ref: "v1"
           },
           hasWithKey: false,
+          indentSize: 2,
           insertPosition: {
             character: 0,
             line: 7

languageservice/src/validate.ts

@@ -8,6 +8,7 @@ import {splitAllowedContext} from "@actions/workflow-parser/templates/allowed-co
 import {BasicExpressionToken} from "@actions/workflow-parser/templates/tokens/basic-expression-token";
 import {StringToken} from "@actions/workflow-parser/templates/tokens/string-token";
 import {TemplateToken} from "@actions/workflow-parser/templates/tokens/template-token";
+import {MappingToken} from "@actions/workflow-parser/templates/tokens/mapping-token";
 import {TokenRange} from "@actions/workflow-parser/templates/tokens/token-range";
 import {File} from "@actions/workflow-parser/workflows/file";
 import {FileProvider} from "@actions/workflow-parser/workflows/file-provider";
@@ -155,7 +156,8 @@ async function additionalValidations(
     // Validate action metadata (inputs, required fields) for regular steps
     if (token.definition?.key === "regular-step" && token.range) {
       const context = getProviderContext(documentUri, template, root, token.range);
-      await validateAction(diagnostics, token, context.step, config);
+      const indentSize = detectIndentSize(root as MappingToken);
+      await validateAction(diagnostics, token, context.step, config, indentSize);
     }
 
     // Validate job-level reusable workflow uses field format
@@ -783,3 +785,26 @@ function getStaticConcurrencyGroup(token: TemplateToken | undefined): StringToke
 
   return undefined;
 }
+
+function detectIndentSize(rootToken: MappingToken): number {
+  // Find the "jobs" key in the root mapping to get its column position
+  let jobsKeyColumn: number | undefined;
+  let jobsValue: TemplateToken | undefined;
+
+  for (const {key, value} of rootToken) {
+    if (key.toString() === "jobs") {
+      jobsKeyColumn = key.range?.start.column;
+      jobsValue = value;
+      break;
+    }
+  }
+
+  if (jobsKeyColumn !== undefined && jobsValue && isMapping(jobsValue) && jobsValue.count > 0) {
+    const firstJob = jobsValue.get(0);
+    if (firstJob?.key.range) {
+      return firstJob.key.range.start.column - jobsKeyColumn;
+    }
+  }
+
+  return 2; // fallback
+}