diff --git a/src/analysis/blocks-analyzer.ts b/src/analysis/blocks-analyzer.ts new file mode 100644 index 0000000..ddf59b7 --- /dev/null +++ b/src/analysis/blocks-analyzer.ts @@ -0,0 +1,66 @@ +/*! + * Copyright 2019 Omar Tawfik. Please see LICENSE file at the root of this repository. + */ + +import { BoundNodeVisitor } from "../binding/bound-node-visitor"; +import { DiagnosticBag } from "../util/diagnostics"; +import { BoundDocument, BoundAction, BoundWorkflow } from "../binding/bound-nodes"; +import { MAXIMUM_SUPPORTED_ACTIONS } from "../util/constants"; + +export function analyzeBlocks( + document: BoundDocument, + bag: DiagnosticBag, +): { + workflows: ReadonlySet; + actions: ReadonlySet; +} { + const instance = new BlocksAnalyzer(document, bag); + return { + workflows: instance.workflows, + actions: instance.actions, + }; +} + +class BlocksAnalyzer extends BoundNodeVisitor { + private readonly allWorkflows = new Set(); + private readonly allActions = new Set(); + private actionsExceededMaximum = false; + + public constructor(document: BoundDocument, private readonly bag: DiagnosticBag) { + super(); + this.visit(document); + } + + public get workflows(): ReadonlySet { + return this.allWorkflows; + } + + public get actions(): ReadonlySet { + return this.allActions; + } + + protected visitAction(node: BoundAction): void { + if (this.allActions.has(node.name) || this.allWorkflows.has(node.name)) { + this.bag.duplicateBlock(node.name, node.syntax.name.range); + } else { + this.allActions.add(node.name); + } + + if (!this.actionsExceededMaximum && this.allActions.size > MAXIMUM_SUPPORTED_ACTIONS) { + this.bag.tooManyActions(node.syntax.name.range); + this.actionsExceededMaximum = true; + } + + super.visitAction(node); + } + + protected visitWorkflow(node: BoundWorkflow): void { + if (this.allActions.has(node.name) || this.allWorkflows.has(node.name)) { + this.bag.duplicateBlock(node.name, node.syntax.name.range); + } else { + this.allWorkflows.add(node.name); + } + + super.visitWorkflow(node); + } +} diff --git a/src/binding/visitors/resolves-analyzer.ts b/src/analysis/resolves-analyzer.ts similarity index 55% rename from src/binding/visitors/resolves-analyzer.ts rename to src/analysis/resolves-analyzer.ts index 1d3711b..3002179 100644 --- a/src/binding/visitors/resolves-analyzer.ts +++ b/src/analysis/resolves-analyzer.ts @@ -2,12 +2,16 @@ * 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"; +import { BoundNodeVisitor } from "../binding/bound-node-visitor"; +import { DiagnosticBag } from "../util/diagnostics"; +import { BoundDocument, BoundResolves } from "../binding/bound-nodes"; -export class ResolvesAnalyzer extends BoundNodeVisitor { - private constructor( +export function analyzeResolves(document: BoundDocument, actions: ReadonlySet, bag: DiagnosticBag): void { + new ResolvesAnalyzer(document, actions, bag); +} + +class ResolvesAnalyzer extends BoundNodeVisitor { + public constructor( document: BoundDocument, private readonly actions: ReadonlySet, private readonly bag: DiagnosticBag, @@ -16,10 +20,6 @@ export class ResolvesAnalyzer extends BoundNodeVisitor { 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)) { diff --git a/src/binding/visitors/secrets-analyzer.ts b/src/analysis/secrets-analyzer.ts similarity index 63% rename from src/binding/visitors/secrets-analyzer.ts rename to src/analysis/secrets-analyzer.ts index 2bb5e4a..06b8887 100644 --- a/src/binding/visitors/secrets-analyzer.ts +++ b/src/analysis/secrets-analyzer.ts @@ -2,24 +2,24 @@ * 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 { BoundSecrets, BoundDocument } from "../bound-nodes"; -import { MAXIMUM_SUPPORTED_SECRETS } from "../../util/constants"; +import { BoundNodeVisitor } from "../binding/bound-node-visitor"; +import { DiagnosticBag } from "../util/diagnostics"; +import { BoundSecrets, BoundDocument } from "../binding/bound-nodes"; +import { MAXIMUM_SUPPORTED_SECRETS } from "../util/constants"; -export class SecretsAnalyzer extends BoundNodeVisitor { +export function analyzeSecrets(document: BoundDocument, bag: DiagnosticBag): void { + new SecretsAnalyzer(document, bag); +} + +class SecretsAnalyzer extends BoundNodeVisitor { private exceededMaximum = false; private secrets = new Set(); - private constructor(document: BoundDocument, private readonly bag: DiagnosticBag) { + public constructor(document: BoundDocument, private readonly bag: DiagnosticBag) { super(); this.visit(document); } - public static analyze(document: BoundDocument, bag: DiagnosticBag): void { - new SecretsAnalyzer(document, bag); - } - protected visitSecrets(node: BoundSecrets): void { const localSecrets = new Set(); for (const secret of node.secrets) { diff --git a/src/binding/visitors/actions-analyzer.ts b/src/binding/visitors/actions-analyzer.ts deleted file mode 100644 index ce626c3..0000000 --- a/src/binding/visitors/actions-analyzer.ts +++ /dev/null @@ -1,38 +0,0 @@ -/*! - * 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, BoundAction } from "../bound-nodes"; -import { MAXIMUM_SUPPORTED_ACTIONS } from "../../util/constants"; - -export class ActionsAnalyzer extends BoundNodeVisitor { - private exceededMaximum = false; - private actions = new Set(); - - private constructor(document: BoundDocument, private readonly bag: DiagnosticBag) { - super(); - this.visit(document); - } - - public static analyze(document: BoundDocument, bag: DiagnosticBag): ReadonlySet { - const instance = new ActionsAnalyzer(document, bag); - return instance.actions; - } - - protected visitAction(node: BoundAction): void { - if (this.actions.has(node.name)) { - this.bag.duplicateActions(node.name, node.syntax.name.range); - } else { - this.actions.add(node.name); - } - - if (!this.exceededMaximum && this.actions.size > MAXIMUM_SUPPORTED_ACTIONS) { - this.bag.tooManyActions(node.syntax.name.range); - this.exceededMaximum = true; - } - - super.visitAction(node); - } -} diff --git a/src/util/compilation.ts b/src/util/compilation.ts index ae13d45..8f89b8d 100644 --- a/src/util/compilation.ts +++ b/src/util/compilation.ts @@ -9,9 +9,9 @@ import { DocumentSyntax } from "../parsing/syntax-nodes"; import { parseTokens } from "../parsing/parser"; 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"; +import { analyzeSecrets } from "../analysis/secrets-analyzer"; +import { analyzeBlocks } from "../analysis/blocks-analyzer"; +import { analyzeResolves } from "../analysis/resolves-analyzer"; export class Compilation { private readonly bag: DiagnosticBag; @@ -21,13 +21,14 @@ export class Compilation { public constructor(public readonly text: string) { this.bag = new DiagnosticBag(); + this.tokens = scanText(text, this.bag); this.syntax = parseTokens(this.tokens, this.bag); this.document = bindDocument(this.syntax, this.bag); - const actions = ActionsAnalyzer.analyze(this.document, this.bag); - ResolvesAnalyzer.analyze(this.document, actions, this.bag); - SecretsAnalyzer.analyze(this.document, this.bag); + const { actions } = analyzeBlocks(this.document, this.bag); + analyzeResolves(this.document, actions, this.bag); + analyzeSecrets(this.document, this.bag); } public get diagnostics(): ReadonlyArray { diff --git a/src/util/diagnostics.ts b/src/util/diagnostics.ts index dc416bb..db47c4e 100644 --- a/src/util/diagnostics.ts +++ b/src/util/diagnostics.ts @@ -32,7 +32,7 @@ export enum DiagnosticCode { TooManySecrets, DuplicateSecrets, TooManyActions, - DuplicateActions, + DuplicateBlock, ActionDoesNotExist, } @@ -209,11 +209,11 @@ export class DiagnosticBag { }); } - public duplicateActions(duplicate: string, range: TextRange): void { + public duplicateBlock(duplicate: string, range: TextRange): void { this.items.push({ range, - code: DiagnosticCode.DuplicateActions, - message: `This file already defines another action with the name '${duplicate}'.`, + code: DiagnosticCode.DuplicateBlock, + message: `This file already defines another workflow or action with the name '${duplicate}'.`, }); } diff --git a/test/analysis-errors.spec.ts b/test/analysis-errors.spec.ts index 746f7bb..df754b0 100644 --- a/test/analysis-errors.spec.ts +++ b/test/analysis-errors.spec.ts @@ -80,7 +80,7 @@ ERROR: This 'secrets' property has duplicate 'S1' secrets. `); }); - it("reports errors on duplicate actions", () => { + it("reports errors on two actions with the same name", () => { expectDiagnostics(` action "a" { uses = "./ci" @@ -91,12 +91,9 @@ action "b" { action "a" { uses = "./ci" } -action "d" { - uses = "./ci" -} `).toMatchInlineSnapshot(` " -ERROR: This file already defines another action with the name 'a'. +ERROR: This file already defines another workflow or action with the name 'a'. 6 | uses = \\"./ci\\" 7 | } 8 | action \\"a\\" { @@ -107,6 +104,53 @@ ERROR: This file already defines another action with the name 'a'. `); }); + it("reports errors on two workflows with the same name", () => { + expectDiagnostics(` +workflow "a" { + on = "fork" +} +workflow "b" { + on = "fork" +} +workflow "a" { + on = "fork" +} +`).toMatchInlineSnapshot(` +" +ERROR: This file already defines another workflow or action with the name 'a'. + 6 | on = \\"fork\\" + 7 | } + 8 | workflow \\"a\\" { + | ^^^ + 9 | on = \\"fork\\" + 10 | } +" +`); + }); + + it("reports errors on a workflow and an action with the same name", () => { + expectDiagnostics(` +action "a" { + uses = "./ci" +} +workflow "b" { + on = "fork" +} +workflow "a" { + on = "fork" +} +`).toMatchInlineSnapshot(` +" +ERROR: This file already defines another workflow or action with the name 'a'. + 1 | + 2 | action \\"a\\" { + | ^^^ + 3 | uses = \\"./ci\\" + 4 | } +" +`); + }); + it("reports errors on too many actions", () => { expectDiagnostics(` action "action001" { uses = "./ci" }