From 3fbe81be026b920330b73521391d6bff160d33b4 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Tue, 10 Oct 2023 15:28:37 +0200 Subject: [PATCH 1/3] feat: error is null safe access is needed --- .../other/expressions/memberAccesses.ts | 26 ++++++++++++ src/language/validation/safe-ds-validator.ts | 6 ++- .../missing safe access/main.sdstest | 40 +++++++++++++++++++ .../unnecessary safe access/main.sdstest | 30 +++++++++++--- 4 files changed, 95 insertions(+), 7 deletions(-) create mode 100644 src/language/validation/other/expressions/memberAccesses.ts create mode 100644 tests/resources/validation/other/expressions/member accesses/missing safe access/main.sdstest diff --git a/src/language/validation/other/expressions/memberAccesses.ts b/src/language/validation/other/expressions/memberAccesses.ts new file mode 100644 index 000000000..7cf5fc578 --- /dev/null +++ b/src/language/validation/other/expressions/memberAccesses.ts @@ -0,0 +1,26 @@ +import {SafeDsServices} from "../../../safe-ds-module.js"; +import {SdsMemberAccess} from "../../../generated/ast.js"; +import {ValidationAcceptor} from "langium"; +import {UnknownType} from "../../../typing/model.js"; + +export const CODE_MEMBER_ACCESS_MISSING_NULL_SAFETY = 'member-access/missing-null-safety'; + +export const memberAccessMustBeNullSafeIfReceiverIsNullable = + (services: SafeDsServices) => + (node: SdsMemberAccess, accept: ValidationAcceptor): void => { + if (node.isNullSafe) { + return; + } + + const receiverType = services.types.TypeComputer.computeType(node.receiver); + if (receiverType === UnknownType) { + return; + } + + if (receiverType.isNullable) { + accept('error', "The receiver can be null so a safe access must be used.", { + node, + code: CODE_MEMBER_ACCESS_MISSING_NULL_SAFETY, + }); + } + }; diff --git a/src/language/validation/safe-ds-validator.ts b/src/language/validation/safe-ds-validator.ts index 444240c2b..0fddf3313 100644 --- a/src/language/validation/safe-ds-validator.ts +++ b/src/language/validation/safe-ds-validator.ts @@ -71,6 +71,7 @@ import { callableTypeResultsMustNotBeAnnotated, lambdaParametersMustNotBeAnnotated, } from './other/declarations/annotationCalls.js'; +import {memberAccessMustBeNullSafeIfReceiverIsNullable} from "./other/expressions/memberAccesses.js"; /** * Register custom validation checks. @@ -119,7 +120,10 @@ export const registerValidationChecks = function (services: SafeDsServices) { SdsFunction: [functionMustContainUniqueNames, functionResultListShouldNotBeEmpty], SdsIndexedAccess: [indexedAccessesShouldBeUsedWithCaution], SdsLambda: [lambdaParametersMustNotBeAnnotated, lambdaParameterMustNotHaveConstModifier], - SdsMemberAccess: [memberAccessNullSafetyShouldBeNeeded(services)], + SdsMemberAccess: [ + memberAccessMustBeNullSafeIfReceiverIsNullable(services), + memberAccessNullSafetyShouldBeNeeded(services) + ], SdsModule: [moduleDeclarationsMustMatchFileKind, moduleWithDeclarationsMustStatePackage], SdsNamedType: [ namedTypeDeclarationShouldNotBeDeprecated(services), diff --git a/tests/resources/validation/other/expressions/member accesses/missing safe access/main.sdstest b/tests/resources/validation/other/expressions/member accesses/missing safe access/main.sdstest new file mode 100644 index 000000000..a16559d4c --- /dev/null +++ b/tests/resources/validation/other/expressions/member accesses/missing safe access/main.sdstest @@ -0,0 +1,40 @@ +package validation.other.expressions.memberAccesses.missingSafeAccess + +class MyClass() { + attr a: Int +} + +fun nullableMyClass() -> instance: MyClass? + +pipeline test { + // $TEST$ no error "The receiver can be null so a safe access must be used." + »1.a«(); + + // $TEST$ error "The receiver can be null so a safe access must be used." + »null.a«(); + + // $TEST$ no error "The receiver can be null so a safe access must be used." + »nullableMyClass().a«(); // This should be where the error is reported. Remove the "no" on the bug in Langium is fixed. + + // $TEST$ error "The receiver can be null so a safe access must be used." + nullableMyClass()».a«(); // Langium currently computes the wrong range for a member access + + // $TEST$ no error "The receiver can be null so a safe access must be used." + »unresolved.a«(); + + + // $TEST$ no error "The receiver can be null so a safe access must be used." + »1?.a«(); + + // $TEST$ no error "The receiver can be null so a safe access must be used." + »null?.a«(); + + // $TEST$ no error "The receiver can be null so a safe access must be used." + »nullableMyClass()?.a«(); + + // $TEST$ no error "The receiver can be null so a safe access must be used." + nullableMyClass()»?.a«(); // Langium currently computes the wrong range for a member access + + // $TEST$ no error "The receiver can be null so a safe access must be used." + »unresolved?.a«(); +} diff --git a/tests/resources/validation/style/unnecessary safe access/main.sdstest b/tests/resources/validation/style/unnecessary safe access/main.sdstest index 35154efb8..e84742fb0 100644 --- a/tests/resources/validation/style/unnecessary safe access/main.sdstest +++ b/tests/resources/validation/style/unnecessary safe access/main.sdstest @@ -1,22 +1,40 @@ package tests.validation.style.unnecessarySafeAccess -class MyClass +class MyClass { + attr a: Int +} fun nullableMyClass() -> result: MyClass? pipeline test { // $TEST$ info "The receiver is never null, so the safe access is unnecessary." - »1?.toString«(); + »1?.a«(); + + // $TEST$ no info "The receiver is never null, so the safe access is unnecessary." + »null?.a«(); + + // $TEST$ no info "The receiver is never null, so the safe access is unnecessary." + »nullableMyClass()?.a«(); + + // $TEST$ no info "The receiver is never null, so the safe access is unnecessary." + nullableMyClass()»?.a«(); // Langium currently computes the wrong range for a member access + + // $TEST$ no info "The receiver is never null, so the safe access is unnecessary." + »unresolved?.a«(); + + + // $TEST$ no info "The receiver is never null, so the safe access is unnecessary." + »1.a«(); // $TEST$ no info "The receiver is never null, so the safe access is unnecessary." - »null?.toString«(); + »null.a«(); // $TEST$ no info "The receiver is never null, so the safe access is unnecessary." - »nullableMyClass()?.toString«(); + »nullableMyClass().a«(); // $TEST$ no info "The receiver is never null, so the safe access is unnecessary." - nullableMyClass()»?.toString«(); // Langium currently computes the wrong range for a member access + nullableMyClass()».a«(); // Langium currently computes the wrong range for a member access // $TEST$ no info "The receiver is never null, so the safe access is unnecessary." - »unresolved?.toString«(); + »unresolved.a«(); } From eeda84526910d5d9d35f40660faf8e8ce21484c4 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Tue, 10 Oct 2023 15:33:03 +0200 Subject: [PATCH 2/3] feat: point to class members even if receiver of member access is nullable and the member access is not null safe --- src/language/scoping/safe-ds-scope-provider.ts | 3 --- .../to class members/instance attributes/main.sdstest | 6 +++--- .../to class members/instance methods/main.sdstest | 6 +++--- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/language/scoping/safe-ds-scope-provider.ts b/src/language/scoping/safe-ds-scope-provider.ts index 9c9ec45a2..1839a2d09 100644 --- a/src/language/scoping/safe-ds-scope-provider.ts +++ b/src/language/scoping/safe-ds-scope-provider.ts @@ -205,9 +205,6 @@ export class SafeDsScopeProvider extends DefaultScopeProvider { // Members let receiverType = this.typeComputer.computeType(node.receiver); - if (receiverType.isNullable && !node.isNullSafe) { - return resultScope; - } if (receiverType instanceof ClassType) { const ownInstanceMembers = classMembersOrEmpty(receiverType.sdsClass, (it) => !isStatic(it)); diff --git a/tests/resources/scoping/member accesses/to class members/instance attributes/main.sdstest b/tests/resources/scoping/member accesses/to class members/instance attributes/main.sdstest index f7699f7a9..4a5ddbcb2 100644 --- a/tests/resources/scoping/member accesses/to class members/instance attributes/main.sdstest +++ b/tests/resources/scoping/member accesses/to class members/instance attributes/main.sdstest @@ -87,6 +87,9 @@ pipeline myPipeline { // $TEST$ references declaredPreviouslyAsStaticMethod MyClass().»declaredPreviouslyAsStaticMethod«; + // $TEST$ references myInstanceAttribute + nullableMyClass().»myInstanceAttribute«; + // $TEST$ references myInstanceAttribute nullableMyClass()?.»myInstanceAttribute«; @@ -97,9 +100,6 @@ pipeline myPipeline { // $TEST$ unresolved AnotherClass().»myInstanceAttribute«; - // $TEST$ unresolved - nullableMyClass().»myInstanceAttribute«; - // $TEST$ unresolved unresolved.»myInstanceAttribute«; diff --git a/tests/resources/scoping/member accesses/to class members/instance methods/main.sdstest b/tests/resources/scoping/member accesses/to class members/instance methods/main.sdstest index 693bdbf49..109f228e3 100644 --- a/tests/resources/scoping/member accesses/to class members/instance methods/main.sdstest +++ b/tests/resources/scoping/member accesses/to class members/instance methods/main.sdstest @@ -87,6 +87,9 @@ pipeline myPipeline { // $TEST$ references declaredPreviouslyAsStaticMethod MyClass().»declaredPreviouslyAsStaticMethod«(); + // $TEST$ references myInstanceMethod + nullableMyClass().»myInstanceMethod«; + // $TEST$ references myInstanceMethod nullableMyClass()?.»myInstanceMethod«(); @@ -97,9 +100,6 @@ pipeline myPipeline { // $TEST$ unresolved AnotherClass().»myInstanceMethod«; - // $TEST$ unresolved - nullableMyClass().»myInstanceAttribute«; - // $TEST$ unresolved unresolved.»myInstanceMethod«; From bdb0a5abf73f710e489f5f6a88cf3f81dcfba42f Mon Sep 17 00:00:00 2001 From: megalinter-bot <129584137+megalinter-bot@users.noreply.github.com> Date: Tue, 10 Oct 2023 13:36:31 +0000 Subject: [PATCH 3/3] style: apply automated linter fixes --- .../other/expressions/memberAccesses.ts | 38 +++++++++---------- src/language/validation/safe-ds-validator.ts | 4 +- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/language/validation/other/expressions/memberAccesses.ts b/src/language/validation/other/expressions/memberAccesses.ts index 7cf5fc578..7a4d19066 100644 --- a/src/language/validation/other/expressions/memberAccesses.ts +++ b/src/language/validation/other/expressions/memberAccesses.ts @@ -1,26 +1,26 @@ -import {SafeDsServices} from "../../../safe-ds-module.js"; -import {SdsMemberAccess} from "../../../generated/ast.js"; -import {ValidationAcceptor} from "langium"; -import {UnknownType} from "../../../typing/model.js"; +import { SafeDsServices } from '../../../safe-ds-module.js'; +import { SdsMemberAccess } from '../../../generated/ast.js'; +import { ValidationAcceptor } from 'langium'; +import { UnknownType } from '../../../typing/model.js'; export const CODE_MEMBER_ACCESS_MISSING_NULL_SAFETY = 'member-access/missing-null-safety'; export const memberAccessMustBeNullSafeIfReceiverIsNullable = (services: SafeDsServices) => - (node: SdsMemberAccess, accept: ValidationAcceptor): void => { - if (node.isNullSafe) { - return; - } + (node: SdsMemberAccess, accept: ValidationAcceptor): void => { + if (node.isNullSafe) { + return; + } - const receiverType = services.types.TypeComputer.computeType(node.receiver); - if (receiverType === UnknownType) { - return; - } + const receiverType = services.types.TypeComputer.computeType(node.receiver); + if (receiverType === UnknownType) { + return; + } - if (receiverType.isNullable) { - accept('error', "The receiver can be null so a safe access must be used.", { - node, - code: CODE_MEMBER_ACCESS_MISSING_NULL_SAFETY, - }); - } - }; + if (receiverType.isNullable) { + accept('error', 'The receiver can be null so a safe access must be used.', { + node, + code: CODE_MEMBER_ACCESS_MISSING_NULL_SAFETY, + }); + } + }; diff --git a/src/language/validation/safe-ds-validator.ts b/src/language/validation/safe-ds-validator.ts index 0fddf3313..f6d837666 100644 --- a/src/language/validation/safe-ds-validator.ts +++ b/src/language/validation/safe-ds-validator.ts @@ -71,7 +71,7 @@ import { callableTypeResultsMustNotBeAnnotated, lambdaParametersMustNotBeAnnotated, } from './other/declarations/annotationCalls.js'; -import {memberAccessMustBeNullSafeIfReceiverIsNullable} from "./other/expressions/memberAccesses.js"; +import { memberAccessMustBeNullSafeIfReceiverIsNullable } from './other/expressions/memberAccesses.js'; /** * Register custom validation checks. @@ -122,7 +122,7 @@ export const registerValidationChecks = function (services: SafeDsServices) { SdsLambda: [lambdaParametersMustNotBeAnnotated, lambdaParameterMustNotHaveConstModifier], SdsMemberAccess: [ memberAccessMustBeNullSafeIfReceiverIsNullable(services), - memberAccessNullSafetyShouldBeNeeded(services) + memberAccessNullSafetyShouldBeNeeded(services), ], SdsModule: [moduleDeclarationsMustMatchFileKind, moduleWithDeclarationsMustStatePackage], SdsNamedType: [