From 8cc576a65898eb27c2343af8f5080be86fa1306b Mon Sep 17 00:00:00 2001 From: Omar Tawfik Date: Sat, 9 Mar 2019 22:48:55 -0800 Subject: [PATCH] feat: report errors on unresolved actions Fixes $5 --- src/binding/visitors/actions-analyzer.ts | 13 ++++---- src/binding/visitors/resolves-analyzer.ts | 32 +++++++++++++++++++ src/binding/visitors/secrets-analyzer.ts | 6 ++-- src/util/compilation.ts | 4 ++- src/util/diagnostics.ts | 9 ++++++ test/analysis-errors.spec.ts | 29 +++++++++++++++++ test/parsing-errors.spec.ts | 38 +++++++++++++---------- 7 files changed, 105 insertions(+), 26 deletions(-) create mode 100644 src/binding/visitors/resolves-analyzer.ts diff --git a/src/binding/visitors/actions-analyzer.ts b/src/binding/visitors/actions-analyzer.ts index e3c7f24..ce626c3 100644 --- a/src/binding/visitors/actions-analyzer.ts +++ b/src/binding/visitors/actions-analyzer.ts @@ -9,25 +9,26 @@ import { MAXIMUM_SUPPORTED_ACTIONS } from "../../util/constants"; export class ActionsAnalyzer extends BoundNodeVisitor { private exceededMaximum = false; - private allActions = new Set(); + private actions = new Set(); private constructor(document: BoundDocument, private readonly bag: DiagnosticBag) { super(); this.visit(document); } - public static analyze(document: BoundDocument, bag: DiagnosticBag): void { - new ActionsAnalyzer(document, bag); + public static analyze(document: BoundDocument, bag: DiagnosticBag): ReadonlySet { + const instance = new ActionsAnalyzer(document, bag); + return instance.actions; } protected visitAction(node: BoundAction): void { - if (this.allActions.has(node.name)) { + if (this.actions.has(node.name)) { this.bag.duplicateActions(node.name, node.syntax.name.range); } else { - this.allActions.add(node.name); + this.actions.add(node.name); } - if (!this.exceededMaximum && this.allActions.size > MAXIMUM_SUPPORTED_ACTIONS) { + if (!this.exceededMaximum && this.actions.size > MAXIMUM_SUPPORTED_ACTIONS) { this.bag.tooManyActions(node.syntax.name.range); this.exceededMaximum = true; } diff --git a/src/binding/visitors/resolves-analyzer.ts b/src/binding/visitors/resolves-analyzer.ts new file mode 100644 index 0000000..1d3711b --- /dev/null +++ b/src/binding/visitors/resolves-analyzer.ts @@ -0,0 +1,32 @@ +/*! + * Copyright 2019 Omar Tawfik. Please see LICENSE file at the root of this repository. + */ + +import { BoundNodeVisitor } from "../bound-node-visitor"; +import { DiagnosticBag } from "../../util/diagnostics"; +import { BoundDocument, BoundResolves } from "../bound-nodes"; + +export class ResolvesAnalyzer extends BoundNodeVisitor { + private constructor( + document: BoundDocument, + private readonly actions: ReadonlySet, + private readonly bag: DiagnosticBag, + ) { + super(); + this.visit(document); + } + + public static analyze(document: BoundDocument, actions: ReadonlySet, bag: DiagnosticBag): void { + new ResolvesAnalyzer(document, actions, bag); + } + + protected visitResolves(node: BoundResolves): void { + for (const action of node.actions) { + if (!this.actions.has(action.value)) { + this.bag.actionDoesNotExist(action.value, action.syntax.range); + } + } + + super.visitResolves(node); + } +} diff --git a/src/binding/visitors/secrets-analyzer.ts b/src/binding/visitors/secrets-analyzer.ts index aa9ca24..2bb5e4a 100644 --- a/src/binding/visitors/secrets-analyzer.ts +++ b/src/binding/visitors/secrets-analyzer.ts @@ -9,7 +9,7 @@ import { MAXIMUM_SUPPORTED_SECRETS } from "../../util/constants"; export class SecretsAnalyzer extends BoundNodeVisitor { private exceededMaximum = false; - private allSecrets = new Set(); + private secrets = new Set(); private constructor(document: BoundDocument, private readonly bag: DiagnosticBag) { super(); @@ -32,8 +32,8 @@ export class SecretsAnalyzer extends BoundNodeVisitor { if (!this.exceededMaximum) { for (const secret of node.secrets) { - this.allSecrets.add(secret.value); - if (this.allSecrets.size > MAXIMUM_SUPPORTED_SECRETS) { + this.secrets.add(secret.value); + if (this.secrets.size > MAXIMUM_SUPPORTED_SECRETS) { this.bag.tooManySecrets(secret.syntax.range); this.exceededMaximum = true; break; diff --git a/src/util/compilation.ts b/src/util/compilation.ts index eaa15e3..ae13d45 100644 --- a/src/util/compilation.ts +++ b/src/util/compilation.ts @@ -11,6 +11,7 @@ import { BoundDocument } from "../binding/bound-nodes"; import { bindDocument } from "../binding/binder"; import { SecretsAnalyzer } from "../binding/visitors/secrets-analyzer"; import { ActionsAnalyzer } from "../binding/visitors/actions-analyzer"; +import { ResolvesAnalyzer } from "../binding/visitors/resolves-analyzer"; export class Compilation { private readonly bag: DiagnosticBag; @@ -24,7 +25,8 @@ export class Compilation { this.syntax = parseTokens(this.tokens, this.bag); this.document = bindDocument(this.syntax, this.bag); - ActionsAnalyzer.analyze(this.document, this.bag); + const actions = ActionsAnalyzer.analyze(this.document, this.bag); + ResolvesAnalyzer.analyze(this.document, actions, this.bag); SecretsAnalyzer.analyze(this.document, this.bag); } diff --git a/src/util/diagnostics.ts b/src/util/diagnostics.ts index 485ca53..dc416bb 100644 --- a/src/util/diagnostics.ts +++ b/src/util/diagnostics.ts @@ -33,6 +33,7 @@ export enum DiagnosticCode { DuplicateSecrets, TooManyActions, DuplicateActions, + ActionDoesNotExist, } export interface Diagnostic { @@ -215,4 +216,12 @@ export class DiagnosticBag { message: `This file already defines another action with the name '${duplicate}'.`, }); } + + public actionDoesNotExist(action: string, range: TextRange): void { + this.items.push({ + range, + code: DiagnosticCode.ActionDoesNotExist, + message: `The action '${action}' does not exist in the same workflow file.`, + }); + } } diff --git a/test/analysis-errors.spec.ts b/test/analysis-errors.spec.ts index 0bec07f..746f7bb 100644 --- a/test/analysis-errors.spec.ts +++ b/test/analysis-errors.spec.ts @@ -219,6 +219,35 @@ ERROR: Too many actions defined. The maximum currently supported is '100'. | ^^^^^^^^^^^ 103 | " +`); + }); + + it("reports error on resolving a non-existing action", () => { + expectDiagnostics(` +action "a" { + uses = "./ci" +} +action "b" { + uses = "./ci" +} +workflow "c" { + on = "fork" + resolves = [ + "a", + "b", + "not_found" + ] +} +`).toMatchInlineSnapshot(` +" +ERROR: The action 'not_found' does not exist in the same workflow file. + 11 | \\"a\\", + 12 | \\"b\\", + 13 | \\"not_found\\" + | ^^^^^^^^^^^ + 14 | ] + 15 | } +" `); }); }); diff --git a/test/parsing-errors.spec.ts b/test/parsing-errors.spec.ts index 35730d9..084e157 100644 --- a/test/parsing-errors.spec.ts +++ b/test/parsing-errors.spec.ts @@ -89,28 +89,34 @@ ERROR: A token of kind 'string' or '{' or '[' was expected here. it("reports errors on extra commas in a string array", () => { expectDiagnostics(` +action "a" { + uses = "./ci" +} +action "b" { + uses = "./ci" +} workflow "x" { - on = "fork" - resolves = [ - "a", "b", , , "c", - ] + on = "fork" + resolves = [ + , "a", , "b", + ] } `).toMatchInlineSnapshot(` " ERROR: A token of kind ',' was not expected here. - 3 | on = \\"fork\\" - 4 | resolves = [ - 5 | \\"a\\", \\"b\\", , , \\"c\\", - | ^ - 6 | ] - 7 | } + 9 | on = \\"fork\\" + 10 | resolves = [ + 11 | , \\"a\\", , \\"b\\", + | ^ + 12 | ] + 13 | } ERROR: A token of kind ',' was not expected here. - 3 | on = \\"fork\\" - 4 | resolves = [ - 5 | \\"a\\", \\"b\\", , , \\"c\\", - | ^ - 6 | ] - 7 | } + 9 | on = \\"fork\\" + 10 | resolves = [ + 11 | , \\"a\\", , \\"b\\", + | ^ + 12 | ] + 13 | } " `); });