diff --git a/client/tests/e2e/suite/extension.ga.e2e.ts b/client/tests/e2e/suite/extension.ga.e2e.ts index d5bafb9..20d8c26 100644 --- a/client/tests/e2e/suite/extension.ga.e2e.ts +++ b/client/tests/e2e/suite/extension.ga.e2e.ts @@ -59,12 +59,12 @@ suite("Native (JSON) Workflows", () => { { 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, + severity: vscode.DiagnosticSeverity.Warning, }, { 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, + severity: vscode.DiagnosticSeverity.Warning, }, { message: "Missing label in workflow output.", diff --git a/server/gx-workflow-ls-format2/src/profiles.ts b/server/gx-workflow-ls-format2/src/profiles.ts index 65ee38c..5e621c4 100644 --- a/server/gx-workflow-ls-format2/src/profiles.ts +++ b/server/gx-workflow-ls-format2/src/profiles.ts @@ -3,7 +3,10 @@ import { BasicCommonValidationProfile, IWCCommonValidationProfile, } from "@gxwf/server-common/src/providers/validation/profiles"; -import { RequiredPropertyValidationRule } from "@gxwf/server-common/src/providers/validation/rules"; +import { + ChildrenRequiredPropertyValidationRule, + RequiredPropertyValidationRule, +} from "@gxwf/server-common/src/providers/validation/rules"; import { InputTypeValidationRule } from "./validation/rules/InputTypeValidationRule"; /** @@ -33,10 +36,18 @@ export class GxFormat2IWCValidationProfile extends IWCCommonValidationProfile { new RequiredPropertyValidationRule( "doc", true, - DiagnosticSeverity.Error, + DiagnosticSeverity.Warning, "The workflow is not documented. Documenting workflows helps users understand the purpose of the workflow." ), new InputTypeValidationRule(DiagnosticSeverity.Error), + new ChildrenRequiredPropertyValidationRule( + "steps", + "doc", + true, + DiagnosticSeverity.Warning, + "The step is not documented. Documenting steps helps users understand the purpose of the step." + ), + // Add more custom rules specific to gxformat2 workflows here... ]); diff --git a/server/gx-workflow-ls-format2/tests/integration/validation.test.ts b/server/gx-workflow-ls-format2/tests/integration/validation.test.ts index 8ff41c5..7cba2cd 100644 --- a/server/gx-workflow-ls-format2/tests/integration/validation.test.ts +++ b/server/gx-workflow-ls-format2/tests/integration/validation.test.ts @@ -1,5 +1,8 @@ import { Diagnostic, ValidationRule } from "@gxwf/server-common/src/languageTypes"; -import { StepExportErrorValidationRule } from "@gxwf/server-common/src/providers/validation/rules"; +import { + ChildrenRequiredPropertyValidationRule, + StepExportErrorValidationRule, +} from "@gxwf/server-common/src/providers/validation/rules"; import { GalaxyWorkflowFormat2SchemaLoader } from "../../src/schema"; import { GxFormat2SchemaValidationService } from "../../src/services/schemaValidationService"; import { InputTypeValidationRule } from "../../src/validation/rules/InputTypeValidationRule"; @@ -274,5 +277,68 @@ steps: expect(diagnostics).toHaveLength(0); }); }); + + describe("ChildrenRequiredPropertyValidationRule", () => { + beforeAll(() => { + rule = new ChildrenRequiredPropertyValidationRule("steps", "doc"); + }); + + it("should report error when step is missing required property", async () => { + const content = ` +class: GalaxyWorkflow +steps: + step: + tool_id: tool_id + `; + const diagnostics = await validateRule(content); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0].message).toBe('Missing required property "doc".'); + }); + + it("should not report error when all steps have required property", async () => { + const content = ` +class: GalaxyWorkflow +steps: + step: + tool_id: tool_id + doc: step doc + step2: + tool_id: tool_id + doc: step2 doc + `; + const diagnostics = await validateRule(content); + expect(diagnostics).toHaveLength(0); + }); + + it("should report error when step has empty required property", async () => { + const content = ` +class: GalaxyWorkflow +steps: + step: + tool_id: tool_id + doc: + `; + const diagnostics = await validateRule(content); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0].message).toBe('Missing required value in property "doc".'); + }); + + it("should not report error when there are no steps", async () => { + const content = ` +class: GalaxyWorkflow +steps: + `; + const diagnostics = await validateRule(content); + expect(diagnostics).toHaveLength(0); + }); + + it("should not report error when the steps property does not exist", async () => { + const content = ` +class: GalaxyWorkflow + `; + const diagnostics = await validateRule(content); + expect(diagnostics).toHaveLength(0); + }); + }); }); }); diff --git a/server/gx-workflow-ls-native/src/profiles.ts b/server/gx-workflow-ls-native/src/profiles.ts index 1a44e6e..50f0a52 100644 --- a/server/gx-workflow-ls-native/src/profiles.ts +++ b/server/gx-workflow-ls-native/src/profiles.ts @@ -33,7 +33,7 @@ export class NativeIWCValidationProfile extends IWCCommonValidationProfile { new RequiredPropertyValidationRule( "annotation", true, - DiagnosticSeverity.Error, + DiagnosticSeverity.Warning, "The workflow is not annotated. Annotating workflows helps users understand the purpose of the workflow." ), new WorkflowOutputLabelValidationRule(DiagnosticSeverity.Error), diff --git a/server/packages/server-common/src/providers/validation/profiles.ts b/server/packages/server-common/src/providers/validation/profiles.ts index 53046e4..5e3cc4b 100644 --- a/server/packages/server-common/src/providers/validation/profiles.ts +++ b/server/packages/server-common/src/providers/validation/profiles.ts @@ -47,13 +47,13 @@ export class IWCCommonValidationProfile implements ValidationProfile { new RequiredPropertyValidationRule( "creator", true, - DiagnosticSeverity.Error, + DiagnosticSeverity.Warning, "The workflow does not specify a creator." ), new RequiredPropertyValidationRule( "license", true, - DiagnosticSeverity.Error, + DiagnosticSeverity.Warning, "The workflow does not specify a license." ), // Add more common rules here... diff --git a/server/packages/server-common/src/providers/validation/rules/ChildrenRequiredPropertyValidationRule.ts b/server/packages/server-common/src/providers/validation/rules/ChildrenRequiredPropertyValidationRule.ts new file mode 100644 index 0000000..96f5787 --- /dev/null +++ b/server/packages/server-common/src/providers/validation/rules/ChildrenRequiredPropertyValidationRule.ts @@ -0,0 +1,53 @@ +import { DocumentContext, ValidationRule } from "@gxwf/server-common/src/languageTypes"; +import { Diagnostic, DiagnosticSeverity } from "vscode-languageserver-types"; + +/** + * Validation rule to check that all children of a node have a required property. + */ +export class ChildrenRequiredPropertyValidationRule implements ValidationRule { + constructor( + private nodePath: string, + private propertyName: 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) { + // The target node does not exist, so we can't check for children. + return Promise.resolve(result); + } + + const children = documentContext.nodeManager.getChildren(targetNode); + for (const child of children) { + if (child.type !== "object") { + continue; + } + for (const childProperty of child.properties) { + const targetChildNode = documentContext.nodeManager.getPropertyNodeByName(childProperty, this.propertyName); + if (!targetChildNode) { + result.push({ + message: this.message ?? `Missing required property "${this.propertyName}".`, + range: documentContext.nodeManager.getNodeRange(childProperty.keyNode), + severity: this.severity, + }); + } + + if (this.valueRequired && targetChildNode) { + const missingValue = documentContext.nodeManager.isNodeEmpty(targetChildNode); + if (missingValue) { + result.push({ + message: `Missing required value in property "${this.propertyName}".`, + range: documentContext.nodeManager.getNodeRange(targetChildNode), + severity: this.severity, + }); + } + } + } + } + return Promise.resolve(result); + } +} diff --git a/server/packages/server-common/src/providers/validation/rules/index.ts b/server/packages/server-common/src/providers/validation/rules/index.ts index 294f5cf..50a0135 100644 --- a/server/packages/server-common/src/providers/validation/rules/index.ts +++ b/server/packages/server-common/src/providers/validation/rules/index.ts @@ -1,9 +1,11 @@ +import { ChildrenRequiredPropertyValidationRule } from "./ChildrenRequiredPropertyValidationRule"; import { RequiredArrayPropertyValidationRule } from "./RequiredArrayPropertyValidationRule"; import { RequiredPropertyValidationRule } from "./RequiredPropertyValidationRule"; import { StepExportErrorValidationRule } from "./StepErrorValidationRule"; import { TestToolshedValidationRule } from "./TestToolShedValidationRule"; export { + ChildrenRequiredPropertyValidationRule, RequiredArrayPropertyValidationRule, RequiredPropertyValidationRule, StepExportErrorValidationRule,