diff --git a/actions-languageservice/src/complete.ts b/actions-languageservice/src/complete.ts index ca9d58c..db85d6c 100644 --- a/actions-languageservice/src/complete.ts +++ b/actions-languageservice/src/complete.ts @@ -84,7 +84,7 @@ export async function complete( const expressionInput = (getExpressionInput(currentInput, relCharPos) || "").trim(); - const allowedContext = getAllowedContext(token, parent!); + const allowedContext = getAllowedContext(token, parent); const context = await getContext(allowedContext, contextProviderConfig, workflowContext); return completeExpression(expressionInput, context, []); diff --git a/actions-languageservice/src/context-providers/default.ts b/actions-languageservice/src/context-providers/default.ts index 2ab205a..784c693 100644 --- a/actions-languageservice/src/context-providers/default.ts +++ b/actions-languageservice/src/context-providers/default.ts @@ -1,4 +1,5 @@ import {data} from "@github/actions-expressions"; +import {Kind} from "@github/actions-expressions/data/expressiondata"; import {WorkflowContext} from "../context/workflow-context"; import {ContextProviderConfig} from "./config"; import {getInputsContext} from "./inputs"; @@ -7,6 +8,10 @@ import {getNeedsContext} from "./needs"; import {getStepsContext} from "./steps"; import {getStrategyContext} from "./strategy"; +// ContextValue is the type of the value returned by a context provider +// Null indicates that the context provider doesn't have any value to provide +export type ContextValue = data.Dictionary | data.Null; + export async function getContext( names: string[], config: ContextProviderConfig | undefined, @@ -17,6 +22,10 @@ export async function getContext( const filteredNames = filterContextNames(names, workflowContext); for (const contextName of filteredNames) { let value = (await getDefaultContext(contextName, workflowContext)) || new data.Dictionary(); + if (value.kind === Kind.Null) { + context.add(contextName, value); + continue; + } value = (await config?.getContext(contextName, value)) || value; @@ -26,7 +35,7 @@ export async function getContext( return context; } -async function getDefaultContext(name: string, workflowContext: WorkflowContext): Promise { +async function getDefaultContext(name: string, workflowContext: WorkflowContext): Promise { switch (name) { case "runner": return objectToDictionary({ diff --git a/actions-languageservice/src/context-providers/matrix.test.ts b/actions-languageservice/src/context-providers/matrix.test.ts index b1a0d48..a1e473c 100644 --- a/actions-languageservice/src/context-providers/matrix.test.ts +++ b/actions-languageservice/src/context-providers/matrix.test.ts @@ -88,7 +88,7 @@ describe("matrix context", () => { const workflowContext = contextFromStrategy(strategy); const context = getMatrixContext(workflowContext); - expect(context).toEqual(new data.Dictionary()); + expect(context).toEqual(new data.Null()); }); it("matrix is not a mapping token", () => { @@ -97,7 +97,7 @@ describe("matrix context", () => { const workflowContext = contextFromStrategy(strategy); const context = getMatrixContext(workflowContext); - expect(context).toEqual(new data.Dictionary()); + expect(context).toEqual(new data.Null()); }); it("empty matrix", () => { @@ -118,7 +118,7 @@ describe("matrix context", () => { const workflowContext = contextFromStrategy(strategy); const context = getMatrixContext(workflowContext); - expect(context).toEqual(new data.Dictionary()); + expect(context).toEqual(new data.Null()); }); it("matrix with include expression", () => { @@ -138,7 +138,7 @@ describe("matrix context", () => { const workflowContext = contextFromStrategy(strategy); const context = getMatrixContext(workflowContext); - expect(context).toEqual(new data.Dictionary()); + expect(context).toEqual(new data.Null()); }); it("matrix with expression within property", () => { diff --git a/actions-languageservice/src/context-providers/matrix.ts b/actions-languageservice/src/context-providers/matrix.ts index c13f6ac..f3d8b4c 100644 --- a/actions-languageservice/src/context-providers/matrix.ts +++ b/actions-languageservice/src/context-providers/matrix.ts @@ -4,8 +4,9 @@ import {KeyValuePair} from "@github/actions-workflow-parser/templates/tokens/key import {MappingToken} from "@github/actions-workflow-parser/templates/tokens/mapping-token"; import {SequenceToken} from "@github/actions-workflow-parser/templates/tokens/sequence-token"; import {WorkflowContext} from "../context/workflow-context"; +import {ContextValue} from "./default"; -export function getMatrixContext(workflowContext: WorkflowContext): data.Dictionary { +export function getMatrixContext(workflowContext: WorkflowContext): ContextValue { // https://docs.github.com/en/actions/learn-github-actions/contexts#matrix-context const strategy = workflowContext.job?.strategy; if (!strategy || !isMapping(strategy)) { @@ -15,13 +16,13 @@ export function getMatrixContext(workflowContext: WorkflowContext): data.Diction const matrix = strategy.find("matrix"); if (!matrix || !isMapping(matrix)) { // Matrix could be an expression, so there's no context we can provide - return new data.Dictionary(); + return new data.Null(); } const properties = matrixProperties(matrix); if (!properties) { // Matrix included an expression, so there's no context we can provide - return new data.Dictionary(); + return new data.Null(); } const d = new data.Dictionary(); diff --git a/actions-languageservice/src/utils/allowed-context.ts b/actions-languageservice/src/utils/allowed-context.ts index 0eb8704..36a5d91 100644 --- a/actions-languageservice/src/utils/allowed-context.ts +++ b/actions-languageservice/src/utils/allowed-context.ts @@ -1,11 +1,11 @@ import {TemplateToken} from "@github/actions-workflow-parser/templates/tokens/template-token"; -export function getAllowedContext(token: TemplateToken, parent: TemplateToken): string[] { +export function getAllowedContext(token: TemplateToken, parent: TemplateToken | null | undefined): string[] { // Workaround for https://github.com/github/c2c-actions-experience/issues/6876 // Context is inherited from the parent const allowedContext = new Set(); for (const t of [token, parent]) { - if (t.definition?.readerContext) { + if (t?.definition?.readerContext) { for (const context of t.definition.readerContext) { allowedContext.add(context); } diff --git a/actions-languageservice/src/validate.expressions.test.ts b/actions-languageservice/src/validate.expressions.test.ts index 152cfe6..0039471 100644 --- a/actions-languageservice/src/validate.expressions.test.ts +++ b/actions-languageservice/src/validate.expressions.test.ts @@ -305,4 +305,354 @@ jobs: ]); }); }); + + describe("matrix context", () => { + it("reference within a matrix job", async () => { + const input = ` +on: push + +jobs: + test: + runs-on: ubuntu-latest + strategy: + matrix: + node: [14, 16] + steps: + - uses: actions/checkout@v3 + - run: echo \${{ matrix.node }} +`; + + const result = await validate(createDocument("wf.yaml", input)); + + expect(result).toEqual([]); + }); + + it("reference outside of a matrix job", async () => { + const input = ` +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - run: echo \${{ matrix.node }} +`; + + const result = await validate(createDocument("wf.yaml", input)); + + expect(result).toEqual([ + { + message: "Context access might be invalid: matrix", + range: { + end: { + character: 36, + line: 8 + }, + start: { + character: 18, + line: 8 + } + }, + severity: DiagnosticSeverity.Warning + } + ]); + }); + + it("basic matrix", async () => { + const input = ` +on: push + +jobs: + test: + runs-on: \${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-latest, windows-latest] + node: [14, 16] + steps: + - uses: actions/checkout@v3 + - run: echo \${{ matrix.node }} +`; + + const result = await validate(createDocument("wf.yaml", input)); + + expect(result).toEqual([]); + }); + + it("invalid property reference", async () => { + const input = ` +on: push + +jobs: + test: + runs-on: \${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-latest, windows-latest] + node: [14, 16] + include: + - os: macos-latest + node: 14 + exclude: + - os: windows-latest + node: 14 + steps: + - uses: actions/checkout@v3 + - run: echo \${{ matrix.goversion }} +`; + + const result = await validate(createDocument("wf.yaml", input)); + + expect(result).toEqual([ + { + message: "Context access might be invalid: goversion", + range: { + end: { + character: 41, + line: 18 + }, + start: { + character: 18, + line: 18 + } + }, + severity: DiagnosticSeverity.Warning + } + ]); + }); + + it("matrix with include", async () => { + const input = ` +on: push + +jobs: + test: + runs-on: \${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-latest, windows-latest] + node: [14, 16] + include: + - os: macos-latest + node: 14 + steps: + - uses: actions/checkout@v3 + - run: echo \${{ matrix.node }} +`; + + const result = await validate(createDocument("wf.yaml", input)); + + expect(result).toEqual([]); + }); + + it("matrix with only include", async () => { + const input = ` +on: push + +jobs: + test: + runs-on: ubuntu-latest + strategy: + matrix: + include: + - os: windows-latest + node: 14 + steps: + - uses: actions/checkout@v3 + - run: echo \${{ matrix.os }} + - run: echo \${{ matrix.node }} +`; + + const result = await validate(createDocument("wf.yaml", input)); + + expect(result).toEqual([]); + }); + + it("matrix with exclude", async () => { + const input = ` +on: push + +jobs: + test: + runs-on: \${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-latest, windows-latest] + node: [14, 16] + include: + - os: macos-latest + node: 14 + exclude: + - os: windows-latest + node: 14 + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: \${{ matrix.node }} +`; + + const result = await validate(createDocument("wf.yaml", input)); + + expect(result).toEqual([]); + }); + + it("matrix with only exclude", async () => { + const input = ` +on: push + +jobs: + test: + runs-on: \${{ matrix.os }} + strategy: + matrix: + exclude: + - os: windows-latest + node: 14 + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: \${{ matrix.node }} +`; + + const result = await validate(createDocument("wf.yaml", input)); + + expect(result).toEqual([ + { + message: "Context access might be invalid: os", + range: { + end: { + character: 29, + line: 5 + }, + start: { + character: 13, + line: 5 + } + }, + severity: DiagnosticSeverity.Warning + }, + { + message: "Context access might be invalid: node", + range: { + end: { + character: 42, + line: 15 + }, + start: { + character: 24, + line: 15 + } + }, + severity: DiagnosticSeverity.Warning + } + ]); + }); + + it("matrix from expression", async () => { + const input = ` +on: push + +jobs: + test: + runs-on: ubuntu-latest + strategy: + matrix: \${{ fromJSON('{"color":["green","blue"]}') }} + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: \${{ matrix.ANYVALUE }} +`; + + const result = await validate(createDocument("wf.yaml", input)); + + expect(result).toEqual([]); + }); + + it("matrix with include expression", async () => { + const input = ` +on: push + +jobs: + test: + runs-on: ubuntu-latest + strategy: + matrix: + fruit: [apple, pear] + animal: [cat, dog] + include: \${{ fromJSON('{"color":"green"}') }} + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: \${{ matrix.ANYVALUE }} +`; + + const result = await validate(createDocument("wf.yaml", input)); + + expect(result).toEqual([]); + }); + + it("matrix with property expression", async () => { + const input = ` +on: push + +jobs: + test: + runs-on: ubuntu-latest + strategy: + matrix: + color: \${{ fromJSON('["green","blue"]') }} + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: \${{ matrix.color }} +`; + + const result = await validate(createDocument("wf.yaml", input)); + + expect(result).toEqual([]); + }); + + it("matrix with property expression and invalid property reference", async () => { + const input = ` +on: push + +jobs: + test: + runs-on: ubuntu-latest + strategy: + matrix: + color: \${{ fromJSON('["green","blue"]') }} + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: \${{ matrix.shape }} +`; + + const result = await validate(createDocument("wf.yaml", input)); + + expect(result).toEqual([ + { + message: "Context access might be invalid: shape", + range: { + end: { + character: 43, + line: 13 + }, + start: { + character: 24, + line: 13 + } + }, + severity: DiagnosticSeverity.Warning + } + ]); + }); + }); }); diff --git a/actions-languageservice/src/validate.ts b/actions-languageservice/src/validate.ts index 39e1177..99962c9 100644 --- a/actions-languageservice/src/validate.ts +++ b/actions-languageservice/src/validate.ts @@ -24,6 +24,7 @@ import {getWorkflowContext, WorkflowContext} from "./context/workflow-context"; import {AccessError, wrapDictionary} from "./expression-validation/error-dictionary"; import {error} from "./log"; import {nullTrace} from "./nulltrace"; +import {getAllowedContext} from "./utils/allowed-context"; import {findToken} from "./utils/find-token"; import {mapRange} from "./utils/range"; import {ValueProviderConfig, ValueProviderKind} from "./value-providers/config"; @@ -95,12 +96,14 @@ async function additionalValidations( const validationToken = key || parent || token; const validationDefinition = validationToken.definition; + const allowedContext = getAllowedContext(validationToken, parent); + // If this is an expression, validate it if (isBasicExpression(token)) { await validateExpression( diagnostics, token, - validationDefinition, + allowedContext, contextProviderConfig, getProviderContext(documentUri, template, root, token) ); @@ -171,14 +174,13 @@ function getProviderContext( async function validateExpression( diagnostics: Diagnostic[], token: BasicExpressionToken, - definition: Definition | undefined, + allowedContext: string[], contextProviderConfig: ContextProviderConfig | undefined, workflowContext: WorkflowContext ) { // Validate the expression for (const expression of token.originalExpressions || [token]) { - const allowedContexts = definition?.readerContext || []; - const {namedContexts, functions} = splitAllowedContext(allowedContexts); + const {namedContexts, functions} = splitAllowedContext(allowedContext); let expr: Expr | undefined;