From 272dec83cef4aeae27e4b923da1316219d1e0f65 Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Fri, 17 Mar 2023 11:17:18 -0400 Subject: [PATCH] Lint the workflow parser package --- workflow-parser/.eslintrc.cjs | 4 ++++ workflow-parser/src/expressions.test.ts | 15 ++++++++------- workflow-parser/src/index.test.ts | 2 +- .../src/model/converter/container.ts | 2 +- .../src/model/converter/id-builder.ts | 2 +- workflow-parser/src/model/converter/job.ts | 2 +- .../src/model/converter/job/inputs.ts | 8 ++++---- .../src/model/converter/job/secrets.ts | 10 +++++----- .../templates/schema/boolean-definition.ts | 5 +++-- .../src/templates/schema/null-definition.ts | 5 +++-- .../src/templates/schema/number-definition.ts | 5 +++-- .../src/templates/schema/string-definition.ts | 3 +-- .../src/templates/schema/template-schema.ts | 1 + .../templates/tokens/template-token.test.ts | 5 +++-- workflow-parser/src/xlang.test.ts | 19 ++++++++----------- 15 files changed, 47 insertions(+), 41 deletions(-) diff --git a/workflow-parser/.eslintrc.cjs b/workflow-parser/.eslintrc.cjs index 76e2889..558a252 100644 --- a/workflow-parser/.eslintrc.cjs +++ b/workflow-parser/.eslintrc.cjs @@ -4,4 +4,8 @@ module.exports = { project: './tsconfig.json', tsconfigRootDir: __dirname, }, + rules: { + // TypeScript doesn't correctly detect toString() implementations on the token classes + "@typescript-eslint/restrict-template-expressions": "off", + } } \ No newline at end of file diff --git a/workflow-parser/src/expressions.test.ts b/workflow-parser/src/expressions.test.ts index 1e46282..cf22c62 100644 --- a/workflow-parser/src/expressions.test.ts +++ b/workflow-parser/src/expressions.test.ts @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/no-non-null-assertion */ import {StringToken} from "./templates/tokens"; import {isBasicExpression, isString} from "./templates/tokens/type-guards"; import {nullTrace} from "./test-utils/null-trace"; @@ -25,7 +26,7 @@ jobs: const runNameMapping = run.get(1)!; expect(runNameMapping?.key?.assertString("run-name key").value).toBe("run-name"); - const v = runNameMapping.value!; + const v = runNameMapping.value; expect(v).not.toBeUndefined(); if (!isBasicExpression(v)) { @@ -61,14 +62,14 @@ jobs: const jobsMapping = run.get(1)!; expect(jobsMapping?.key?.assertString("jobs").value).toBe("jobs"); - const job = jobsMapping.value!.assertMapping("jobs")!.get(0)!.value!.assertMapping("job"); + const job = jobsMapping.value.assertMapping("jobs").get(0).value.assertMapping("job"); const stepRun = job .get(1) - .value!.assertSequence("steps") + .value.assertSequence("steps") .get(0) .assertMapping("step") .get(0) - .value!.assertScalar("step-run"); + .value.assertScalar("step-run"); if (!isBasicExpression(stepRun)) { throw new Error("expected run-name to be a basic expression"); @@ -123,14 +124,14 @@ jobs: const jobsMapping = run.get(1)!; expect(jobsMapping?.key?.assertString("jobs").value).toBe("jobs"); - const job = jobsMapping.value!.assertMapping("jobs")!.get(0)!.value!.assertMapping("job"); + const job = jobsMapping.value.assertMapping("jobs").get(0).value.assertMapping("job"); const stepRun = job .get(1) - .value!.assertSequence("steps") + .value.assertSequence("steps") .get(0) .assertMapping("step") .get(0) - .value!.assertScalar("step-run"); + .value.assertScalar("step-run"); expect(isString(stepRun)).toBe(true); expect((stepRun as StringToken).value).toContain("${{"); diff --git a/workflow-parser/src/index.test.ts b/workflow-parser/src/index.test.ts index c1e2372..ba2b654 100644 --- a/workflow-parser/src/index.test.ts +++ b/workflow-parser/src/index.test.ts @@ -72,7 +72,7 @@ jobs: expect(result.context.errors.getErrors()).toHaveLength(0); expect(result.value).not.toBeUndefined(); - const root = result.value!.assertMapping("root"); + const root = result.value!.assertMapping("root"); // eslint-disable-line @typescript-eslint/no-non-null-assertion expect(root.description).toBe("Workflow file with strict validation"); for (const pair of root) { const key = pair.key.assertString("key").value; diff --git a/workflow-parser/src/model/converter/container.ts b/workflow-parser/src/model/converter/container.ts index a17a85c..a9fea78 100644 --- a/workflow-parser/src/model/converter/container.ts +++ b/workflow-parser/src/model/converter/container.ts @@ -12,7 +12,7 @@ export function convertToJobContainer(context: TemplateContext, container: Templ // Skip validation for expressions for now to match // behavior of the other parsers - for (const [_, token, __] of TemplateToken.traverse(container)) { + for (const [, token] of TemplateToken.traverse(container)) { if (token.isExpression) { return; } diff --git a/workflow-parser/src/model/converter/id-builder.ts b/workflow-parser/src/model/converter/id-builder.ts index c59559c..c816c74 100644 --- a/workflow-parser/src/model/converter/id-builder.ts +++ b/workflow-parser/src/model/converter/id-builder.ts @@ -47,7 +47,7 @@ export class IdBuilder { if (attempt === 1) { suffix = ""; } else { - suffix = "_" + attempt; + suffix = `_${attempt}`; } const candidate = original.substring(0, Math.min(original.length, MAX_LENGTH - suffix.length)) + suffix; diff --git a/workflow-parser/src/model/converter/job.ts b/workflow-parser/src/model/converter/job.ts index 6d650f6..1886207 100644 --- a/workflow-parser/src/model/converter/job.ts +++ b/workflow-parser/src/model/converter/job.ts @@ -1,6 +1,6 @@ import {TemplateContext} from "../../templates/template-context"; import {BasicExpressionToken, MappingToken, ScalarToken, StringToken, TemplateToken} from "../../templates/tokens"; -import {isMapping, isSequence, isString} from "../../templates/tokens/type-guards"; +import {isSequence, isString} from "../../templates/tokens/type-guards"; import {Step, WorkflowJob} from "../workflow-template"; import {convertConcurrency} from "./concurrency"; import {convertToJobContainer, convertToJobServices} from "./container"; diff --git a/workflow-parser/src/model/converter/job/inputs.ts b/workflow-parser/src/model/converter/job/inputs.ts index f8d6c57..a72fe06 100644 --- a/workflow-parser/src/model/converter/job/inputs.ts +++ b/workflow-parser/src/model/converter/job/inputs.ts @@ -13,10 +13,10 @@ export function convertWorkflowJobInputs(context: TemplateContext, job: Reusable const inputValues = createTokenMap(job["input-values"]?.assertMapping("workflow job input values"), "with"); if (inputDefinitions !== undefined) { - for (const [_, [name, value]] of inputDefinitions) { - const inputSpec = createTokenMap(value.assertMapping(`input ${name}`), `input ${name} key`)!; + for (const [, [name, value]] of inputDefinitions) { + const inputSpec = createTokenMap(value.assertMapping(`input ${name}`), `input ${name} key`); - const inputTypeToken = inputSpec.get("type")?.[1]; + const inputTypeToken = inputSpec?.get("type")?.[1]; if (!inputTypeToken) { // This should be validated by the template reader per the schema continue; @@ -31,7 +31,7 @@ export function convertWorkflowJobInputs(context: TemplateContext, job: Reusable } if (inputValues !== undefined) { - for (const [_, [name, value]] of inputValues) { + for (const [, [name, value]] of inputValues) { if (!inputDefinitions?.has(name.toLowerCase())) { context.error(value, `Invalid input, ${name} is not defined in the referenced workflow.`); } diff --git a/workflow-parser/src/model/converter/job/secrets.ts b/workflow-parser/src/model/converter/job/secrets.ts index bc4c9a5..31a1d96 100644 --- a/workflow-parser/src/model/converter/job/secrets.ts +++ b/workflow-parser/src/model/converter/job/secrets.ts @@ -5,7 +5,7 @@ import {createTokenMap} from "./inputs"; export function convertWorkflowJobSecrets(context: TemplateContext, job: ReusableWorkflowJob) { // No validation if job passes all secrets - if (!!job["inherit-secrets"]) { + if (job["inherit-secrets"]) { return; } @@ -17,14 +17,14 @@ export function convertWorkflowJobSecrets(context: TemplateContext, job: Reusabl const secretValues = createTokenMap(job["secret-values"]?.assertMapping("workflow job secret values"), "secrets"); if (secretDefinitions !== undefined) { - for (const [_, [name, value]] of secretDefinitions) { + for (const [, [name, value]] of secretDefinitions) { if (value instanceof NullToken) { continue; } - const secretSpec = createTokenMap(value.assertMapping(`secret ${name}`), `secret ${name} key`)!; + const secretSpec = createTokenMap(value.assertMapping(`secret ${name}`), `secret ${name} key`); - const required = secretSpec.get("required")?.[1].assertBoolean(`secret ${name} required`).value; + const required = secretSpec?.get("required")?.[1].assertBoolean(`secret ${name} required`).value; if (required) { if (secretValues == undefined || !secretValues.has(name.toLowerCase())) { context.error(job.ref, `Secret ${name} is required, but not provided while calling.`); @@ -34,7 +34,7 @@ export function convertWorkflowJobSecrets(context: TemplateContext, job: Reusabl } if (secretValues !== undefined) { - for (const [_, [name, value]] of secretValues) { + for (const [, [name, value]] of secretValues) { if (!secretDefinitions?.has(name.toLowerCase())) { context.error(value, `Invalid secret, ${name} is not defined in the referenced workflow.`); } diff --git a/workflow-parser/src/templates/schema/boolean-definition.ts b/workflow-parser/src/templates/schema/boolean-definition.ts index aa59859..2c1a6eb 100644 --- a/workflow-parser/src/templates/schema/boolean-definition.ts +++ b/workflow-parser/src/templates/schema/boolean-definition.ts @@ -1,4 +1,3 @@ -import {TemplateSchema} from "."; import {DEFINITION, BOOLEAN} from "../template-constants"; import {MappingToken, LiteralToken} from "../tokens"; import {TokenType} from "../tokens/types"; @@ -40,5 +39,7 @@ export class BooleanDefinition extends ScalarDefinition { return literal.templateTokenType === TokenType.Boolean; } - public override validate(schema: TemplateSchema, name: string): void {} + public override validate(): void { + // no-op + } } diff --git a/workflow-parser/src/templates/schema/null-definition.ts b/workflow-parser/src/templates/schema/null-definition.ts index 8b4389c..f410aae 100644 --- a/workflow-parser/src/templates/schema/null-definition.ts +++ b/workflow-parser/src/templates/schema/null-definition.ts @@ -1,4 +1,3 @@ -import {TemplateSchema} from "./template-schema"; import {DEFINITION, NULL} from "../template-constants"; import {MappingToken, LiteralToken} from "../tokens"; import {DefinitionType} from "./definition-type"; @@ -40,5 +39,7 @@ export class NullDefinition extends ScalarDefinition { return literal.templateTokenType === TokenType.Null; } - public override validate(schema: TemplateSchema, name: string): void {} + public override validate(): void { + // no-op + } } diff --git a/workflow-parser/src/templates/schema/number-definition.ts b/workflow-parser/src/templates/schema/number-definition.ts index a7e90b0..002374d 100644 --- a/workflow-parser/src/templates/schema/number-definition.ts +++ b/workflow-parser/src/templates/schema/number-definition.ts @@ -1,4 +1,3 @@ -import {TemplateSchema} from "./template-schema"; import {DEFINITION, NUMBER} from "../template-constants"; import {MappingToken, LiteralToken} from "../tokens"; import {DefinitionType} from "./definition-type"; @@ -40,5 +39,7 @@ export class NumberDefinition extends ScalarDefinition { return literal.templateTokenType === TokenType.Number; } - public override validate(schema: TemplateSchema, name: string): void {} + public override validate(): void { + // no-op + } } diff --git a/workflow-parser/src/templates/schema/string-definition.ts b/workflow-parser/src/templates/schema/string-definition.ts index f1812ad..48edb14 100644 --- a/workflow-parser/src/templates/schema/string-definition.ts +++ b/workflow-parser/src/templates/schema/string-definition.ts @@ -3,7 +3,6 @@ import {LiteralToken, MappingToken, StringToken} from "../tokens"; import {TokenType} from "../tokens/types"; import {DefinitionType} from "./definition-type"; import {ScalarDefinition} from "./scalar-definition"; -import {TemplateSchema} from "./template-schema"; export class StringDefinition extends ScalarDefinition { public constant = ""; @@ -80,7 +79,7 @@ export class StringDefinition extends ScalarDefinition { return false; } - public override validate(schema: TemplateSchema, name: string): void { + public override validate(): void { if (this.constant && this.requireNonEmpty) { throw new Error(`Properties '${CONSTANT}' and '${REQUIRE_NON_EMPTY}' cannot both be set`); } diff --git a/workflow-parser/src/templates/schema/template-schema.ts b/workflow-parser/src/templates/schema/template-schema.ts index 5255b07..ca78670 100644 --- a/workflow-parser/src/templates/schema/template-schema.ts +++ b/workflow-parser/src/templates/schema/template-schema.ts @@ -324,6 +324,7 @@ export class TemplateSchema { const template = readTemplate(context, TEMPLATE_SCHEMA, objectReader, undefined); context.errors.check(); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const mapping = template!.assertMapping(TEMPLATE_SCHEMA); const schema = new TemplateSchema(mapping); schema.validate(); diff --git a/workflow-parser/src/templates/tokens/template-token.test.ts b/workflow-parser/src/templates/tokens/template-token.test.ts index 34afdab..a0acdcf 100644 --- a/workflow-parser/src/templates/tokens/template-token.test.ts +++ b/workflow-parser/src/templates/tokens/template-token.test.ts @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion */ import {nullTrace} from "../../test-utils/null-trace"; import {parseWorkflow} from "../../workflows/workflow-parser"; import {StringToken} from "./string-token"; @@ -20,13 +21,13 @@ describe("traverse", () => { expect(traverser.next()!.value).toEqual([undefined, root, undefined]); // On - const onResult = traverser.next()!.value!; + const onResult = traverser.next().value!; expect(onResult[0]).toBe(root); expect(getValue(onResult[1])).toEqual("on"); expect(onResult[2]).toBeUndefined(); // Push - const pushResult = traverser.next()!.value!; + const pushResult = traverser.next().value!; expect(pushResult[0]).toBe(root); expect(getValue(pushResult[1])).toEqual("push"); expect(getValue(pushResult[2])).toEqual("on"); diff --git a/workflow-parser/src/xlang.test.ts b/workflow-parser/src/xlang.test.ts index 5aeb845..c07bdbc 100644 --- a/workflow-parser/src/xlang.test.ts +++ b/workflow-parser/src/xlang.test.ts @@ -2,7 +2,7 @@ import * as fs from "fs"; import * as path from "path"; import * as YAML from "yaml"; import {convertWorkflowTemplate} from "./model/convert"; -import {TraceWriter} from "./templates/trace-writer"; +import {NoOperationTraceWriter} from "./templates/trace-writer"; import {File} from "./workflows/file"; import {FileProvider} from "./workflows/file-provider"; import {fileIdentifier, FileReference} from "./workflows/file-reference"; @@ -13,12 +13,7 @@ interface TestOptions { skip?: string[]; } -const nullTrace: TraceWriter = { - info: x => {}, - verbose: x => {}, - error: x => {} -}; - +const nullTrace = new NoOperationTraceWriter(); const testFiles = "./testdata/reader"; const skippedTestsFile = "./testdata/skipped-tests.txt"; @@ -44,7 +39,7 @@ describe("x-lang tests", () => { const testDocs: string[] = inputFile.split(/\r?\n---\r?\n/); expect(testDocs.length).toBeGreaterThanOrEqual(3); - const testOptions: TestOptions = YAML.parse(testDocs[0]); + const testOptions = YAML.parse(testDocs[0]) as TestOptions; const unsupportedTest = skippedTests.has(file); const test = async () => { @@ -65,10 +60,10 @@ describe("x-lang tests", () => { } const testFileProvider: FileProvider = { - getFileContent: async (ref: FileReference) => { + getFileContent: (ref: FileReference) => { const file = reusableWorkflows[fileIdentifier(ref)]; if (file) { - return file; + return Promise.resolve(file); } throw new Error("File not found: " + fileName); @@ -87,7 +82,7 @@ describe("x-lang tests", () => { const workflowTemplate = await convertWorkflowTemplate( parseResult.context, - parseResult.value!, + parseResult.value!, // eslint-disable-line @typescript-eslint/no-non-null-assertion testFileProvider, { fetchReusableWorkflowDepth: 1 @@ -99,11 +94,13 @@ describe("x-lang tests", () => { const includeEvents = testOptions.skip !== undefined && contains(testOptions.skip, "Go") && contains(testOptions.skip, "C#"); if (!includeEvents) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access delete (workflowTemplate as any).events; } // Other parsers don't have a partial template when there are errors if (workflowTemplate.errors) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access delete (workflowTemplate as any).jobs; }