fix: report errors on duplicate actions or workflows (#44)

Fixes #41

Author: OmarTawfik

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<string>;
+  actions: ReadonlySet<string>;
+} {
+  const instance = new BlocksAnalyzer(document, bag);
+  return {
+    workflows: instance.workflows,
+    actions: instance.actions,
+  };
+}
+
+class BlocksAnalyzer extends BoundNodeVisitor {
+  private readonly allWorkflows = new Set<string>();
+  private readonly allActions = new Set<string>();
+  private actionsExceededMaximum = false;
+
+  public constructor(document: BoundDocument, private readonly bag: DiagnosticBag) {
+    super();
+    this.visit(document);
+  }
+
+  public get workflows(): ReadonlySet<string> {
+    return this.allWorkflows;
+  }
+
+  public get actions(): ReadonlySet<string> {
+    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);
+  }
+}

src/binding/visitors/resolves-analyzer.ts renamed to 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<string>, bag: DiagnosticBag): void {
+  new ResolvesAnalyzer(document, actions, bag);
+}
+
+class ResolvesAnalyzer extends BoundNodeVisitor {
+  public constructor(
     document: BoundDocument,
     private readonly actions: ReadonlySet<string>,
     private readonly bag: DiagnosticBag,
@@ -16,10 +20,6 @@ export class ResolvesAnalyzer extends BoundNodeVisitor {
     this.visit(document);
   }
 
-  public static analyze(document: BoundDocument, actions: ReadonlySet<string>, 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)) {

src/binding/visitors/secrets-analyzer.ts renamed to 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<string>();
 
-  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<string>();
     for (const secret of node.secrets) {

src/binding/visitors/actions-analyzer.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<Diagnostic> {

src/util/compilation.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}'.`,
     });
   }
 

src/util/diagnostics.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" }