Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve validation profiles #71

Merged
merged 43 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
068d354
Refactor move validation rules to own package
davelopez Jun 15, 2024
bfeff48
Set default severity for MissingPropertyValidationRule
davelopez Jun 15, 2024
24962e3
Add MissingPropertyValidationRule tests
davelopez Jun 15, 2024
8b2966a
add more missing properties rules to IWCValidationProfile
davelopez Jun 15, 2024
e0bfe10
Increase test coverage for MissingPropertyValidationRule
davelopez Jun 15, 2024
23fe709
Add isNodeEmpty method to ASTNodeManager class
davelopez Jun 15, 2024
0380ac8
Add valueRequired parameter to MissingPropertyValidationRule
davelopez Jun 15, 2024
fe09ee2
Refactor MissingPropertyValidationRule to accept custom error messages
davelopez Jun 15, 2024
ff9ff91
Refactor and improve validation rules package
davelopez Jun 15, 2024
3e03dd9
Display profile name on diagnostics + refactoring
davelopez Jun 15, 2024
9a9720c
Refactor move WorkflowOutputLabelValidationRule to native server
davelopez Jun 15, 2024
deac9f1
Add missing property validation rule for documentation and annotation
davelopez Jun 15, 2024
c3ef32c
Refactor schema validation sources to include source information
davelopez Jun 16, 2024
e919067
Refactor and improve validation rules package
davelopez Jun 16, 2024
40bb41a
Convert workflow tests documents semantic validation to rules
davelopez Jun 16, 2024
b0252aa
Setup basic validation profiles for all language services
davelopez Jun 16, 2024
4860bc5
Refactor validation rule tests for wf test documents
davelopez Jun 16, 2024
5b2e93b
Refactor tests
davelopez Jun 16, 2024
9b65f9f
Adapt e2e tests
davelopez Jun 16, 2024
0730fd7
Refactor rename validation rule from missing to required
davelopez Jun 16, 2024
c21813b
Consider null value as empty value
davelopez Jun 18, 2024
53d1f46
Add RequiredArrayPropertyValidationRule + tests
davelopez Jun 18, 2024
6581ae4
Refactor parsing of format2 workflow inputs and outputs
davelopez Jun 19, 2024
24c494b
Fix completionService to suggest available properties for 'report'
davelopez Jun 19, 2024
7fcc8f7
Improve schema validation for format2 workflows
davelopez Jun 19, 2024
2b65796
Add some tests for format2 validation
davelopez Jun 19, 2024
269feed
Add InputTypeValidationRule
davelopez Jun 19, 2024
bb57bb7
Add InputTypeValidationRule to IWC profile for gxformat2
davelopez Jun 19, 2024
3f63367
Add tests for InputTypeValidationRule
davelopez Jun 19, 2024
de29176
Refactor use getPropertyNodeByName
davelopez Jun 19, 2024
cccdddd
Update WorkflowInput definition to include default value + tests
davelopez Jun 19, 2024
e598454
Update WorkflowInput definition to include optional flag + tests
davelopez Jun 19, 2024
a2e9b59
Refactor extract utility function for type checking
davelopez Jun 19, 2024
023917b
Update test file wf to include default value and optional flag
davelopez Jun 19, 2024
0299815
Update isCompatibleType to use WorkflowDataType for expectedType para…
davelopez Jun 20, 2024
1674bf4
Add check for invalid type in WorkflowInputsValidationRule
davelopez Jun 20, 2024
d442ffc
Increase test coverage for input validation
davelopez Jun 20, 2024
5ae87d2
Fix type validation for workflow inputs
davelopez Jun 20, 2024
d117fb8
Refactor InputTypeValidationRule to skip type validation for invalid …
davelopez Jun 20, 2024
6ab97c0
Fix getAllPropertyNodesByName
davelopez Jun 20, 2024
815b7d9
Validate required inputs in tests documents + tests
davelopez Jun 20, 2024
f75858d
Refactor validation profiles to include descriptive names
davelopez Jun 20, 2024
f4841ba
Refactor hoverService to use getNodeRange from nodeManager
davelopez Jun 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion client/tests/e2e/suite/extension.ga.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
99 changes: 57 additions & 42 deletions server/gx-workflow-ls-format2/src/gxFormat2WorkflowDocument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {
TextDocument,
WorkflowDataType,
WorkflowDocument,
WorkflowInput,
WorkflowOutput,
} from "@gxwf/server-common/src/languageTypes";
import { YAMLDocument } from "@gxwf/yaml-language-service/src";

Expand All @@ -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 {
Expand All @@ -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;
}
}
32 changes: 19 additions & 13 deletions server/gx-workflow-ls-format2/src/languageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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,
Expand All @@ -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 {
Expand All @@ -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[] {
Expand Down
46 changes: 46 additions & 0 deletions server/gx-workflow-ls-format2/src/profiles.ts
Original file line number Diff line number Diff line change
@@ -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;
}
}
12 changes: 8 additions & 4 deletions server/gx-workflow-ls-format2/src/schema/definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}

Expand Down
4 changes: 2 additions & 2 deletions server/gx-workflow-ls-format2/src/schema/index.ts
Original file line number Diff line number Diff line change
@@ -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 };
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -104,6 +104,8 @@ export class GxFormat2CompletionService {
};
result.push(item);
});
} else if (schemaRecord instanceof RecordSchemaNode) {
return this.getProposedItems(schemaRecord, textBuffer, exclude, offset);
}
}
return result;
Expand Down
6 changes: 1 addition & 5 deletions server/gx-workflow-ls-format2/src/services/hoverService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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) {}
Expand Down Expand Up @@ -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":
Expand All @@ -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,
Expand All @@ -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) {
Expand Down
Loading