diff --git a/packages/safe-ds-lang/src/language/flow/safe-ds-call-graph-computer.ts b/packages/safe-ds-lang/src/language/flow/safe-ds-call-graph-computer.ts index e70aecbaf..df45eb84c 100644 --- a/packages/safe-ds-lang/src/language/flow/safe-ds-call-graph-computer.ts +++ b/packages/safe-ds-lang/src/language/flow/safe-ds-call-graph-computer.ts @@ -61,6 +61,11 @@ export class SafeDsCallGraphComputer { */ private readonly callCache: WorkspaceCache; + /** + * Stores the call graph for the callable with the given ID if it is called without substitutions. + */ + private readonly callGraphCache: WorkspaceCache; + constructor(services: SafeDsServices) { this.astNodeLocator = services.workspace.AstNodeLocator; this.nodeMapper = services.helpers.NodeMapper; @@ -68,6 +73,7 @@ export class SafeDsCallGraphComputer { this.typeComputer = services.types.TypeComputer; this.callCache = new WorkspaceCache(services.shared); + this.callGraphCache = new WorkspaceCache(services.shared); } /** @@ -98,6 +104,22 @@ export class SafeDsCallGraphComputer { * of any containing callables, i.e. the context of the call/callable. */ getCallGraph(node: SdsCall | SdsCallable, substitutions: ParameterSubstitutions = NO_SUBSTITUTIONS): CallGraph { + // Cache the result if no substitutions are given + if (isEmpty(substitutions)) { + const key = this.getNodeId(node); + return this.callGraphCache.get(key, () => { + return this.doGetCallGraph(node, substitutions); + }); + } else { + /* c8 ignore next 2 */ + return this.doGetCallGraph(node, substitutions); + } + } + + private doGetCallGraph( + node: SdsCall | SdsCallable, + substitutions: ParameterSubstitutions = NO_SUBSTITUTIONS, + ): CallGraph { if (isSdsCall(node)) { const call = this.createSyntheticCallForCall(node, substitutions); return this.getCallGraphWithRecursionCheck(call, []); diff --git a/packages/safe-ds-lang/src/language/validation/other/statements/statements.ts b/packages/safe-ds-lang/src/language/validation/other/statements/statements.ts new file mode 100644 index 000000000..49ac06913 --- /dev/null +++ b/packages/safe-ds-lang/src/language/validation/other/statements/statements.ts @@ -0,0 +1,57 @@ +import { + isSdsAssignment, + isSdsExpressionStatement, + isSdsWildcard, + SdsExpression, + SdsStatement, +} from '../../../generated/ast.js'; +import { ValidationAcceptor } from 'langium'; +import { SafeDsServices } from '../../../safe-ds-module.js'; +import { getAssignees } from '../../../helpers/nodeProperties.js'; + +export const CODE_STATEMENT_HAS_NO_EFFECT = 'statement/has-no-effect'; + +export const statementMustDoSomething = (services: SafeDsServices) => { + const statementDoesSomething = statementDoesSomethingProvider(services); + + return (node: SdsStatement, accept: ValidationAcceptor): void => { + if (!statementDoesSomething(node)) { + accept('warning', 'This statement does nothing.', { + node, + code: CODE_STATEMENT_HAS_NO_EFFECT, + }); + } + }; +}; + +const statementDoesSomethingProvider = (services: SafeDsServices) => { + const statementExpressionDoesSomething = statementExpressionDoesSomethingProvider(services); + + return (node: SdsStatement): boolean => { + if (isSdsAssignment(node)) { + return !getAssignees(node).every(isSdsWildcard) || statementExpressionDoesSomething(node.expression); + } else if (isSdsExpressionStatement(node)) { + return statementExpressionDoesSomething(node.expression); + } else { + /* c8 ignore next 2 */ + return false; + } + }; +}; + +const statementExpressionDoesSomethingProvider = (services: SafeDsServices) => { + const callGraphComputer = services.flow.CallGraphComputer; + const purityComputer = services.purity.PurityComputer; + + return (node: SdsExpression | undefined): boolean => { + if (!node) { + /* c8 ignore next 2 */ + return false; + } + + return ( + callGraphComputer.getAllContainedCalls(node).some((it) => callGraphComputer.isRecursive(it)) || + !purityComputer.isPureExpression(node) + ); + }; +}; 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 index de3e35116..b7ef7b5d5 100644 --- a/packages/safe-ds-lang/src/language/validation/safe-ds-validator.ts +++ b/packages/safe-ds-lang/src/language/validation/safe-ds-validator.ts @@ -175,6 +175,7 @@ import { resultMustHaveTypeHint, yieldTypeMustMatchResultType, } from './types.js'; +import { statementMustDoSomething } from './other/statements/statements.js'; /** * Register custom validation checks. @@ -339,6 +340,7 @@ export const registerValidationChecks = function (services: SafeDsServices) { segmentResultListShouldNotBeEmpty, segmentShouldBeUsed(services), ], + SdsStatement: [statementMustDoSomething(services)], SdsTemplateString: [templateStringMustHaveExpressionBetweenTwoStringParts], SdsTypeArgumentList: [typeArgumentListsShouldBeUsedWithCaution], SdsTypeParameter: [ diff --git a/packages/safe-ds-lang/tests/resources/validation/skip-old/assignments/hasNoEffect.sdstest b/packages/safe-ds-lang/tests/resources/validation/other/statements/assignments/has no effect/main.sdstest similarity index 83% rename from packages/safe-ds-lang/tests/resources/validation/skip-old/assignments/hasNoEffect.sdstest rename to packages/safe-ds-lang/tests/resources/validation/other/statements/assignments/has no effect/main.sdstest index 729ecb48d..fa35045cc 100644 --- a/packages/safe-ds-lang/tests/resources/validation/skip-old/assignments/hasNoEffect.sdstest +++ b/packages/safe-ds-lang/tests/resources/validation/other/statements/assignments/has no effect/main.sdstest @@ -1,4 +1,6 @@ -step myFunction() -> a: Int { +package tests.validation.other.statements.assignments.hasNoEffect + +segment mySegment() -> a: Int { // $TEST$ warning "This statement does nothing." »_ = 1 + 2;« diff --git a/packages/safe-ds-lang/tests/resources/validation/skip-old/expressionStatements/hasNoEffect.sdstest b/packages/safe-ds-lang/tests/resources/validation/other/statements/expression statements/has no effect/hasNoEffect.sdstest similarity index 82% rename from packages/safe-ds-lang/tests/resources/validation/skip-old/expressionStatements/hasNoEffect.sdstest rename to packages/safe-ds-lang/tests/resources/validation/other/statements/expression statements/has no effect/hasNoEffect.sdstest index 9e2e40079..640d48025 100644 --- a/packages/safe-ds-lang/tests/resources/validation/skip-old/expressionStatements/hasNoEffect.sdstest +++ b/packages/safe-ds-lang/tests/resources/validation/other/statements/expression statements/has no effect/hasNoEffect.sdstest @@ -1,30 +1,35 @@ -package tests.validation.statements.expressionStatements.hasNoEffect +package tests.validation.other.statements.expressionStatements.hasNoEffect +@Impure([ImpurityReason.Other]) fun impureFunction() -@Pure fun pureFunction() -> a: Int + +@Pure +fun pureFunction() -> a: Int class MyClass() { + @Impure([ImpurityReason.Other]) fun impureFunction() + @Pure fun pureFunction() } -step pureStep() { +segment pureSegment() { val a = pureFunction(); } -step impureStep() { +segment impureSegment() { impureFunction(); } -step recursiveA() { +segment recursiveA() { recursiveB(); } -step recursiveB() { +segment recursiveB() { recursiveA(); } -step myStep() { +segment mySegment() { // $TEST$ warning "This statement does nothing." »1 + 2;« // $TEST$ warning "This statement does nothing." @@ -51,15 +56,18 @@ step myStep() { »MyClass().impureFunction();« }; + // $TEST$ warning "This statement does nothing." + »pureSegment();« + + // $TEST$ no warning "This statement does nothing." + »impureSegment();« + // $TEST$ warning "This statement does nothing." »(() { pureFunction(); MyClass().pureFunction(); })();« - // $TEST$ warning "This statement does nothing." - »pureStep();« - // $TEST$ no warning "This statement does nothing." »(() { impureFunction(); @@ -70,9 +78,6 @@ step myStep() { MyClass().impureFunction(); })();« - // $TEST$ no warning "This statement does nothing." - »impureStep();« - // $TEST$ no warning "This statement does nothing." »recursiveA();« } diff --git a/packages/safe-ds-lang/tests/resources/validation/skip-old/staticAnalysis/sideEffects.sdstest b/packages/safe-ds-lang/tests/resources/validation/skip-old/staticAnalysis/sideEffects.sdstest deleted file mode 100644 index 38ea9255a..000000000 --- a/packages/safe-ds-lang/tests/resources/validation/skip-old/staticAnalysis/sideEffects.sdstest +++ /dev/null @@ -1,147 +0,0 @@ -package tests.staticAnalysis.sideEffects - -// Positive examples ----------------------------------------------------------- - -annotation ShouldHaveNoSideEffects - -// Call to class constructor - -class C() - -@ShouldHaveNoSideEffects -step callOfClassConstructor() { - C(); -} - -// Call to enum variant constructor - -enum MyEnum { - Variant -} - -@ShouldHaveNoSideEffects -step callOfEnumVariantConstructor() { - MyEnum.Variant(); -} - -// Function without side effects - -@Pure -fun pureFunction() - -@NoSideEffects -fun functionWithoutSideEffects() - -@ShouldHaveNoSideEffects -step callToPureFunction() { - pureFunction(); - functionWithoutSideEffects(); -} - -// Lambdas without side effects - -@ShouldHaveNoSideEffects -step callToPureLambdas() { - (() {})(); - (() -> null)(); - - () { - (() {})(); - }; - - () -> (() -> null)(); -} - -// Steps without side effects - -step pureStep() {} - -@ShouldHaveNoSideEffects -step callToPureSteps() { - pureStep(); -} - -// Uncalled lambdas - -step pureStepWithUncalledLambdas() { - () -> impureFunction(); -} - -@ShouldHaveNoSideEffects -step uncalledLambdas() { - pureStepWithUncalledLambdas(); -} - -// Function as result - -@ShouldHaveNoSideEffects -step pureFunctionAsResult() { - (() -> pureFunction)()(); -} - -// Negative examples ----------------------------------------------------------- - -annotation ShouldHaveSideEffects - -// Callable type - -@ShouldHaveSideEffects -step callToCallableType(f: () -> ()) { - f(); -} - -// Function with side effects - -fun impureFunction() - -@ShouldHaveSideEffects -step callToImpureFunction() { - impureFunction(); -} - -// Lambdas with side effects - -@ShouldHaveSideEffects -step callToImpureLambdas() { - (() { impureFunction(); })(); - (() -> impureFunction())(); - - () { - (() { impureFunction(); })(); - }; - - () -> (() -> impureFunction())(); -} - -// Steps with side effects - -step impureStep() { - impureFunction(); -} - -@ShouldHaveSideEffects -step callToImpureSteps() { - impureStep(); -} - -// Recursion - -@ShouldHaveSideEffects -step recursion() { - recursion(); -} - -// Unresolved callable - -@ShouldHaveSideEffects -step unresolvedCallable() { - unresolved(); -} - -// Function as parameter - -@ShouldHaveSideEffects -step impureFunctionAsParameter() { - ((f) -> f())(pureFunction); // This is actually pure, but we match in a conservative manner. Can be improved later. - ((f) -> f())(impureFunction); -}