feat: various checks for annotations on parameters and results (#625)

Closes partially #543 ### Summary of Changes * Annotations must not be used on parameters of callable types * Annotations must not be used on results of callable types * Annotations must not be used on parameters of lambdas * `@Deprecated` must not be used on required parameters * `@Expert` must not be used on required parameters
Safe-DS · Oct 10, 2023 · e77037e · e77037e
1 parent 090fcc3
commit e77037e
Show file tree

Hide file tree

Showing 12 changed files with 265 additions and 22 deletions.
diff --git a/src/language/builtins/safe-ds-annotations.ts b/src/language/builtins/safe-ds-annotations.ts
@@ -1,34 +1,44 @@
-import { isSdsAnnotation, SdsAnnotatedObject, SdsAnnotation } from '../generated/ast.js';
+import { isSdsAnnotation, SdsAnnotatedObject, SdsAnnotation, SdsParameter } from '../generated/ast.js';
 import { annotationCallsOrEmpty } from '../helpers/nodeProperties.js';
 import { SafeDsModuleMembers } from './safe-ds-module-members.js';
 import { resourceNameToUri } from '../../helpers/resources.js';
+import { URI } from 'langium';
 
 const CORE_ANNOTATIONS_URI = resourceNameToUri('builtins/safeds/lang/coreAnnotations.sdsstub');
 
 export class SafeDsAnnotations extends SafeDsModuleMembers<SdsAnnotation> {
     isDeprecated(node: SdsAnnotatedObject | undefined): boolean {
-        return annotationCallsOrEmpty(node).some((it) => {
-            const annotation = it.annotation?.ref;
-            return annotation === this.Deprecated;
-        });
+        return this.hasAnnotationCallOf(node, this.Deprecated);
     }
 
-    isExperimental(node: SdsAnnotatedObject | undefined): boolean {
-        return annotationCallsOrEmpty(node).some((it) => {
-            const annotation = it.annotation?.ref;
-            return annotation === this.Experimental;
-        });
+    private get Deprecated(): SdsAnnotation | undefined {
+        return this.getAnnotation(CORE_ANNOTATIONS_URI, 'Deprecated');
     }
 
-    private get Deprecated(): SdsAnnotation | undefined {
-        return this.getAnnotation('Deprecated');
+    isExperimental(node: SdsAnnotatedObject | undefined): boolean {
+        return this.hasAnnotationCallOf(node, this.Experimental);
     }
 
     private get Experimental(): SdsAnnotation | undefined {
-        return this.getAnnotation('Experimental');
+        return this.getAnnotation(CORE_ANNOTATIONS_URI, 'Experimental');
+    }
+
+    isExpert(node: SdsParameter | undefined): boolean {
+        return this.hasAnnotationCallOf(node, this.Expert);
+    }
+
+    private get Expert(): SdsAnnotation | undefined {
+        return this.getAnnotation(CORE_ANNOTATIONS_URI, 'Expert');
+    }
+
+    private hasAnnotationCallOf(node: SdsAnnotatedObject | undefined, expected: SdsAnnotation | undefined): boolean {
+        return annotationCallsOrEmpty(node).some((it) => {
+            const actual = it.annotation?.ref;
+            return actual === expected;
+        });
     }
 
-    private getAnnotation(name: string): SdsAnnotation | undefined {
-        return this.getModuleMember(CORE_ANNOTATIONS_URI, name, isSdsAnnotation);
+    private getAnnotation(uri: URI, name: string): SdsAnnotation | undefined {
+        return this.getModuleMember(uri, name, isSdsAnnotation);
     }
 }
diff --git a/src/language/validation/builtins/deprecated.ts b/src/language/validation/builtins/deprecated.ts
@@ -7,14 +7,18 @@ import {
     SdsArgument,
     SdsAssignee,
     SdsNamedType,
+    SdsParameter,
     SdsReference,
 } from '../../generated/ast.js';
 import { SafeDsServices } from '../../safe-ds-module.js';
+import { isRequiredParameter } from '../../helpers/nodeProperties.js';
+import { parameterCanBeAnnotated } from '../other/declarations/annotationCalls.js';
 
 export const CODE_DEPRECATED_ASSIGNED_RESULT = 'deprecated/assigned-result';
 export const CODE_DEPRECATED_CALLED_ANNOTATION = 'deprecated/called-annotation';
 export const CODE_DEPRECATED_CORRESPONDING_PARAMETER = 'deprecated/corresponding-parameter';
 export const CODE_DEPRECATED_REFERENCED_DECLARATION = 'deprecated/referenced-declaration';
+export const CODE_DEPRECATED_REQUIRED_PARAMETER = 'deprecated/required-parameter';
 
 export const assigneeAssignedResultShouldNotBeDeprecated =
     (services: SafeDsServices) => (node: SdsAssignee, accept: ValidationAcceptor) => {
@@ -95,3 +99,16 @@ export const referenceTargetShouldNotBeDeprecated =
             });
         }
     };
+
+export const requiredParameterMustNotBeDeprecated =
+    (services: SafeDsServices) => (node: SdsParameter, accept: ValidationAcceptor) => {
+        if (isRequiredParameter(node) && parameterCanBeAnnotated(node)) {
+            if (services.builtins.Annotations.isDeprecated(node)) {
+                accept('error', 'A deprecated parameter must be optional.', {
+                    node,
+                    property: 'name',
+                    code: CODE_DEPRECATED_REQUIRED_PARAMETER,
+                });
+            }
+        }
+    };
diff --git a/src/language/validation/builtins/expert.ts b/src/language/validation/builtins/expert.ts
@@ -0,0 +1,20 @@
+import { ValidationAcceptor } from 'langium';
+import { SdsParameter } from '../../generated/ast.js';
+import { SafeDsServices } from '../../safe-ds-module.js';
+import { isRequiredParameter } from '../../helpers/nodeProperties.js';
+import { parameterCanBeAnnotated } from '../other/declarations/annotationCalls.js';
+
+export const CODE_EXPERT_TARGET_PARAMETER = 'expert/target-parameter';
+
+export const requiredParameterMustNotBeExpert =
+    (services: SafeDsServices) => (node: SdsParameter, accept: ValidationAcceptor) => {
+        if (isRequiredParameter(node) && parameterCanBeAnnotated(node)) {
+            if (services.builtins.Annotations.isExpert(node)) {
+                accept('error', 'An expert parameter must be optional.', {
+                    node,
+                    property: 'name',
+                    code: CODE_EXPERT_TARGET_PARAMETER,
+                });
+            }
+        }
+    };
diff --git a/src/language/validation/other/declarations/annotationCalls.ts b/src/language/validation/other/declarations/annotationCalls.ts
@@ -0,0 +1,51 @@
+import {
+    isSdsCallable,
+    isSdsCallableType,
+    isSdsLambda,
+    SdsCallableType,
+    SdsLambda,
+    SdsParameter,
+} from '../../../generated/ast.js';
+import { getContainerOfType, ValidationAcceptor } from 'langium';
+import { annotationCallsOrEmpty, parametersOrEmpty, resultsOrEmpty } from '../../../helpers/nodeProperties.js';
+
+export const CODE_ANNOTATION_CALL_TARGET_PARAMETER = 'annotation-call/target-parameter';
+export const CODE_ANNOTATION_CALL_TARGET_RESULT = 'annotation-call/target-result';
+
+export const callableTypeParametersMustNotBeAnnotated = (node: SdsCallableType, accept: ValidationAcceptor) => {
+    for (const parameter of parametersOrEmpty(node)) {
+        for (const annotationCall of annotationCallsOrEmpty(parameter)) {
+            accept('error', 'Parameters of callable types must not be annotated.', {
+                node: annotationCall,
+                code: CODE_ANNOTATION_CALL_TARGET_PARAMETER,
+            });
+        }
+    }
+};
+
+export const callableTypeResultsMustNotBeAnnotated = (node: SdsCallableType, accept: ValidationAcceptor) => {
+    for (const result of resultsOrEmpty(node.resultList)) {
+        for (const annotationCall of annotationCallsOrEmpty(result)) {
+            accept('error', 'Results of callable types must not be annotated.', {
+                node: annotationCall,
+                code: CODE_ANNOTATION_CALL_TARGET_RESULT,
+            });
+        }
+    }
+};
+
+export const lambdaParametersMustNotBeAnnotated = (node: SdsLambda, accept: ValidationAcceptor) => {
+    for (const parameter of parametersOrEmpty(node)) {
+        for (const annotationCall of annotationCallsOrEmpty(parameter)) {
+            accept('error', 'Lambda parameters must not be annotated.', {
+                node: annotationCall,
+                code: CODE_ANNOTATION_CALL_TARGET_PARAMETER,
+            });
+        }
+    }
+};
+
+export const parameterCanBeAnnotated = (node: SdsParameter) => {
+    const containingCallable = getContainerOfType(node, isSdsCallable);
+    return !isSdsCallableType(containingCallable) && !isSdsLambda(containingCallable);
+};
diff --git a/src/language/validation/safe-ds-validator.ts b/src/language/validation/safe-ds-validator.ts
@@ -52,6 +52,7 @@ import {
     assigneeAssignedResultShouldNotBeDeprecated,
     namedTypeDeclarationShouldNotBeDeprecated,
     referenceTargetShouldNotBeDeprecated,
+    requiredParameterMustNotBeDeprecated,
 } from './builtins/deprecated.js';
 import {
     annotationCallAnnotationShouldNotBeExperimental,
@@ -64,6 +65,12 @@ import { placeholderShouldBeUsed } from './other/declarations/placeholders.js';
 import { segmentParameterShouldBeUsed, segmentResultMustBeAssignedExactlyOnce } from './other/declarations/segments.js';
 import { lambdaParameterMustNotHaveConstModifier } from './other/expressions/lambdas.js';
 import { indexedAccessesShouldBeUsedWithCaution } from './experimentalLanguageFeature.js';
+import { requiredParameterMustNotBeExpert } from './builtins/expert.js';
+import {
+    callableTypeParametersMustNotBeAnnotated,
+    callableTypeResultsMustNotBeAnnotated,
+    lambdaParametersMustNotBeAnnotated,
+} from './other/declarations/annotationCalls.js';
 
 /**
  * Register custom validation checks.
@@ -97,7 +104,9 @@ export const registerValidationChecks = function (services: SafeDsServices) {
         SdsCallableType: [
             callableTypeMustContainUniqueNames,
             callableTypeMustNotHaveOptionalParameters,
+            callableTypeParametersMustNotBeAnnotated,
             callableTypeParameterMustNotHaveConstModifier,
+            callableTypeResultsMustNotBeAnnotated,
         ],
         SdsClass: [classMustContainUniqueNames],
         SdsClassBody: [classBodyShouldNotBeEmpty],
@@ -109,15 +118,19 @@ export const registerValidationChecks = function (services: SafeDsServices) {
         SdsExpressionLambda: [expressionLambdaMustContainUniqueNames],
         SdsFunction: [functionMustContainUniqueNames, functionResultListShouldNotBeEmpty],
         SdsIndexedAccess: [indexedAccessesShouldBeUsedWithCaution],
-        SdsLambda: [lambdaParameterMustNotHaveConstModifier],
+        SdsLambda: [lambdaParametersMustNotBeAnnotated, lambdaParameterMustNotHaveConstModifier],
         SdsMemberAccess: [memberAccessNullSafetyShouldBeNeeded(services)],
         SdsModule: [moduleDeclarationsMustMatchFileKind, moduleWithDeclarationsMustStatePackage],
         SdsNamedType: [
             namedTypeDeclarationShouldNotBeDeprecated(services),
             namedTypeDeclarationShouldNotBeExperimental(services),
             namedTypeTypeArgumentListShouldBeNeeded,
         ],
-        SdsParameter: [parameterMustHaveTypeHint],
+        SdsParameter: [
+            parameterMustHaveTypeHint,
+            requiredParameterMustNotBeDeprecated(services),
+            requiredParameterMustNotBeExpert(services),
+        ],
         SdsParameterList: [parameterListMustNotHaveRequiredParametersAfterOptionalParameters],
         SdsPipeline: [pipelineMustContainUniqueNames],
         SdsPlaceholder: [placeholderShouldBeUsed(services)],

diff --git a/src/resources/builtins/safeds/lang/coreAnnotations.sdsstub b/src/resources/builtins/safeds/lang/coreAnnotations.sdsstub
@@ -90,6 +90,11 @@ annotation Deprecated(
 ])
 annotation Experimental
 
+@Experimental
+@Description("This parameter should only be used by expert users.")
+@Target([AnnotationTarget.Parameter])
+annotation Expert
+
 @Experimental
 @Description("The function has no side effects and returns the same results for the same arguments.")
 @Target([AnnotationTarget.Function])

diff --git a/src/resources/builtins/safeds/lang/documentation.sdsstub b/src/resources/builtins/safeds/lang/documentation.sdsstub
@@ -11,8 +11,3 @@ annotation Since(
     @Description("The version in which a declaration was added.")
     version: String
 )
-
-@Experimental
-@Description("This parameter should only be used by expert users.")
-@Target([AnnotationTarget.Parameter])
-annotation Expert
diff --git a/...tion/builtins/annotations/deprecated/must not be used on required parameters/main.sdstest b/...tion/builtins/annotations/deprecated/must not be used on required parameters/main.sdstest
@@ -0,0 +1,48 @@
+package tests.validation.builtins.deprecated.mustNotBeUsedOnRequiredParameters
+
+// $TEST$ error "A deprecated parameter must be optional."
+// $TEST$ no error "A deprecated parameter must be optional."
+annotation MyAnnotation(@Deprecated »a«: Int, @Deprecated »b«: Int = 3)
+
+// $TEST$ error "A deprecated parameter must be optional."
+// $TEST$ no error "A deprecated parameter must be optional."
+class MyClass(@Deprecated »a«: Int, @Deprecated »b«: Int = 3) {
+
+    // $TEST$ error "A deprecated parameter must be optional."
+    // $TEST$ no error "A deprecated parameter must be optional."
+    class MyClass(@Deprecated »a«: Int, @Deprecated »b«: Int = 3)
+
+    // $TEST$ error "A deprecated parameter must be optional."
+    // $TEST$ no error "A deprecated parameter must be optional."
+    fun myFunction(@Deprecated »a«: Int, @Deprecated »b«: Int = 3)
+}
+
+enum MyEnum {
+
+    // $TEST$ error "A deprecated parameter must be optional."
+    // $TEST$ no error "A deprecated parameter must be optional."
+    MyEnumVariant(@Deprecated »a«: Int, @Deprecated »b«: Int = 3)
+}
+
+// $TEST$ error "A deprecated parameter must be optional."
+// $TEST$ no error "A deprecated parameter must be optional."
+fun myFunction(@Deprecated »a«: Int, @Deprecated »b«: Int = 3)
+
+// $TEST$ error "A deprecated parameter must be optional."
+// $TEST$ no error "A deprecated parameter must be optional."
+segment mySegment1(@Deprecated »a«: Int, @Deprecated »b«: Int = 3) {}
+
+// $TEST$ no error "A deprecated parameter must be optional."
+// $TEST$ no error "A deprecated parameter must be optional."
+segment mySegment2(
+    f: (@Deprecated »a«: Int, @Deprecated »b«: Int = 3) -> ()
+) {
+
+    // $TEST$ no error "A deprecated parameter must be optional."
+    // $TEST$ no error "A deprecated parameter must be optional."
+    val g = (@Deprecated »a«: Int, @Deprecated »b«: Int = 3) {};
+
+    // $TEST$ no error "A deprecated parameter must be optional."
+    // $TEST$ no error "A deprecated parameter must be optional."
+    val h = (@Deprecated »a«: Int, @Deprecated »b«: Int = 3) -> 1;
+}
diff --git a/...lidation/builtins/annotations/expert/must not be used on required parameters/main.sdstest b/...lidation/builtins/annotations/expert/must not be used on required parameters/main.sdstest
@@ -0,0 +1,48 @@
+package tests.validation.builtins.expert.mustNotBeUsedOnRequiredParameters
+
+// $TEST$ error "An expert parameter must be optional."
+// $TEST$ no error "An expert parameter must be optional."
+annotation MyAnnotation(@Expert »a«: Int, @Expert »b«: Int = 3)
+
+// $TEST$ error "An expert parameter must be optional."
+// $TEST$ no error "An expert parameter must be optional."
+class MyClass(@Expert »a«: Int, @Expert »b«: Int = 3) {
+
+    // $TEST$ error "An expert parameter must be optional."
+    // $TEST$ no error "An expert parameter must be optional."
+    class MyClass(@Expert »a«: Int, @Expert »b«: Int = 3)
+
+    // $TEST$ error "An expert parameter must be optional."
+    // $TEST$ no error "An expert parameter must be optional."
+    fun myFunction(@Expert »a«: Int, @Expert »b«: Int = 3)
+}
+
+enum MyEnum {
+
+    // $TEST$ error "An expert parameter must be optional."
+    // $TEST$ no error "An expert parameter must be optional."
+    MyEnumVariant(@Expert »a«: Int, @Expert »b«: Int = 3)
+}
+
+// $TEST$ error "An expert parameter must be optional."
+// $TEST$ no error "An expert parameter must be optional."
+fun myFunction(@Expert »a«: Int, @Expert »b«: Int = 3)
+
+// $TEST$ error "An expert parameter must be optional."
+// $TEST$ no error "An expert parameter must be optional."
+segment mySegment1(@Expert »a«: Int, @Expert »b«: Int = 3) {}
+
+// $TEST$ no error "An expert parameter must be optional."
+// $TEST$ no error "An expert parameter must be optional."
+segment mySegment2(
+    f: (@Expert »a«: Int, @Expert »b«: Int = 3) -> ()
+) {
+
+    // $TEST$ no error "An expert parameter must be optional."
+    // $TEST$ no error "An expert parameter must be optional."
+    val g = (@Expert »a«: Int, @Expert »b«: Int = 3) {};
+
+    // $TEST$ no error "An expert parameter must be optional."
+    // $TEST$ no error "An expert parameter must be optional."
+    val h = (@Expert »a«: Int, @Expert »b«: Int = 3) -> 1;
+}
diff --git a/...on/other/declarations/annotation calls/must not be used on lambda parameters/main.sdstest b/...on/other/declarations/annotation calls/must not be used on lambda parameters/main.sdstest
@@ -0,0 +1,16 @@
+package tests.validation.other.declarations.annotationCalls.mustNotBeUsedOnLambdaParameters
+
+annotation MyAnnotation
+
+pipeline myPipeline {
+
+    // $TEST$ error "Lambda parameters must not be annotated."
+    // $TEST$ error "Lambda parameters must not be annotated."
+    // $TEST$ error "Lambda parameters must not be annotated."
+    val f = (»@MyAnnotation« »@MyAnnotation« a: Int, »@MyAnnotation« b: Int = 3) {};
+
+    // $TEST$ error "Lambda parameters must not be annotated."
+    // $TEST$ error "Lambda parameters must not be annotated."
+    // $TEST$ error "Lambda parameters must not be annotated."
+    val g = (»@MyAnnotation« »@MyAnnotation« a: Int, »@MyAnnotation« b: Int = 3) -> 1;
+}
diff --git a/...clarations/annotation calls/must not be used on parameters of callable types/main.sdstest b/...clarations/annotation calls/must not be used on parameters of callable types/main.sdstest
@@ -0,0 +1,10 @@
+package tests.validation.other.declarations.annotationCalls.mustNotBeUsedOnParametersOfCallableTypes
+
+annotation MyAnnotation
+
+// $TEST$ error "Parameters of callable types must not be annotated."
+// $TEST$ error "Parameters of callable types must not be annotated."
+// $TEST$ error "Parameters of callable types must not be annotated."
+segment mySegment(
+    f: (»@MyAnnotation« »@MyAnnotation« a: Int, »@MyAnnotation« b: Int = 3) -> ()
+) {}
diff --git a/.../declarations/annotation calls/must not be used on results of callable types/main.sdstest b/.../declarations/annotation calls/must not be used on results of callable types/main.sdstest
@@ -0,0 +1,10 @@
+package tests.validation.other.declarations.annotationCalls.mustNotBeUsedOnResultsOfCallableTypes
+
+annotation MyAnnotation
+
+// $TEST$ error "Results of callable types must not be annotated."
+// $TEST$ error "Results of callable types must not be annotated."
+// $TEST$ error "Results of callable types must not be annotated."
+segment mySegment(
+    f: () -> (»@MyAnnotation« »@MyAnnotation« a: Int, »@MyAnnotation« b: Int)
+) {}