From 8b076e7d67aef0a622779b166db572b6af3f3025 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Tue, 17 Oct 2023 09:53:53 +0200 Subject: [PATCH] feat: error if class or enum are statically referenced (#643) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes partially #543 ### Summary of Changes Show an error if a class or enum is statically referenced. They must only be referenced to access one of their members/variants or to call them¹. ----- ¹ If they are not callable, we already show another error. --------- Co-authored-by: megalinter-bot <129584137+megalinter-bot@users.noreply.github.com> --- src/language/typing/safe-ds-type-computer.ts | 3 +- .../other/expressions/references.ts | 53 ++++++++++++++++--- src/language/validation/safe-ds-validator.ts | 2 + .../member accesses/to other/main.sdstest | 29 +++++++--- .../static class reference/main.sdstest | 44 +++++++++++++++ .../static enum reference/main.sdstest | 38 +++++++++++++ 6 files changed, 154 insertions(+), 15 deletions(-) create mode 100644 tests/resources/validation/other/expressions/references/static class reference/main.sdstest create mode 100644 tests/resources/validation/other/expressions/references/static enum reference/main.sdstest diff --git a/src/language/typing/safe-ds-type-computer.ts b/src/language/typing/safe-ds-type-computer.ts index 4e0894ded..2032d7411 100644 --- a/src/language/typing/safe-ds-type-computer.ts +++ b/src/language/typing/safe-ds-type-computer.ts @@ -419,7 +419,8 @@ export class SafeDsTypeComputer { } } - return memberType.copyWithNullability(node.isNullSafe || memberType.isNullable); + const receiverType = this.computeType(node.receiver); + return memberType.copyWithNullability((receiverType.isNullable && node.isNullSafe) || memberType.isNullable); } private computeTypeOfArithmeticPrefixOperation(node: SdsPrefixOperation): Type { diff --git a/src/language/validation/other/expressions/references.ts b/src/language/validation/other/expressions/references.ts index 26f260d7a..767a59151 100644 --- a/src/language/validation/other/expressions/references.ts +++ b/src/language/validation/other/expressions/references.ts @@ -1,6 +1,8 @@ import { isSdsAnnotation, isSdsCall, + isSdsClass, + isSdsEnum, isSdsFunction, isSdsMemberAccess, isSdsPipeline, @@ -11,20 +13,23 @@ import { import { AstNode, ValidationAcceptor } from 'langium'; export const CODE_REFERENCE_FUNCTION_POINTER = 'reference/function-pointer'; +export const CODE_REFERENCE_STATIC_CLASS_REFERENCE = 'reference/static-class-reference'; +export const CODE_REFERENCE_STATIC_ENUM_REFERENCE = 'reference/static-enum-reference'; export const CODE_REFERENCE_TARGET = 'reference/target'; export const referenceMustNotBeFunctionPointer = (node: SdsReference, accept: ValidationAcceptor): void => { - const target = node.target?.ref; + const target = node.target.ref; if (!isSdsFunction(target) && !isSdsSegment(target)) { return; } - let container: AstNode | undefined = node.$container; - if (isSdsMemberAccess(container) && node.$containerProperty === 'member') { - container = container.$container; + // Get the containing member access if the node is on its right side + let nodeOrContainer: AstNode | undefined = node; + if (isSdsMemberAccess(node.$container) && node.$containerProperty === 'member') { + nodeOrContainer = nodeOrContainer.$container; } - if (!isSdsCall(container)) { + if (!isSdsCall(nodeOrContainer?.$container)) { accept( 'error', 'Function pointers are not allowed to provide a cleaner graphical view. Use a lambda instead.', @@ -36,11 +41,47 @@ export const referenceMustNotBeFunctionPointer = (node: SdsReference, accept: Va } }; +export const referenceMustNotBeStaticClassOrEnumReference = (node: SdsReference, accept: ValidationAcceptor) => { + const target = node.target.ref; + if (!isSdsClass(target) && !isSdsEnum(target)) { + return; + } + + // Get the containing member access if the node is on its right side + let nodeOrContainer: AstNode | undefined = node; + if (isSdsMemberAccess(node.$container) && node.$containerProperty === 'member') { + nodeOrContainer = nodeOrContainer.$container; + } + + // Access to a member of the class or enum + if (isSdsMemberAccess(nodeOrContainer?.$container) && nodeOrContainer?.$containerProperty === 'receiver') { + return; + } + + // Call of the class or enum + if (isSdsCall(nodeOrContainer?.$container)) { + return; + } + + // Static reference to the class or enum + if (isSdsClass(target)) { + accept('error', 'A class must not be statically referenced.', { + node, + code: CODE_REFERENCE_STATIC_CLASS_REFERENCE, + }); + } else if (isSdsEnum(target)) { + accept('error', 'An enum must not be statically referenced.', { + node, + code: CODE_REFERENCE_STATIC_ENUM_REFERENCE, + }); + } +}; + export const referenceTargetMustNotBeAnnotationPipelineOrSchema = ( node: SdsReference, accept: ValidationAcceptor, ): void => { - const target = node.target?.ref; + const target = node.target.ref; if (isSdsAnnotation(target)) { accept('error', 'An annotation must not be the target of a reference.', { diff --git a/src/language/validation/safe-ds-validator.ts b/src/language/validation/safe-ds-validator.ts index 46a4bca42..4dcea904f 100644 --- a/src/language/validation/safe-ds-validator.ts +++ b/src/language/validation/safe-ds-validator.ts @@ -60,6 +60,7 @@ import { import { argumentListMustNotHavePositionalArgumentsAfterNamedArguments } from './other/argumentLists.js'; import { referenceMustNotBeFunctionPointer, + referenceMustNotBeStaticClassOrEnumReference, referenceTargetMustNotBeAnnotationPipelineOrSchema, } from './other/expressions/references.js'; import { @@ -197,6 +198,7 @@ export const registerValidationChecks = function (services: SafeDsServices) { SdsPlaceholder: [placeholdersMustNotBeAnAlias, placeholderShouldBeUsed(services)], SdsReference: [ referenceMustNotBeFunctionPointer, + referenceMustNotBeStaticClassOrEnumReference, referenceTargetMustNotBeAnnotationPipelineOrSchema, referenceTargetShouldNotBeDeprecated(services), referenceTargetShouldNotExperimental(services), diff --git a/tests/resources/typing/expressions/member accesses/to other/main.sdstest b/tests/resources/typing/expressions/member accesses/to other/main.sdstest index 32058b9f4..0da3c4671 100644 --- a/tests/resources/typing/expressions/member accesses/to other/main.sdstest +++ b/tests/resources/typing/expressions/member accesses/to other/main.sdstest @@ -1,21 +1,34 @@ package tests.typing.expressions.memberAccesses.toOther -class C { +class C() { // $TEST$ equivalence_class nonNullableMember - static attr »nonNullableMember«: Int + attr »nonNullableMember«: Int // $TEST$ equivalence_class nullableMember - static attr »nullableMember«: Any? + attr »nullableMember«: Any? } +fun nullableC() -> result: C? + pipeline myPipeline { // $TEST$ equivalence_class nonNullableMember - »C.nonNullableMember«; + »C().nonNullableMember«; // $TEST$ equivalence_class nullableMember - »C.nullableMember«; + »C().nullableMember«; + + // $TEST$ equivalence_class nonNullableMember + »C()?.nonNullableMember«; + // $TEST$ equivalence_class nullableMember + »C()?.nullableMember«; + + + // $TEST$ equivalence_class nonNullableMember + »nullableC().nonNullableMember«; + // $TEST$ equivalence_class nullableMember + »nullableC().nullableMember«; // $TEST$ serialization Int? - »C?.nonNullableMember«; - // $TEST$ serialization Any? - »C?.nullableMember«; + »nullableC()?.nonNullableMember«; + // $TEST$ equivalence_class nullableMember + »nullableC()?.nullableMember«; } diff --git a/tests/resources/validation/other/expressions/references/static class reference/main.sdstest b/tests/resources/validation/other/expressions/references/static class reference/main.sdstest new file mode 100644 index 000000000..729e2e535 --- /dev/null +++ b/tests/resources/validation/other/expressions/references/static class reference/main.sdstest @@ -0,0 +1,44 @@ +package tests.validation.other.expressions.references.staticClassReference + +class ClassWithConstructor() + +class ClassWithoutConstructor + +class ClassWithStaticMembers { + static attr myAttribute: Int + + class InnerClassWithConstructor() { + static attr myAttribute: Int + } + + class InnerClassWithoutConstructor +} + +pipeline test { + // $TEST$ no error "A class must not be statically referenced." + »Unresolved«; + // $TEST$ error "A class must not be statically referenced." + »ClassWithConstructor«; + // $TEST$ error "A class must not be statically referenced." + »ClassWithoutConstructor«; + // $TEST$ no error "A class must not be statically referenced." + »ClassWithoutConstructor«(); + // $TEST$ no error "A class must not be statically referenced." + »ClassWithConstructor«(); + // $TEST$ no error "A class must not be statically referenced." + »ClassWithStaticMembers«.myAttribute; + // $TEST$ no error "A class must not be statically referenced." + »ClassWithStaticMembers«.unresolved; + // $TEST$ no error "A class must not be statically referenced." + // $TEST$ error "A class must not be statically referenced." + »ClassWithStaticMembers«.»InnerClassWithConstructor«; + // $TEST$ no error "A class must not be statically referenced." + // $TEST$ error "A class must not be statically referenced." + »ClassWithStaticMembers«.»InnerClassWithoutConstructor«; + // $TEST$ no error "A class must not be statically referenced." + // $TEST$ no error "A class must not be statically referenced." + »ClassWithStaticMembers«.»InnerClassWithConstructor«(); + // $TEST$ no error "A class must not be statically referenced." + // $TEST$ no error "A class must not be statically referenced." + »ClassWithStaticMembers«.»InnerClassWithConstructor«.myAttribute; +} diff --git a/tests/resources/validation/other/expressions/references/static enum reference/main.sdstest b/tests/resources/validation/other/expressions/references/static enum reference/main.sdstest new file mode 100644 index 000000000..60fc554a4 --- /dev/null +++ b/tests/resources/validation/other/expressions/references/static enum reference/main.sdstest @@ -0,0 +1,38 @@ +package tests.validation.other.expressions.references.staticEnumReference + +enum Enum { + Variant +} + +class ClassWithEnum { + enum Enum { + Variant + } + + class ClassWithEnum { + enum Enum { + Variant + } + } +} + +pipeline test { + // $TEST$ no error "An enum must not be statically referenced." + »Unresolved«; + // $TEST$ error "An enum must not be statically referenced." + »Enum«; + // $TEST$ no error "An enum must not be statically referenced." + »Enum«(); + // $TEST$ no error "An enum must not be statically referenced." + »Enum«.Variant; + // $TEST$ no error "An enum must not be statically referenced." + »Enum«.unresolved; + // $TEST$ error "An enum must not be statically referenced." + ClassWithEnum.»Enum«; + // $TEST$ no error "An enum must not be statically referenced." + ClassWithEnum.»Enum«.Variant; + // $TEST$ error "An enum must not be statically referenced." + ClassWithEnum.ClassWithEnum.»Enum«; + // $TEST$ no error "An enum must not be statically referenced." + ClassWithEnum.ClassWithEnum.»Enum«.Variant; +}