feat: mark unused internal/private segments as unnecessary (#710)

Closes #682 ### Summary of Changes Show a warning if internal/private segments are unused and mark them as unnecessary.
Safe-DS · Oct 30, 2023 · 3ba8698 · 3ba8698
1 parent 9d342e4
commit 3ba8698
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 7 deletions.
diff --git a/packages/safe-ds-lang/src/language/validation/other/declarations/segments.ts b/packages/safe-ds-lang/src/language/validation/other/declarations/segments.ts
@@ -6,13 +6,16 @@ import { DiagnosticTag } from 'vscode-languageserver';
 
 export const CODE_SEGMENT_DUPLICATE_YIELD = 'segment/duplicate-yield';
 export const CODE_SEGMENT_UNASSIGNED_RESULT = 'segment/unassigned-result';
+export const CODE_SEGMENT_UNUSED = 'segment/unused';
 export const CODE_SEGMENT_UNUSED_PARAMETER = 'segment/unused-parameter';
 
-export const segmentResultMustBeAssignedExactlyOnce =
-    (services: SafeDsServices) => (node: SdsSegment, accept: ValidationAcceptor) => {
+export const segmentResultMustBeAssignedExactlyOnce = (services: SafeDsServices) => {
+    const nodeMapper = services.helpers.NodeMapper;
+
+    return (node: SdsSegment, accept: ValidationAcceptor) => {
         const results = getResults(node.resultList);
         for (const result of results) {
-            const yields = services.helpers.NodeMapper.resultToYields(result);
+            const yields = nodeMapper.resultToYields(result);
             if (yields.isEmpty()) {
                 accept('error', 'Nothing is assigned to this result.', {
                     node: result,
@@ -32,11 +35,35 @@ export const segmentResultMustBeAssignedExactlyOnce =
             }
         }
     };
+};
+
+export const segmentShouldBeUsed = (services: SafeDsServices) => {
+    const referenceProvider = services.references.References;
+
+    return (node: SdsSegment, accept: ValidationAcceptor) => {
+        // Don't show this warning for public segments
+        if (node.visibility === undefined) {
+            return;
+        }
+
+        const references = referenceProvider.findReferences(node, {});
+        if (references.isEmpty()) {
+            accept('warning', 'This segment is unused and can be removed.', {
+                node,
+                property: 'name',
+                code: CODE_SEGMENT_UNUSED,
+                tags: [DiagnosticTag.Unnecessary],
+            });
+        }
+    };
+};
+
+export const segmentParameterShouldBeUsed = (services: SafeDsServices) => {
+    const nodeMapper = services.helpers.NodeMapper;
 
-export const segmentParameterShouldBeUsed =
-    (services: SafeDsServices) => (node: SdsSegment, accept: ValidationAcceptor) => {
+    return (node: SdsSegment, accept: ValidationAcceptor) => {
         for (const parameter of getParameters(node)) {
-            const usages = services.helpers.NodeMapper.parameterToReferences(parameter);
+            const usages = nodeMapper.parameterToReferences(parameter);
 
             if (usages.isEmpty()) {
                 accept('warning', 'This parameter is unused and can be removed.', {
@@ -48,3 +75,4 @@ export const segmentParameterShouldBeUsed =
             }
         }
     };
+};
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
@@ -93,7 +93,11 @@ import {
     referenceTargetShouldNotExperimental,
 } from './builtins/experimental.js';
 import { placeholderShouldBeUsed, placeholdersMustNotBeAnAlias } from './other/declarations/placeholders.js';
-import { segmentParameterShouldBeUsed, segmentResultMustBeAssignedExactlyOnce } from './other/declarations/segments.js';
+import {
+    segmentParameterShouldBeUsed,
+    segmentResultMustBeAssignedExactlyOnce,
+    segmentShouldBeUsed,
+} from './other/declarations/segments.js';
 import {
     lambdaMustBeAssignedToTypedParameter,
     lambdaParameterMustNotHaveConstModifier,
@@ -282,6 +286,7 @@ export const registerValidationChecks = function (services: SafeDsServices) {
             segmentParameterShouldBeUsed(services),
             segmentResultMustBeAssignedExactlyOnce(services),
             segmentResultListShouldNotBeEmpty,
+            segmentShouldBeUsed(services),
         ],
         SdsTemplateString: [templateStringMustHaveExpressionBetweenTwoStringParts],
         SdsTypeParameter: [typeParameterMustHaveSufficientContext],

diff --git a/...s/safe-ds-lang/tests/resources/validation/other/declarations/segments/unused/main.sdstest b/...s/safe-ds-lang/tests/resources/validation/other/declarations/segments/unused/main.sdstest
@@ -0,0 +1,17 @@
+package tests.validation.other.declarations.segments.unused
+
+// $TEST$ warning "This segment is unused and can be removed."
+private segment »myUnusedPrivateSegment«() {}
+
+// $TEST$ no warning "This segment is unused and can be removed."
+private segment »myUsedPrivateSegment«() {}
+
+// $TEST$ warning "This segment is unused and can be removed."
+internal segment »myUnusedInternalSegment«() {}
+
+// $TEST$ no warning "This segment is unused and can be removed."
+internal segment »myUsedInternalSegment«() {}
+
+pipeline myPipeline1 {
+    myUsedPrivateSegment();
+}
diff --git a/...s-lang/tests/resources/validation/other/declarations/segments/unused/same package.sdstest b/...s-lang/tests/resources/validation/other/declarations/segments/unused/same package.sdstest
@@ -0,0 +1,5 @@
+package tests.validation.other.declarations.segments.unused
+
+pipeline myPipeline2 {
+    myUsedInternalSegment();
+}