feat: warn if parameters or placeholders are unused

src/language/helpers/safe-ds-node-mapper.ts

@@ -27,7 +27,7 @@ import {
     SdsYield,
 } from '../generated/ast.js';
 import { CallableType, StaticType } from '../typing/model.js';
-import { findLocalReferences, getContainerOfType } from 'langium';
+import { findLocalReferences, getContainerOfType, Stream, stream } from 'langium';
 import {
     abstractResultsOrEmpty,
     argumentsOrEmpty,
@@ -148,62 +148,59 @@ export class SafeDsNodeMapper {
     /**
      * Returns all references that target the given parameter.
      */
-    parameterToReferences(node: SdsParameter | undefined): SdsReference[] {
+    parameterToReferences(node: SdsParameter | undefined): Stream<SdsReference> {
         if (!node) {
-            return [];
+            return stream();
         }
 
         const containingCallable = getContainerOfType(node, isSdsCallable);
         /* c8 ignore start */
         if (!containingCallable) {
-            return [];
+            return stream();
         }
         /* c8 ignore stop */
 
         return findLocalReferences(node, containingCallable)
             .map((it) => it.$refNode?.astNode)
-            .filter(isSdsReference)
-            .toArray();
+            .filter(isSdsReference);
     }
 
     /**
      * Returns all references that target the given placeholder.
      */
-    placeholderToReferences(node: SdsPlaceholder | undefined): SdsReference[] {
+    placeholderToReferences(node: SdsPlaceholder | undefined): Stream<SdsReference> {
         if (!node) {
-            return [];
+            return stream();
         }
 
         const containingBlock = getContainerOfType(node, isSdsBlock);
         /* c8 ignore start */
         if (!containingBlock) {
-            return [];
+            return stream();
         }
         /* c8 ignore stop */
 
         return findLocalReferences(node, containingBlock)
             .map((it) => it.$refNode?.astNode)
-            .filter(isSdsReference)
-            .toArray();
+            .filter(isSdsReference);
     }
 
     /**
      * Returns all yields that assign to the given result.
      */
-    resultToYields(node: SdsResult | undefined): SdsYield[] {
+    resultToYields(node: SdsResult | undefined): Stream<SdsYield> {
         if (!node) {
-            return [];
+            return stream();
         }
 
         const containingSegment = getContainerOfType(node, isSdsSegment);
         if (!containingSegment) {
-            return [];
+            return stream();
         }
 
         return findLocalReferences(node, containingSegment)
             .map((it) => it.$refNode?.astNode)
-            .filter(isSdsYield)
-            .toArray();
+            .filter(isSdsYield);
     }
 
     /**

src/language/validation/other/declarations/parameters.ts

@@ -1,6 +1,9 @@
-import { SdsParameter } from '../../../generated/ast.js';
+import { SdsParameter, SdsSegment } from '../../../generated/ast.js';
 import { ValidationAcceptor } from 'langium';
+import { parametersOrEmpty } from '../../../helpers/nodeProperties.js';
+import { SafeDsServices } from '../../../safe-ds-module.js';
 
+export const CODE_PARAMETER_UNUSED = 'parameter/unused';
 export const CODE_PARAMETER_VARIADIC_AND_OPTIONAL = 'parameter/variadic-and-optional';
 
 export const parameterMustNotBeVariadicAndOptional = (node: SdsParameter, accept: ValidationAcceptor) => {
@@ -12,3 +15,18 @@ export const parameterMustNotBeVariadicAndOptional = (node: SdsParameter, accept
         });
     }
 };
+
+export const segmentParameterShouldBeUsed =
+    (services: SafeDsServices) => (node: SdsSegment, accept: ValidationAcceptor) => {
+        for (const parameter of parametersOrEmpty(node)) {
+            const usages = services.helpers.NodeMapper.parameterToReferences(parameter);
+
+            if (usages.isEmpty()) {
+                accept('warning', 'This parameter is unused and can be removed.', {
+                    node: parameter,
+                    property: 'name',
+                    code: CODE_PARAMETER_UNUSED,
+                });
+            }
+        }
+    };

src/language/validation/other/declarations/placeholders.ts

@@ -0,0 +1,29 @@
+import {isSdsBlock, isSdsStatement, SdsPlaceholder} from '../../../generated/ast.js';
+import {getContainerOfType, ValidationAcceptor} from 'langium';
+import { SafeDsServices } from '../../../safe-ds-module.js';
+import {statementsOrEmpty} from "../../../helpers/nodeProperties.js";
+import {last} from "radash";
+
+export const CODE_PLACEHOLDER_UNUSED = 'placeholder/unused';
+
+export const placeholderShouldBeUsed =
+    (services: SafeDsServices) => (node: SdsPlaceholder, accept: ValidationAcceptor) => {
+        const usages = services.helpers.NodeMapper.placeholderToReferences(node);
+        if (!usages.isEmpty()) {
+            return;
+        }
+
+        // Don't show a warning if the placeholder is declared in the last statement in the block
+        const containingStatement = getContainerOfType(node, isSdsStatement);
+        const containingBlock = getContainerOfType(containingStatement, isSdsBlock);
+        const allStatementsInBlock = statementsOrEmpty(containingBlock);
+        if (last(allStatementsInBlock) === containingStatement) {
+            return;
+        }
+
+        accept('warning', 'This placeholder is unused and can be removed.', {
+            node,
+            property: 'name',
+            code: CODE_PLACEHOLDER_UNUSED,
+        });
+    };

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

@@ -45,7 +45,7 @@ import { unionTypeMustHaveTypeArguments } from './other/types/unionTypes.js';
 import { callableTypeMustNotHaveOptionalParameters } from './other/types/callableTypes.js';
 import { typeArgumentListMustNotHavePositionalArgumentsAfterNamedArguments } from './other/types/typeArgumentLists.js';
 import { argumentListMustNotHavePositionalArgumentsAfterNamedArguments } from './other/argumentLists.js';
-import { parameterMustNotBeVariadicAndOptional } from './other/declarations/parameters.js';
+import {parameterMustNotBeVariadicAndOptional, segmentParameterShouldBeUsed} from './other/declarations/parameters.js';
 import { referenceTargetMustNotBeAnnotationPipelineOrSchema } from './other/expressions/references.js';
 import {
     annotationCallAnnotationShouldNotBeDeprecated,
@@ -61,6 +61,7 @@ import {
     namedTypeDeclarationShouldNotBeExperimental,
     referenceTargetShouldNotExperimental,
 } from './builtins/experimental.js';
+import {placeholderShouldBeUsed} from "./other/declarations/placeholders.js";
 
 /**
  * Register custom validation checks.
@@ -111,13 +112,20 @@ export const registerValidationChecks = function (services: SafeDsServices) {
             parameterListVariadicParameterMustBeLast,
         ],
         SdsPipeline: [pipelineMustContainUniqueNames],
+        SdsPlaceholder: [
+            placeholderShouldBeUsed(services),
+        ],
         SdsReference: [
             referenceTargetMustNotBeAnnotationPipelineOrSchema,
             referenceTargetShouldNotBeDeprecated(services),
             referenceTargetShouldNotExperimental(services),
         ],
         SdsResult: [resultMustHaveTypeHint],
-        SdsSegment: [segmentMustContainUniqueNames, segmentResultListShouldNotBeEmpty],
+        SdsSegment: [
+            segmentMustContainUniqueNames,
+            segmentParameterShouldBeUsed(services),
+            segmentResultListShouldNotBeEmpty
+        ],
         SdsTemplateString: [templateStringMustHaveExpressionBetweenTwoStringParts],
         SdsTypeArgumentList: [typeArgumentListMustNotHavePositionalArgumentsAfterNamedArguments],
         SdsTypeParameterConstraint: [typeParameterConstraintLeftOperandMustBeOwnTypeParameter],

tests/language/helpers/safe-ds-node-mapper/parameterToReferences.test.ts

@@ -15,7 +15,7 @@ describe('SafeDsNodeMapper', () => {
 
     describe('parameterToReferences', () => {
         it('should return an empty list if passed undefined', async () => {
-            expect(nodeMapper.parameterToReferences(undefined)).toStrictEqual([]);
+            expect(nodeMapper.parameterToReferences(undefined).toArray()).toStrictEqual([]);
         });
 
         it('should return references in default values', async () => {
@@ -24,7 +24,7 @@ describe('SafeDsNodeMapper', () => {
             `;
 
             const parameter = await getNodeOfType(services, code, isSdsParameter);
-            expect(nodeMapper.parameterToReferences(parameter)).toHaveLength(1);
+            expect(nodeMapper.parameterToReferences(parameter).toArray()).toHaveLength(1);
         });
 
         it('should return references directly in body', async () => {
@@ -36,7 +36,7 @@ describe('SafeDsNodeMapper', () => {
             `;
 
             const parameter = await getNodeOfType(services, code, isSdsParameter);
-            expect(nodeMapper.parameterToReferences(parameter)).toHaveLength(2);
+            expect(nodeMapper.parameterToReferences(parameter).toArray()).toHaveLength(2);
         });
 
         it('should return references nested in body', async () => {
@@ -50,7 +50,7 @@ describe('SafeDsNodeMapper', () => {
             `;
 
             const parameter = await getNodeOfType(services, code, isSdsParameter);
-            expect(nodeMapper.parameterToReferences(parameter)).toHaveLength(2);
+            expect(nodeMapper.parameterToReferences(parameter).toArray()).toHaveLength(2);
         });
 
         it('should return references in own parameter list', async () => {
@@ -59,7 +59,7 @@ describe('SafeDsNodeMapper', () => {
             `;
 
             const parameter = await getNodeOfType(services, code, isSdsParameter);
-            expect(nodeMapper.parameterToReferences(parameter)).toHaveLength(1);
+            expect(nodeMapper.parameterToReferences(parameter).toArray()).toHaveLength(1);
         });
 
         it('should return references in nested parameter list', async () => {
@@ -71,7 +71,7 @@ describe('SafeDsNodeMapper', () => {
             `;
 
             const parameter = await getNodeOfType(services, code, isSdsParameter);
-            expect(nodeMapper.parameterToReferences(parameter)).toHaveLength(2);
+            expect(nodeMapper.parameterToReferences(parameter).toArray()).toHaveLength(2);
         });
 
         it('should not return references to other parameters', async () => {
@@ -83,7 +83,7 @@ describe('SafeDsNodeMapper', () => {
             `;
 
             const parameter = await getNodeOfType(services, code, isSdsParameter);
-            expect(nodeMapper.parameterToReferences(parameter)).toHaveLength(1);
+            expect(nodeMapper.parameterToReferences(parameter).toArray()).toHaveLength(1);
         });
     });
 });

tests/language/helpers/safe-ds-node-mapper/placeholdersToReferences.test.ts

@@ -3,7 +3,7 @@ import { createSafeDsServices } from '../../../../src/language/safe-ds-module.js
 import { clearDocuments } from 'langium/test';
 import { EmptyFileSystem } from 'langium';
 import { getNodeOfType } from '../../../helpers/nodeFinder.js';
-import {isSdsParameter, isSdsPlaceholder} from '../../../../src/language/generated/ast.js';
+import { isSdsPlaceholder } from '../../../../src/language/generated/ast.js';
 
 const services = createSafeDsServices(EmptyFileSystem).SafeDs;
 const nodeMapper = services.helpers.NodeMapper;
@@ -15,7 +15,7 @@ describe('SafeDsNodeMapper', () => {
 
     describe('placeholderToReferences', () => {
         it('should return an empty list if passed undefined', async () => {
-            expect(nodeMapper.placeholderToReferences(undefined)).toStrictEqual([]);
+            expect(nodeMapper.placeholderToReferences(undefined).toArray()).toStrictEqual([]);
         });
 
         it('should return references in default values', async () => {
@@ -27,7 +27,7 @@ describe('SafeDsNodeMapper', () => {
             `;
 
             const placeholder = await getNodeOfType(services, code, isSdsPlaceholder);
-            expect(nodeMapper.placeholderToReferences(placeholder)).toHaveLength(1);
+            expect(nodeMapper.placeholderToReferences(placeholder).toArray()).toHaveLength(1);
         });
 
         it('should return references directly in body', async () => {
@@ -41,7 +41,7 @@ describe('SafeDsNodeMapper', () => {
             `;
 
             const placeholder = await getNodeOfType(services, code, isSdsPlaceholder);
-            expect(nodeMapper.placeholderToReferences(placeholder)).toHaveLength(2);
+            expect(nodeMapper.placeholderToReferences(placeholder).toArray()).toHaveLength(2);
         });
 
         it('should return references nested in body', async () => {
@@ -59,7 +59,7 @@ describe('SafeDsNodeMapper', () => {
             `;
 
             const placeholder = await getNodeOfType(services, code, isSdsPlaceholder);
-            expect(nodeMapper.placeholderToReferences(placeholder)).toHaveLength(2);
+            expect(nodeMapper.placeholderToReferences(placeholder).toArray()).toHaveLength(2);
         });
 
         it('should return references in nested parameter list', async () => {
@@ -73,7 +73,7 @@ describe('SafeDsNodeMapper', () => {
             `;
 
             const placeholder = await getNodeOfType(services, code, isSdsPlaceholder);
-            expect(nodeMapper.placeholderToReferences(placeholder)).toHaveLength(2);
+            expect(nodeMapper.placeholderToReferences(placeholder).toArray()).toHaveLength(2);
         });
 
         it('should not return references to other placeholders', async () => {
@@ -88,7 +88,7 @@ describe('SafeDsNodeMapper', () => {
             `;
 
             const placeholder = await getNodeOfType(services, code, isSdsPlaceholder);
-            expect(nodeMapper.placeholderToReferences(placeholder)).toHaveLength(1);
+            expect(nodeMapper.placeholderToReferences(placeholder).toArray()).toHaveLength(1);
         });
     });
 });

tests/language/helpers/safe-ds-node-mapper/resultToYields.test.ts

@@ -15,14 +15,14 @@ describe('SafeDsNodeMapper', () => {
 
     describe('resultToYields', () => {
         it('should return an empty list if passed undefined', async () => {
-            expect(nodeMapper.resultToYields(undefined)).toStrictEqual([]);
+            expect(nodeMapper.resultToYields(undefined).toArray()).toStrictEqual([]);
         });
 
         it('should return an empty list if result is not in a segment', async () => {
             const code = `fun myFunction() -> r1: Int`;
 
             const result = await getNodeOfType(services, code, isSdsResult);
-            expect(nodeMapper.resultToYields(result)).toStrictEqual([]);
+            expect(nodeMapper.resultToYields(result).toArray()).toStrictEqual([]);
         });
 
         it('should return all yields that refer to a result', async () => {
@@ -34,7 +34,7 @@ describe('SafeDsNodeMapper', () => {
             `;
 
             const result = await getNodeOfType(services, code, isSdsResult);
-            expect(nodeMapper.resultToYields(result)).toHaveLength(2);
+            expect(nodeMapper.resultToYields(result).toArray()).toHaveLength(2);
         });
 
         it('should not return yields that refer to another result', async () => {
@@ -47,7 +47,7 @@ describe('SafeDsNodeMapper', () => {
             `;
 
             const result = await getNodeOfType(services, code, isSdsResult);
-            expect(nodeMapper.resultToYields(result)).toHaveLength(1);
+            expect(nodeMapper.resultToYields(result).toArray()).toHaveLength(1);
         });
     });
 });

tests/resources/validation/other/declarations/parameter/unused/main.sdstest

@@ -0,0 +1,49 @@
+package tests.validation.declarations.parameters.unused
+
+segment mySegment(
+    // $TEST$ warning "This parameter is unused and can be removed."
+    »unused«: Int,
+    // $TEST$ no warning "This parameter is unused and can be removed."
+    »used«: Int
+) {
+    used;
+
+    /*
+     * Since we only allow lambdas as arguments, there signature is predefined, so no warning should be emitted.
+     */
+
+    (
+        // $TEST$ no warning "This parameter is unused and can be removed."
+        »unused«: Int,
+        // $TEST$ no warning "This parameter is unused and can be removed."
+        »used«: Int
+    ) {
+        used;
+    };
+
+    (
+        // $TEST$ no warning "This parameter is unused and can be removed."
+        »unused«: Int,
+        // $TEST$ no warning "This parameter is unused and can be removed."
+        »used«: Int
+    ) -> used;
+}
+
+// $TEST$ no warning "This parameter is unused and can be removed."
+annotation MyAnnotation(»unused«: Int)
+
+// $TEST$ no warning "This parameter is unused and can be removed."
+class MyClass(»unused«: Int)
+
+enum MyEnum {
+    // $TEST$ no warning "This parameter is unused and can be removed."
+    MyEnumVariant(»unused«: Int)
+}
+
+fun myFunction(
+    // $TEST$ no warning "This parameter is unused and can be removed."
+    »unused«: Int,
+
+    // $TEST$ no warning "This parameter is unused and can be removed."
+    myCallableType: (»unused«: Int) -> (),
+)