feat: report errors on unknown event types (#45)

Fixes #4

package.json

@@ -87,6 +87,7 @@
     "vscode": "1.1.30"
   },
   "dependencies": {
+    "@octokit/webhooks-definitions": "1.1.2",
     "chalk": "2.4.2",
     "vscode-languageclient": "3.3.0",
     "vscode-languageserver": "3.3.0"

src/analysis/blocks-analyzer.ts

@@ -50,8 +50,6 @@ class BlocksAnalyzer extends BoundNodeVisitor {
       this.bag.tooManyActions(node.syntax.name.range);
       this.actionsExceededMaximum = true;
     }
-
-    super.visitAction(node);
   }
 
   protected visitWorkflow(node: BoundWorkflow): void {
@@ -60,7 +58,5 @@ class BlocksAnalyzer extends BoundNodeVisitor {
     } else {
       this.allWorkflows.add(node.name);
     }
-
-    super.visitWorkflow(node);
   }
 }

src/analysis/properties-analyzer.ts

@@ -0,0 +1,71 @@
+/*!
+ * 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, BoundResolves, BoundSecrets, BoundOn, BoundEnv } from "../binding/bound-nodes";
+import { MAXIMUM_SUPPORTED_SECRETS } from "../util/constants";
+import * as webhooks from "@octokit/webhooks-definitions";
+
+export function analyzeProperties(document: BoundDocument, actions: ReadonlySet<string>, bag: DiagnosticBag): void {
+  new PropertiesAnalyzer(document, actions, bag);
+}
+
+class PropertiesAnalyzer extends BoundNodeVisitor {
+  private exceededMaximum = false;
+  private allSecrets = new Set<string>();
+
+  public constructor(
+    document: BoundDocument,
+    private readonly actions: ReadonlySet<string>,
+    private readonly bag: DiagnosticBag,
+  ) {
+    super();
+    this.visit(document);
+  }
+
+  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);
+      }
+    }
+  }
+
+  protected visitSecrets(node: BoundSecrets): void {
+    const localSecrets = new Set<string>();
+    for (const secret of node.secrets) {
+      if (localSecrets.has(secret.value)) {
+        this.bag.duplicateSecrets(secret.value, secret.syntax.range);
+      } else {
+        localSecrets.add(secret.value);
+      }
+    }
+
+    if (!this.exceededMaximum) {
+      for (const secret of node.secrets) {
+        this.allSecrets.add(secret.value);
+        if (this.allSecrets.size > MAXIMUM_SUPPORTED_SECRETS) {
+          this.bag.tooManySecrets(secret.syntax.range);
+          this.exceededMaximum = true;
+          break;
+        }
+      }
+    }
+  }
+
+  protected visitOn(node: BoundOn): void {
+    if (node.event && !webhooks.some(definition => definition.name === node.event!.value)) {
+      this.bag.unrecognizedEvent(node.event.value, node.event.syntax.range);
+    }
+  }
+
+  protected visitEnv(node: BoundEnv): void {
+    for (const variable of node.variables) {
+      if (variable.name.startsWith("GITHUB_")) {
+        this.bag.reservedEnvironmentVariable(variable.syntax.name.range);
+      }
+    }
+  }
+}

src/binding/binder.ts

@@ -166,11 +166,6 @@ export function bindDocument(root: DocumentSyntax, bag: DiagnosticBag): BoundDoc
             bag.propertyAlreadyDefined(property.key);
           } else {
             env = new BoundEnv(bindObject(property), property);
-            for (const variable of env.variables) {
-              if (variable.name.startsWith("GITHUB_")) {
-                bag.reservedEnvironmentVariable(variable.syntax.name.range);
-              }
-            }
           }
           break;
         }
@@ -203,7 +198,7 @@ export function bindDocument(root: DocumentSyntax, bag: DiagnosticBag): BoundDoc
     switch (syntax.kind) {
       case SyntaxKind.StringProperty: {
         const property = syntax as StringPropertySyntax;
-        if (property.value) {
+        if (property.value && property.value.kind !== TokenKind.Missing) {
           const value = removeDoubleQuotes(property.value.text);
           return new BoundStringValue(value, property.value);
         }
@@ -231,16 +226,16 @@ export function bindDocument(root: DocumentSyntax, bag: DiagnosticBag): BoundDoc
     switch (syntax.kind) {
       case SyntaxKind.StringProperty: {
         const property = syntax as StringPropertySyntax;
-        if (property.value) {
+        if (property.value && property.value.kind !== TokenKind.Missing) {
           const value = removeDoubleQuotes(property.value.text);
           return [new BoundStringValue(value, property.value)];
         }
         return [];
       }
       case SyntaxKind.ArrayProperty: {
-        return (syntax as ArrayPropertySyntax).items.map(
-          item => new BoundStringValue(removeDoubleQuotes(item.value.text), item.value),
-        );
+        return (syntax as ArrayPropertySyntax).items
+          .filter(item => item.value.kind !== TokenKind.Missing)
+          .map(item => new BoundStringValue(removeDoubleQuotes(item.value.text), item.value));
       }
       case SyntaxKind.ObjectProperty: {
         bag.valueIsNotStringOrArray(syntax.key.range);
@@ -267,9 +262,9 @@ export function bindDocument(root: DocumentSyntax, bag: DiagnosticBag): BoundDoc
         return [];
       }
       case SyntaxKind.ObjectProperty: {
-        return (syntax as ObjectPropertySyntax).members.map(
-          member => new BoundObjectMember(member.name.text, removeDoubleQuotes(member.value.text), member),
-        );
+        return (syntax as ObjectPropertySyntax).members
+          .filter(member => member.name.kind !== TokenKind.Missing && member.value.kind !== TokenKind.Missing)
+          .map(member => new BoundObjectMember(member.name.text, removeDoubleQuotes(member.value.text), member));
       }
       default: {
         throw new Error(`Unexpected Syntax kind '${syntax.kind}'`);

src/util/compilation.ts

@@ -9,9 +9,8 @@ import { DocumentSyntax } from "../parsing/syntax-nodes";
 import { parseTokens } from "../parsing/parser";
 import { BoundDocument } from "../binding/bound-nodes";
 import { bindDocument } from "../binding/binder";
-import { analyzeSecrets } from "../analysis/secrets-analyzer";
 import { analyzeBlocks } from "../analysis/blocks-analyzer";
-import { analyzeResolves } from "../analysis/resolves-analyzer";
+import { analyzeProperties } from "../analysis/properties-analyzer";
 
 export class Compilation {
   private readonly bag: DiagnosticBag;
@@ -27,8 +26,7 @@ export class Compilation {
     this.document = bindDocument(this.syntax, this.bag);
 
     const { actions } = analyzeBlocks(this.document, this.bag);
-    analyzeResolves(this.document, actions, this.bag);
-    analyzeSecrets(this.document, this.bag);
+    analyzeProperties(this.document, actions, this.bag);
   }
 
   public get diagnostics(): ReadonlyArray<Diagnostic> {

src/util/diagnostics.ts

@@ -34,6 +34,7 @@ export enum DiagnosticCode {
   TooManyActions,
   DuplicateBlock,
   ActionDoesNotExist,
+  UnrecognizedEvent,
 }
 
 export interface Diagnostic {
@@ -224,4 +225,12 @@ export class DiagnosticBag {
       message: `The action '${action}' does not exist in the same workflow file.`,
     });
   }
+
+  public unrecognizedEvent(event: string, range: TextRange): void {
+    this.items.push({
+      range,
+      code: DiagnosticCode.UnrecognizedEvent,
+      message: `The event '${event}' is not a known event type.`,
+    });
+  }
 }

test/binding-errors.spec.ts → test/binding.spec.ts

@@ -223,28 +223,6 @@ ERROR: Value must be an object.
   5 | }
   6 | 
 "
-`);
-  });
-
-  it("reports errors on reserved environment variables", () => {
-    expectDiagnostics(`
-action "x" {
-  uses = "./ci"
-  env = {
-    GITHUB_ACTION = "1"
-    GITHUBNOUNDERSCORE = "2"
-    SOMETHING_ELSE = "3"
-  }
-}`).toMatchInlineSnapshot(`
-"
-ERROR: Environment variables starting with 'GITHUB_' are reserved.
-  3 |   uses = \\"./ci\\"
-  4 |   env = {
-  5 |     GITHUB_ACTION = \\"1\\"
-    |     ^^^^^^^^^^^^^
-  6 |     GITHUBNOUNDERSCORE = \\"2\\"
-  7 |     SOMETHING_ELSE = \\"3\\"
-"
 `);
   });
 });