actions/languageservices
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove isExpression flag and implement convertToIfCondition to align with Go parser architecture (#217)

Browse Source
This commit is contained in:
eric sciple
2025-11-24 09:12:26 -06:00
committed by GitHub
parent 7f8bba4305
commit bad1fb96af
12 changed files with 232 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
languageservice/src/expression-hover/expression-pos.ts
+41 -1
View File
@@ -1,6 +1,6 @@
import {Pos} from "@actions/expressions/lexer";
import {TemplateToken} from "@actions/workflow-parser/templates/tokens/template-token";
import {isBasicExpression} from "@actions/workflow-parser/templates/tokens/type-guards";
import {isBasicExpression, isString} from "@actions/workflow-parser/templates/tokens/type-guards";
import {Position, Range as LSPRange} from "vscode-languageserver-textdocument";
import {mapRange} from "../utils/range";
import {posWithinRange} from "./pos-range";
@@ -16,12 +16,52 @@ export type ExpressionPos = {
documentRange: LSPRange;
};
/**
* Maps a document position to an expression position for hover/completion features.
*
* This handles both explicit expressions (with ${{ }}) and implicit expressions (like if conditions).
* For if conditions without ${{ }}, this applies the same conversion as the parser's convertToIfCondition,
* wrapping them in `success() && (...)` when no status function is present.
*
* @param token The template token at the position
* @param position The position in the document
* @returns Expression and adjusted position, or undefined if not an expression
*/
export function mapToExpressionPos(token: TemplateToken, position: Position): ExpressionPos | undefined {
const pos: Pos = {
line: position.line + 1,
column: position.character + 1
};
// Handle if conditions that are string tokens (job-if, step-if, snapshot-if)
const definitionKey = token.definition?.key;
if (
isString(token) &&
token.range &&
(definitionKey === "job-if" || definitionKey === "step-if" || definitionKey === "snapshot-if")
) {
const condition = token.value.trim();
if (condition) {
// Check if the condition already contains a status function
const hasStatusFunction = /\b(success|failure|cancelled|always)\s*\(/.test(condition);
const finalCondition = hasStatusFunction ? condition : `success() && (${condition})`;
const exprRange = mapRange(token.range);
// Calculate offset for wrapping
const offset = hasStatusFunction ? 0 : "success() && (".length;
return {
expression: finalCondition,
position: {
line: pos.line - exprRange.start.line - 1,
column: pos.column - exprRange.start.character - 1 + offset
},
documentRange: exprRange
};
}
}
if (!isBasicExpression(token)) {
return undefined;
}
languageservice/src/hover.expressions.test.ts
+2 -2
View File
@@ -155,8 +155,8 @@ jobs:
contents:
"Causes the step to always execute, and returns `true`, even when canceled. The `always` expression is best used at the step level or on tasks that you expect to run even when a job is canceled. For example, you can use `always` to send logs even when a job is canceled.",
range: {
start: {line: 3, character: 11},
end: {line: 3, character: 17}
start: {line: 3, character: 8},
end: {line: 3, character: 14}
}
});
});
languageservice/src/utils/expression-detection.ts
+4 -5
View File
@@ -1,12 +1,11 @@
import {isString} from "@actions/workflow-parser";
import {DefinitionType} from "@actions/workflow-parser/templates/schema/definition-type";
import {StringDefinition} from "@actions/workflow-parser/templates/schema/string-definition";
import {OPEN_EXPRESSION} from "@actions/workflow-parser/templates/template-constants";
import {TemplateToken} from "@actions/workflow-parser/templates/tokens/index";
export function isPotentiallyExpression(token: TemplateToken): boolean {
const isAlwaysExpression =
token.definition?.definitionType === DefinitionType.String && (token.definition as StringDefinition).isExpression;
const containsExpression = isString(token) && token.value != null && token.value.indexOf(OPEN_EXPRESSION) >= 0;
return isAlwaysExpression || containsExpression;
// If conditions are always expressions (job-if, step-if, snapshot-if)
const definitionKey = token.definition?.key;
const isIfCondition = definitionKey === "job-if" || definitionKey === "step-if" || definitionKey === "snapshot-if";
return containsExpression || isIfCondition;
}
languageservice/src/validate.ts
+34
View File
@@ -108,6 +108,40 @@ async function additionalValidations(
);
}
// If this is a job-if, step-if, or snapshot-if field (which are strings that should be treated as expressions), validate it
const definitionKey = token.definition?.key;
if (
isString(token) &&
token.range &&
(definitionKey === "job-if" || definitionKey === "step-if" || definitionKey === "snapshot-if")
) {
// Convert the string to an expression token for validation
const condition = token.value.trim();
if (condition) {
// Check if the condition already contains a status function
const hasStatusFunction = /\b(success|failure|cancelled|always)\s*\(/.test(condition);
const finalCondition = hasStatusFunction ? condition : `success() && (${condition})`;
// Create a BasicExpressionToken for validation
const expressionToken = new BasicExpressionToken(
token.file,
token.range,
finalCondition,
token.definitionInfo,
undefined,
token.source
);
await validateExpression(
diagnostics,
expressionToken,
validationToken.definitionInfo?.allowedContext || [],
config?.contextProviderConfig,
getProviderContext(documentUri, template, root, token.range)
);
}
}
if (token.definition?.key === "regular-step" && token.range) {
const context = getProviderContext(documentUri, template, root, token.range);
await validateAction(diagnostics, token, context.step, config);
workflow-parser/src/expressions.test.ts
+4 -3
View File
@@ -194,10 +194,11 @@ jobs:
const jobs = workflowRoot.get(1).value.assertMapping("jobs");
const build = jobs.get(0).value.assertMapping("job");
const ifToken = build.get(1).value;
expect(ifToken.toString()).toEqual("${{ github.event_name == 'push' }}");
// Without isExpression: true, the value is kept as a string until convertToIfCondition processes it
expect(ifToken.toString()).toEqual("github.event_name == 'push'");
if (!isBasicExpression(ifToken)) {
throw new Error("expected if to be a basic expression");
if (!isString(ifToken)) {
throw new Error("expected if to be a string (will be converted to expression later)");
}
});
});
workflow-parser/src/model/convert.test.ts
+2 -2
View File
@@ -74,7 +74,7 @@ jobs:
{
id: "build",
if: {
expr: "success()",
expr: "success() && (true)",
type: 3
},
name: "build",
@@ -85,7 +85,7 @@ jobs:
{
id: "deploy",
if: {
expr: "success()",
expr: "success() && (true)",
type: 3
},
name: "deploy",
workflow-parser/src/model/converter/if-condition.ts
+119
View File
@@ -0,0 +1,119 @@
import {TemplateContext} from "../../templates/template-context";
import {BasicExpressionToken, ExpressionToken, TemplateToken} from "../../templates/tokens";
/**
* Converts an if condition token to a BasicExpressionToken.
* Similar to Go's convertToIfCondition - treats the value as a string and parses it as an expression.
* Wraps the condition in success() && (...) if it doesn't already contain a status function.
* This allows both 'if: success()' and 'if: ${{ success() }}' to work correctly.
*
* @param context The template context for error reporting
* @param token The token containing the if condition
* @param allowedContext The allowed expression contexts (varies by job-if vs step-if vs snapshot-if)
* @returns A BasicExpressionToken with the processed condition, or undefined on error
*/
export function convertToIfCondition(
context: TemplateContext,
token: TemplateToken,
allowedContext: string[]
): BasicExpressionToken | undefined {
const scalar = token.assertScalar("if condition");
// If it's already an expression, use its value
let condition: string;
let source: string | undefined;
if (scalar instanceof BasicExpressionToken) {
condition = scalar.expression;
source = scalar.source;
} else {
// Otherwise, treat it as a string
const stringToken = scalar.assertString("if condition");
condition = stringToken.value.trim();
source = stringToken.source;
}
if (!condition) {
// Empty condition defaults to success()
return new BasicExpressionToken(token.file, token.range, "success()", token.definitionInfo, undefined, undefined);
}
// Check if the condition already contains a status function (success, failure, cancelled, always)
// This is a simple check - just look for these function names
const hasStatusFunction = /\b(success|failure|cancelled|always)\s*\(/.test(condition);
let finalCondition: string;
if (hasStatusFunction) {
finalCondition = condition;
} else {
// Wrap in success() && (condition)
finalCondition = `success() && (${condition})`;
}
// Validate the expression before creating the token
try {
ExpressionToken.validateExpression(finalCondition, allowedContext);
} catch (err) {
context.error(token, err as Error);
return undefined;
}
// Create a BasicExpressionToken with the final condition
return new BasicExpressionToken(token.file, token.range, finalCondition, token.definitionInfo, undefined, source);
}
/**
* Allowed context for job-level if conditions
*/
export const JOB_IF_CONTEXT = [
"github",
"inputs",
"vars",
"needs",
"always(0,0)",
"failure(0,MAX)",
"cancelled(0,0)",
"success(0,MAX)"
];
/**
* Allowed context for step-level if conditions
*/
export const STEP_IF_CONTEXT = [
"github",
"inputs",
"vars",
"needs",
"strategy",
"matrix",
"steps",
"job",
"runner",
"env",
"always(0,0)",
"failure(0,0)",
"cancelled(0,0)",
"success(0,0)",
"hashFiles(1,255)"
];
/**
* Allowed context for snapshot-level if conditions
*/
export const SNAPSHOT_IF_CONTEXT = [
"github",
"inputs",
"vars",
"needs",
"strategy",
"matrix",
"steps",
"job",
"runner",
"env",
"always(0,0)",
"failure(0,0)",
"cancelled(0,0)",
"success(0,0)",
"hashFiles(1,255)"
];
workflow-parser/src/model/converter/job.ts
+8 -2
View File
@@ -2,6 +2,7 @@ import {TemplateContext} from "../../templates/template-context";
import {BasicExpressionToken, MappingToken, ScalarToken, StringToken, TemplateToken} from "../../templates/tokens";
import {isSequence, isString} from "../../templates/tokens/type-guards";
import {Step, WorkflowJob} from "../workflow-template";
import {convertToIfCondition, JOB_IF_CONTEXT} from "./if-condition";
import {convertConcurrency} from "./concurrency";
import {convertToJobContainer, convertToJobServices} from "./container";
import {handleTemplateTokenErrors} from "./handle-errors";
@@ -20,6 +21,7 @@ export function convertJob(context: TemplateContext, jobKey: StringToken, token:
container,
env,
environment,
ifCondition,
name,
outputs,
runsOn,
@@ -59,6 +61,10 @@ export function convertJob(context: TemplateContext, jobKey: StringToken, token:
environment = item.value;
break;
case "if":
ifCondition = convertToIfCondition(context, item.value, JOB_IF_CONTEXT);
break;
case "name":
name = item.value.assertScalar("job name");
break;
@@ -134,7 +140,7 @@ export function convertJob(context: TemplateContext, jobKey: StringToken, token:
id: jobKey,
name: jobName(name, jobKey),
needs: needs || [],
if: new BasicExpressionToken(undefined, undefined, "success()", undefined, undefined, undefined),
if: ifCondition || new BasicExpressionToken(undefined, undefined, "success()", undefined, undefined, undefined),
ref: workflowJobRef,
"input-definitions": undefined,
"input-values": workflowJobInputs,
@@ -151,7 +157,7 @@ export function convertJob(context: TemplateContext, jobKey: StringToken, token:
id: jobKey,
name: jobName(name, jobKey),
needs,
if: new BasicExpressionToken(undefined, undefined, "success()", undefined, undefined, undefined),
if: ifCondition || new BasicExpressionToken(undefined, undefined, "success()", undefined, undefined, undefined),
env,
concurrency,
environment,
workflow-parser/src/model/converter/steps.ts
+14 -3
View File
@@ -2,6 +2,7 @@ import {TemplateContext} from "../../templates/template-context";
import {BasicExpressionToken, MappingToken, ScalarToken, StringToken, TemplateToken} from "../../templates/tokens";
import {isSequence} from "../../templates/tokens/type-guards";
import {isActionStep} from "../type-guards";
import {convertToIfCondition, STEP_IF_CONTEXT} from "./if-condition";
import {ActionStep, Step} from "../workflow-template";
import {handleTemplateTokenErrors} from "./handle-errors";
import {IdBuilder} from "./id-builder";
@@ -52,7 +53,7 @@ function convertStep(context: TemplateContext, idBuilder: IdBuilder, step: Templ
let uses: StringToken | undefined;
let continueOnError: boolean | ScalarToken | undefined;
let env: MappingToken | undefined;
const ifCondition = new BasicExpressionToken(undefined, undefined, "success()", undefined, undefined, undefined);
let ifCondition: BasicExpressionToken | undefined;
for (const item of mapping) {
const key = item.key.assertString("steps item key");
switch (key.value) {
@@ -77,6 +78,9 @@ function convertStep(context: TemplateContext, idBuilder: IdBuilder, step: Templ
case "env":
env = item.value.assertMapping("step env");
break;
case "if":
ifCondition = convertToIfCondition(context, item.value, STEP_IF_CONTEXT);
break;
case "continue-on-error":
if (!item.value.isExpression) {
continueOnError = item.value.assertBoolean("steps item continue-on-error").value;
@@ -90,7 +94,7 @@ function convertStep(context: TemplateContext, idBuilder: IdBuilder, step: Templ
return {
id: id?.value || "",
name,
if: ifCondition,
if: ifCondition || new BasicExpressionToken(undefined, undefined, "success()", undefined, undefined, undefined),
"continue-on-error": continueOnError,
env,
run
@@ -101,7 +105,7 @@ function convertStep(context: TemplateContext, idBuilder: IdBuilder, step: Templ
return {
id: id?.value || "",
name,
if: ifCondition,
if: ifCondition || new BasicExpressionToken(undefined, undefined, "success()", undefined, undefined, undefined),
"continue-on-error": continueOnError,
env,
uses
@@ -134,3 +138,10 @@ function createActionStepId(step: ActionStep): string {
return "";
}
/**
* Converts an if condition token to a BasicExpressionToken.
* Similar to Go's convertToIfCondition - treats the value as a string and parses it as an expression.
* Wraps the condition in success() && (...) if it doesn't already contain a status function.
* This allows both 'if: success()' and 'if: ${{ success() }}' to work correctly.
*/
workflow-parser/src/templates/template-reader.ts
+1 -9
View File
@@ -8,7 +8,6 @@ import {DefinitionType} from "./schema/definition-type";
import {MappingDefinition} from "./schema/mapping-definition";
import {ScalarDefinition} from "./schema/scalar-definition";
import {SequenceDefinition} from "./schema/sequence-definition";
import {StringDefinition} from "./schema/string-definition";
import {ANY, CLOSE_EXPRESSION, INSERT_DIRECTIVE, OPEN_EXPRESSION} from "./template-constants";
import {TemplateContext} from "./template-context";
import {
@@ -456,14 +455,7 @@ class TemplateReader {
let startExpression: number = raw.indexOf(OPEN_EXPRESSION);
if (startExpression < 0) {
// Doesn't contain "${{"
// Check if value should still be evaluated as an expression
if (definitionInfo.definition instanceof StringDefinition && definitionInfo.definition.isExpression) {
const expression = this.parseIntoExpressionToken(token.range!, raw, allowedContext, token, definitionInfo);
if (expression) {
return expression;
}
}
// Doesn't contain "{{"
return token;
}
workflow-parser/src/workflow-v1.0.json
+3 -9
View File
@@ -1842,9 +1842,7 @@
"cancelled(0,0)",
"success(0,MAX)"
],
"string": {
"is-expression": true
}
"string": {}
},
"job-if-result": {
"context": [
@@ -1949,9 +1947,7 @@
"matrix"
],
"description": "Use the if conditional to prevent a snapshot from being taken unless a condition is met. Any supported context and expression can be used to create a conditional. Expressions in an `if` conditional do not require the bracketed expression syntax. When you use expressions in an `if` conditional, you may omit the expression syntax because GitHub automatically evaluates the `if` conditional as an expression.",
"string": {
"is-expression": true
}
"string": {}
},
"runs-on": {
"description": "Use `runs-on` to define the type of machine to run the job on.\n* The destination machine can be either a GitHub-hosted runner, larger runner, or a self-hosted runner.\n* You can target runners based on the labels assigned to them, or their group membership, or a combination of these.\n* You can provide `runs-on` as a single string or as an array of strings.\n* If you specify an array of strings, your workflow will execute on any runner that matches all of the specified `runs-on` values.\n* If you would like to run your workflow on multiple machines, use `jobs.<job_id>.strategy`.",
@@ -2216,9 +2212,7 @@
"hashFiles(1,255)"
],
"description": "Use the `if` conditional to prevent a step from running unless a condition is met. Any supported context and expression can be used to create a conditional. Expressions in an `if` conditional do not require the bracketed expression syntax. When you use expressions in an `if` conditional, you may omit the expression syntax because GitHub automatically evaluates the `if` conditional as an expression.",
"string": {
"is-expression": true
}
"string": {}
},
"step-if-result": {
"context": [
workflow-parser/testdata/skipped-tests.txt
Vendored
-2
View File
@@ -50,7 +50,6 @@ errors-step-uses-syntax.yml
errors-unclosed-tokens.yml
errors-yaml-invalid-style.yml
errors-yaml-tags-explicit-unsupported.yml
escape-html-values.yml
float-folded-style.yml
insert.yml
is-partial-rerun.yml
@@ -59,7 +58,6 @@ job-cancel-timeout-minutes.yml
job-concurrency.yml
job-continue-on-error.yml
job-defaults.yml
job-if.yml
job-permissions.yml
job-timeout-minutes.yml
matrix-basic.yml