-
Notifications
You must be signed in to change notification settings - Fork 53
Setup CodeActions and add quickfix for missing inputs #254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
5517920
Setup CodeActions and add quickfix for missing inputs
rentziass 5d05f29
PR feedback
rentziass f6750a6
Update languageservice/src/code-actions/quickfix/add-missing-inputs.ts
rentziass fa9be15
Fix indentSize detection for code actions after rebase
rentziass 745ad89
Merge branch 'main' into rentziass/codeactions2
rentziass a186d7a
update typescript
rentziass 576419e
formatting
rentziass df2127b
linting
rentziass 612cfdb
Merge branch 'main' into rentziass/codeactions2
rentziass 1ba0f24
Gate missing inputs quickfix behind feature flag
rentziass 1116f79
Address PR review: rename files, move position calculation to quickfix
rentziass 1efba34
wip
rentziass 2897a40
Remove pointless comment
rentziass File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| import {FeatureFlags} from "@actions/expressions"; | ||
| import {CodeAction, CodeActionKind, Diagnostic} from "vscode-languageserver-types"; | ||
| import {CodeActionContext, CodeActionProvider} from "./types.js"; | ||
| import {getQuickfixProviders} from "./quickfix/quickfix-providers.js"; | ||
|
|
||
| export interface CodeActionParams { | ||
| uri: string; | ||
| documentContent: string; | ||
| diagnostics: Diagnostic[]; | ||
| only?: string[]; | ||
| featureFlags?: FeatureFlags; | ||
| } | ||
|
|
||
| export function getCodeActions(params: CodeActionParams): CodeAction[] { | ||
| const actions: CodeAction[] = []; | ||
| const context: CodeActionContext = { | ||
| uri: params.uri, | ||
| documentContent: params.documentContent, | ||
| featureFlags: params.featureFlags | ||
| }; | ||
|
|
||
| // Build providers map based on feature flags | ||
| const providersByKind: Map<string, CodeActionProvider[]> = new Map([ | ||
| [CodeActionKind.QuickFix, getQuickfixProviders(params.featureFlags)] | ||
| // [CodeActionKind.Refactor, getRefactorProviders(params.featureFlags)], | ||
| // [CodeActionKind.Source, getSourceProviders(params.featureFlags)], | ||
| // etc | ||
| ]); | ||
|
|
||
| // Filter to requested kinds, or use all if none specified | ||
| const requestedKinds = params.only; | ||
| const kindsToCheck = requestedKinds | ||
| ? [...providersByKind.keys()].filter(kind => requestedKinds.some(requested => kind.startsWith(requested))) | ||
| : [...providersByKind.keys()]; | ||
|
|
||
| for (const diagnostic of params.diagnostics) { | ||
| for (const kind of kindsToCheck) { | ||
| const providers = providersByKind.get(kind) ?? []; | ||
| for (const provider of providers) { | ||
| if (provider.diagnosticCodes.includes(diagnostic.code)) { | ||
| const action = provider.createCodeAction(context, diagnostic); | ||
| if (action) { | ||
| action.kind = kind; | ||
| action.diagnostics = [diagnostic]; | ||
| actions.push(action); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return actions; | ||
| } | ||
|
|
||
| export type {CodeActionContext, CodeActionProvider} from "./types.js"; | ||
245 changes: 245 additions & 0 deletions
245
languageservice/src/code-actions/quickfix/add-missing-inputs.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,245 @@ | ||
| import {isMapping} from "@actions/workflow-parser"; | ||
| import {MappingToken} from "@actions/workflow-parser/templates/tokens/mapping-token"; | ||
| import {ScalarToken} from "@actions/workflow-parser/templates/tokens/scalar-token"; | ||
| import {TemplateToken} from "@actions/workflow-parser/templates/tokens/template-token"; | ||
| import {CodeAction, Position, TextEdit} from "vscode-languageserver-types"; | ||
| import {error} from "../../log.js"; | ||
| import {findToken} from "../../utils/find-token.js"; | ||
| import {getOrParseWorkflow} from "../../utils/workflow-cache.js"; | ||
| import {DiagnosticCode, MissingInputsDiagnosticData} from "../../validate-action-reference.js"; | ||
| import {CodeActionContext, CodeActionProvider} from "../types.js"; | ||
|
|
||
| /** | ||
| * Information extracted from a step token needed to generate edits | ||
| */ | ||
| interface StepInfo { | ||
| /** Column where step keys start (1-indexed), e.g., the column of "uses:" */ | ||
| stepKeyColumn: number; | ||
| /** End line of the step (1-indexed) */ | ||
| stepEndLine: number; | ||
| /** Detected indent size (spaces per level) */ | ||
| indentSize: number; | ||
| /** Information about existing with: block, if present */ | ||
| withInfo?: { | ||
| keyColumn: number; | ||
| keyEndLine: number; | ||
| valueEndLine: number; | ||
| hasChildren: boolean; | ||
| /** Column of first child input (1-indexed), for indentation detection */ | ||
| firstChildColumn?: number; | ||
| }; | ||
| } | ||
|
|
||
| export const addMissingInputsProvider: CodeActionProvider = { | ||
| diagnosticCodes: [DiagnosticCode.MissingRequiredInputs], | ||
|
|
||
| createCodeAction(context: CodeActionContext, diagnostic): CodeAction | undefined { | ||
| const data = diagnostic.data as MissingInputsDiagnosticData | undefined; | ||
| if (!data) { | ||
| return undefined; | ||
| } | ||
|
|
||
| // Parse the document to get the step token | ||
| const stepInfo = getStepInfo(context, diagnostic.range.start); | ||
| if (!stepInfo) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const edits = createInputEdits(data.missingInputs, stepInfo); | ||
| if (!edits || edits.length === 0) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const inputNames = data.missingInputs.map(i => i.name).join(", "); | ||
|
|
||
| return { | ||
| title: `Add missing input${data.missingInputs.length > 1 ? "s" : ""}: ${inputNames}`, | ||
| edit: { | ||
| changes: { | ||
| [context.uri]: edits | ||
| } | ||
| } | ||
| }; | ||
| } | ||
| }; | ||
|
|
||
| /** | ||
| * Parse the document and extract step information needed for generating edits. | ||
| * Returns undefined if parsing fails or the step token cannot be found. | ||
| */ | ||
| function getStepInfo(context: CodeActionContext, diagnosticPosition: Position): StepInfo | undefined { | ||
| // Parse the document (uses cache if available from validation) | ||
| const file = {name: context.uri, content: context.documentContent}; | ||
| const parseResult = getOrParseWorkflow(file, context.uri); | ||
|
|
||
| if (!parseResult.value) { | ||
| error("Failed to parse workflow for missing inputs quickfix"); | ||
| return undefined; | ||
| } | ||
|
|
||
| // Find the token at the diagnostic position | ||
| const {path} = findToken(diagnosticPosition, parseResult.value); | ||
|
|
||
| // Walk up the path to find the step token (regular-step) | ||
| const stepToken = findStepInPath(path); | ||
| if (!stepToken) { | ||
| error("Could not find step token for missing inputs quickfix"); | ||
| return undefined; | ||
| } | ||
|
|
||
| return extractStepInfo(stepToken); | ||
| } | ||
|
|
||
| /** | ||
| * Find the step token (regular-step) in the token path | ||
| */ | ||
| function findStepInPath(path: TemplateToken[]): MappingToken | undefined { | ||
| // Walk backwards through path to find the step | ||
| for (let i = path.length - 1; i >= 0; i--) { | ||
| if (path[i].definition?.key === "regular-step" && isMapping(path[i])) { | ||
| return path[i] as MappingToken; | ||
| } | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| /** | ||
| * Extract position and indentation info from a step token | ||
| */ | ||
| function extractStepInfo(stepToken: MappingToken): StepInfo | undefined { | ||
| if (!stepToken.range) { | ||
| return undefined; | ||
| } | ||
|
|
||
| // Get the column of the first key in the step | ||
| let stepKeyColumn = stepToken.range.start.column; | ||
| if (stepToken.count > 0) { | ||
| const firstEntry = stepToken.get(0); | ||
| if (firstEntry?.key.range) { | ||
| stepKeyColumn = firstEntry.key.range.start.column; | ||
| } | ||
| } | ||
|
|
||
| // Find the with: block if present | ||
| let withKey: ScalarToken | undefined; | ||
| let withToken: TemplateToken | undefined; | ||
| for (const {key, value} of stepToken) { | ||
| if (key.toString() === "with") { | ||
| withKey = key; | ||
| withToken = value; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // Calculate indent size | ||
| let indentSize = 2; // Default | ||
| let withInfo: StepInfo["withInfo"]; | ||
|
|
||
| if (withKey?.range && withToken?.range) { | ||
| // Has with: block - extract its info | ||
| const hasChildren = isMapping(withToken) && withToken.count > 0; | ||
| let firstChildColumn: number | undefined; | ||
|
|
||
| if (hasChildren) { | ||
| const firstChild = (withToken as MappingToken).get(0); | ||
| if (firstChild?.key.range) { | ||
| firstChildColumn = firstChild.key.range.start.column; | ||
| // Detect indent size from with: children | ||
| indentSize = firstChildColumn - withKey.range.start.column; | ||
| } | ||
| } | ||
|
|
||
| withInfo = { | ||
| keyColumn: withKey.range.start.column, | ||
| keyEndLine: withKey.range.end.line, | ||
| valueEndLine: withToken.range.end.line, | ||
| hasChildren, | ||
| firstChildColumn | ||
| }; | ||
| } else { | ||
| // No with: block - detect indent size using heuristics | ||
| // Based on the step key column position, estimate indent size | ||
| // 2-space indent files typically have step keys at column 7 | ||
| // 4-space indent files typically have step keys at column 15 | ||
| const zeroIndexedCol = stepKeyColumn - 1; | ||
| if (zeroIndexedCol >= 10) { | ||
| indentSize = 4; | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| stepKeyColumn, | ||
| stepEndLine: stepToken.range.end.line, | ||
| indentSize, | ||
| withInfo | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Generate text edits to add missing inputs | ||
| */ | ||
| function createInputEdits(missingInputs: MissingInputsDiagnosticData["missingInputs"], stepInfo: StepInfo): TextEdit[] { | ||
| const formatInputLines = (indent: string) => | ||
| missingInputs.map(input => { | ||
| const value = input.default ?? '""'; | ||
| return `${indent}${input.name}: ${value}`; | ||
| }); | ||
|
|
||
| if (stepInfo.withInfo) { | ||
| // `with:` exists - add inputs to existing block | ||
| const withIndent = stepInfo.withInfo.keyColumn - 1; // 0-indexed | ||
| const inputIndentSize = stepInfo.withInfo.firstChildColumn | ||
| ? stepInfo.withInfo.firstChildColumn - stepInfo.withInfo.keyColumn | ||
| : stepInfo.indentSize; | ||
|
|
||
| const inputIndent = " ".repeat(withIndent + inputIndentSize); | ||
| const inputLines = formatInputLines(inputIndent); | ||
|
|
||
| // Calculate insert position | ||
| let insertLine: number; | ||
| if (stepInfo.withInfo.hasChildren) { | ||
| // Insert after the last child (at end of with: block) | ||
| // valueEndLine is 1-indexed, we want 0-indexed for Position | ||
| insertLine = stepInfo.withInfo.valueEndLine - 1; | ||
| } else { | ||
| // Empty with: block - insert on the next line after with: | ||
| // keyEndLine is 1-indexed, convert to 0-indexed and go to next line | ||
| insertLine = stepInfo.withInfo.keyEndLine; | ||
| } | ||
|
|
||
| const insertPosition: Position = { | ||
| line: insertLine, | ||
| character: 0 | ||
| }; | ||
|
|
||
| return [ | ||
| { | ||
| range: {start: insertPosition, end: insertPosition}, | ||
| newText: inputLines.map(line => line + "\n").join("") | ||
| } | ||
| ]; | ||
| } else { | ||
| // No `with:` key - add `with:` at the same level as other step keys | ||
| const withKeyIndent = stepInfo.stepKeyColumn - 1; // 0-indexed (columns are 1-based) | ||
|
|
||
| const withIndent = " ".repeat(withKeyIndent); | ||
| const inputIndent = " ".repeat(withKeyIndent + stepInfo.indentSize); | ||
| const inputLines = formatInputLines(inputIndent); | ||
|
|
||
| const newText = `${withIndent}with:\n` + inputLines.map(line => `${line}\n`).join(""); | ||
|
|
||
| // Insert at end of step | ||
| // stepEndLine is 1-indexed, we want 0-indexed and insert before the line after | ||
| const insertPosition: Position = { | ||
| line: stepInfo.stepEndLine - 1, | ||
| character: 0 | ||
| }; | ||
|
|
||
| return [ | ||
| { | ||
| range: {start: insertPosition, end: insertPosition}, | ||
| newText | ||
| } | ||
| ]; | ||
| } | ||
| } |
13 changes: 13 additions & 0 deletions
13
languageservice/src/code-actions/quickfix/quickfix-providers.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| import {FeatureFlags} from "@actions/expressions"; | ||
| import {CodeActionProvider} from "../types.js"; | ||
| import {addMissingInputsProvider} from "./add-missing-inputs.js"; | ||
|
|
||
| export function getQuickfixProviders(featureFlags?: FeatureFlags): CodeActionProvider[] { | ||
| const providers: CodeActionProvider[] = []; | ||
|
|
||
| if (featureFlags?.isEnabled("missingInputsQuickfix")) { | ||
| providers.push(addMissingInputsProvider); | ||
| } | ||
|
|
||
| return providers; | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Subdirectory⚠️
index.tsfiles break repo patternThe existing repo pattern imports directly from specific files in the main
index.ts:No subdirectory has its own
index.ts- the main package-levelindex.tsimports directly from the implementation files (e.g.,value-providers/config.js, notvalue-providers/index.js).This PR introduces:
code-actions/index.ts(new subdirectory index with implementation code)code-actions/quickfix/index.ts(another subdirectory index)index.tsimports from"./code-actions/index.js"instead of a specific fileQuestions:
index.tsfiles contain implementation, or be exports-only like the mainindex.ts?Recommendation: Follow existing pattern - use meaningful file names instead of
index.ts:code-actions/index.ts→code-actions/code-actions.tscode-actions/quickfix/index.ts→code-actions/quickfix/quickfix-providers.tsThen update the main
index.ts:This avoids having many files named
index.tswith implementation code, making the codebase easier to navigate.This is a minor consistency issue, not a blocker.