feat: check usages of @PythonName and @PythonCall on overriding m…

…ethods (#1100) ### Summary of Changes Add some checks: * Must not override method with `@PythonCall` annotation * Overriding method must not have `@PythonCall` annotation * Python name of overriding method must equal the python name of the overridden method
Safe-DS · Apr 24, 2024 · 3021166 · 3021166
1 parent 4fe4daa
commit 3021166
Show file tree

Hide file tree

Showing 8 changed files with 256 additions and 6 deletions.
diff --git a/packages/safe-ds-lang/src/language/validation/inheritance.ts b/packages/safe-ds-lang/src/language/validation/inheritance.ts
@@ -1,6 +1,6 @@
 import { AstUtils, ValidationAcceptor } from 'langium';
 import { isEmpty, isEqualSet } from '../../helpers/collections.js';
-import { isSdsClass, isSdsFunction, SdsClass, type SdsClassMember } from '../generated/ast.js';
+import { isSdsClass, isSdsFunction, SdsClass, type SdsClassMember, SdsFunction } from '../generated/ast.js';
 import { getParentTypes, getQualifiedName } from '../helpers/nodeProperties.js';
 import { SafeDsServices } from '../safe-ds-module.js';
 import { ClassType, Type, UnknownType } from '../typing/model.js';
@@ -13,6 +13,8 @@ export const CODE_INHERITANCE_IDENTICAL_TO_OVERRIDDEN_MEMBER = 'inheritance/iden
 export const CODE_INHERITANCE_INCOMPATIBLE_TO_OVERRIDDEN_MEMBER = 'inheritance/incompatible-to-overridden-member';
 export const CODE_INHERITANCE_NOT_A_CLASS = 'inheritance/not-a-class';
 export const CODE_INHERITANCE_NULLABLE = 'inheritance/nullable';
+export const CODE_INHERITANCE_PYTHON_CALL = 'inheritance/python-call';
+export const CODE_INHERITANCE_PYTHON_NAME = 'inheritance/python-name';
 
 export const classMemberMustMatchOverriddenMemberAndShouldBeNeeded = (services: SafeDsServices) => {
     const builtinAnnotations = services.builtins.Annotations;
@@ -32,6 +34,18 @@ export const classMemberMustMatchOverriddenMemberAndShouldBeNeeded = (services:
             computeMemberTypes(node, overriddenMember, typeComputer);
 
         // Check whether the overriding is legal and needed
+        if (
+            // It's an error if the overriding member calls the `@PythonCall` annotation
+            (isSdsFunction(node) && builtinAnnotations.getPythonCall(node)) ||
+            // It's an error if the overridden member calls the `@PythonCall` annotation
+            (isSdsFunction(overriddenMember) && builtinAnnotations.getPythonCall(overriddenMember)) ||
+            // It's an error if the overriding member has a different Python name than the overridden member
+            (builtinAnnotations.getPythonName(node) ?? node.name) !==
+                (builtinAnnotations.getPythonName(overriddenMember) ?? overriddenMember.name)
+        ) {
+            return;
+        }
+
         if (!typeChecker.isSubtypeOf(substitutedOwnMemberType, overriddenMemberType)) {
             accept(
                 'error',
@@ -49,6 +63,7 @@ export const classMemberMustMatchOverriddenMemberAndShouldBeNeeded = (services:
         } else if (typeChecker.isSubtypeOf(substitutedOverriddenMemberType, ownMemberType)) {
             // Prevents the info from showing when editing the builtin files
             if (isInSafedsLangAnyClass(services, node)) {
+                /* c8 ignore next 2 */
                 return;
             }
 
@@ -201,3 +216,70 @@ export const classMustNotInheritItself = (services: SafeDsServices) => {
         }
     };
 };
+
+export const overridingAndOverriddenMethodsMustNotHavePythonCall = (services: SafeDsServices) => {
+    const builtinAnnotations = services.builtins.Annotations;
+
+    return (node: SdsFunction, accept: ValidationAcceptor): void => {
+        // Prevents the errors from showing when editing the builtin files
+        if (isInSafedsLangAnyClass(services, node)) {
+            /* c8 ignore next 2 */
+            return;
+        }
+
+        // Check whether the function overrides something
+        const overriddenMember = services.typing.ClassHierarchy.getOverriddenMember(node);
+        if (!overriddenMember) {
+            return;
+        }
+
+        // Check whether the function calls the `@PythonCall` annotation
+        const ownPythonCall = builtinAnnotations.getPythonCall(node);
+        if (ownPythonCall !== undefined) {
+            accept('error', "An overriding method must not call the '@PythonCall' annotation.", {
+                node,
+                property: 'name',
+                code: CODE_INHERITANCE_PYTHON_CALL,
+            });
+            return;
+        }
+
+        // Check whether the overridden function calls the `@PythonCall` annotation
+        if (!isSdsFunction(overriddenMember)) {
+            return;
+        }
+
+        const overriddenPythonCall = builtinAnnotations.getPythonCall(overriddenMember);
+        if (overriddenPythonCall !== undefined) {
+            accept('error', "Cannot override a method that calls the '@PythonCall' annotation.", {
+                node,
+                property: 'name',
+                code: CODE_INHERITANCE_PYTHON_CALL,
+            });
+        }
+    };
+};
+
+export const overridingMemberPythonNameMustMatchOverriddenMember = (services: SafeDsServices) => {
+    const builtinAnnotations = services.builtins.Annotations;
+
+    return (node: SdsClassMember, accept: ValidationAcceptor): void => {
+        // Check whether the function overrides something
+        const overriddenMember = services.typing.ClassHierarchy.getOverriddenMember(node);
+        if (!overriddenMember) {
+            return;
+        }
+
+        // Check whether the function has a different Python name than the overridden member
+        if (
+            (builtinAnnotations.getPythonName(node) ?? node.name) !==
+            (builtinAnnotations.getPythonName(overriddenMember) ?? overriddenMember.name)
+        ) {
+            accept('error', 'The Python name must match the overridden member.', {
+                node,
+                property: 'name',
+                code: CODE_INHERITANCE_PYTHON_NAME,
+            });
+        }
+    };
+};
diff --git a/packages/safe-ds-lang/src/language/validation/safe-ds-validator.ts b/packages/safe-ds-lang/src/language/validation/safe-ds-validator.ts
@@ -35,6 +35,8 @@ import {
     classMemberMustMatchOverriddenMemberAndShouldBeNeeded,
     classMustNotInheritItself,
     classMustOnlyInheritASingleClass,
+    overridingAndOverriddenMethodsMustNotHavePythonCall,
+    overridingMemberPythonNameMustMatchOverriddenMember,
 } from './inheritance.js';
 import {
     annotationMustContainUniqueNames,
@@ -260,7 +262,10 @@ export const registerValidationChecks = function (services: SafeDsServices) {
             staticClassMemberNamesMustNotCollideWithInheritedMembers(services),
         ],
         SdsClassBody: [classBodyShouldNotBeEmpty(services)],
-        SdsClassMember: [classMemberMustMatchOverriddenMemberAndShouldBeNeeded(services)],
+        SdsClassMember: [
+            classMemberMustMatchOverriddenMemberAndShouldBeNeeded(services),
+            overridingMemberPythonNameMustMatchOverriddenMember(services),
+        ],
         SdsConstraintList: [constraintListsShouldBeUsedWithCaution(services), constraintListShouldNotBeEmpty(services)],
         SdsDeclaration: [
             nameMustNotOccurOnCoreDeclaration(services),
@@ -283,6 +288,7 @@ export const registerValidationChecks = function (services: SafeDsServices) {
             impurityReasonShouldNotBeSetMultipleTimes(services),
             pythonCallMustOnlyContainValidTemplateExpressions(services),
             pythonNameMustNotBeSetIfPythonCallIsSet(services),
+            overridingAndOverriddenMethodsMustNotHavePythonCall(services),
         ],
         SdsImport: [importPackageMustExist(services), importPackageShouldNotBeEmpty(services)],
         SdsImportedDeclaration: [importedDeclarationAliasShouldDifferFromDeclarationName(services)],

diff --git a/packages/safe-ds-lang/tests/resources/validation/builtins/annotations/pythonCall/main.sdsdev b/packages/safe-ds-lang/tests/resources/validation/builtins/annotations/pythonCall/main.sdsdev
@@ -33,7 +33,3 @@ fun myFunction3()
 // $TEST$ error "The template expressions '$param1', '$param2' cannot be interpreted."
 @»PythonCall«("myFunction4($param1, $param2)")
 fun myFunction4()
-
-// $TEST$ no error "An expert parameter must be optional."
-@»PythonCall«("$this")
-annotation MyAnnotation()
diff --git a/...s/validation/inheritance/must not override method with python call annotation/main.sdsdev b/...s/validation/inheritance/must not override method with python call annotation/main.sdsdev
@@ -0,0 +1,36 @@
+package tests.validation.inheritance.mustNotOverrideMethodWithPythonCallAnnotation
+
+class MyClass1 {
+    attr myAttribute: Int
+
+    // $TEST$ no error "Cannot override a method that calls the '@PythonCall' annotation."
+    @Pure
+    @PythonCall("some_other_function")
+    fun »withPythonCall«()
+
+    // $TEST$ no error "Cannot override a method that calls the '@PythonCall' annotation."
+    @Pure
+    fun »withoutPythonCall«()
+}
+
+class MyClass2 sub MyClass1 {
+    // $TEST$ no error "Cannot override a method that calls the '@PythonCall' annotation."
+    @Pure
+    @PythonCall("some_other_function")
+    fun »withPythonCall«()
+
+    // $TEST$ no error "Cannot override a method that calls the '@PythonCall' annotation."
+    @Pure
+    @PythonCall("some_other_function")
+    fun »withoutPythonCall«()
+}
+
+class MyClass3 sub MyClass1 {
+    // $TEST$ no error "Cannot override a method that calls the '@PythonCall' annotation."
+    @Pure
+    fun »myAttribute«()
+
+    // $TEST$ error "Cannot override a method that calls the '@PythonCall' annotation."
+    @Pure
+    fun »withPythonCall«()
+}
diff --git a/...validation/inheritance/overriding method must not have python call annotation/main.sdsdev b/...validation/inheritance/overriding method must not have python call annotation/main.sdsdev
@@ -0,0 +1,24 @@
+package tests.validation.inheritance.overridingMethodMustNotHavePythonCallAnnotation
+
+class MyClass1 {
+    // $TEST$ no error "An overriding method must not call the '@PythonCall' annotation."
+    @Pure
+    @PythonCall("some_other_function")
+    fun »withPythonCall«()
+
+    // $TEST$ no error "An overriding method must not call the '@PythonCall' annotation."
+    @Pure
+    fun »withoutPythonCall«()
+}
+
+class MyClass2 sub MyClass1 {
+    // $TEST$ error "An overriding method must not call the '@PythonCall' annotation."
+    @Pure
+    @PythonCall("some_other_function")
+    fun »withPythonCall«()
+
+    // $TEST$ error "An overriding method must not call the '@PythonCall' annotation."
+    @Pure
+    @PythonCall("some_other_function")
+    fun »withoutPythonCall«()
+}
diff --git a/...nheritance/overriding method should differ from overridden method/with python call.sdsdev b/...nheritance/overriding method should differ from overridden method/with python call.sdsdev
@@ -0,0 +1,13 @@
+package tests.validation.inheritance.overridingMethodShouldDifferFromOverriddenMethod.withPythonCall
+
+class MyClass1 {
+    @Pure
+    fun f()
+}
+
+class MyClass2 {
+    // $TEST$ no info "Overriding member is identical to overridden member and can be removed."
+    @Pure
+    @PythonCall("")
+    fun »f«()
+}
diff --git a/...nheritance/overriding method should differ from overridden method/with python name.sdsdev b/...nheritance/overriding method should differ from overridden method/with python name.sdsdev
@@ -0,0 +1,34 @@
+package tests.validation.inheritance.overridingMethodShouldDifferFromOverriddenMethod.withPythonName
+
+class MyClass1 {
+    @Pure
+    fun myFunction1()
+
+    @Pure
+    @PythonName("my_function_2")
+    fun myFunction2()
+}
+
+class MyClass2 sub MyClass1 {
+    // $TEST$ info "Overriding member is identical to overridden member and can be removed."
+    @Pure
+    @PythonName("myFunction1")
+    fun »myFunction1«()
+
+    // $TEST$ info "Overriding member is identical to overridden member and can be removed."
+    @Pure
+    @PythonName("my_function_2")
+    fun »myFunction2«()
+}
+
+class MyClass3 sub MyClass1 {
+    // $TEST$ no info "Overriding member is identical to overridden member and can be removed."
+    @Pure
+    @PythonName("anotherFunction1")
+    fun »myFunction1«()
+
+    // $TEST$ no info "Overriding member is identical to overridden member and can be removed."
+    @Pure
+    @PythonName("anotherFunction2")
+    fun »myFunction2«()
+}
diff --git a/...sts/resources/validation/inheritance/python name must match overridden method/main.sdsdev b/...sts/resources/validation/inheritance/python name must match overridden method/main.sdsdev
@@ -0,0 +1,59 @@
+package tests.validation.inheritance.pythonNameMustMatchOverriddenMethod
+
+class MyClass1 {
+    // $TEST$ no error "The Python name must match the overridden member."
+    attr »myAttribute1«: Int
+
+    // $TEST$ no error "The Python name must match the overridden member."
+    @PythonName("my_Attribute_2")
+    attr »myAttribute2«: Int
+
+    // $TEST$ no error "The Python name must match the overridden member."
+    @Pure
+    fun »myFunction1«()
+
+    // $TEST$ no error "The Python name must match the overridden member."
+    @Pure
+    @PythonName("my_function_2")
+    fun »myFunction2«()
+}
+
+class MyClass2 sub MyClass1 {
+    // $TEST$ no error "The Python name must match the overridden member."
+    @PythonName("myAttribute1")
+    attr »myAttribute1«: Int
+
+    // $TEST$ no error "The Python name must match the overridden member."
+    @PythonName("my_Attribute_2")
+    attr »myAttribute2«: Int
+
+    // $TEST$ no error "The Python name must match the overridden member."
+    @Pure
+    @PythonName("myFunction1")
+    fun »myFunction1«()
+
+    // $TEST$ no error "The Python name must match the overridden member."
+    @Pure
+    @PythonName("my_function_2")
+    fun »myFunction2«()
+}
+
+class MyClass3 sub MyClass1 {
+    // $TEST$ error "The Python name must match the overridden member."
+    @PythonName("anotherAttribute1")
+    attr »myAttribute1«: Int
+
+    // $TEST$ error "The Python name must match the overridden member."
+    @PythonName("another_attribute_2")
+    attr »myAttribute2«: Int
+
+    // $TEST$ error "The Python name must match the overridden member."
+    @Pure
+    @PythonName("anotherFunction1")
+    fun »myFunction1«()
+
+    // $TEST$ error "The Python name must match the overridden member."
+    @Pure
+    @PythonName("anotherFunction2")
+    fun »myFunction2«()
+}