diff --git a/client/tests/e2e/suite/extension.ga.e2e.ts b/client/tests/e2e/suite/extension.ga.e2e.ts index 1f387db..d5bafb9 100644 --- a/client/tests/e2e/suite/extension.ga.e2e.ts +++ b/client/tests/e2e/suite/extension.ga.e2e.ts @@ -52,7 +52,17 @@ suite("Native (JSON) Workflows", () => { await waitForDiagnostics(docUri); await assertDiagnostics(docUri, [ { - message: 'Missing property "release".', + message: "The workflow must have a release version.", + range: new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 1)), + severity: vscode.DiagnosticSeverity.Error, + }, + { + message: "The workflow does not specify a creator.", + range: new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 1)), + severity: vscode.DiagnosticSeverity.Error, + }, + { + message: "The workflow does not specify a license.", range: new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 1)), severity: vscode.DiagnosticSeverity.Error, }, diff --git a/server/gx-workflow-ls-format2/src/gxFormat2WorkflowDocument.ts b/server/gx-workflow-ls-format2/src/gxFormat2WorkflowDocument.ts index 72168e5..245a0f4 100644 --- a/server/gx-workflow-ls-format2/src/gxFormat2WorkflowDocument.ts +++ b/server/gx-workflow-ls-format2/src/gxFormat2WorkflowDocument.ts @@ -5,6 +5,8 @@ import { TextDocument, WorkflowDataType, WorkflowDocument, + WorkflowInput, + WorkflowOutput, } from "@gxwf/server-common/src/languageTypes"; import { YAMLDocument } from "@gxwf/yaml-language-service/src"; @@ -23,58 +25,60 @@ export class GxFormat2WorkflowDocument extends WorkflowDocument { } public getWorkflowInputs(): GetWorkflowInputsResult { - const result: GetWorkflowInputsResult = { inputs: [] }; - const inputs = this.nodeManager.getNodeFromPath("inputs"); - if (inputs?.type === "property") { - const inputList = inputs.valueNode?.children; - if (inputList) { - inputList.forEach((input) => { - if (input.type !== "property" || !input.keyNode) return; - const inputName = String(input.keyNode.value); - const inputType = this.extractInputType(input); - const inputDocNode = input.valueNode?.children?.find( - (prop) => prop.type === "property" && prop.keyNode.value === "doc" - ) as PropertyASTNode; - const inputDescription = String(inputDocNode?.valueNode?.value ?? ""); - result.inputs.push({ - name: inputName, - doc: inputDescription, - type: inputType, - }); - }); - } - } - return result; + return { + inputs: this.getRawInputNodes().map((input) => this.parseInputDefinition(input)), + }; } public getWorkflowOutputs(): GetWorkflowOutputsResult { - const result: GetWorkflowOutputsResult = { outputs: [] }; - const output = this.nodeManager.getNodeFromPath("outputs"); - if (output?.type === "property") { - const outputList = output.valueNode?.children; - if (outputList) { - outputList.forEach((output) => { - if (output.type !== "property" || !output.keyNode) return; - const outputName = String(output.keyNode.value); - const outputDocNode = output.valueNode?.children?.find( - (prop) => prop.type === "property" && prop.keyNode.value === "doc" - ) as PropertyASTNode; - const outputDoc = String(outputDocNode?.valueNode?.value ?? ""); - result.outputs.push({ - name: outputName, - doc: outputDoc, - }); + return { + outputs: this.getRawOutputNodes().map((output) => this.parseOutputDefinition(output)), + }; + } + + public getRawInputNodes(): PropertyASTNode[] { + return this.getAllPropertyNodesAtPath("inputs"); + } + + public getRawOutputNodes(): PropertyASTNode[] { + return this.getAllPropertyNodesAtPath("outputs"); + } + + private getAllPropertyNodesAtPath(path: string): PropertyASTNode[] { + const result: PropertyASTNode[] = []; + const nodeAtPath = this.nodeManager.getNodeFromPath(path); + if (nodeAtPath?.type === "property") { + const propertyNodes = nodeAtPath.valueNode?.children; + if (propertyNodes) { + propertyNodes.forEach((node) => { + if (node.type !== "property" || !node.keyNode) return; + result.push(node); }); } } return result; } - private extractInputType(input: PropertyASTNode): WorkflowDataType { + private parseInputDefinition(input: PropertyASTNode): WorkflowInput { + const inputName = String(input.keyNode.value); + const inputType = this.parseInputType(input); + const inputDocNode = this.nodeManager.getPropertyNodeByName(input, "doc"); + const inputDescription = String(inputDocNode?.valueNode?.value ?? ""); + const defaultValueNode = this.nodeManager.getPropertyNodeByName(input, "default"); + const optionalValue = this.nodeManager.getPropertyValueByName(input, "optional"); + const inputDefinition: WorkflowInput = { + name: inputName, + doc: inputDescription, + type: inputType, + default: defaultValueNode?.valueNode?.value, + optional: optionalValue === undefined ? undefined : optionalValue === true, + }; + return inputDefinition; + } + + private parseInputType(input: PropertyASTNode): WorkflowDataType { let inputType: WorkflowDataType = "data"; - const inputTypeNode = input.valueNode?.children?.find( - (prop) => prop.type === "property" && prop.keyNode.value === "type" - ) as PropertyASTNode; + const inputTypeNode = this.nodeManager.getPropertyNodeByName(input, "type"); if (inputTypeNode) { inputType = String(inputTypeNode.valueNode?.value) as WorkflowDataType; } else { @@ -83,4 +87,15 @@ export class GxFormat2WorkflowDocument extends WorkflowDocument { } return inputType; } + + private parseOutputDefinition(output: PropertyASTNode): WorkflowOutput { + const outputName = String(output.keyNode.value); + const outputDocNode = this.nodeManager.getPropertyNodeByName(output, "doc"); + const outputDoc = String(outputDocNode?.valueNode?.value ?? ""); + const outputDefinition = { + name: outputName, + doc: outputDoc, + }; + return outputDefinition; + } } diff --git a/server/gx-workflow-ls-format2/src/languageService.ts b/server/gx-workflow-ls-format2/src/languageService.ts index edef90a..f923c97 100644 --- a/server/gx-workflow-ls-format2/src/languageService.ts +++ b/server/gx-workflow-ls-format2/src/languageService.ts @@ -12,16 +12,16 @@ import { TYPES, TextDocument, TextEdit, - WorkflowValidator, } from "@gxwf/server-common/src/languageTypes"; import { TYPES as YAML_TYPES } from "@gxwf/yaml-language-service/src/inversify.config"; import { YAMLLanguageService } from "@gxwf/yaml-language-service/src/yamlLanguageService"; import { inject, injectable } from "inversify"; import { GxFormat2WorkflowDocument } from "./gxFormat2WorkflowDocument"; +import { GxFormat2BasicValidationProfile, GxFormat2IWCValidationProfile } from "./profiles"; import { GalaxyWorkflowFormat2SchemaLoader } from "./schema"; import { GxFormat2CompletionService } from "./services/completionService"; import { GxFormat2HoverService } from "./services/hoverService"; -import { GxFormat2SchemaValidationService, WorkflowValidationService } from "./services/validation"; +import { GxFormat2SchemaValidationService } from "./services/schemaValidationService"; const LANGUAGE_ID = "gxformat2"; @@ -40,7 +40,7 @@ export class GxFormat2WorkflowLanguageServiceImpl private _schemaLoader: GalaxyWorkflowFormat2SchemaLoader; private _hoverService: GxFormat2HoverService; private _completionService: GxFormat2CompletionService; - private _validationServices: WorkflowValidator[]; + private _schemaValidationService: GxFormat2SchemaValidationService; constructor( @inject(YAML_TYPES.YAMLLanguageService) yamlLanguageService: YAMLLanguageService, @@ -51,10 +51,7 @@ export class GxFormat2WorkflowLanguageServiceImpl this._yamlLanguageService = yamlLanguageService; this._hoverService = new GxFormat2HoverService(this._schemaLoader.nodeResolver); this._completionService = new GxFormat2CompletionService(this._schemaLoader.nodeResolver); - this._validationServices = [ - new GxFormat2SchemaValidationService(this._schemaLoader.nodeResolver), - new WorkflowValidationService(), - ]; + this._schemaValidationService = new GxFormat2SchemaValidationService(this._schemaLoader.nodeResolver); } public override parseDocument(document: TextDocument): GxFormat2WorkflowDocument { @@ -77,13 +74,22 @@ export class GxFormat2WorkflowLanguageServiceImpl return this._completionService.doComplete(documentContext, position); } + protected override initializeValidationProfiles(): void { + super.initializeValidationProfiles(); + this.validationProfiles.set("basic", new GxFormat2BasicValidationProfile()); + this.validationProfiles.set("iwc", new GxFormat2IWCValidationProfile()); + } + protected override async doValidation(documentContext: GxFormat2WorkflowDocument): Promise { - const diagnostics = await this._yamlLanguageService.doValidation(documentContext.yamlDocument); - for (const validator of this._validationServices) { - const results = await validator.doValidation(documentContext); - diagnostics.push(...results); - } - return diagnostics; + const syntaxDiagnostics = await this._yamlLanguageService.doValidation(documentContext.yamlDocument); + syntaxDiagnostics.forEach((diagnostic) => { + diagnostic.source = "YAML Syntax"; + }); + const schemaDiagnostics = await this._schemaValidationService.doValidation(documentContext); + schemaDiagnostics.forEach((diagnostic) => { + diagnostic.source = "Format2 Schema"; + }); + return syntaxDiagnostics.concat(schemaDiagnostics); } public override getSymbols(documentContext: GxFormat2WorkflowDocument): DocumentSymbol[] { diff --git a/server/gx-workflow-ls-format2/src/profiles.ts b/server/gx-workflow-ls-format2/src/profiles.ts new file mode 100644 index 0000000..65ee38c --- /dev/null +++ b/server/gx-workflow-ls-format2/src/profiles.ts @@ -0,0 +1,46 @@ +import { DiagnosticSeverity, ValidationRule } from "@gxwf/server-common/src/languageTypes"; +import { + BasicCommonValidationProfile, + IWCCommonValidationProfile, +} from "@gxwf/server-common/src/providers/validation/profiles"; +import { RequiredPropertyValidationRule } from "@gxwf/server-common/src/providers/validation/rules"; +import { InputTypeValidationRule } from "./validation/rules/InputTypeValidationRule"; + +/** + * Defines the minimal set of validation rules for gxformat2 Galaxy workflows. + */ +export class GxFormat2BasicValidationProfile extends BasicCommonValidationProfile { + public readonly name: string = "GxFormat2 Validation"; + public static readonly RULES = new Set([ + ...super.RULES, + // Add more custom rules specific to gxformat2 workflows here... + ]); + + public get rules(): Set { + return GxFormat2BasicValidationProfile.RULES; + } +} + +/** + * *Intergalactic Workflow Commission* (IWC) validation profile for gxformat2 Galaxy workflows. + * This profile extends the basic validation profile and adds additional rules to comply + * with the IWC best practices guidelines. + */ +export class GxFormat2IWCValidationProfile extends IWCCommonValidationProfile { + protected static readonly RULES = new Set([ + ...super.RULES, + ...GxFormat2BasicValidationProfile.RULES, + new RequiredPropertyValidationRule( + "doc", + true, + DiagnosticSeverity.Error, + "The workflow is not documented. Documenting workflows helps users understand the purpose of the workflow." + ), + new InputTypeValidationRule(DiagnosticSeverity.Error), + // Add more custom rules specific to gxformat2 workflows here... + ]); + + public get rules(): Set { + return GxFormat2IWCValidationProfile.RULES; + } +} diff --git a/server/gx-workflow-ls-format2/src/schema/definitions.ts b/server/gx-workflow-ls-format2/src/schema/definitions.ts index b05a01e..14b87df 100644 --- a/server/gx-workflow-ls-format2/src/schema/definitions.ts +++ b/server/gx-workflow-ls-format2/src/schema/definitions.ts @@ -259,7 +259,7 @@ export class FieldSchemaNode implements SchemaNode, IdMapper { } public get canBeObject(): boolean { - return this.canBeAny || this._allowedTypes.some((t) => this.isObjectType(t.typeName)); + return this.canBeAny || this._allowedTypes.some((t) => this.isRecordType(t.typeName)); } public matchesType(typeName: string): boolean { @@ -313,11 +313,15 @@ export class FieldSchemaNode implements SchemaNode, IdMapper { return undefined; } - private isPrimitiveType(typeName: string): boolean { - return FieldSchemaNode.definitions.primitiveTypes.has(typeName); + public get isPrimitiveType(): boolean { + return FieldSchemaNode.definitions.primitiveTypes.has(this.typeRef); } - private isObjectType(typeName: string): boolean { + public get isObjectType(): boolean { + return this.isRecordType(this.typeRef); + } + + private isRecordType(typeName: string): boolean { return FieldSchemaNode.definitions.records.has(typeName); } diff --git a/server/gx-workflow-ls-format2/src/schema/index.ts b/server/gx-workflow-ls-format2/src/schema/index.ts index a4239af..d1ebff3 100644 --- a/server/gx-workflow-ls-format2/src/schema/index.ts +++ b/server/gx-workflow-ls-format2/src/schema/index.ts @@ -1,5 +1,5 @@ +import { FieldSchemaNode, RecordSchemaNode, SchemaNode } from "./definitions"; import { GalaxyWorkflowFormat2SchemaLoader } from "./schemaLoader"; -import { RecordSchemaNode, FieldSchemaNode, SchemaNode } from "./definitions"; import { SchemaNodeResolver } from "./schemaNodeResolver"; -export { GalaxyWorkflowFormat2SchemaLoader, SchemaNodeResolver, SchemaNode, RecordSchemaNode, FieldSchemaNode }; +export { FieldSchemaNode, GalaxyWorkflowFormat2SchemaLoader, RecordSchemaNode, SchemaNode, SchemaNodeResolver }; diff --git a/server/gx-workflow-ls-format2/src/services/completionService.ts b/server/gx-workflow-ls-format2/src/services/completionService.ts index 5764eba..c419ce6 100644 --- a/server/gx-workflow-ls-format2/src/services/completionService.ts +++ b/server/gx-workflow-ls-format2/src/services/completionService.ts @@ -68,7 +68,7 @@ export class GxFormat2CompletionService { result.push(item); }); } else if (schemaNode instanceof FieldSchemaNode) { - if (this.schemaNodeResolver.definitions.primitiveTypes.has(schemaNode.typeRef)) { + if (schemaNode.isPrimitiveType) { const defaultValue = String(schemaNode.default ?? ""); if (defaultValue) { const item: CompletionItem = { @@ -104,6 +104,8 @@ export class GxFormat2CompletionService { }; result.push(item); }); + } else if (schemaRecord instanceof RecordSchemaNode) { + return this.getProposedItems(schemaRecord, textBuffer, exclude, offset); } } return result; diff --git a/server/gx-workflow-ls-format2/src/services/hoverService.ts b/server/gx-workflow-ls-format2/src/services/hoverService.ts index f04cea7..ef0cfbc 100644 --- a/server/gx-workflow-ls-format2/src/services/hoverService.ts +++ b/server/gx-workflow-ls-format2/src/services/hoverService.ts @@ -33,11 +33,7 @@ export class GxFormat2HoverService { } } - const hoverRange = Range.create( - textDocument.positionAt(hoverRangeNode.offset), - textDocument.positionAt(hoverRangeNode.offset + hoverRangeNode.length) - ); - + const hoverRange = nodeManager.getNodeRange(hoverRangeNode); const location = nodeManager.getPathFromNode(hoverRangeNode); const schemaNode = this.schemaNodeResolver.resolveSchemaContext(location); const contents = this.getHoverMarkdownContentsForNode(schemaNode); diff --git a/server/gx-workflow-ls-format2/src/services/validation/schemaValidationService.ts b/server/gx-workflow-ls-format2/src/services/schemaValidationService.ts similarity index 71% rename from server/gx-workflow-ls-format2/src/services/validation/schemaValidationService.ts rename to server/gx-workflow-ls-format2/src/services/schemaValidationService.ts index 4705302..d4425fe 100644 --- a/server/gx-workflow-ls-format2/src/services/validation/schemaValidationService.ts +++ b/server/gx-workflow-ls-format2/src/services/schemaValidationService.ts @@ -1,5 +1,5 @@ import { ASTNodeManager } from "@gxwf/server-common/src/ast/nodeManager"; -import { ASTNode, ObjectASTNode } from "@gxwf/server-common/src/ast/types"; +import { ASTNode, ObjectASTNode, StringASTNode } from "@gxwf/server-common/src/ast/types"; import { Diagnostic, DiagnosticSeverity, @@ -7,8 +7,8 @@ import { WorkflowDocument, WorkflowValidator, } from "@gxwf/server-common/src/languageTypes"; -import { SchemaNode, SchemaNodeResolver } from "../../schema"; -import { RecordSchemaNode, IdMapper } from "../../schema/definitions"; +import { SchemaNode, SchemaNodeResolver } from "../schema"; +import { EnumSchemaNode, IdMapper, RecordSchemaNode } from "../schema/definitions"; export class GxFormat2SchemaValidationService implements WorkflowValidator { constructor(protected readonly schemaNodeResolver: SchemaNodeResolver) {} @@ -38,6 +38,10 @@ export class GxFormat2SchemaValidationService implements WorkflowValidator { parenSchemaNode?: SchemaNode ): void { const range = this.getRange(nodeManager, node); + const schemaRecord = this.schemaNodeResolver.getSchemaNodeByTypeRef(schemaNode.typeRef); + if (schemaRecord instanceof EnumSchemaNode && node.type === "string") { + this.validateEnumValue(node, schemaRecord, range, diagnostics); + } if (schemaNode instanceof RecordSchemaNode) { switch (node.type) { case "object": @@ -50,6 +54,22 @@ export class GxFormat2SchemaValidationService implements WorkflowValidator { } } } + private validateEnumValue( + node: StringASTNode, + schemaRecord: EnumSchemaNode, + range: Range, + diagnostics: Diagnostic[] + ): void { + if (!schemaRecord.symbols.includes(node.value)) { + diagnostics.push( + Diagnostic.create( + range, + `The value is not a valid '${schemaRecord.name}'. Allowed values are: ${schemaRecord.symbols.join(", ")}.`, + DiagnosticSeverity.Error + ) + ); + } + } private validateObjectNode( nodeManager: ASTNodeManager, @@ -67,6 +87,17 @@ export class GxFormat2SchemaValidationService implements WorkflowValidator { ); } if (nodeFound) { + if (schemaFieldNode.isPrimitiveType && propertyNode?.valueNode?.type) { + if (!schemaFieldNode.matchesType(propertyNode.valueNode.type)) { + diagnostics.push( + Diagnostic.create( + range, + `Type mismatch for field '${schemaFieldNode.name}'. Expected '${schemaFieldNode.typeRef}' but found '${propertyNode.valueNode.type}'.`, + DiagnosticSeverity.Error + ) + ); + } + } const childSchemaNode = this.schemaNodeResolver.getSchemaNodeByTypeRef(schemaFieldNode.typeRef); if (childSchemaNode && propertyNode.valueNode) { if (schemaFieldNode.canBeArray) { diff --git a/server/gx-workflow-ls-format2/src/services/validation/index.ts b/server/gx-workflow-ls-format2/src/services/validation/index.ts deleted file mode 100644 index 904820f..0000000 --- a/server/gx-workflow-ls-format2/src/services/validation/index.ts +++ /dev/null @@ -1,4 +0,0 @@ -import { GxFormat2SchemaValidationService } from "./schemaValidationService"; -import { WorkflowValidationService } from "./workflowValidationService"; - -export { GxFormat2SchemaValidationService, WorkflowValidationService }; diff --git a/server/gx-workflow-ls-format2/src/services/validation/workflowValidationService.ts b/server/gx-workflow-ls-format2/src/services/validation/workflowValidationService.ts deleted file mode 100644 index 9e1f0e9..0000000 --- a/server/gx-workflow-ls-format2/src/services/validation/workflowValidationService.ts +++ /dev/null @@ -1,56 +0,0 @@ -import { - Diagnostic, - DiagnosticSeverity, - WorkflowDocument, - WorkflowValidator, -} from "@gxwf/server-common/src/languageTypes"; - -export class WorkflowValidationService implements WorkflowValidator { - public doValidation(workflowDocument: WorkflowDocument): Promise { - const diagnostics: Diagnostic[] = [ - ...collectStepErrorDiagnostics(workflowDocument), - ...collectToolDiagnostics(workflowDocument), - ]; - return Promise.resolve(diagnostics); - } -} - -export function collectStepErrorDiagnostics(workflowDocument: WorkflowDocument): Diagnostic[] { - const diagnostics: Diagnostic[] = []; - const steps = workflowDocument.nodeManager.getStepNodes(true); - steps.forEach((step) => { - const errors = step.properties.find((p) => p.keyNode.value === "errors"); - if (errors) { - const range = workflowDocument.nodeManager.getNodeRange(errors); - diagnostics.push( - Diagnostic.create( - range, - `Tool step contains error indicated during Galaxy export - ${errors}`, - DiagnosticSeverity.Warning - ) - ); - } - }); - return diagnostics; -} - -export function collectToolDiagnostics(workflowDocument: WorkflowDocument): Diagnostic[] { - const diagnostics: Diagnostic[] = []; - const steps = workflowDocument.nodeManager.getStepNodes(true); - steps.forEach((step) => { - const tool_id = step.properties.find((p) => p.keyNode.value === "tool_id"); - if (tool_id) { - if (tool_id.valueNode?.value?.toString().includes("testtoolshed")) { - const range = workflowDocument.nodeManager.getNodeRange(tool_id); - diagnostics.push( - Diagnostic.create( - range, - `Step references a tool from the test tool shed, this should be replaced with a production tool`, - DiagnosticSeverity.Warning - ) - ); - } - } - }); - return diagnostics; -} diff --git a/server/gx-workflow-ls-format2/src/validation/rules/InputTypeValidationRule.ts b/server/gx-workflow-ls-format2/src/validation/rules/InputTypeValidationRule.ts new file mode 100644 index 0000000..fa10849 --- /dev/null +++ b/server/gx-workflow-ls-format2/src/validation/rules/InputTypeValidationRule.ts @@ -0,0 +1,42 @@ +import { ValidationRule } from "@gxwf/server-common/src/languageTypes"; +import { isCompatibleType, isWorkflowDataType } from "@gxwf/server-common/src/utils"; +import { Diagnostic, DiagnosticSeverity } from "vscode-languageserver-types"; +import { GxFormat2WorkflowDocument } from "../../gxFormat2WorkflowDocument"; + +/** + * Validation rule to check that all inputs that define a default value have the correct type. + */ +export class InputTypeValidationRule implements ValidationRule { + constructor(readonly severity: DiagnosticSeverity = DiagnosticSeverity.Error) {} + + public async validate(documentContext: GxFormat2WorkflowDocument): Promise { + const result: Diagnostic[] = []; + + const inputNodes = documentContext.getRawInputNodes(); + for (const input of inputNodes) { + const inputName = String(input.keyNode.value); + const inputType = documentContext.nodeManager.getPropertyValueByName(input, "type") as string; + + if (!isWorkflowDataType(inputType)) { + // Skip type validation for invalid types. Those will be caught by the schema validation. + continue; + } + + const defaultValueNode = documentContext.nodeManager.getPropertyNodeByName(input, "default"); + const defaultValue = defaultValueNode?.valueNode?.value; + const defaultValueType = typeof defaultValue; + + if (inputType && defaultValue) { + const defaultTypeMatchesValue = isCompatibleType(inputType, defaultValueType); + if (!defaultTypeMatchesValue) { + result.push({ + message: `Input '${inputName}' default value has invalid type. Expected '${inputType}' but found '${defaultValueType}'.`, + range: documentContext.nodeManager.getNodeRange(defaultValueNode), + severity: this.severity, + }); + } + } + } + return Promise.resolve(result); + } +} diff --git a/server/gx-workflow-ls-format2/tests/integration/completion.test.ts b/server/gx-workflow-ls-format2/tests/integration/completion.test.ts index a4421f5..2174957 100644 --- a/server/gx-workflow-ls-format2/tests/integration/completion.test.ts +++ b/server/gx-workflow-ls-format2/tests/integration/completion.test.ts @@ -301,4 +301,18 @@ outputs:`; const completionLabels = getCompletionItemsLabels(completions); expect(completionLabels).toEqual(EXPECTED_COMPLETION_LABELS); }); + + it("should suggest the list of available properties for 'report' when the cursor is inside the property", async () => { + const template = ` +class: GalaxyWorkflow +report: + $`; + const EXPECTED_COMPLETION_LABELS = ["markdown"]; + const { contents, position } = parseTemplate(template); + + const completions = await getCompletions(contents, position); + + const completionLabels = getCompletionItemsLabels(completions); + expect(completionLabels).toEqual(EXPECTED_COMPLETION_LABELS); + }); }); diff --git a/server/gx-workflow-ls-format2/tests/integration/document.test.ts b/server/gx-workflow-ls-format2/tests/integration/document.test.ts index d449b3c..1e4440a 100644 --- a/server/gx-workflow-ls-format2/tests/integration/document.test.ts +++ b/server/gx-workflow-ls-format2/tests/integration/document.test.ts @@ -9,6 +9,7 @@ inputs: input_2: type: File doc: This is the input 2 + optional: false the_collection: type: collection doc: This is a collection @@ -34,6 +35,7 @@ inputs: name: "input_2", doc: "This is the input 2", type: "File", + optional: false, }, { name: "the_collection", @@ -49,6 +51,8 @@ inputs: name: "text_param", doc: "", type: "text", + default: "text value", + optional: true, }, ]); }); diff --git a/server/gx-workflow-ls-format2/tests/integration/validation.test.ts b/server/gx-workflow-ls-format2/tests/integration/validation.test.ts new file mode 100644 index 0000000..7256941 --- /dev/null +++ b/server/gx-workflow-ls-format2/tests/integration/validation.test.ts @@ -0,0 +1,127 @@ +import { Diagnostic, ValidationRule } from "@gxwf/server-common/src/languageTypes"; +import { GalaxyWorkflowFormat2SchemaLoader } from "../../src/schema"; +import { GxFormat2SchemaValidationService } from "../../src/services/schemaValidationService"; +import { InputTypeValidationRule } from "../../src/validation/rules/InputTypeValidationRule"; +import { createFormat2WorkflowDocument } from "../testHelpers"; + +describe("Schema validation", () => { + let validator: GxFormat2SchemaValidationService; + + function validateDocument(documentContents: string): Promise { + const document = createFormat2WorkflowDocument(documentContents); + return validator.doValidation(document); + } + + beforeAll(() => { + const schemaNodeResolver = new GalaxyWorkflowFormat2SchemaLoader().nodeResolver; + validator = new GxFormat2SchemaValidationService(schemaNodeResolver); + }); + + it("should validate a correct workflow", async () => { + const content = ` +class: GalaxyWorkflow +doc: | + Simple workflow that no-op cats a file. +inputs: + the_input: + type: File + doc: input doc +outputs: + the_output: + outputSource: cat/out_file1 +steps: + cat: + tool_id: cat1 + doc: cat doc + in: + input1: the_input + `; + const diagnostics = await validateDocument(content); + expect(diagnostics).toHaveLength(0); + }); + + it("should report error when a property has incorrect type value", async () => { + const content = ` +class: null +inputs: +outputs: +steps: + `; + const diagnostics = await validateDocument(content); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0].message).toBe("Type mismatch for field 'class'. Expected 'string' but found 'null'."); + }); + + it("should report error for missing required properties", async () => { + const content = ` +class: GalaxyWorkflow + `; + const diagnostics = await validateDocument(content); + expect(diagnostics).toHaveLength(3); + expect(diagnostics[0].message).toBe("The 'steps' field is required."); + expect(diagnostics[1].message).toBe("The 'inputs' field is required."); + expect(diagnostics[2].message).toBe("The 'outputs' field is required."); + }); + + it("should report error for invalid enum value", async () => { + const content = ` +class: GalaxyWorkflow +inputs: + the_input: + type: unknown +outputs: +steps: + `; + const diagnostics = await validateDocument(content); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0].message).toBe( + "The value is not a valid 'GalaxyType'. Allowed values are: integer, text, File, data, collection, null, boolean, int, long, float, double, string." + ); + }); + + describe("Custom Rules", () => { + let rule: ValidationRule; + + function validateRule(documentContents: string): Promise { + const document = createFormat2WorkflowDocument(documentContents); + return rule.validate(document); + } + + describe("InputTypeValidationRule", () => { + beforeAll(() => { + rule = new InputTypeValidationRule(); + }); + + it("should report error when input default value has invalid type", async () => { + const content = ` +class: GalaxyWorkflow +inputs: + the_input: + type: int + default: this is not a number +outputs: +steps: + `; + const diagnostics = await validateRule(content); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0].message).toBe( + "Input 'the_input' default value has invalid type. Expected 'int' but found 'string'." + ); + }); + + it("should not report error when input default value has valid type", async () => { + const content = ` +class: GalaxyWorkflow +inputs: + the_input: + type: int + default: 42 +outputs: +steps: + `; + const diagnostics = await validateRule(content); + expect(diagnostics).toHaveLength(0); + }); + }); + }); +}); diff --git a/server/gx-workflow-ls-native/src/languageService.ts b/server/gx-workflow-ls-native/src/languageService.ts index 47cb073..1f0b35e 100644 --- a/server/gx-workflow-ls-native/src/languageService.ts +++ b/server/gx-workflow-ls-native/src/languageService.ts @@ -25,6 +25,7 @@ import { } from "vscode-json-languageservice"; import NativeWorkflowSchema from "../../../workflow-languages/schemas/native.schema.json"; import { NativeWorkflowDocument } from "./nativeWorkflowDocument"; +import { NativeBasicValidationProfile, NativeIWCValidationProfile } from "./profiles"; const LANGUAGE_ID = "galaxyworkflow"; @@ -86,6 +87,12 @@ export class NativeWorkflowLanguageServiceImpl return completionResult; } + protected override initializeValidationProfiles(): void { + super.initializeValidationProfiles(); + this.validationProfiles.set("basic", new NativeBasicValidationProfile()); + this.validationProfiles.set("iwc", new NativeIWCValidationProfile()); + } + protected override async doValidation(workflowDocument: NativeWorkflowDocument): Promise { const nativeWorkflowDocument = workflowDocument as NativeWorkflowDocument; const schemaValidationResults = await this._jsonLanguageService.doValidation( @@ -94,6 +101,9 @@ export class NativeWorkflowLanguageServiceImpl this._documentSettings, this.schema ); + schemaValidationResults.forEach((diagnostic) => { + diagnostic.source = "Native Workflow Schema"; + }); return schemaValidationResults; } diff --git a/server/gx-workflow-ls-native/src/nativeWorkflowDocument.ts b/server/gx-workflow-ls-native/src/nativeWorkflowDocument.ts index 287b059..83d5f75 100644 --- a/server/gx-workflow-ls-native/src/nativeWorkflowDocument.ts +++ b/server/gx-workflow-ls-native/src/nativeWorkflowDocument.ts @@ -5,6 +5,8 @@ import { TextDocument, WorkflowDataType, WorkflowDocument, + WorkflowInput, + WorkflowOutput, } from "@gxwf/server-common/src/languageTypes"; import { JSONDocument } from "vscode-json-languageservice"; import { ToolState, isWorkflowInputType, type ParameterInputToolState } from "./utils"; @@ -63,11 +65,14 @@ export class NativeWorkflowDocument extends WorkflowDocument { const toolStateValue = JSON.parse( toolStateNode?.valueNode?.value ? String(toolStateNode?.valueNode?.value) : "{}" ); - result.inputs.push({ + const inputDefinition: WorkflowInput = { name: String(labelValue), doc: String(annotationValue ?? ""), type: this.getInputType(stepTypeValue, toolStateValue), - }); + default: toolStateValue.default, + optional: toolStateValue.optional, + }; + result.inputs.push(inputDefinition); } }); return result; @@ -95,10 +100,11 @@ export class NativeWorkflowDocument extends WorkflowDocument { } const uuidNode = workflowOutput.properties.find((property) => property.keyNode.value === "uuid"); const uuidValue = String(uuidNode?.valueNode?.value); - result.outputs.push({ + const outputDefinition: WorkflowOutput = { name: String(labelValue), uuid: uuidValue, - }); + }; + result.outputs.push(outputDefinition); }); } }); diff --git a/server/gx-workflow-ls-native/src/profiles.ts b/server/gx-workflow-ls-native/src/profiles.ts new file mode 100644 index 0000000..1a44e6e --- /dev/null +++ b/server/gx-workflow-ls-native/src/profiles.ts @@ -0,0 +1,46 @@ +import { DiagnosticSeverity, ValidationRule } from "@gxwf/server-common/src/languageTypes"; +import { + BasicCommonValidationProfile, + IWCCommonValidationProfile, +} from "@gxwf/server-common/src/providers/validation/profiles"; +import { RequiredPropertyValidationRule } from "@gxwf/server-common/src/providers/validation/rules"; +import { WorkflowOutputLabelValidationRule } from "./validation/rules/WorkflowOutputLabelValidationRule"; + +/** + * Defines the minimal set of validation rules for Native Galaxy workflows. + */ +export class NativeBasicValidationProfile extends BasicCommonValidationProfile { + public readonly name: string = "Workflow Validation"; + public static readonly RULES = new Set([ + ...super.RULES, + // Add more custom rules specific to native workflows here... + ]); + + public get rules(): Set { + return NativeBasicValidationProfile.RULES; + } +} + +/** + * *Intergalactic Workflow Commission* (IWC) validation profile for Native Galaxy workflows. + * This profile extends the basic validation profile and adds additional rules to comply + * with the IWC best practices guidelines. + */ +export class NativeIWCValidationProfile extends IWCCommonValidationProfile { + protected static readonly RULES = new Set([ + ...super.RULES, + ...NativeBasicValidationProfile.RULES, + new RequiredPropertyValidationRule( + "annotation", + true, + DiagnosticSeverity.Error, + "The workflow is not annotated. Annotating workflows helps users understand the purpose of the workflow." + ), + new WorkflowOutputLabelValidationRule(DiagnosticSeverity.Error), + // Add more custom rules specific to native workflows here... + ]); + + public get rules(): Set { + return NativeIWCValidationProfile.RULES; + } +} diff --git a/server/packages/server-common/src/providers/validation/WorkflowOutputLabelValidation.ts b/server/gx-workflow-ls-native/src/validation/rules/WorkflowOutputLabelValidationRule.ts similarity index 72% rename from server/packages/server-common/src/providers/validation/WorkflowOutputLabelValidation.ts rename to server/gx-workflow-ls-native/src/validation/rules/WorkflowOutputLabelValidationRule.ts index 1c54742..887b01d 100644 --- a/server/packages/server-common/src/providers/validation/WorkflowOutputLabelValidation.ts +++ b/server/gx-workflow-ls-native/src/validation/rules/WorkflowOutputLabelValidationRule.ts @@ -1,15 +1,15 @@ +import { DocumentContext, ValidationRule } from "@gxwf/server-common/src/languageTypes"; import { Diagnostic, DiagnosticSeverity } from "vscode-languageserver-types"; -import { ValidationRule, WorkflowDocument } from "../../languageTypes"; /** * Validation rule to check that all defined `workflow_outputs` have a `label`. */ -export class WorkflowOutputLabelValidation implements ValidationRule { +export class WorkflowOutputLabelValidationRule implements ValidationRule { constructor(readonly severity: DiagnosticSeverity = DiagnosticSeverity.Error) {} - validate(workflowDocument: WorkflowDocument): Promise { + public async validate(documentContext: DocumentContext): Promise { const result: Diagnostic[] = []; - const stepNodes = workflowDocument.nodeManager.getStepNodes(); + const stepNodes = documentContext.nodeManager.getStepNodes(); stepNodes.forEach((step) => { const workflowOutputs = step.properties.find((property) => property.keyNode.value === "workflow_outputs"); if (workflowOutputs && workflowOutputs.valueNode && workflowOutputs.valueNode.type === "array") { @@ -19,7 +19,7 @@ export class WorkflowOutputLabelValidation implements ValidationRule { if (!labelNode?.valueNode?.value) { result.push({ message: `Missing label in workflow output.`, - range: workflowDocument.nodeManager.getNodeRange(outputNode), + range: documentContext.nodeManager.getNodeRange(outputNode), severity: this.severity, }); } diff --git a/server/gx-workflow-ls-native/tests/unit/customValidationRules.test.ts b/server/gx-workflow-ls-native/tests/unit/customValidationRules.test.ts index 95d53ef..2d2dc3d 100644 --- a/server/gx-workflow-ls-native/tests/unit/customValidationRules.test.ts +++ b/server/gx-workflow-ls-native/tests/unit/customValidationRules.test.ts @@ -1,13 +1,15 @@ +import { DiagnosticSeverity, ValidationRule } from "@gxwf/server-common/src/languageTypes"; +import { RequiredPropertyValidationRule } from "@gxwf/server-common/src/providers/validation/rules"; +import { WorkflowOutputLabelValidationRule } from "../../src/validation/rules/WorkflowOutputLabelValidationRule"; import { createNativeWorkflowDocument } from "../testHelpers"; -import { WorkflowOutputLabelValidation } from "@gxwf/server-common/src/providers/validation/WorkflowOutputLabelValidation"; import { TestWorkflowProvider } from "../testWorkflowProvider"; describe("Custom Validation Rules", () => { - describe("WorkflowOutputLabelValidation Rule", () => { - let rule: WorkflowOutputLabelValidation; + let rule: ValidationRule; - beforeEach(() => { - rule = new WorkflowOutputLabelValidation(); + describe("WorkflowOutputLabelValidation Rule", () => { + beforeAll(() => { + rule = new WorkflowOutputLabelValidationRule(); }); it("should not provide diagnostics when there are no steps", async () => { @@ -41,4 +43,200 @@ describe("Custom Validation Rules", () => { }); }); }); + + describe("Required Propery Validation Rule", () => { + beforeAll(() => { + rule = new RequiredPropertyValidationRule("release"); + }); + + it("should not provide diagnostics when the property is present", async () => { + const wfContents = `{ + "a_galaxy_workflow": "true", + "release": "0.1", + }`; + const wfDocument = createNativeWorkflowDocument(wfContents); + const diagnostics = await rule.validate(wfDocument); + expect(diagnostics).toHaveLength(0); + }); + + it("should provide error diagnostics when the property is missing", async () => { + const wfContents = `{ + "a_galaxy_workflow": "true", + }`; + const wfDocument = createNativeWorkflowDocument(wfContents); + const diagnostics = await rule.validate(wfDocument); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0].message).toBe('Missing required property "release".'); + expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error); + }); + + it("should provide error diagnostics when the property is present but has no value", async () => { + const wfContents = `{ + "a_galaxy_workflow": "true", + "release": "", + }`; + const wfDocument = createNativeWorkflowDocument(wfContents); + const diagnostics = await rule.validate(wfDocument); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0].message).toBe('Missing required value in property "release".'); + expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error); + }); + + it("should provide warning diagnostics when the property is missing and severity is set to warning", async () => { + rule = new RequiredPropertyValidationRule("release", true, DiagnosticSeverity.Warning); + const wfContents = `{ + "a_galaxy_workflow": "true", + }`; + const wfDocument = createNativeWorkflowDocument(wfContents); + const diagnostics = await rule.validate(wfDocument); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0].message).toBe('Missing required property "release".'); + expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Warning); + }); + + it("should display a custom message when provided", async () => { + rule = new RequiredPropertyValidationRule("release", true, DiagnosticSeverity.Warning, "Custom message"); + const wfContents = `{ + "a_galaxy_workflow": "true", + }`; + const wfDocument = createNativeWorkflowDocument(wfContents); + const diagnostics = await rule.validate(wfDocument); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0].message).toBe("Custom message"); + expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Warning); + }); + + describe("when valueRequired is false", () => { + beforeAll(() => { + rule = new RequiredPropertyValidationRule("release", false); + }); + + it("should not provide diagnostics when the property is present", async () => { + const wfContents = `{ + "a_galaxy_workflow": "true", + "release": "0.1", + }`; + const wfDocument = createNativeWorkflowDocument(wfContents); + const diagnostics = await rule.validate(wfDocument); + expect(diagnostics).toHaveLength(0); + }); + + it("should provide diagnostics when the property is missing", async () => { + const wfContents = `{ + "a_galaxy_workflow": "true", + }`; + const wfDocument = createNativeWorkflowDocument(wfContents); + const diagnostics = await rule.validate(wfDocument); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0].message).toBe('Missing required property "release".'); + expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error); + }); + + it("should not provide diagnostics when the property is present but has no value", async () => { + const wfContents = `{ + "a_galaxy_workflow": "true", + "release": "", + }`; + const wfDocument = createNativeWorkflowDocument(wfContents); + const diagnostics = await rule.validate(wfDocument); + expect(diagnostics).toHaveLength(0); + }); + }); + + describe("when the property is an array", () => { + beforeAll(() => { + rule = new RequiredPropertyValidationRule("creator"); + }); + + it("should not provide diagnostics when the property has a value", async () => { + const wfContents = `{ + "a_galaxy_workflow": "true", + "creator": [{ "name": "John Doe" }], + }`; + const wfDocument = createNativeWorkflowDocument(wfContents); + const diagnostics = await rule.validate(wfDocument); + expect(diagnostics).toHaveLength(0); + }); + + it("should provide diagnostics when the property is present but has empty value", async () => { + const wfContents = `{ + "a_galaxy_workflow": "true", + "creator": [], + }`; + const wfDocument = createNativeWorkflowDocument(wfContents); + const diagnostics = await rule.validate(wfDocument); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0].message).toBe('Missing required value in property "creator".'); + expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error); + }); + }); + + describe("when the property is an object", () => { + beforeAll(() => { + rule = new RequiredPropertyValidationRule("steps"); + }); + + it("should not provide diagnostics when the property has a value", async () => { + const wfContents = `{ + "a_galaxy_workflow": "true", + "steps": { "0": { "tool_id": "tool1" } }, + }`; + const wfDocument = createNativeWorkflowDocument(wfContents); + const diagnostics = await rule.validate(wfDocument); + expect(diagnostics).toHaveLength(0); + }); + + it("should provide diagnostics when the property is present but has empty value", async () => { + const wfContents = `{ + "a_galaxy_workflow": "true", + "steps": {}, + }`; + const wfDocument = createNativeWorkflowDocument(wfContents); + const diagnostics = await rule.validate(wfDocument); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0].message).toBe('Missing required value in property "steps".'); + expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error); + }); + }); + + describe("when the property is nested", () => { + beforeAll(() => { + rule = new RequiredPropertyValidationRule("steps/0/tool_id"); + }); + + it("should not provide diagnostics when the property has a value", async () => { + const wfContents = `{ + "a_galaxy_workflow": "true", + "steps": { "0": { "tool_id": "tool1" } }, + }`; + const wfDocument = createNativeWorkflowDocument(wfContents); + const diagnostics = await rule.validate(wfDocument); + expect(diagnostics).toHaveLength(0); + }); + + it("should provide diagnostics when the property is missing", async () => { + const wfContents = `{ + "a_galaxy_workflow": "true", + "steps": { "0": {} }, + }`; + const wfDocument = createNativeWorkflowDocument(wfContents); + const diagnostics = await rule.validate(wfDocument); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0].message).toBe('Missing required property "steps/0/tool_id".'); + expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error); + }); + + it("should provide diagnostics when the property is present but has empty value", async () => { + const wfContents = `{ + "a_galaxy_workflow": "true", + "steps": { "0": { "tool_id": "" } }, + }`; + const wfDocument = createNativeWorkflowDocument(wfContents); + const diagnostics = await rule.validate(wfDocument); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0].message).toBe('Missing required value in property "steps/0/tool_id".'); + expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error); + }); + }); + }); }); diff --git a/server/gx-workflow-ls-native/tests/unit/nativeWorkflowDocument.test.ts b/server/gx-workflow-ls-native/tests/unit/nativeWorkflowDocument.test.ts index 96c30bf..0ab3134 100644 --- a/server/gx-workflow-ls-native/tests/unit/nativeWorkflowDocument.test.ts +++ b/server/gx-workflow-ls-native/tests/unit/nativeWorkflowDocument.test.ts @@ -50,13 +50,13 @@ describe("NativeWorkflowDocument", () => { [ TestWorkflowProvider.workflows.validation.withOnlyInputs, [ - { doc: "", name: "Dataset Input", type: "data" }, - { doc: "", name: "Collection Input", type: "collection" }, - { doc: "", name: "Text Param", type: "text" }, - { doc: "", name: "Integer Param", type: "integer" }, - { doc: "", name: "Float Param", type: "float" }, - { doc: "", name: "Boolean Param", type: "boolean" }, - { doc: "", name: "Color Param", type: "color" }, + { default: undefined, doc: "", name: "Dataset Input", optional: false, type: "data" }, + { default: undefined, doc: "", name: "Collection Input", optional: false, type: "collection" }, + { default: undefined, doc: "", name: "Text Param", optional: false, type: "text" }, + { default: 10, doc: "", name: "Integer Param", optional: true, type: "integer" }, + { default: undefined, doc: "", name: "Float Param", optional: false, type: "float" }, + { default: undefined, doc: "", name: "Boolean Param", optional: false, type: "boolean" }, + { default: undefined, doc: "", name: "Color Param", optional: false, type: "color" }, ], ], ])("returns the expected inputs", (wfContent: string, expectedInputs: WorkflowInput[]) => { diff --git a/server/gx-workflow-ls-native/tsconfig.json b/server/gx-workflow-ls-native/tsconfig.json index 198d885..6a81fc7 100644 --- a/server/gx-workflow-ls-native/tsconfig.json +++ b/server/gx-workflow-ls-native/tsconfig.json @@ -11,7 +11,6 @@ "esModuleInterop": true, "sourceMap": true, "strict": true, - "rootDirs": ["src", "tests"], "paths": { "@schemas/*": ["../../workflow-languages/schemas/*"] } diff --git a/server/packages/server-common/src/ast/nodeManager.ts b/server/packages/server-common/src/ast/nodeManager.ts index e9bbdd6..db2d485 100644 --- a/server/packages/server-common/src/ast/nodeManager.ts +++ b/server/packages/server-common/src/ast/nodeManager.ts @@ -1,5 +1,5 @@ import { Position, Range, TextDocument } from "../languageTypes"; -import { ASTNode, NodePath, ObjectASTNode, ParsedDocument, PropertyASTNode, Segment } from "./types"; +import { ASTNode, NodePath, ObjectASTNode, ParsedDocument, PropertyASTNode, Segment, ValueTypes } from "./types"; import { getPropertyNodeFromPath } from "./utils"; export class ASTNodeManager { @@ -109,7 +109,7 @@ export class ASTNodeManager { if (!root) return result; this.visit((node) => { - if (node.type === "property" && node.keyNode.value === name && node.valueNode?.type === "object") { + if (node.type === "property" && node.keyNode.value === name) { result.push(node); } return true; @@ -190,4 +190,34 @@ export class ASTNodeManager { doVisit(this.root); } } + + public isNodeEmpty(node: ASTNode): boolean { + switch (node.type) { + case "object": + return node.properties.length === 0; + case "array": + return node.items.length === 0; + case "property": + return !node.valueNode || this.isNodeEmpty(node.valueNode); + case "string": + return node.value === ""; + case "null": + return true; + default: + return false; + } + } + + public getPropertyNodeByName(node: PropertyASTNode, propertyName: string): PropertyASTNode | undefined { + const targetProperty = node.valueNode?.children?.find( + (prop) => prop.type === "property" && prop.keyNode.value === propertyName + ) as PropertyASTNode; + return targetProperty; + } + + public getPropertyValueByName(node: PropertyASTNode, propertyName: string): ValueTypes | undefined { + const targetProperty = this.getPropertyNodeByName(node, propertyName); + const targetValue = targetProperty?.valueNode?.value; + return targetValue; + } } diff --git a/server/packages/server-common/src/ast/types.ts b/server/packages/server-common/src/ast/types.ts index 2c3ae8c..b869597 100644 --- a/server/packages/server-common/src/ast/types.ts +++ b/server/packages/server-common/src/ast/types.ts @@ -7,13 +7,15 @@ export type ASTNode = | BooleanASTNode | NullASTNode; +export type ValueTypes = string | boolean | number | null; + export interface BaseASTNode { readonly type: "object" | "array" | "property" | "string" | "number" | "boolean" | "null"; readonly parent?: ASTNode; readonly offset: number; readonly length: number; readonly children?: ASTNode[]; - readonly value?: string | boolean | number | null; + readonly value?: ValueTypes; readonly internalNode: unknown; getNodeFromOffsetEndInclusive(offset: number): ASTNode | undefined; } diff --git a/server/packages/server-common/src/languageTypes.ts b/server/packages/server-common/src/languageTypes.ts index 7e49115..3c1fd1c 100644 --- a/server/packages/server-common/src/languageTypes.ts +++ b/server/packages/server-common/src/languageTypes.ts @@ -67,6 +67,7 @@ import { ASTNodeManager } from "./ast/nodeManager"; import { ConfigService } from "./configService"; import { WorkflowDocument } from "./models/workflowDocument"; import { WorkflowTestsDocument } from "./models/workflowTestsDocument"; +import { NoOpValidationProfile } from "./providers/validation/profiles"; export { CleanWorkflowContentsParams, @@ -150,15 +151,17 @@ export interface ValidationRule { validate(documentContext: DocumentContext): Promise; } +export type ValidationProfileIdentifier = "basic" | "iwc"; + /** * Interface representing a validation profile which contains a set of custom rules. */ export interface ValidationProfile { - /** The unique identifier of this validation profile. */ - get id(): string; - /** The set of rules defining this validation profile. */ get rules(): Set; + + /** The human-readable name of the profile. */ + name: string; } /** @@ -197,7 +200,8 @@ export interface LanguageService { * Validates the document and reports all the diagnostics found. * An optional validation profile can be used to provide additional custom diagnostics. */ - validate(documentContext: T, useProfile?: ValidationProfile): Promise; + validate(documentContext: T, useProfile?: ValidationProfileIdentifier): Promise; + getValidationProfile(profileId: ValidationProfileIdentifier): ValidationProfile; setServer(server: GalaxyWorkflowLanguageServer): void; } @@ -208,8 +212,12 @@ export interface LanguageService { */ @injectable() export abstract class LanguageServiceBase implements LanguageService { - constructor(@unmanaged() public readonly languageId: string) {} protected server?: GalaxyWorkflowLanguageServer; + protected validationProfiles = new Map(); + + constructor(@unmanaged() public readonly languageId: string) { + this.initializeValidationProfiles(); + } public abstract parseDocument(document: TextDocument): T; public abstract format(document: TextDocument, range: Range, options: FormattingOptions): TextEdit[]; @@ -220,21 +228,44 @@ export abstract class LanguageServiceBase implements /** Performs basic syntax and semantic validation based on the document schema. */ protected abstract doValidation(documentContext: T): Promise; + /** + * Initializes the validation profiles for this language service. + * Subclasses should override this method to provide custom validation profiles. + * The default implementation does nothing. + */ + protected initializeValidationProfiles(): void { + const defaultProfile = new NoOpValidationProfile(); + this.validationProfiles.set("basic", defaultProfile); + this.validationProfiles.set("iwc", defaultProfile); + } + /** * Validates the document and reports all the diagnostics found. * An optional validation profile can be used to provide additional custom diagnostics. */ - public async validate(documentContext: T, useProfile?: ValidationProfile): Promise { + public async validate(documentContext: T, useProfile?: ValidationProfileIdentifier): Promise { const diagnostics = await this.doValidation(documentContext); if (useProfile) { - useProfile.rules.forEach(async (validationRule) => { - const contributedDiagnostics = await validationRule.validate(documentContext); + const profile = this.getValidationProfile(useProfile); + for (const rule of profile.rules) { + const contributedDiagnostics = await rule.validate(documentContext); + contributedDiagnostics.forEach((diagnostic) => { + diagnostic.source = diagnostic.source ?? profile.name; + }); diagnostics.push(...contributedDiagnostics); - }); + } } return diagnostics; } + public getValidationProfile(profileId: ValidationProfileIdentifier): ValidationProfile { + const profile = this.validationProfiles.get(profileId); + if (!profile) { + throw new Error(`Validation profile not found for id: ${profileId}`); + } + return profile; + } + public setServer(server: GalaxyWorkflowLanguageServer): void { this.server = server; } diff --git a/server/packages/server-common/src/providers/validation/MissingPropertyValidation.ts b/server/packages/server-common/src/providers/validation/MissingPropertyValidation.ts deleted file mode 100644 index c0c702d..0000000 --- a/server/packages/server-common/src/providers/validation/MissingPropertyValidation.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { Diagnostic, DiagnosticSeverity } from "vscode-languageserver-types"; -import { ValidationRule, WorkflowDocument } from "../../languageTypes"; - -/** - * Validation rule to check that a particular property exists in a workflow. - * The property can be specified by a path, i.e: "prop1/prop2" will check - * that 'prop1' contains a 'prop2' property defined. - */ -export class MissingPropertyValidationRule implements ValidationRule { - constructor( - readonly nodePath: string, - readonly severity?: DiagnosticSeverity | undefined - ) {} - - validate(workflowDocument: WorkflowDocument): Promise { - const result: Diagnostic[] = []; - const targetNode = workflowDocument.nodeManager.getNodeFromPath(this.nodePath); - if (!targetNode) { - result.push({ - message: `Missing property "${this.nodePath}".`, - range: workflowDocument.nodeManager.getDefaultRange(), - severity: this.severity, - }); - } - return Promise.resolve(result); - } -} diff --git a/server/packages/server-common/src/providers/validation/profiles.ts b/server/packages/server-common/src/providers/validation/profiles.ts index 84c79a6..53046e4 100644 --- a/server/packages/server-common/src/providers/validation/profiles.ts +++ b/server/packages/server-common/src/providers/validation/profiles.ts @@ -1,46 +1,65 @@ import { DiagnosticSeverity, ValidationProfile, ValidationRule } from "../../languageTypes"; -import { MissingPropertyValidationRule } from "./MissingPropertyValidation"; -import { WorkflowOutputLabelValidation } from "./WorkflowOutputLabelValidation"; +import { RequiredPropertyValidationRule, StepExportErrorValidationRule, TestToolshedValidationRule } from "./rules"; /** - * The default validation profile. + * The *NoOp* validation profile. * It doesn't apply any additional custom rules. */ -export class BasicValidationProfile implements ValidationProfile { - protected static readonly ID = "basic"; +export class NoOpValidationProfile implements ValidationProfile { + public readonly name: string = "No Validation"; protected static readonly NO_RULES = new Set([]); - public get id(): string { - return BasicValidationProfile.ID; - } - public get rules(): Set { - return BasicValidationProfile.NO_RULES; + return NoOpValidationProfile.NO_RULES; } } /** - * *Intergalactic Workflow Commission* (IWC) validation profile. - * Defines custom validation rules to comply with the IWC best practices guidelines. + * Common set of validation rules for basic validation of any workflow format. */ -export class IWCValidationProfile implements ValidationProfile { - protected static readonly ID = "iwc"; - protected static readonly RULES = new Set([ - new MissingPropertyValidationRule("release"), - new WorkflowOutputLabelValidation(DiagnosticSeverity.Error), +export class BasicCommonValidationProfile implements ValidationProfile { + public readonly name: string = "Workflow Validator"; + + protected static readonly RULES: Set = new Set([ + new TestToolshedValidationRule(DiagnosticSeverity.Error), + new StepExportErrorValidationRule(DiagnosticSeverity.Error), + // Add common basic rules here... ]); - public get id(): string { - return IWCValidationProfile.ID; + public get rules(): Set { + return BasicCommonValidationProfile.RULES; } +} + +/** + * Common set of validation rules for IWC best practices. + */ +export class IWCCommonValidationProfile implements ValidationProfile { + public readonly name: string = "IWC Best Practices"; + + protected static readonly RULES: Set = new Set([ + new RequiredPropertyValidationRule( + "release", + true, + DiagnosticSeverity.Error, + "The workflow must have a release version." + ), + new RequiredPropertyValidationRule( + "creator", + true, + DiagnosticSeverity.Error, + "The workflow does not specify a creator." + ), + new RequiredPropertyValidationRule( + "license", + true, + DiagnosticSeverity.Error, + "The workflow does not specify a license." + ), + // Add more common rules here... + ]); public get rules(): Set { - return IWCValidationProfile.RULES; + return IWCCommonValidationProfile.RULES; } } - -/** Contains all the available validation profiles. */ -export const ValidationProfiles = new Map([ - ["basic", new BasicValidationProfile()], - ["iwc", new IWCValidationProfile()], -]); diff --git a/server/packages/server-common/src/providers/validation/rules/RequiredArrayPropertyValidationRule.ts b/server/packages/server-common/src/providers/validation/rules/RequiredArrayPropertyValidationRule.ts new file mode 100644 index 0000000..3e95f9e --- /dev/null +++ b/server/packages/server-common/src/providers/validation/rules/RequiredArrayPropertyValidationRule.ts @@ -0,0 +1,42 @@ +import { Diagnostic, DiagnosticSeverity } from "vscode-languageserver-types"; +import { DocumentContext, ValidationRule } from "../../../languageTypes"; + +export class RequiredArrayPropertyValidationRule implements ValidationRule { + constructor( + private propertyName: string, + private valueRequired: boolean = true, + private severity: DiagnosticSeverity = DiagnosticSeverity.Error, + private message?: string + ) {} + + public async validate(documentContext: DocumentContext): Promise { + const result: Diagnostic[] = []; + if (documentContext.nodeManager.root?.type !== "array") { + return Promise.resolve(result); + } + for (const node of documentContext.nodeManager.root.items) { + if (node.type !== "object") { + continue; + } + const targetNode = node.properties.find((p) => p.keyNode.value === this.propertyName); + if (!targetNode) { + result.push({ + message: this.message ?? `Missing required property "${this.propertyName}".`, + range: documentContext.nodeManager.getNodeRange(node), + severity: this.severity, + }); + } + if (this.valueRequired && targetNode) { + const missingValue = documentContext.nodeManager.isNodeEmpty(targetNode); + if (missingValue) { + result.push({ + message: `Missing required value in property "${this.propertyName}".`, + range: documentContext.nodeManager.getNodeRange(targetNode), + severity: this.severity, + }); + } + } + } + return Promise.resolve(result); + } +} diff --git a/server/packages/server-common/src/providers/validation/rules/RequiredPropertyValidationRule.ts b/server/packages/server-common/src/providers/validation/rules/RequiredPropertyValidationRule.ts new file mode 100644 index 0000000..66986fb --- /dev/null +++ b/server/packages/server-common/src/providers/validation/rules/RequiredPropertyValidationRule.ts @@ -0,0 +1,45 @@ +import { Diagnostic, DiagnosticSeverity } from "vscode-languageserver-types"; +import { DocumentContext, ValidationRule } from "../../../languageTypes"; + +/** + * Validation rule to check that a particular property exists in a workflow. + * The property can be specified by a path, i.e: "prop1/prop2" will check + * that 'prop1' contains a 'prop2' property defined. + * + * By default, the rule will also check that the property has a value, but this + * can be disabled by setting the `valueRequired` parameter to `false`. If the + * property is an object or an array, the rule will also check that it has at + * least one property or item. + */ +export class RequiredPropertyValidationRule implements ValidationRule { + constructor( + private nodePath: string, + private valueRequired: boolean = true, + private severity: DiagnosticSeverity = DiagnosticSeverity.Error, + private message?: string + ) {} + + public async validate(documentContext: DocumentContext): Promise { + const result: Diagnostic[] = []; + const targetNode = documentContext.nodeManager.getNodeFromPath(this.nodePath); + if (!targetNode) { + result.push({ + message: this.message ?? `Missing required property "${this.nodePath}".`, + range: documentContext.nodeManager.getDefaultRange(), + severity: this.severity, + }); + } + + if (this.valueRequired && targetNode) { + const missingValue = documentContext.nodeManager.isNodeEmpty(targetNode); + if (missingValue) { + result.push({ + message: `Missing required value in property "${this.nodePath}".`, + range: documentContext.nodeManager.getNodeRange(targetNode), + severity: this.severity, + }); + } + } + return Promise.resolve(result); + } +} diff --git a/server/packages/server-common/src/providers/validation/rules/StepErrorValidationRule.ts b/server/packages/server-common/src/providers/validation/rules/StepErrorValidationRule.ts new file mode 100644 index 0000000..96cfe58 --- /dev/null +++ b/server/packages/server-common/src/providers/validation/rules/StepErrorValidationRule.ts @@ -0,0 +1,28 @@ +import { DocumentContext, ValidationRule } from "@gxwf/server-common/src/languageTypes"; +import { Diagnostic, DiagnosticSeverity } from "vscode-languageserver-types"; + +/** + * Validation rule to check if step nodes contain export errors. + */ +export class StepExportErrorValidationRule implements ValidationRule { + constructor(readonly severity: DiagnosticSeverity = DiagnosticSeverity.Error) {} + + public async validate(documentContext: DocumentContext): Promise { + const diagnostics: Diagnostic[] = []; + const steps = documentContext.nodeManager.getStepNodes(true); + steps.forEach((step) => { + const errors = step.properties.find((p) => p.keyNode.value === "errors"); + if (errors) { + const range = documentContext.nodeManager.getNodeRange(errors); + diagnostics.push( + Diagnostic.create( + range, + `Tool step contains error indicated during Galaxy export - ${errors}`, + DiagnosticSeverity.Warning + ) + ); + } + }); + return Promise.resolve(diagnostics); + } +} diff --git a/server/packages/server-common/src/providers/validation/rules/TestToolShedValidationRule.ts b/server/packages/server-common/src/providers/validation/rules/TestToolShedValidationRule.ts new file mode 100644 index 0000000..65b9ad7 --- /dev/null +++ b/server/packages/server-common/src/providers/validation/rules/TestToolShedValidationRule.ts @@ -0,0 +1,30 @@ +import { DocumentContext, ValidationRule } from "@gxwf/server-common/src/languageTypes"; +import { Diagnostic, DiagnosticSeverity } from "vscode-languageserver-types"; + +/** + * Validation rule to check if step nodes contain a reference to a tool from the test Tool Shed. + */ +export class TestToolshedValidationRule implements ValidationRule { + constructor(readonly severity: DiagnosticSeverity = DiagnosticSeverity.Error) {} + + public async validate(documentContext: DocumentContext): Promise { + const diagnostics: Diagnostic[] = []; + const steps = documentContext.nodeManager.getStepNodes(true); + steps.forEach((step) => { + const tool_id = step.properties.find((p) => p.keyNode.value === "tool_id"); + if (tool_id) { + if (tool_id.valueNode?.value?.toString().includes("testtoolshed")) { + const range = documentContext.nodeManager.getNodeRange(tool_id); + diagnostics.push( + Diagnostic.create( + range, + `Step references a tool from the test tool shed, this should be replaced with a production tool`, + DiagnosticSeverity.Warning + ) + ); + } + } + }); + return Promise.resolve(diagnostics); + } +} diff --git a/server/packages/server-common/src/providers/validation/rules/index.ts b/server/packages/server-common/src/providers/validation/rules/index.ts new file mode 100644 index 0000000..294f5cf --- /dev/null +++ b/server/packages/server-common/src/providers/validation/rules/index.ts @@ -0,0 +1,11 @@ +import { RequiredArrayPropertyValidationRule } from "./RequiredArrayPropertyValidationRule"; +import { RequiredPropertyValidationRule } from "./RequiredPropertyValidationRule"; +import { StepExportErrorValidationRule } from "./StepErrorValidationRule"; +import { TestToolshedValidationRule } from "./TestToolShedValidationRule"; + +export { + RequiredArrayPropertyValidationRule, + RequiredPropertyValidationRule, + StepExportErrorValidationRule, + TestToolshedValidationRule, +}; diff --git a/server/packages/server-common/src/server.ts b/server/packages/server-common/src/server.ts index c712918..4becb7c 100644 --- a/server/packages/server-common/src/server.ts +++ b/server/packages/server-common/src/server.ts @@ -26,7 +26,6 @@ import { inject, injectable } from "inversify"; import { ConfigService } from "./configService"; import { CompletionHandler } from "./providers/completionHandler"; import { ServerEventHandler } from "./providers/handler"; -import { ValidationProfiles } from "./providers/validation/profiles"; @injectable() export class GalaxyWorkflowLanguageServerImpl implements GalaxyWorkflowLanguageServer { @@ -144,9 +143,8 @@ export class GalaxyWorkflowLanguageServerImpl implements GalaxyWorkflowLanguageS return; } const settings = await this.configService.getDocumentSettings(documentContext.textDocument.uri); - const validationProfile = ValidationProfiles.get(settings.validation.profile); const languageService = this.getLanguageServiceById(documentContext.languageId); - languageService.validate(documentContext, validationProfile).then((diagnostics) => { + languageService.validate(documentContext, settings.validation.profile).then((diagnostics) => { this.connection.sendDiagnostics({ uri: documentContext.textDocument.uri, diagnostics }); }); } diff --git a/server/packages/server-common/src/utils.ts b/server/packages/server-common/src/utils.ts new file mode 100644 index 0000000..8019952 --- /dev/null +++ b/server/packages/server-common/src/utils.ts @@ -0,0 +1,42 @@ +import { workflowDataTypes } from "../../../../shared/src/requestsDefinitions"; +import { WorkflowDataType } from "./languageTypes"; + +/** + * Check if the actual type can be mapped to the expected type. + * Usefull to validate properties types. + * @param expectedType The expected type. Usually a type supported by the schema. + * @param actualType The actual type. You can use the `typeof` operator to get this value. + */ +export function isCompatibleType(expectedType: WorkflowDataType, actualType: string): boolean { + let isCompatible = true; + switch (expectedType) { + case "int": + case "integer": + case "long": + case "double": + case "float": + isCompatible = actualType === "number"; + break; + case "boolean": + isCompatible = actualType === "boolean"; + break; + case "text": + case "string": + isCompatible = actualType === "string"; + break; + case "File": + isCompatible = actualType === "string" || actualType === "object"; + break; + case "null": + isCompatible = actualType === null; + break; + } + return isCompatible; +} + +export function isWorkflowDataType(type?: string): type is WorkflowDataType { + if (!type) { + return false; + } + return type in workflowDataTypes; +} diff --git a/server/packages/server-common/tests/testHelpers.ts b/server/packages/server-common/tests/testHelpers.ts index 019143e..3ac5e32 100644 --- a/server/packages/server-common/tests/testHelpers.ts +++ b/server/packages/server-common/tests/testHelpers.ts @@ -57,6 +57,7 @@ export const FAKE_DATASET_INPUT: WorkflowInput = { name: "My fake dataset", doc: "This is a simple dataset", type: "data", + optional: true, }; export const EXPECTED_WORKFLOW_INPUTS: WorkflowInput[] = [ @@ -65,11 +66,33 @@ export const EXPECTED_WORKFLOW_INPUTS: WorkflowInput[] = [ name: "Input dataset: fake", doc: "This is a simple dataset with a colon in the name", type: "File", + optional: true, }, { name: "My fake collection", doc: "This is a collection", type: "collection", + optional: true, + }, + { + name: "My fake string", + doc: "This is an optional string with a default value", + type: "string", + default: "default string", + optional: true, + }, + { + name: "My fake number", + doc: "This is a required number parameter", + type: "int", + optional: false, + }, + { + name: "My fake boolean", + doc: "This is a required boolean parameter with a default value", + type: "boolean", + default: true, + optional: false, }, ]; diff --git a/server/packages/workflow-tests-language-service/src/languageService.ts b/server/packages/workflow-tests-language-service/src/languageService.ts index 8885147..1bf1947 100644 --- a/server/packages/workflow-tests-language-service/src/languageService.ts +++ b/server/packages/workflow-tests-language-service/src/languageService.ts @@ -16,6 +16,7 @@ import { TYPES as YAML_TYPES } from "@gxwf/yaml-language-service/src/inversify.c import { YAMLLanguageService } from "@gxwf/yaml-language-service/src/yamlLanguageService"; import { inject, injectable } from "inversify"; import { GxWorkflowTestsDocument } from "./document"; +import { TestDocumentBasicValidationProfile, TestDocumentIWCValidationProfile } from "./profiles"; import { WorkflowTestsCompletionService } from "./services/completion"; import { WorkflowTestsHoverService } from "./services/hover"; import { WorkflowTestsValidationService } from "./services/validation"; @@ -60,6 +61,12 @@ export class GxWorkflowTestsLanguageServiceImpl extends LanguageServiceBase { return this.validationService.doValidation(documentContext); } diff --git a/server/packages/workflow-tests-language-service/src/profiles.ts b/server/packages/workflow-tests-language-service/src/profiles.ts new file mode 100644 index 0000000..ef8121d --- /dev/null +++ b/server/packages/workflow-tests-language-service/src/profiles.ts @@ -0,0 +1,43 @@ +import { DiagnosticSeverity, ValidationProfile, ValidationRule } from "@gxwf/server-common/src/languageTypes"; +import { IWCCommonValidationProfile } from "@gxwf/server-common/src/providers/validation/profiles"; +import { RequiredArrayPropertyValidationRule } from "@gxwf/server-common/src/providers/validation/rules"; +import { WorkflowInputsValidationRule } from "./validation/rules/WorkflowInputsValidationRule"; +import { WorkflowOutputsValidationRule } from "./validation/rules/WorkflowOutputsValidationRule"; + +/** + * Defines the minimal set of validation rules for Galaxy Workflow Tests Documents. + */ +export class TestDocumentBasicValidationProfile implements ValidationProfile { + public readonly name: string = "Test Document Validator"; + public static readonly RULES = new Set([ + new WorkflowInputsValidationRule(DiagnosticSeverity.Error), + new WorkflowOutputsValidationRule(DiagnosticSeverity.Error), + // Add more custom rules specific to native workflows here... + ]); + + public get rules(): Set { + return TestDocumentBasicValidationProfile.RULES; + } +} + +/** + * *Intergalactic Workflow Commission* (IWC) validation profile for Galaxy Workflow Tests Documents. + * This profile extends the basic validation profile and adds additional rules to comply + * with the IWC best practices guidelines. + */ +export class TestDocumentIWCValidationProfile extends IWCCommonValidationProfile { + protected static readonly RULES = new Set([ + ...TestDocumentBasicValidationProfile.RULES, + new RequiredArrayPropertyValidationRule( + "doc", + true, + DiagnosticSeverity.Warning, + "The workflow test is not documented. Documenting the tests helps reviewers understand the purpose of the tests." + ), + // Add more custom rules specific to native workflows here... + ]); + + public get rules(): Set { + return TestDocumentIWCValidationProfile.RULES; + } +} diff --git a/server/packages/workflow-tests-language-service/src/schema/adapter.ts b/server/packages/workflow-tests-language-service/src/schema/adapter.ts index 9ca59f7..5648b64 100644 --- a/server/packages/workflow-tests-language-service/src/schema/adapter.ts +++ b/server/packages/workflow-tests-language-service/src/schema/adapter.ts @@ -227,7 +227,7 @@ enum ProblemType { } const ProblemTypeMessages: Record = { - [ProblemType.missingRequiredPropWarning]: 'Missing property "{0}".', + [ProblemType.missingRequiredPropWarning]: 'Missing required property "{0}".', [ProblemType.typeMismatchWarning]: 'Incorrect type. Expected "{0}".', [ProblemType.constWarning]: "Value must be {0}.", }; diff --git a/server/packages/workflow-tests-language-service/src/services/hover.ts b/server/packages/workflow-tests-language-service/src/services/hover.ts index 9e9205a..6032d7f 100644 --- a/server/packages/workflow-tests-language-service/src/services/hover.ts +++ b/server/packages/workflow-tests-language-service/src/services/hover.ts @@ -47,11 +47,7 @@ export class WorkflowTestsHoverServiceImpl implements WorkflowTestsHoverService return Promise.resolve(null); } - const hoverRange = Range.create( - documentContext.textDocument.positionAt(hoverRangeNode.offset), - documentContext.textDocument.positionAt(hoverRangeNode.offset + hoverRangeNode.length) - ); - + const hoverRange = documentContext.nodeManager.getNodeRange(hoverRangeNode); if (this.parentPropertyMatchesKey(node, "job")) { const inputHover = await this.getHoverForWorkflowInput(documentContext, node, hoverRange); if (inputHover) { diff --git a/server/packages/workflow-tests-language-service/src/services/validation.ts b/server/packages/workflow-tests-language-service/src/services/validation.ts index 5e24874..2d1de15 100644 --- a/server/packages/workflow-tests-language-service/src/services/validation.ts +++ b/server/packages/workflow-tests-language-service/src/services/validation.ts @@ -1,10 +1,4 @@ -import { - Diagnostic, - DiagnosticSeverity, - DocumentContext, - Range, - WorkflowTestsDocument, -} from "@gxwf/server-common/src/languageTypes"; +import { Diagnostic, DiagnosticSeverity, DocumentContext } from "@gxwf/server-common/src/languageTypes"; import { inject, injectable } from "inversify"; import { ResolvedSchema } from "../schema/jsonSchema"; import { WorkflowTestsSchemaProvider } from "../schema/provider"; @@ -43,16 +37,10 @@ export class WorkflowTestsValidationServiceImpl implements WorkflowTestsValidati const property = astRoot.type === "object" ? astRoot.properties[0] : undefined; if (property && property.keyNode.value === "$schema") { const node = property.valueNode || property; - const range = Range.create( - documentContext.textDocument.positionAt(node.offset), - documentContext.textDocument.positionAt(node.offset + node.length) - ); + const range = documentContext.nodeManager.getNodeRange(node); addProblem(Diagnostic.create(range, errorMessage, severity)); } else { - const range = Range.create( - documentContext.textDocument.positionAt(astRoot.offset), - documentContext.textDocument.positionAt(astRoot.offset + 1) - ); + const range = documentContext.nodeManager.getDefaultRange(); addProblem(Diagnostic.create(range, errorMessage, severity)); } } @@ -73,59 +61,9 @@ export class WorkflowTestsValidationServiceImpl implements WorkflowTestsValidati const schema = this.schemaProvider.getResolvedSchema(); const schemaValidation = getDiagnostics(schema); - const semanticValidation = await this.doSemanticValidation(documentContext); - return schemaValidation.concat(semanticValidation); - } - - async doSemanticValidation(documentContext: DocumentContext): Promise { - const testDocument = documentContext as WorkflowTestsDocument; - const inputDiagnostics = await this.validateWorkflowInputs(testDocument); - const outputDiagnostics = await this.validateWorkflowOutputs(testDocument); - return inputDiagnostics.concat(outputDiagnostics); - } - - private async validateWorkflowInputs(testDocument: WorkflowTestsDocument): Promise { - const diagnostics: Diagnostic[] = []; - const workflowInputs = await testDocument.getWorkflowInputs(); - const documentInputNodes = testDocument.nodeManager.getAllPropertyNodesByName("job")[0]?.valueNode?.children ?? []; - documentInputNodes.forEach((inputNode) => { - if (inputNode.type !== "property") { - return; - } - const inputName = inputNode.keyNode.value as string; - const input = workflowInputs.find((i) => i.name === inputName); - if (!input) { - const range = Range.create( - testDocument.textDocument.positionAt(inputNode.offset), - testDocument.textDocument.positionAt(inputNode.offset + inputNode.length) - ); - const message = `Input "${inputName}" is not defined in the associated workflow.`; - diagnostics.push(Diagnostic.create(range, message, DiagnosticSeverity.Error)); - } - }); - return diagnostics; - } - - private async validateWorkflowOutputs(testDocument: WorkflowTestsDocument): Promise { - const diagnostics: Diagnostic[] = []; - const workflowOutputs = await testDocument.getWorkflowOutputs(); - const documentOutputNodes = - testDocument.nodeManager.getAllPropertyNodesByName("outputs")[0]?.valueNode?.children ?? []; - documentOutputNodes.forEach((outputNode) => { - if (outputNode.type !== "property") { - return; - } - const outputName = outputNode.keyNode.value as string; - const output = workflowOutputs.find((o) => o.name === outputName); - if (!output) { - const range = Range.create( - testDocument.textDocument.positionAt(outputNode.offset), - testDocument.textDocument.positionAt(outputNode.offset + outputNode.length) - ); - const message = `Output "${outputName}" is not defined in the associated workflow.`; - diagnostics.push(Diagnostic.create(range, message, DiagnosticSeverity.Error)); - } + schemaValidation.forEach((diagnostic) => { + diagnostic.source = "Workflow Tests Schema"; }); - return diagnostics; + return schemaValidation; } } diff --git a/server/packages/workflow-tests-language-service/src/validation/rules/WorkflowInputsValidationRule.ts b/server/packages/workflow-tests-language-service/src/validation/rules/WorkflowInputsValidationRule.ts new file mode 100644 index 0000000..bd418c3 --- /dev/null +++ b/server/packages/workflow-tests-language-service/src/validation/rules/WorkflowInputsValidationRule.ts @@ -0,0 +1,70 @@ +import { PropertyASTNode } from "@gxwf/server-common/src/ast/types"; +import { ValidationRule, WorkflowInput, WorkflowTestsDocument } from "@gxwf/server-common/src/languageTypes"; +import { isCompatibleType } from "@gxwf/server-common/src/utils"; +import { Diagnostic, DiagnosticSeverity } from "vscode-languageserver-types"; + +/** + * Validation rule to check if the inputs defined in the test document are present in the associated workflow. + */ +export class WorkflowInputsValidationRule implements ValidationRule { + constructor(readonly severity: DiagnosticSeverity = DiagnosticSeverity.Error) {} + + public async validate(documentContext: WorkflowTestsDocument): Promise { + const diagnostics: Diagnostic[] = []; + const workflowInputs = await documentContext.getWorkflowInputs(); + const jobNodes = documentContext.nodeManager.getAllPropertyNodesByName("job"); + for (const jobNode of jobNodes) { + const jobDiagnostics = await this.validateInputsInJobNode(jobNode, workflowInputs, documentContext); + diagnostics.push(...jobDiagnostics); + } + return Promise.resolve(diagnostics); + } + + private async validateInputsInJobNode( + jobNode: PropertyASTNode, + workflowInputs: WorkflowInput[], + documentContext: WorkflowTestsDocument + ): Promise { + const diagnostics: Diagnostic[] = []; + if (!jobNode) { + // Skip validation if the job node is not present. This will be caught by schema validation. + return Promise.resolve(diagnostics); + } + const documentInputNodes = jobNode?.valueNode?.children ?? []; + const definedInputsInTestDocument = new Set(); + for (const inputNode of documentInputNodes) { + if (inputNode.type !== "property") { + continue; + } + const inputName = inputNode.keyNode.value as string; + definedInputsInTestDocument.add(inputName); + const input = workflowInputs.find((i) => i.name === inputName); + if (!input) { + const range = documentContext.nodeManager.getNodeRange(inputNode); + const message = `Input "${inputName}" is not defined in the associated workflow.`; + diagnostics.push(Diagnostic.create(range, message, DiagnosticSeverity.Error)); + } else { + if (inputNode.valueNode) { + const inputType = input.type; + const inputTypeValue = inputNode.valueNode.type; + if (!isCompatibleType(inputType, inputTypeValue)) { + const range = documentContext.nodeManager.getNodeRange(inputNode); + const message = `Input "${inputName}" has an invalid type. Expected "${inputType}" but found "${inputTypeValue}".`; + diagnostics.push(Diagnostic.create(range, message, DiagnosticSeverity.Error)); + } + } + } + } + const requiredWorkflowInputs = workflowInputs + .filter((i) => !i.optional && i.default === undefined) + .map((i) => i.name); + const range = documentContext.nodeManager.getNodeRange(jobNode); + for (const requiredInput of requiredWorkflowInputs) { + if (!definedInputsInTestDocument.has(requiredInput)) { + const message = `Input "${requiredInput}" is required but no value or default was provided.`; + diagnostics.push(Diagnostic.create(range, message, DiagnosticSeverity.Error)); + } + } + return Promise.resolve(diagnostics); + } +} diff --git a/server/packages/workflow-tests-language-service/src/validation/rules/WorkflowOutputsValidationRule.ts b/server/packages/workflow-tests-language-service/src/validation/rules/WorkflowOutputsValidationRule.ts new file mode 100644 index 0000000..883c391 --- /dev/null +++ b/server/packages/workflow-tests-language-service/src/validation/rules/WorkflowOutputsValidationRule.ts @@ -0,0 +1,29 @@ +import { ValidationRule, WorkflowTestsDocument } from "@gxwf/server-common/src/languageTypes"; +import { Diagnostic, DiagnosticSeverity } from "vscode-languageserver-types"; + +/** + * Validation rule to check if the outputs defined in the test document are present in the associated workflow. + */ +export class WorkflowOutputsValidationRule implements ValidationRule { + constructor(readonly severity: DiagnosticSeverity = DiagnosticSeverity.Error) {} + + public async validate(documentContext: WorkflowTestsDocument): Promise { + const diagnostics: Diagnostic[] = []; + const workflowOutputs = await documentContext.getWorkflowOutputs(); + const documentOutputNodes = + documentContext.nodeManager.getAllPropertyNodesByName("outputs")[0]?.valueNode?.children ?? []; + documentOutputNodes.forEach((outputNode) => { + if (outputNode.type !== "property") { + return; + } + const outputName = outputNode.keyNode.value as string; + const output = workflowOutputs.find((o) => o.name === outputName); + if (!output) { + const range = documentContext.nodeManager.getNodeRange(outputNode); + const message = `Output "${outputName}" is not defined in the associated workflow.`; + diagnostics.push(Diagnostic.create(range, message, DiagnosticSeverity.Error)); + } + }); + return Promise.resolve(diagnostics); + } +} diff --git a/server/packages/workflow-tests-language-service/tests/unit/validation.test.ts b/server/packages/workflow-tests-language-service/tests/unit/validation.test.ts index c7f0692..13e2172 100644 --- a/server/packages/workflow-tests-language-service/tests/unit/validation.test.ts +++ b/server/packages/workflow-tests-language-service/tests/unit/validation.test.ts @@ -1,10 +1,18 @@ import { container } from "@gxwf/server-common/src/inversify.config"; -import { Diagnostic, DiagnosticSeverity, WorkflowDataProvider } from "@gxwf/server-common/src/languageTypes"; +import { + Diagnostic, + DiagnosticSeverity, + ValidationRule, + WorkflowDataProvider, +} from "@gxwf/server-common/src/languageTypes"; +import { RequiredArrayPropertyValidationRule } from "@gxwf/server-common/src/providers/validation/rules"; import { FAKE_WORKFLOW_DATA_PROVIDER } from "@gxwf/server-common/tests/testHelpers"; import { WorkflowTestsLanguageServiceContainerModule } from "@gxwf/workflow-tests-language-service/src/inversify.config"; import { WorkflowTestsValidationService } from "@gxwf/workflow-tests-language-service/src/services/validation"; import "reflect-metadata"; import { TYPES } from "../../src/types"; +import { WorkflowInputsValidationRule } from "../../src/validation/rules/WorkflowInputsValidationRule"; +import { WorkflowOutputsValidationRule } from "../../src/validation/rules/WorkflowOutputsValidationRule"; import { createGxWorkflowTestsDocument } from "../testHelpers"; describe("Workflow Tests Validation Service", () => { @@ -30,46 +38,158 @@ describe("Workflow Tests Validation Service", () => { const diagnostics = await validate(testDocumentContents); expect(diagnostics.length).toBe(2); - expect(diagnostics[0].message).toBe('Missing property "job".'); + expect(diagnostics[0].message).toBe('Missing required property "job".'); expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Warning); - expect(diagnostics[1].message).toBe('Missing property "outputs".'); + expect(diagnostics[1].message).toBe('Missing required property "outputs".'); expect(diagnostics[1].severity).toBe(DiagnosticSeverity.Warning); }); +}); + +describe("Workflow Tests Validation Rules", () => { + let rule: ValidationRule; - describe("Workflow Inputs/Outputs Validation", () => { - it("should pass validation when the inputs and outputs are defined in the workflow", async () => { + async function validate( + contents: string, + workflowDataProvider: WorkflowDataProvider = FAKE_WORKFLOW_DATA_PROVIDER + ): Promise { + const documentContext = createGxWorkflowTestsDocument(contents, workflowDataProvider); + return await rule.validate(documentContext); + } + + describe("WorkflowInputsValidationRule", () => { + beforeAll(() => { + rule = new WorkflowInputsValidationRule(); + }); + + it("should pass validation when a valid input is defined in the workflow (inline)", async () => { const testDocumentContents = ` -- doc: The docs - job: - My fake dataset: data/input.txt - outputs: - My output: out/output.txt`; + - doc: The docs + job: + My fake dataset: data/input.txt + My fake number: 5 + `; const diagnostics = await validate(testDocumentContents); expect(diagnostics).not.toBeNull(); + expect(diagnostics.length).toBe(0); + }); + + it("should pass validation when a valid input is defined in the workflow", async () => { + const testDocumentContents = ` + - doc: The docs + job: + My fake dataset: + class: File + My fake number: 5 + `; + + const diagnostics = await validate(testDocumentContents); + + expect(diagnostics).not.toBeNull(); + expect(diagnostics.length).toBe(0); }); it("should error when an input is not defined in the workflow", async () => { const testDocumentContents = ` -- doc: The docs - job: - Missing input: data/input.txt - outputs: - My output: out/output.txt`; + - doc: The docs + job: + My fake number: 5 + Unknown input: data/input.txt + `; const diagnostics = await validate(testDocumentContents); expect(diagnostics.length).toBe(1); - expect(diagnostics[0].message).toBe('Input "Missing input" is not defined in the associated workflow.'); + expect(diagnostics[0].message).toBe('Input "Unknown input" is not defined in the associated workflow.'); expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error); }); + it("should error when an input has an invalid type (inline)", async () => { + const testDocumentContents = ` + - doc: The docs + job: + My fake number: 5 + My fake string: 5 + `; + + const diagnostics = await validate(testDocumentContents); + + expect(diagnostics.length).toBe(1); + expect(diagnostics[0].message).toBe( + 'Input "My fake string" has an invalid type. Expected "string" but found "number".' + ); + expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error); + }); + + it("should error when an input has an invalid type", async () => { + const testDocumentContents = ` + - doc: The docs + job: + My fake number: 5 + My fake string: + class: string + `; + + const diagnostics = await validate(testDocumentContents); + + expect(diagnostics.length).toBe(1); + expect(diagnostics[0].message).toBe( + 'Input "My fake string" has an invalid type. Expected "string" but found "object".' + ); + expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error); + }); + + it("should error when an input is required but not defined", async () => { + const testDocumentContents = ` + - doc: The docs + job: + `; + + const diagnostics = await validate(testDocumentContents); + + expect(diagnostics.length).toBe(1); + expect(diagnostics[0].message).toBe('Input "My fake number" is required but no value or default was provided.'); + expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error); + }); + + it("should error when an input is required in multiple tests but not defined in one of them", async () => { + const testDocumentContents = ` + - doc: The docs + job: + My fake number: 5 + - doc: The docs + job: + `; + + const diagnostics = await validate(testDocumentContents); + + expect(diagnostics.length).toBe(1); + expect(diagnostics[0].message).toBe('Input "My fake number" is required but no value or default was provided.'); + expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error); + }); + }); + + describe("WorkflowOutputsValidationRule", () => { + beforeAll(() => { + rule = new WorkflowOutputsValidationRule(); + }); + + it("should pass validation when a valid outputs is defined in the workflow", async () => { + const testDocumentContents = ` + - doc: The docs + outputs: + My output: out/output.txt`; + + const diagnostics = await validate(testDocumentContents); + + expect(diagnostics).not.toBeNull(); + expect(diagnostics.length).toBe(0); + }); + it("should error when an output is not defined in the workflow", async () => { const testDocumentContents = ` - doc: The docs - job: - My fake dataset: data/input.txt outputs: Missing output: out/output.txt`; @@ -80,4 +200,79 @@ describe("Workflow Tests Validation Service", () => { expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error); }); }); + + describe("RequiredArrayPropertyValidationRule", () => { + beforeAll(() => { + rule = new RequiredArrayPropertyValidationRule("doc"); + }); + + it("should pass validation when the required property is defined in all items of an array", async () => { + const testDocumentContents = ` +- doc: The docs1 +- doc: The docs2 +- doc: The docs3 +`; + const diagnostics = await validate(testDocumentContents); + + expect(diagnostics).not.toBeNull(); + expect(diagnostics.length).toBe(0); + }); + + it("should error when a required property is missing", async () => { + const testDocumentContents = `- job:`; + + const diagnostics = await validate(testDocumentContents); + + expect(diagnostics.length).toBe(1); + expect(diagnostics[0].message).toBe('Missing required property "doc".'); + }); + + it("should error when a required property is empty", async () => { + const testDocumentContents = `- doc:`; + + const diagnostics = await validate(testDocumentContents); + + expect(diagnostics.length).toBe(1); + expect(diagnostics[0].message).toBe('Missing required value in property "doc".'); + }); + + it("should error when a required property is missing in some items of an array", async () => { + const testDocumentContents = ` +- doc: The docs1 +- job: +- doc: The docs3 +`; + const diagnostics = await validate(testDocumentContents); + + expect(diagnostics.length).toBe(1); + expect(diagnostics[0].message).toBe('Missing required property "doc".'); + }); + + it("should error when a required property is missing or empty in some items of an array", async () => { + const testDocumentContents = ` +- doc: The docs1 +- doc: +- job: +`; + const diagnostics = await validate(testDocumentContents); + + expect(diagnostics.length).toBe(2); + expect(diagnostics[0].message).toBe('Missing required value in property "doc".'); + expect(diagnostics[1].message).toBe('Missing required property "doc".'); + }); + + it("should error when a required property is missing in all items of an array", async () => { + const testDocumentContents = ` +- job: +- job: +- job: +`; + const diagnostics = await validate(testDocumentContents); + + expect(diagnostics.length).toBe(3); + for (const diagnostic of diagnostics) { + expect(diagnostic.message).toBe('Missing required property "doc".'); + } + }); + }); }); diff --git a/server/packages/workflow-tests-language-service/tsconfig.json b/server/packages/workflow-tests-language-service/tsconfig.json index fe8c62d..ae51428 100644 --- a/server/packages/workflow-tests-language-service/tsconfig.json +++ b/server/packages/workflow-tests-language-service/tsconfig.json @@ -12,7 +12,6 @@ "sourceMap": true, "strict": true, "composite": true, - "rootDirs": ["src", "tests"], "rootDir": "../../../" }, "include": ["src/**/*", "../../../workflow-languages/schemas/*.json"], diff --git a/server/packages/yaml-language-service/src/services/yamlValidation.ts b/server/packages/yaml-language-service/src/services/yamlValidation.ts index 57cec03..a74935d 100644 --- a/server/packages/yaml-language-service/src/services/yamlValidation.ts +++ b/server/packages/yaml-language-service/src/services/yamlValidation.ts @@ -20,8 +20,6 @@ export class YAMLValidation { return Promise.resolve([]); } const diagnostics: Diagnostic[] = [...yamlDocument.syntaxDiagnostics]; - // TODO: add schema validation diagnostics - return Promise.resolve(diagnostics); } } diff --git a/shared/src/requestsDefinitions.ts b/shared/src/requestsDefinitions.ts index 41ff0be..633b830 100644 --- a/shared/src/requestsDefinitions.ts +++ b/shared/src/requestsDefinitions.ts @@ -28,25 +28,40 @@ export interface TargetWorkflowDocumentParams { uri: string; } -export type WorkflowDataType = - | "color" //TODO: this type seems to be missing in format2 schema - | "null" - | "boolean" - | "int" - | "long" - | "float" - | "double" - | "string" - | "integer" - | "text" - | "File" - | "data" - | "collection"; +/** + * This contains all the supported data types for workflow inputs. + * + * **Important**: This definition must be kept in sync with the schema definition. + * + * Note: Is defined as a const object to be used as a map and to be able to use the keys as a union type. + * This way we can maintain a single source of truth for the supported data types and generate WorkflowDataType + * type dynamically from it. + */ +export const workflowDataTypes = { + boolean: true, + collection: true, + color: true, //TODO: this type seems to be missing in format2 schema + data: true, + double: true, + File: true, + float: true, + int: true, + integer: true, + long: true, + null: true, + string: true, + text: true, +} as const; + +// Extract the keys of the object to form the union type +export type WorkflowDataType = keyof typeof workflowDataTypes; export interface WorkflowInput { name: string; type: WorkflowDataType; doc: string; + default?: unknown; + optional?: boolean; } export interface GetWorkflowInputsResult { diff --git a/test-data/json/validation/test_wf_05.ga b/test-data/json/validation/test_wf_05.ga index 3bf04dd..8374f75 100644 --- a/test-data/json/validation/test_wf_05.ga +++ b/test-data/json/validation/test_wf_05.ga @@ -106,7 +106,7 @@ "top": 374 }, "tool_id": null, - "tool_state": "{\"parameter_type\": \"integer\", \"optional\": false}", + "tool_state": "{\"default\": 10, \"parameter_type\": \"integer\", \"optional\": true}", "tool_version": null, "type": "parameter_input", "uuid": "847d6938-c7aa-41fa-a139-611f4aaa900d",