Merge pull request #71 from davelopez/improve_validation_profiles

Improve validation profiles

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,
         },

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;
+  }
 }

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<Diagnostic[]> {
-    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[] {

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<ValidationRule> {
+    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<ValidationRule> {
+    return GxFormat2IWCValidationProfile.RULES;
+  }
+}

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);
   }
 

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 };

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;

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);

server/gx-workflow-ls-format2/src/services/validation/schemaValidationService.ts → server/gx-workflow-ls-format2/src/services/schemaValidationService.ts

@@ -1,14 +1,14 @@
 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,
   Range,
   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) {