feat: port additional checks (#567)

Closes partially #543

### Summary of Changes

Port additional checks from old Xtext version:
* Template string without expression between string parts
* Yield in pipeline
* Missing type hints
* Missing package in module with declarations
* Declarations in module that are not allowed by the file type

---------

Co-authored-by: megalinter-bot <[email protected]>

src/language/validation/names.ts

@@ -1,8 +1,8 @@
 import { SdsDeclaration } from '../generated/ast.js';
 import { ValidationAcceptor } from 'langium';
 
-export const CODE_NAMES_BLOCK_LAMBDA_PREFIX = 'names/block-lambda-prefix';
-export const CODE_NAMES_CASING = 'names/casing';
+export const CODE_NAME_BLOCK_LAMBDA_PREFIX = 'name/block-lambda-prefix';
+export const CODE_NAME_CASING = 'name/casing';
 
 export const nameMustNotStartWithBlockLambdaPrefix = (node: SdsDeclaration, accept: ValidationAcceptor) => {
     const name = node.name ?? '';
@@ -14,7 +14,7 @@ export const nameMustNotStartWithBlockLambdaPrefix = (node: SdsDeclaration, acce
             {
                 node,
                 property: 'name',
-                code: CODE_NAMES_BLOCK_LAMBDA_PREFIX,
+                code: CODE_NAME_BLOCK_LAMBDA_PREFIX,
             },
         );
     }
@@ -43,7 +43,7 @@ export const nameShouldHaveCorrectCasing = (node: SdsDeclaration, accept: Valida
                 accept('warning', 'All segments of the qualified name of a package should be lowerCamelCase.', {
                     node,
                     property: 'name',
-                    code: CODE_NAMES_CASING,
+                    code: CODE_NAME_CASING,
                 });
             }
             return;
@@ -95,6 +95,6 @@ const acceptCasingWarning = (
     accept('warning', `Names of ${nodeName} should be ${expectedCasing}.`, {
         node,
         property: 'name',
-        code: CODE_NAMES_CASING,
+        code: CODE_NAME_CASING,
     });
 };

src/language/validation/other/expressions/templateStrings.ts

@@ -0,0 +1,21 @@
+import { isSdsTemplateStringPart, SdsTemplateString } from '../../../generated/ast.js';
+import { ValidationAcceptor } from 'langium';
+
+export const CODE_TEMPLATE_STRING_MISSING_TEMPLATE_EXPRESSION = 'template-string/missing-template-expression';
+
+export const templateStringMustHaveExpressionBetweenTwoStringParts = (
+    node: SdsTemplateString,
+    accept: ValidationAcceptor,
+): void => {
+    for (let i = 0; i < node.expressions.length - 1; i++) {
+        const first = node.expressions[i];
+        const second = node.expressions[i + 1];
+
+        if (isSdsTemplateStringPart(first) && isSdsTemplateStringPart(second)) {
+            accept('error', 'There must be an expression between two string parts.', {
+                node: second,
+                code: CODE_TEMPLATE_STRING_MISSING_TEMPLATE_EXPRESSION,
+            });
+        }
+    }
+};

src/language/validation/other/modules.ts

@@ -0,0 +1,48 @@
+import { ValidationAcceptor } from 'langium';
+import { isSdsDeclaration, isSdsPipeline, isSdsSegment, SdsModule } from '../../generated/ast.js';
+import { isInPipelineFile, isInStubFile } from '../../constants/fileExtensions.js';
+
+export const CODE_MODULE_MISSING_PACKAGE = 'module/missing-package';
+
+export const CODE_MODULE_FORBIDDEN_IN_PIPELINE_FILE = 'module/forbidden-in-pipeline-file';
+
+export const CODE_MODULE_FORBIDDEN_IN_STUB_FILE = 'module/forbidden-in-stub-file';
+
+export const moduleWithDeclarationsMustStatePackage = (node: SdsModule, accept: ValidationAcceptor): void => {
+    if (!node.name) {
+        const declarations = node.members.filter(isSdsDeclaration);
+        if (declarations.length > 0) {
+            accept('error', 'A module with declarations must state its package.', {
+                node: declarations[0],
+                property: 'name',
+                code: CODE_MODULE_MISSING_PACKAGE,
+            });
+        }
+    }
+};
+
+export const moduleDeclarationsMustMatchFileKind = (node: SdsModule, accept: ValidationAcceptor): void => {
+    const declarations = node.members.filter(isSdsDeclaration);
+
+    if (isInPipelineFile(node)) {
+        for (const declaration of declarations) {
+            if (!isSdsPipeline(declaration) && !isSdsSegment(declaration)) {
+                accept('error', 'A pipeline file must only declare pipelines and segments.', {
+                    node: declaration,
+                    property: 'name',
+                    code: CODE_MODULE_FORBIDDEN_IN_PIPELINE_FILE,
+                });
+            }
+        }
+    } else if (isInStubFile(node)) {
+        for (const declaration of declarations) {
+            if (isSdsPipeline(declaration) || isSdsSegment(declaration)) {
+                accept('error', 'A stub file must not declare pipelines or segments.', {
+                    node: declaration,
+                    property: 'name',
+                    code: CODE_MODULE_FORBIDDEN_IN_STUB_FILE,
+                });
+            }
+        }
+    }
+};

src/language/validation/other/statements/assignments.ts

@@ -0,0 +1,15 @@
+import { isSdsPipeline, SdsYield } from '../../../generated/ast.js';
+import { getContainerOfType, ValidationAcceptor } from 'langium';
+
+export const CODE_ASSIGMENT_YIELD_FORBIDDEN_IN_PIPELINE = 'assignment/yield-forbidden-in-pipeline';
+
+export const yieldMustNotBeUsedInPipeline = (node: SdsYield, accept: ValidationAcceptor): void => {
+    const containingPipeline = getContainerOfType(node, isSdsPipeline);
+
+    if (containingPipeline) {
+        accept('error', 'Yield must not be used in a pipeline.', {
+            node,
+            code: CODE_ASSIGMENT_YIELD_FORBIDDEN_IN_PIPELINE,
+        });
+    }
+};

src/language/validation/safe-ds-validator.ts

@@ -14,7 +14,11 @@ import {
     functionTypeParameterListShouldNotBeEmpty,
     segmentResultListShouldNotBeEmpty,
     unionTypeShouldNotHaveASingularTypeArgument,
-} from './unnecessarySyntax.js';
+} from './style.js';
+import { templateStringMustHaveExpressionBetweenTwoStringParts } from './other/expressions/templateStrings.js';
+import { yieldMustNotBeUsedInPipeline } from './other/statements/assignments.js';
+import { attributeMustHaveTypeHint, parameterMustHaveTypeHint, resultMustHaveTypeHint } from './types.js';
+import { moduleDeclarationsMustMatchFileKind, moduleWithDeclarationsMustStatePackage } from './other/modules.js';
 
 /**
  * Register custom validation checks.
@@ -25,13 +29,19 @@ export const registerValidationChecks = function (services: SafeDsServices) {
     const checks: ValidationChecks<SafeDsAstType> = {
         SdsAssignment: [assignmentShouldHaveMoreThanWildcardsAsAssignees],
         SdsAnnotation: [annotationParameterListShouldNotBeEmpty],
+        SdsAttribute: [attributeMustHaveTypeHint],
         SdsClass: [classBodyShouldNotBeEmpty, classTypeParameterListShouldNotBeEmpty],
         SdsDeclaration: [nameMustNotStartWithBlockLambdaPrefix, nameShouldHaveCorrectCasing],
         SdsEnum: [enumBodyShouldNotBeEmpty],
         SdsEnumVariant: [enumVariantParameterListShouldNotBeEmpty, enumVariantTypeParameterListShouldNotBeEmpty],
         SdsFunction: [functionResultListShouldNotBeEmpty, functionTypeParameterListShouldNotBeEmpty],
+        SdsModule: [moduleDeclarationsMustMatchFileKind, moduleWithDeclarationsMustStatePackage],
+        SdsParameter: [parameterMustHaveTypeHint],
+        SdsResult: [resultMustHaveTypeHint],
         SdsSegment: [segmentResultListShouldNotBeEmpty],
+        SdsTemplateString: [templateStringMustHaveExpressionBetweenTwoStringParts],
         SdsUnionType: [unionTypeShouldNotHaveASingularTypeArgument],
+        SdsYield: [yieldMustNotBeUsedInPipeline],
     };
     registry.register(checks, validator);
 };

src/language/validation/types.ts

@@ -0,0 +1,42 @@
+import { getContainerOfType, ValidationAcceptor } from 'langium';
+import { isSdsCallable, isSdsLambda, SdsAttribute, SdsParameter, SdsResult } from '../generated/ast.js';
+
+export const CODE_TYPE_MISSING_TYPE_HINT = 'type/missing-type-hint';
+
+// -----------------------------------------------------------------------------
+// Missing type hints
+// -----------------------------------------------------------------------------
+
+export const attributeMustHaveTypeHint = (node: SdsAttribute, accept: ValidationAcceptor): void => {
+    if (!node.type) {
+        accept('error', 'An attribute must have a type hint.', {
+            node,
+            property: 'name',
+            code: CODE_TYPE_MISSING_TYPE_HINT,
+        });
+    }
+};
+
+export const parameterMustHaveTypeHint = (node: SdsParameter, accept: ValidationAcceptor): void => {
+    if (!node.type) {
+        const containingCallable = getContainerOfType(node, isSdsCallable);
+
+        if (!isSdsLambda(containingCallable)) {
+            accept('error', 'A parameter must have a type hint.', {
+                node,
+                property: 'name',
+                code: CODE_TYPE_MISSING_TYPE_HINT,
+            });
+        }
+    }
+};
+
+export const resultMustHaveTypeHint = (node: SdsResult, accept: ValidationAcceptor): void => {
+    if (!node.type) {
+        accept('error', 'A result must have a type hint.', {
+            node,
+            property: 'name',
+            code: CODE_TYPE_MISSING_TYPE_HINT,
+        });
+    }
+};

tests/resources/validation/other/expressions/template strings/missing template expression/main.sdstest

@@ -0,0 +1,11 @@
+package tests.validation.other.expressions.templateStrings.missingTemplateExpression
+
+pipeline test {
+    // $TEST$ error "There must be an expression between two string parts."
+    // $TEST$ error "There must be an expression between two string parts."
+    "start {{ »}} inner {{« »}} end"«;
+
+    // $TEST$ no error "There must be an expression between two string parts."
+    // $TEST$ no error "There must be an expression between two string parts."
+    "start {{ 1 »}} inner {{« 1 »}} end"«;
+}

tests/resources/validation/other/modules/declarations in pipeline files/main.sdspipe

@@ -0,0 +1,33 @@
+package tests.validation.other.modules.declarationsInPipelineFiles
+
+// $TEST$ error "A pipeline file must only declare pipelines and segments."
+annotation »MyAnnotation«
+// $TEST$ error "A pipeline file must only declare pipelines and segments."
+class »MyClass« {
+
+    // $TEST$ no error "A pipeline file must only declare pipelines and segments."
+    attr »a«: Int
+
+    // $TEST$ no error "A pipeline file must only declare pipelines and segments."
+    class »MyNestedClass«
+
+    // $TEST$ no error "A pipeline file must only declare pipelines and segments."
+    enum »MyEnum«
+
+    // $TEST$ no error "A pipeline file must only declare pipelines and segments."
+    fun »myFunction«()
+}
+// $TEST$ error "A pipeline file must only declare pipelines and segments."
+enum »MyEnum« {
+    // $TEST$ no error "A pipeline file must only declare pipelines and segments."
+    »MyEnumInstance«
+}
+// $TEST$ error "A pipeline file must only declare pipelines and segments."
+fun »myFunction«()
+// $TEST$ error "A pipeline file must only declare pipelines and segments."
+schema »MySchema« {}
+
+// $TEST$ no error "A pipeline file must only declare pipelines and segments."
+pipeline »myPipeline« {}
+// $TEST$ no error "A pipeline file must only declare pipelines and segments."
+segment »mySegment«() {}

tests/resources/validation/other/modules/declarations in stub files/main.sdsstub

@@ -0,0 +1,33 @@
+package tests.validation.other.modules.declarationsInStubFiles
+
+// $TEST$ error "A stub file must not declare pipelines or segments."
+pipeline »myPipeline« {}
+// $TEST$ error "A stub file must not declare pipelines or segments."
+segment »mySegment«() {}
+
+// $TEST$ no error "A pipeline file must only declare pipelines and segments."
+annotation »MyAnnotation«
+// $TEST$ no error "A pipeline file must only declare pipelines and segments."
+class »MyClass« {
+
+    // $TEST$ no error "A pipeline file must only declare pipelines and segments."
+    attr »a«: Int
+
+    // $TEST$ no error "A pipeline file must only declare pipelines and segments."
+    class »MyNestedClass«
+
+    // $TEST$ no error "A pipeline file must only declare pipelines and segments."
+    enum »MyEnum«
+
+    // $TEST$ no error "A pipeline file must only declare pipelines and segments."
+    fun »myFunction«()
+}
+// $TEST$ no error "A pipeline file must only declare pipelines and segments."
+enum »MyEnum« {
+    // $TEST$ no error "A pipeline file must only declare pipelines and segments."
+    »MyEnumInstance«
+}
+// $TEST$ no error "A pipeline file must only declare pipelines and segments."
+fun »myFunction«()
+// $TEST$ no error "A pipeline file must only declare pipelines and segments."
+schema »MySchema« {}

tests/resources/validation/other/modules/declarations in test files/main.sdsstub

@@ -0,0 +1,33 @@
+package tests.validation.other.modules.declarationsInTestFiles
+
+// $TEST$ no error "A pipeline file must only declare pipelines and segments."
+annotation »MyAnnotation«
+// $TEST$ no error "A pipeline file must only declare pipelines and segments."
+class »MyClass« {
+
+    // $TEST$ no error "A pipeline file must only declare pipelines and segments."
+    attr »a«: Int
+
+    // $TEST$ no error "A pipeline file must only declare pipelines and segments."
+    class »MyNestedClass«
+
+    // $TEST$ no error "A pipeline file must only declare pipelines and segments."
+    enum »MyEnum«
+
+    // $TEST$ no error "A pipeline file must only declare pipelines and segments."
+    fun »myFunction«()
+}
+// $TEST$ no error "A pipeline file must only declare pipelines and segments."
+enum »MyEnum« {
+    // $TEST$ no error "A pipeline file must only declare pipelines and segments."
+    »MyEnumInstance«
+}
+// $TEST$ no error "A pipeline file must only declare pipelines and segments."
+fun »myFunction«()
+// $TEST$ no error "A pipeline file must only declare pipelines and segments."
+schema »MySchema« {}
+
+// $TEST$ no error "A pipeline file must only declare pipelines and segments."
+pipeline »myPipeline« {}
+// $TEST$ no error "A pipeline file must only declare pipelines and segments."
+segment »mySegment«() {}

tests/resources/validation/other/modules/must state package/pipeline file (empty).sdspipe

@@ -0,0 +1 @@
+// $TEST$ no error "A module with declarations must state its package."

tests/resources/validation/other/modules/must state package/pipeline file (only annotations).sdspipe

@@ -0,0 +1,3 @@
+// $TEST$ no error "A module with declarations must state its package."
+
+@AnnotationCall

tests/resources/validation/other/modules/must state package/pipeline file (only imports).sdspipe

@@ -0,0 +1,3 @@
+// $TEST$ no error "A module with declarations must state its package."
+
+import myPackage

tests/resources/validation/other/modules/must state package/pipeline file (with declarations and package).sdspipe

@@ -0,0 +1,7 @@
+package tests.validation.other.modules.mustStatePackage
+
+// $TEST$ no error "A module with declarations must state its package."
+segment »s«() {}
+
+// $TEST$ no error "A module with declarations must state its package."
+segment »t«() {}

tests/resources/validation/other/modules/must state package/pipeline file (with declarations but no package).sdspipe

@@ -0,0 +1,5 @@
+// $TEST$ error "A module with declarations must state its package."
+segment »s«() {}
+
+// $TEST$ no error "A module with declarations must state its package."
+segment »t«() {}

tests/resources/validation/other/modules/must state package/stub file (empty).sdsstub

@@ -0,0 +1 @@
+// $TEST$ no error "A module with declarations must state its package."

tests/resources/validation/other/modules/must state package/stub file (only annotations).sdsstub

@@ -0,0 +1,3 @@
+// $TEST$ no error "A module with declarations must state its package."
+
+@AnnotationCall

tests/resources/validation/other/modules/must state package/stub file (only imports).sdsstub

@@ -0,0 +1,3 @@
+// $TEST$ no error "A module with declarations must state its package."
+
+import myPackage

tests/resources/validation/other/modules/must state package/stub file (with declarations and package).sdsstub

@@ -0,0 +1,7 @@
+package tests.validation.other.modules.mustStatePackage
+
+// $TEST$ no error "A module with declarations must state its package."
+class »C«
+
+// $TEST$ no error "A module with declarations must state its package."
+class »D«

tests/resources/validation/other/modules/must state package/stub file (with declarations but no package).sdsstub

@@ -0,0 +1,5 @@
+// $TEST$ error "A module with declarations must state its package."
+class »C«
+
+// $TEST$ no error "A module with declarations must state its package."
+class »D«

tests/resources/validation/other/statements/assignments/yield forbidden in pipeline/main.sdstest

@@ -0,0 +1,16 @@
+package tests.validation.other.statements.assignments.yieldForbiddenInPipeline
+
+segment s() {
+	// $TEST$ no error "Yield must not be used in a pipeline."
+    »yield a« = 1;
+}
+
+pipeline p {
+	// $TEST$ error "Yield must not be used in a pipeline."
+    »yield a« = 1;
+
+    () {
+        // $TEST$ no error "Yield must not be used in a pipeline."
+        »yield a« = 1;
+    };
+}

tests/resources/validation/unnecessary syntax/unnecessary assignment/main.sdstest → tests/resources/validation/style/unnecessary assignment/main.sdstest

@@ -1,4 +1,4 @@
-package tests.validation.unnecessarySyntax.unnecessaryAssignment
+package tests.validation.style.unnecessaryAssignment
 
 fun f() -> (a: Int, b: Int)
 

tests/resources/validation/unnecessary syntax/unnecessary body in class/main.sdstest → tests/resources/validation/style/unnecessary body in class/main.sdstest

@@ -1,4 +1,4 @@
-package tests.validation.unnecessarySyntax.unnecessaryBodyInClass
+package tests.validation.style.unnecessaryBodyInClass
 
 // $TEST$ info "This body can be removed."
 class MyClass1 »{}«

tests/resources/validation/unnecessary syntax/unnecessary body in enum/main.sdstest → tests/resources/validation/style/unnecessary body in enum/main.sdstest

@@ -1,4 +1,4 @@
-package tests.validation.unnecessarySyntax.unnecessaryBodyInEnum
+package tests.validation.style.unnecessaryBodyInEnum
 
 // $TEST$ info "This body can be removed."
 enum MyEnum1 »{}«