From 8ece721882d646109a47aa1f8a48dba3047a3e24 Mon Sep 17 00:00:00 2001 From: Omar Tawfik Date: Wed, 6 Mar 2019 17:13:19 -0800 Subject: [PATCH] feat: report errors on too many secrets (#27) Closes #9 --- gulpfile.ts | 17 +--- scripts/gulp-build.ts | 5 + src/binding/binder.ts | 3 +- src/binding/bound-node-visitor.ts | 121 +++++++++++++++++++++++ src/binding/bound-nodes.ts | 58 +++++++++++ src/binding/visitors/too-many-secrets.ts | 33 +++++++ src/util/array-utils.ts | 15 +++ src/util/compilation.ts | 5 +- src/util/constants.ts | 6 ++ src/util/diagnostics.ts | 13 ++- test/analysis-errors.spec.ts | 58 +++++++++++ 11 files changed, 315 insertions(+), 19 deletions(-) create mode 100644 src/binding/bound-node-visitor.ts create mode 100644 src/binding/visitors/too-many-secrets.ts create mode 100644 src/util/array-utils.ts create mode 100644 src/util/constants.ts create mode 100644 test/analysis-errors.spec.ts diff --git a/gulpfile.ts b/gulpfile.ts index 05642ad..36ef687 100644 --- a/gulpfile.ts +++ b/gulpfile.ts @@ -7,23 +7,14 @@ import { BuildTasks } from "./scripts/gulp-build"; import { PackageTasks } from "./scripts/gulp-linter"; import { VSCodeTasks } from "./scripts/gulp-vscode"; -const npmPackageTask = "create-linter-package"; -gulp.task(npmPackageTask, gulp.parallel([PackageTasks.copyFiles, PackageTasks.generatePackageJson])); - -const vscodePackageTask = "create-vscode-package"; -gulp.task(vscodePackageTask, gulp.parallel([VSCodeTasks.copyFiles, VSCodeTasks.generatePackageJson])); - gulp.task( "ci", gulp.series([ BuildTasks.clean, - BuildTasks.compile, - BuildTasks.prettier, - BuildTasks.tslint, - BuildTasks.jest, - npmPackageTask, - vscodePackageTask, + gulp.parallel([BuildTasks.compile, BuildTasks.prettier, BuildTasks.tslint, BuildTasks.jestCI]), + gulp.parallel([PackageTasks.copyFiles, PackageTasks.generatePackageJson]), + gulp.parallel([VSCodeTasks.copyFiles, VSCodeTasks.generatePackageJson]), ]), ); -gulp.task("default", gulp.parallel([BuildTasks.compile, BuildTasks.jest])); +gulp.task("default", gulp.series(BuildTasks.jest)); diff --git a/scripts/gulp-build.ts b/scripts/gulp-build.ts index 4878baa..691e8d5 100644 --- a/scripts/gulp-build.ts +++ b/scripts/gulp-build.ts @@ -13,6 +13,7 @@ export module BuildTasks { export const clean = "build:clean"; export const compile = "build:compile"; export const jest = "build:jest"; + export const jestCI = "build:jest-ci"; export const prettier = "build:prettier"; export const tslint = "build:tslint"; } @@ -32,6 +33,10 @@ gulp.task(BuildTasks.compile, () => { }); gulp_shell(BuildTasks.jest, () => { + return [path.join(rootPath, "node_modules", ".bin", "jest"), "--verbose"]; +}); + +gulp_shell(BuildTasks.jestCI, () => { return [path.join(rootPath, "node_modules", ".bin", "jest"), "--verbose", "--ci"]; }); diff --git a/src/binding/binder.ts b/src/binding/binder.ts index ff46de8..b1218bb 100644 --- a/src/binding/binder.ts +++ b/src/binding/binder.ts @@ -28,8 +28,7 @@ import { BoundUses, } from "./bound-nodes"; import { TokenKind } from "../scanning/tokens"; - -export const MAXIMUM_SUPPORTED_VERSION = 0; +import { MAXIMUM_SUPPORTED_VERSION } from "../util/constants"; export function bindDocument(root: DocumentSyntax, bag: DiagnosticBag): BoundDocument { let version: BoundVersion | undefined; diff --git a/src/binding/bound-node-visitor.ts b/src/binding/bound-node-visitor.ts new file mode 100644 index 0000000..6bea44b --- /dev/null +++ b/src/binding/bound-node-visitor.ts @@ -0,0 +1,121 @@ +/*! + * Copyright 2019 Omar Tawfik. Please see LICENSE file at the root of this repository. + */ + +import { + BaseBoundNode, + BoundKind, + BoundDocument, + BoundVersion, + BoundWorkflow, + BoundAction, + BoundOn, + BoundResolves, + BoundUses, + BoundNeeds, + BoundRuns, + BoundArgs, + BoundEnv, + BoundSecrets, +} from "./bound-nodes"; +import { BaseSyntaxNode } from "../parsing/syntax-nodes"; + +export abstract class BoundNodeVisitor { + protected visit(node: BaseBoundNode): void { + switch (node.kind) { + case BoundKind.Document: { + return this.visitDocument(node as BoundDocument); + } + case BoundKind.Version: { + return this.visitVersion(node as BoundVersion); + } + case BoundKind.Workflow: { + return this.visitWorkflow(node as BoundWorkflow); + } + case BoundKind.Action: { + return this.visitAction(node as BoundAction); + } + case BoundKind.On: { + return this.visitOn(node as BoundOn); + } + case BoundKind.Resolves: { + return this.visitResolves(node as BoundResolves); + } + case BoundKind.Uses: { + return this.visitUses(node as BoundUses); + } + case BoundKind.Needs: { + return this.visitNeeds(node as BoundNeeds); + } + case BoundKind.Runs: { + return this.visitRuns(node as BoundRuns); + } + case BoundKind.Args: { + return this.visitArgs(node as BoundArgs); + } + case BoundKind.Env: { + return this.visitEnv(node as BoundEnv); + } + case BoundKind.Secrets: { + return this.visitSecrets(node as BoundSecrets); + } + default: { + throw new Error(`Unexpected bound kind: '${node.kind}'`); + } + } + } + + protected visitDocument(node: BoundDocument): void { + return this.visitDefault(node); + } + + protected visitVersion(node: BoundVersion): void { + return this.visitDefault(node); + } + + protected visitWorkflow(node: BoundWorkflow): void { + return this.visitDefault(node); + } + + protected visitAction(node: BoundAction): void { + return this.visitDefault(node); + } + + protected visitOn(node: BoundOn): void { + return this.visitDefault(node); + } + + protected visitResolves(node: BoundResolves): void { + return this.visitDefault(node); + } + + protected visitUses(node: BoundUses): void { + return this.visitDefault(node); + } + + protected visitNeeds(node: BoundNeeds): void { + return this.visitDefault(node); + } + + protected visitRuns(node: BoundRuns): void { + return this.visitDefault(node); + } + + protected visitArgs(node: BoundArgs): void { + return this.visitDefault(node); + } + + protected visitEnv(node: BoundEnv): void { + return this.visitDefault(node); + } + + protected visitSecrets(node: BoundSecrets): void { + return this.visitDefault(node); + } + + private visitDefault(node: BaseBoundNode): void { + node.children.forEach(child => { + this.visit(child); + }); + } +} diff --git a/src/binding/bound-nodes.ts b/src/binding/bound-nodes.ts index 5352ab4..41985e4 100644 --- a/src/binding/bound-nodes.ts +++ b/src/binding/bound-nodes.ts @@ -3,6 +3,7 @@ */ import { DocumentSyntax, BlockSyntax, BaseSyntaxNode, VersionSyntax, PropertySyntax } from "../parsing/syntax-nodes"; +import { filterUndefined } from "../util/array-utils"; export const enum BoundKind { // Top level @@ -24,6 +25,8 @@ export const enum BoundKind { export abstract class BaseBoundNode { protected constructor(public readonly kind: BoundKind, public readonly syntax: TSyntax) {} + + public abstract get children(): ReadonlyArray>; } export class BoundDocument extends BaseBoundNode { @@ -35,12 +38,20 @@ export class BoundDocument extends BaseBoundNode { ) { super(BoundKind.Document, syntax); } + + public get children(): ReadonlyArray> { + return filterUndefined>(this.version, ...this.workflows, ...this.actions); + } } export class BoundVersion extends BaseBoundNode { public constructor(public readonly version: number, syntax: VersionSyntax) { super(BoundKind.Version, syntax); } + + public get children(): ReadonlyArray> { + return []; + } } export class BoundWorkflow extends BaseBoundNode { @@ -52,6 +63,10 @@ export class BoundWorkflow extends BaseBoundNode { ) { super(BoundKind.Workflow, syntax); } + + public get children(): ReadonlyArray> { + return filterUndefined>(this.on, this.resolves); + } } export class BoundAction extends BaseBoundNode { @@ -67,52 +82,95 @@ export class BoundAction extends BaseBoundNode { ) { super(BoundKind.Action, syntax); } + + public get children(): ReadonlyArray> { + return filterUndefined>( + this.uses, + this.needs, + this.runs, + this.args, + this.env, + this.secrets, + ); + } } export class BoundOn extends BaseBoundNode { public constructor(public readonly event: string, syntax: PropertySyntax) { super(BoundKind.On, syntax); } + + public get children(): ReadonlyArray> { + return []; + } } export class BoundResolves extends BaseBoundNode { public constructor(public readonly actions: ReadonlyArray, syntax: PropertySyntax) { super(BoundKind.Resolves, syntax); } + + public get children(): ReadonlyArray> { + return []; + } } export class BoundUses extends BaseBoundNode { public constructor(public readonly value: string, syntax: PropertySyntax) { super(BoundKind.Uses, syntax); } + + public get children(): ReadonlyArray> { + return []; + } } export class BoundNeeds extends BaseBoundNode { public constructor(public readonly actions: ReadonlyArray, syntax: PropertySyntax) { super(BoundKind.Needs, syntax); } + + public get children(): ReadonlyArray> { + return []; + } } export class BoundRuns extends BaseBoundNode { public constructor(public readonly commands: ReadonlyArray, syntax: PropertySyntax) { super(BoundKind.Runs, syntax); } + + public get children(): ReadonlyArray> { + return []; + } } export class BoundArgs extends BaseBoundNode { public constructor(public readonly args: ReadonlyArray, syntax: PropertySyntax) { super(BoundKind.Args, syntax); } + + public get children(): ReadonlyArray> { + return []; + } } export class BoundEnv extends BaseBoundNode { public constructor(public readonly variables: ReadonlyMap, syntax: PropertySyntax) { super(BoundKind.Env, syntax); } + + public get children(): ReadonlyArray> { + return []; + } } export class BoundSecrets extends BaseBoundNode { public constructor(public readonly args: ReadonlyArray, syntax: PropertySyntax) { super(BoundKind.Secrets, syntax); } + + public get children(): ReadonlyArray> { + return []; + } } diff --git a/src/binding/visitors/too-many-secrets.ts b/src/binding/visitors/too-many-secrets.ts new file mode 100644 index 0000000..14a3c8c --- /dev/null +++ b/src/binding/visitors/too-many-secrets.ts @@ -0,0 +1,33 @@ +/*! + * 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"; + +export class TooManySecretsVisitor extends BoundNodeVisitor { + private definedAlready = new Set(); + private alreadyReported = false; + + public constructor(document: BoundDocument, private readonly bag: DiagnosticBag) { + super(); + this.visit(document); + } + + protected visitSecrets(node: BoundSecrets): void { + if (!this.alreadyReported) { + for (const arg of node.args) { + this.definedAlready.add(arg); + if (this.definedAlready.size > MAXIMUM_SUPPORTED_SECRETS) { + this.bag.tooManySecrets(node.syntax.key.range); + this.alreadyReported = true; + break; + } + } + } + + super.visitSecrets(node); + } +} diff --git a/src/util/array-utils.ts b/src/util/array-utils.ts new file mode 100644 index 0000000..af4656f --- /dev/null +++ b/src/util/array-utils.ts @@ -0,0 +1,15 @@ +/*! + * Copyright 2019 Omar Tawfik. Please see LICENSE file at the root of this repository. + */ + +export function filterUndefined(...items: (T | undefined)[]): T[] { + const result = Array(); + + items.forEach(item => { + if (item) { + result.push(item); + } + }); + + return result; +} diff --git a/src/util/compilation.ts b/src/util/compilation.ts index fcfb5f7..c22d82e 100644 --- a/src/util/compilation.ts +++ b/src/util/compilation.ts @@ -9,6 +9,7 @@ import { DocumentSyntax } from "../parsing/syntax-nodes"; import { parseTokens } from "../parsing/parser"; import { BoundDocument } from "../binding/bound-nodes"; import { bindDocument } from "../binding/binder"; +import { TooManySecretsVisitor } from "../binding/visitors/too-many-secrets"; export class Compilation { private readonly bag: DiagnosticBag; @@ -22,9 +23,7 @@ export class Compilation { this.syntax = parseTokens(this.tokens, this.bag); this.document = bindDocument(this.syntax, this.bag); - if (this.document) { - // are we exporting the bound tree? or accessing services through the compilation? - } + new TooManySecretsVisitor(this.document, this.bag); } public get diagnostics(): ReadonlyArray { diff --git a/src/util/constants.ts b/src/util/constants.ts new file mode 100644 index 0000000..16fff7c --- /dev/null +++ b/src/util/constants.ts @@ -0,0 +1,6 @@ +/*! + * Copyright 2019 Omar Tawfik. Please see LICENSE file at the root of this repository. + */ + +export const MAXIMUM_SUPPORTED_VERSION = 0; +export const MAXIMUM_SUPPORTED_SECRETS = 100; diff --git a/src/util/diagnostics.ts b/src/util/diagnostics.ts index 2b8f1e7..5abaa23 100644 --- a/src/util/diagnostics.ts +++ b/src/util/diagnostics.ts @@ -3,7 +3,7 @@ */ import { TokenKind, TextRange, getTokenDescription, Token } from "../scanning/tokens"; -import { MAXIMUM_SUPPORTED_VERSION } from "../binding/binder"; +import { MAXIMUM_SUPPORTED_VERSION, MAXIMUM_SUPPORTED_SECRETS } from "./constants"; export const enum DiagnosticCode { // Scanning @@ -26,6 +26,9 @@ export const enum DiagnosticCode { PropertyMustBeDefined = 13, InvalidProperty = 14, DuplicateKey = 15, + + // Analysis: + TooManySecrets = 16, } export interface Diagnostic { @@ -168,4 +171,12 @@ export class DiagnosticBag { message: `A key with the name '${key}' is already defined.`, }); } + + public tooManySecrets(range: TextRange): void { + this.items.push({ + range, + code: DiagnosticCode.TooManySecrets, + message: `Too many secrets defined. The maximum currently supported is '${MAXIMUM_SUPPORTED_SECRETS}'.`, + }); + } } diff --git a/test/analysis-errors.spec.ts b/test/analysis-errors.spec.ts new file mode 100644 index 0000000..dabea23 --- /dev/null +++ b/test/analysis-errors.spec.ts @@ -0,0 +1,58 @@ +/*! + * Copyright 2019 Omar Tawfik. Please see LICENSE file at the root of this repository. + */ + +import { expectDiagnostics } from "./utils"; + +describe(__filename, () => { + it("reports errors on too many secrets", () => { + expectDiagnostics(` +action "a" { + uses = "./ci" + secrets = [ + "SECRET1", "SECRET2", "SECRET3", "SECRET4", "SECRET5", "SECRET6", "SECRET7", "SECRET8", "SECRET9", "SECRET10", + "SECRET11", "SECRET12", "SECRET13", "SECRET14", "SECRET15", "SECRET16", "SECRET17", "SECRET18", "SECRET19", "SECRET20", + "SECRET21", "SECRET22", "SECRET23", "SECRET24", "SECRET25", "SECRET26", "SECRET27", "SECRET28", "SECRET29", "SECRET30", + "SECRET31", "SECRET32", "SECRET33", "SECRET34", "SECRET35", "SECRET36", "SECRET37", "SECRET38", "SECRET39", "SECRET40", + "SECRET41", "SECRET42", "SECRET43", "SECRET44", "SECRET45", "SECRET46", "SECRET47", "SECRET48", "SECRET49", "SECRET50", + "SECRET51", "SECRET52", "SECRET53", "SECRET54", "SECRET55", "SECRET56", "SECRET57", "SECRET58", "SECRET59", "SECRET60", + "SECRET61", "SECRET62", "SECRET63", "SECRET64", "SECRET65", "SECRET66", "SECRET67", "SECRET68", "SECRET69", "SECRET70", + "SECRET71", "SECRET72", "SECRET73", "SECRET74", "SECRET75", "SECRET76", "SECRET77", "SECRET78", "SECRET79", "SECRET80", + "SECRET81", "SECRET82", "SECRET83", "SECRET84", "SECRET85", "SECRET86", "SECRET87", "SECRET88", "SECRET89", "SECRET90", + "SECRET91", "SECRET92", "SECRET93", "SECRET94", "SECRET95", "SECRET96", "SECRET97", "SECRET98", "SECRET99", "SECRET100" + ] +} + +action "b" { + uses = "./ci" + secrets = [ + "SECRET50" # Not a duplicate + ] +} + +action "c" { + uses = "./ci" + secrets = [ + "EXTRA_1" # should be reported + ] +} + +action "d" { + uses = "./ci" + secrets = [ + "EXTRA_2" # Should not be reported + ] +} +`).toMatchInlineSnapshot(` +" +ERROR: Too many secrets defined. The maximum currently supported is '100'. + 25 | action \\"c\\" { + 26 | uses = \\"./ci\\" + 27 | secrets = [ + | ^^^^^^^ + 28 | \\"EXTRA_1\\" # should be reported + 29 | ] +" +`); + }); +});