feat: validation for results of segments (#613)

Closes partially #543 ### Summary of Changes Ensure that a segment yields exactly one value for each result. --------- Co-authored-by: megalinter-bot <[email protected]>
Safe-DS · Oct 8, 2023 · bf20c7c · bf20c7c
1 parent 3a2e9cc
commit bf20c7c
Show file tree

Hide file tree

Showing 9 changed files with 97 additions and 25 deletions.
diff --git a/src/language/validation/other/declarations/parameters.ts b/src/language/validation/other/declarations/parameters.ts
@@ -1,9 +1,6 @@
-import { SdsParameter, SdsSegment } from '../../../generated/ast.js';
+import { SdsParameter } from '../../../generated/ast.js';
 import { ValidationAcceptor } from 'langium';
-import { parametersOrEmpty } from '../../../helpers/nodeProperties.js';
-import { SafeDsServices } from '../../../safe-ds-module.js';
 
-export const CODE_PARAMETER_UNUSED = 'parameter/unused';
 export const CODE_PARAMETER_VARIADIC_AND_OPTIONAL = 'parameter/variadic-and-optional';
 
 export const parameterMustNotBeVariadicAndOptional = (node: SdsParameter, accept: ValidationAcceptor) => {
@@ -15,18 +12,3 @@ export const parameterMustNotBeVariadicAndOptional = (node: SdsParameter, accept
         });
     }
 };
-
-export const segmentParameterShouldBeUsed =
-    (services: SafeDsServices) => (node: SdsSegment, accept: ValidationAcceptor) => {
-        for (const parameter of parametersOrEmpty(node)) {
-            const usages = services.helpers.NodeMapper.parameterToReferences(parameter);
-
-            if (usages.isEmpty()) {
-                accept('warning', 'This parameter is unused and can be removed.', {
-                    node: parameter,
-                    property: 'name',
-                    code: CODE_PARAMETER_UNUSED,
-                });
-            }
-        }
-    };
diff --git a/src/language/validation/other/declarations/segments.ts b/src/language/validation/other/declarations/segments.ts
@@ -0,0 +1,48 @@
+import { SdsSegment } from '../../../generated/ast.js';
+import { ValidationAcceptor } from 'langium';
+import { parametersOrEmpty, resultsOrEmpty } from '../../../helpers/nodeProperties.js';
+import { SafeDsServices } from '../../../safe-ds-module.js';
+
+export const CODE_SEGMENT_DUPLICATE_YIELD = 'segment/duplicate-yield';
+export const CODE_SEGMENT_UNASSIGNED_RESULT = 'segment/unassigned-result';
+export const CODE_SEGMENT_UNUSED_PARAMETER = 'segment/unused-parameter';
+
+export const segmentResultMustBeAssignedExactlyOnce =
+    (services: SafeDsServices) => (node: SdsSegment, accept: ValidationAcceptor) => {
+        const results = resultsOrEmpty(node.resultList);
+        for (const result of results) {
+            const yields = services.helpers.NodeMapper.resultToYields(result);
+            if (yields.isEmpty()) {
+                accept('error', 'Nothing is assigned to this result.', {
+                    node: result,
+                    property: 'name',
+                    code: CODE_SEGMENT_UNASSIGNED_RESULT,
+                });
+                continue;
+            }
+
+            const duplicateYields = yields.tail(1);
+            for (const duplicate of duplicateYields) {
+                accept('error', `The result '${result.name}' has been assigned already.`, {
+                    node: duplicate,
+                    property: 'result',
+                    code: CODE_SEGMENT_DUPLICATE_YIELD,
+                });
+            }
+        }
+    };
+
+export const segmentParameterShouldBeUsed =
+    (services: SafeDsServices) => (node: SdsSegment, accept: ValidationAcceptor) => {
+        for (const parameter of parametersOrEmpty(node)) {
+            const usages = services.helpers.NodeMapper.parameterToReferences(parameter);
+
+            if (usages.isEmpty()) {
+                accept('warning', 'This parameter is unused and can be removed.', {
+                    node: parameter,
+                    property: 'name',
+                    code: CODE_SEGMENT_UNUSED_PARAMETER,
+                });
+            }
+        }
+    };
diff --git a/src/language/validation/safe-ds-validator.ts b/src/language/validation/safe-ds-validator.ts
@@ -45,10 +45,7 @@ import { unionTypeMustHaveTypeArguments } from './other/types/unionTypes.js';
 import { callableTypeMustNotHaveOptionalParameters } from './other/types/callableTypes.js';
 import { typeArgumentListMustNotHavePositionalArgumentsAfterNamedArguments } from './other/types/typeArgumentLists.js';
 import { argumentListMustNotHavePositionalArgumentsAfterNamedArguments } from './other/argumentLists.js';
