feat: check for unnecessary null-safety

packages/safe-ds-lang/src/language/validation/safe-ds-validator.ts

@@ -144,14 +144,14 @@ import {
     annotationParameterShouldNotHaveConstModifier,
     assignmentShouldHaveMoreThanWildcardsAsAssignees,
     callArgumentListShouldBeNeeded,
+    chainedExpressionNullSafetyShouldBeNeeded,
     classBodyShouldNotBeEmpty,
     constraintListShouldNotBeEmpty,
     elvisOperatorShouldBeNeeded,
     enumBodyShouldNotBeEmpty,
     enumVariantParameterListShouldNotBeEmpty,
     functionResultListShouldNotBeEmpty,
     importedDeclarationAliasShouldDifferFromDeclarationName,
-    memberAccessNullSafetyShouldBeNeeded,
     namedTypeTypeArgumentListShouldBeNeeded,
     segmentResultListShouldNotBeEmpty,
     typeParameterListShouldNotBeEmpty,
@@ -238,7 +238,10 @@ export const registerValidationChecks = function (services: SafeDsServices) {
             callableTypeParameterMustNotHaveConstModifier,
             callableTypeResultsMustNotBeAnnotated,
         ],
-        SdsChainedExpression: [chainedExpressionsMustBeNullSafeIfReceiverIsNullable(services)],
+        SdsChainedExpression: [
+            chainedExpressionsMustBeNullSafeIfReceiverIsNullable(services),
+            chainedExpressionNullSafetyShouldBeNeeded(services),
+        ],
         SdsClass: [
             classMustContainUniqueNames,
             classMustOnlyInheritASingleClass(services),
@@ -294,10 +297,7 @@ export const registerValidationChecks = function (services: SafeDsServices) {
             literalTypeShouldNotHaveDuplicateLiteral(services),
         ],
         SdsMap: [mapMustNotContainNamedTuples(services), mapsShouldBeUsedWithCaution(services)],
-        SdsMemberAccess: [
-            memberAccessNullSafetyShouldBeNeeded(services),
-            memberAccessOfEnumVariantMustNotLackInstantiation,
-        ],
+        SdsMemberAccess: [memberAccessOfEnumVariantMustNotLackInstantiation],
         SdsModule: [
             moduleDeclarationsMustMatchFileKind,
             moduleMemberMustHaveNameThatIsUniqueInPackage(services),

packages/safe-ds-lang/src/language/validation/style.ts

@@ -1,20 +1,23 @@
 import { ValidationAcceptor } from 'langium';
 import { isEmpty } from '../../helpers/collections.js';
 import {
+    isSdsCall,
     isSdsEnumVariant,
+    isSdsIndexedAccess,
+    isSdsMemberAccess,
     isSdsWildcard,
     SdsAnnotation,
     SdsAnnotationCall,
     SdsAssignment,
     SdsCall,
+    SdsChainedExpression,
     SdsClassBody,
     SdsConstraintList,
     SdsEnumBody,
     SdsEnumVariant,
     SdsFunction,
     SdsImportedDeclaration,
     SdsInfixOperation,
-    SdsMemberAccess,
     SdsNamedType,
     SdsSegment,
     SdsTypeParameterList,
@@ -32,9 +35,9 @@ export const CODE_STYLE_UNNECESSARY_CONST_MODIFIER = 'style/unnecessary-const-mo
 export const CODE_STYLE_UNNECESSARY_CONSTRAINT_LIST = 'style/unnecessary-constraint-list';
 export const CODE_STYLE_UNNECESSARY_ELVIS_OPERATOR = 'style/unnecessary-elvis-operator';
 export const CODE_STYLE_UNNECESSARY_IMPORT_ALIAS = 'style/unnecessary-import-alias';
+export const CODE_STYLE_UNNECESSARY_NULL_SAFETY = 'style/unnecessary-null-safety';
 export const CODE_STYLE_UNNECESSARY_PARAMETER_LIST = 'style/unnecessary-parameter-list';
 export const CODE_STYLE_UNNECESSARY_RESULT_LIST = 'style/unnecessary-result-list';
-export const CODE_STYLE_UNNECESSARY_SAFE_ACCESS = 'style/unnecessary-safe-access';
 export const CODE_STYLE_UNNECESSARY_TYPE_ARGUMENT_LIST = 'style/unnecessary-type-argument-list';
 export const CODE_STYLE_UNNECESSARY_TYPE_PARAMETER_LIST = 'style/unnecessary-type-parameter-list';
 export const CODE_STYLE_UNNECESSARY_UNION_TYPE = 'style/unnecessary-union-type';
@@ -299,6 +302,43 @@ export const importedDeclarationAliasShouldDifferFromDeclarationName = (services
     };
 };
 
+// -----------------------------------------------------------------------------
+// Unnecessary null safety
+// -----------------------------------------------------------------------------
+
+export const chainedExpressionNullSafetyShouldBeNeeded = (services: SafeDsServices) => {
+    const settingsProvider = services.workspace.SettingsProvider;
+    const typeChecker = services.types.TypeChecker;
+    const typeComputer = services.types.TypeComputer;
+
+    return async (node: SdsChainedExpression, accept: ValidationAcceptor) => {
+        if (!(await settingsProvider.shouldValidateCodeStyle())) {
+            /* c8 ignore next 2 */
+            return;
+        }
+
+        if (!node.isNullSafe) {
+            return;
+        }
+
+        const receiverType = typeComputer.computeType(node.receiver);
+        if (receiverType === UnknownType) {
+            return;
+        }
+
+        if (
+            (isSdsCall(node) && !receiverType.isNullable && typeChecker.canBeCalled(receiverType)) ||
+            (isSdsIndexedAccess(node) && !receiverType.isNullable && typeChecker.canBeAccessedByIndex(receiverType)) ||
+            (isSdsMemberAccess(node) && !receiverType.isNullable)
+        ) {
+            accept('info', 'The receiver is never null, so null-safety is unnecessary.', {
+                node,
+                code: CODE_STYLE_UNNECESSARY_NULL_SAFETY,
+            });
+        }
+    };
+};
+
 // -----------------------------------------------------------------------------
 // Unnecessary parameter lists
 // -----------------------------------------------------------------------------
@@ -383,37 +423,6 @@ export const segmentResultListShouldNotBeEmpty = (services: SafeDsServices) => {
     };
 };
 
-// -----------------------------------------------------------------------------
-// Unnecessary safe access
-// -----------------------------------------------------------------------------
-
-export const memberAccessNullSafetyShouldBeNeeded = (services: SafeDsServices) => {
-    const settingsProvider = services.workspace.SettingsProvider;
-
-    return async (node: SdsMemberAccess, accept: ValidationAcceptor) => {
-        if (!(await settingsProvider.shouldValidateCodeStyle())) {
-            /* c8 ignore next 2 */
-            return;
-        }
-
-        if (!node.isNullSafe) {
-            return;
-        }
-
-        const receiverType = services.types.TypeComputer.computeType(node.receiver);
-        if (receiverType === UnknownType) {
-            return;
-        }
-
-        if (!receiverType.isNullable) {
-            accept('info', 'The receiver is never null, so the safe access is unnecessary.', {
-                node,
-                code: CODE_STYLE_UNNECESSARY_SAFE_ACCESS,
-            });
-        }
-    };
-};
-
 // -----------------------------------------------------------------------------
 // Unnecessary type argument lists
 // -----------------------------------------------------------------------------

packages/safe-ds-lang/tests/resources/validation/style/unnecessary null safety/main.sdstest

@@ -0,0 +1,130 @@
+package tests.validation.style.unnecessaryNullSafety
+
+class MyClass() {
+    attr a: Int
+    attr l: List<Int>
+    @Pure fun f()
+}
+
+segment call(
+    myClass: MyClass,
+    myClassOrNull: MyClass?,
+) {
+    val myFunction = myClass.f;
+    val myFunctionOrNull = myClassOrNull?.f;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »(() {})()«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »1()«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »null()«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »myFunction()«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »myFunctionOrNull()«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »unresolved()«;
+
+
+    // $TEST$ info "The receiver is never null, so null-safety is unnecessary."
+    »(() {})?()«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »1?()«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »null?()«;
+
+    // $TEST$ info "The receiver is never null, so null-safety is unnecessary."
+    »myFunction?()«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »myFunctionOrNull?()«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »unresolved?()«;
+}
+
+segment indexedAccess(
+    myClass: MyClass,
+    myClassOrNull: MyClass?,
+) {
+    val myList = myClass.l;
+    val myListOrNull = myClassOrNull?.l;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »[1][0]«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »1[0]«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »null[0]«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »myList[0]«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »myListOrNull[0]«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »unresolved[0]«;
+
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »1?[0]«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »null?[0]«;
+
+    // $TEST$ info "The receiver is never null, so null-safety is unnecessary."
+    »myList?[0]«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »myListOrNull?[0]«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »unresolved?[0]«;
+}
+
+segment memberAccess(
+    myClass: MyClass,
+    myClassOrNull: MyClass?,
+) {
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »1.a«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »null.a«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »myClass.a«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »myClassOrNull.a«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »unresolved.a«;
+
+
+    // $TEST$ info "The receiver is never null, so null-safety is unnecessary."
+    »1?.a«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »null?.a«;
+
+    // $TEST$ info "The receiver is never null, so null-safety is unnecessary."
+    »myClass?.a«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »myClassOrNull?.a«;
+
+    // $TEST$ no info "The receiver is never null, so null-safety is unnecessary."
+    »unresolved?.a«;
+}