-import {
-    parameterMustNotBeVariadicAndOptional,
-    segmentParameterShouldBeUsed,
-} from './other/declarations/parameters.js';
+import { parameterMustNotBeVariadicAndOptional } from './other/declarations/parameters.js';
 import { referenceTargetMustNotBeAnnotationPipelineOrSchema } from './other/expressions/references.js';
 import {
     annotationCallAnnotationShouldNotBeDeprecated,
@@ -65,6 +62,7 @@ import {
     referenceTargetShouldNotExperimental,
 } from './builtins/experimental.js';
 import { placeholderShouldBeUsed } from './other/declarations/placeholders.js';
+import { segmentParameterShouldBeUsed, segmentResultMustBeAssignedExactlyOnce } from './other/declarations/segments.js';
 
 /**
  * Register custom validation checks.
@@ -125,6 +123,7 @@ export const registerValidationChecks = function (services: SafeDsServices) {
         SdsSegment: [
             segmentMustContainUniqueNames,
             segmentParameterShouldBeUsed(services),
+            segmentResultMustBeAssignedExactlyOnce(services),
             segmentResultListShouldNotBeEmpty,
         ],
         SdsTemplateString: [templateStringMustHaveExpressionBetweenTwoStringParts],

diff --git a/tests/resources/scoping/references/in same file/to modules/main.sdstest b/tests/resources/scoping/references/in same file/to modules/main.sdstest
@@ -0,0 +1,6 @@
+package tests
+
+pipeline myPipeline {
+    // $TEST$ unresolved
+    »tests«;
+}
diff --git a/...ariadic must not be optional/main.sdstest → ...ariadic must not be optional/main.sdstest b/...ariadic must not be optional/main.sdstest → ...ariadic must not be optional/main.sdstest
diff --git a/...larations/placeholder/unused/main.sdstest → ...arations/placeholders/unused/main.sdstest b/...larations/placeholder/unused/main.sdstest → ...arations/placeholders/unused/main.sdstest
@@ -1,4 +1,4 @@
-package tests.validation.declarations.placeholders.unused
+package tests.validation.other.declarations.placeholders.unused
 
 fun f() -> (r1: Int, r2: Int)
 

diff --git a/tests/resources/validation/other/declarations/segments/duplicate yield/main.sdstest b/tests/resources/validation/other/declarations/segments/duplicate yield/main.sdstest
@@ -0,0 +1,20 @@
+package tests.validation.other.declarations.segments.duplicateYield
+
+segment mySegment() -> (a: Int, b: Int, c: Int) {
+    // $TEST$ no error r"The result '\w*' has been assigned already\."
+    yield »a« = 1;
+
+    // $TEST$ no error r"The result '\w*' has been assigned already\."
+    yield »b« = 1;
+    // $TEST$ error "The result 'b' has been assigned already."
+    yield »b« = 1;
+
+    // $TEST$ no error r"The result '\w*' has been assigned already\."
+    // $TEST$ error "The result 'c' has been assigned already."
+    yield »c«, yield »c« = 1;
+
+    // $TEST$ no error r"The result '\w*' has been assigned already\."
+    yield »unresolved« = 1;
+    // $TEST$ no error r"The result '\w*' has been assigned already\."
+    yield »unresolved« = 1;
+}
diff --git a/tests/resources/validation/other/declarations/segments/unassigned result/main.sdstest b/tests/resources/validation/other/declarations/segments/unassigned result/main.sdstest
@@ -0,0 +1,17 @@
+package tests.validation.other.declarations.segments.unassignedResult
+
+// $TEST$ no error "Nothing is assigned to this result."
+// $TEST$ no error "Nothing is assigned to this result."
+// $TEST$ no error "Nothing is assigned to this result."
+// $TEST$ no error "Nothing is assigned to this result."
+// $TEST$ error "Nothing is assigned to this result."
+segment mySegment() -> (»a«: Int, »b«: Int, »c«: Int, »d«: Int, »e«: Int) {
+    yield b = 1;
+    yield a = 1;
+
+    yield c = 1;
+    yield c = 1;
+
+    // While nothing is assigned to d, the programmer still states their intention to do so. We already show another error for this.
+    _, yield d = 1;
+}
diff --git a/...eclarations/parameter/unused/main.sdstest → ...ns/segments/unused parameter/main.sdstest b/...eclarations/parameter/unused/main.sdstest → ...ns/segments/unused parameter/main.sdstest
@@ -1,4 +1,4 @@
-package tests.validation.declarations.parameters.unused
+package tests.validation.other.declarations.segments.unusedParameters
 
 segment mySegment(
     // $TEST$ warning "This parameter is unused and can be removed."