Skip to content

Commit

Permalink
feat: error if purity of functions is not specified (#768)
Browse files Browse the repository at this point in the history
Closes #731

### Summary of Changes

Show an error if
* a function is marked as `@Impure` and as `@Pure`,
* a function is neither marked as `@Impure` and `@Pure`,
* a function is marked as `@Impure` but not reasons are given.
  • Loading branch information
lars-reimann committed Nov 13, 2023
1 parent bd08ec1 commit a15b0af
Show file tree
Hide file tree
Showing 55 changed files with 644 additions and 305 deletions.
48 changes: 41 additions & 7 deletions packages/safe-ds-lang/src/language/builtins/safe-ds-annotations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@ import {
hasAnnotationCallOf,
} from '../helpers/nodeProperties.js';
import { SafeDsNodeMapper } from '../helpers/safe-ds-node-mapper.js';
import { EvaluatedEnumVariant, EvaluatedList, EvaluatedNode, StringConstant } from '../partialEvaluation/model.js';
import {
EvaluatedEnumVariant,
EvaluatedList,
EvaluatedNode,
StringConstant,
UnknownEvaluatedNode,
} from '../partialEvaluation/model.js';
import { SafeDsPartialEvaluator } from '../partialEvaluation/safe-ds-partial-evaluator.js';
import { SafeDsServices } from '../safe-ds-module.js';
import { SafeDsEnums } from './safe-ds-enums.js';
Expand Down Expand Up @@ -67,13 +73,29 @@ export class SafeDsAnnotations extends SafeDsModuleMembers<SdsAnnotation> {
return this.getAnnotation(IDE_INTEGRATION_URI, 'Expert');
}

getPythonCall(node: SdsFunction | undefined): string | undefined {
const value = this.getArgumentValue(node, this.PythonCall, 'callSpecification');
if (value instanceof StringConstant) {
return value.value;
} else {
return undefined;
isImpure(node: SdsFunction | undefined): boolean {
return hasAnnotationCallOf(node, this.Impure);
}

streamImpurityReasons(node: SdsFunction | undefined): Stream<SdsEnumVariant> {
// If allReasons are specified, but we could not evaluate them to a list, no reasons apply
const value = this.getArgumentValue(node, this.Impure, 'allReasons');
if (!(value instanceof EvaluatedList)) {
return EMPTY_STREAM;
}

// Otherwise, filter the elements of the list and keep only variants of the ImpurityReason enum
return stream(value.elements)
.filter(
(it) =>
it instanceof EvaluatedEnumVariant &&
getContainerOfType(it.variant, isSdsEnum) === this.builtinEnums.ImpurityReason,
)
.map((it) => (<EvaluatedEnumVariant>it).variant);
}

get Impure(): SdsAnnotation | undefined {
return this.getAnnotation(PURITY_URI, 'Impure');
}

isPure(node: SdsFunction | SdsParameter | undefined): boolean {
Expand All @@ -84,6 +106,15 @@ export class SafeDsAnnotations extends SafeDsModuleMembers<SdsAnnotation> {
return this.getAnnotation(PURITY_URI, 'Pure');
}

getPythonCall(node: SdsFunction | undefined): string | undefined {
const value = this.getArgumentValue(node, this.PythonCall, 'callSpecification');
if (value instanceof StringConstant) {
return value.value;
} else {
return undefined;
}
}

get PythonCall(): SdsAnnotation | undefined {
return this.getAnnotation(CODE_GENERATION_URI, 'PythonCall');
}
Expand Down Expand Up @@ -162,6 +193,9 @@ export class SafeDsAnnotations extends SafeDsModuleMembers<SdsAnnotation> {
parameterName: string,
): EvaluatedNode {
const annotationCall = findFirstAnnotationCallOf(node, annotation);
if (!annotationCall) {
return UnknownEvaluatedNode;
}

// Parameter is set explicitly
const argument = getArguments(annotationCall).find(
Expand Down
9 changes: 7 additions & 2 deletions packages/safe-ds-lang/src/language/builtins/safe-ds-enums.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
import { URI } from 'langium';
import { resourceNameToUri } from '../../helpers/resources.js';
import { isSdsEnum, SdsEnum } from '../generated/ast.js';
import { SafeDsModuleMembers } from './safe-ds-module-members.js';
import { resourceNameToUri } from '../../helpers/resources.js';
import { URI } from 'langium';

const ANNOTATION_USAGE_URI = resourceNameToUri('builtins/safeds/lang/annotationUsage.sdsstub');
const PURITY_URI = resourceNameToUri('builtins/safeds/lang/purity.sdsstub');

export class SafeDsEnums extends SafeDsModuleMembers<SdsEnum> {
get AnnotationTarget(): SdsEnum | undefined {
return this.getEnum(ANNOTATION_USAGE_URI, 'AnnotationTarget');
}

get ImpurityReason(): SdsEnum | undefined {
return this.getEnum(PURITY_URI, 'ImpurityReason');
}

private getEnum(uri: URI, name: string): SdsEnum | undefined {
return this.getModuleMember(uri, name, isSdsEnum);
}
Expand Down
121 changes: 121 additions & 0 deletions packages/safe-ds-lang/src/language/purity/model.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import { isSdsParameter, type SdsParameter } from '../generated/ast.js';
import { getQualifiedName } from '../helpers/nodeProperties.js';

/**
* A reason why a function is impure.
*/
export abstract class ImpurityReason {
/**
* Whether the impurity reason is a side effect.
*/
abstract isSideEffect: boolean;

/**
* Returns whether this impurity reason is equal to another object.
*/
abstract equals(other: unknown): boolean;

/**
* Returns a string representation of this impurity reason.
*/
abstract toString(): string;
}

/**
* The function reads from a file.
*
* @param path The path of the read file. This can either be a parameter or a constant string.
*/
export class FileRead extends ImpurityReason {
override isSideEffect = false;

constructor(readonly path: SdsParameter | string | undefined) {
super();
}

override equals(other: unknown): boolean {
return other instanceof FileRead && this.path === other.path;
}

override toString(): string {
if (isSdsParameter(this.path)) {
return `File read from ${getQualifiedName(this.path)}`;
} else if (typeof this.path === 'string') {
return `File read from "${this.path}"`;
} else {
return 'File read from ?';
}
}
}

/**
* The function writes to a file.
*
* @param path The path of the written file. This can either be a parameter or a constant string.
*/
export class FileWrite extends ImpurityReason {
override isSideEffect = true;

constructor(readonly path: SdsParameter | string | undefined) {
super();
}

override equals(other: unknown): boolean {
return other instanceof FileWrite && this.path === other.path;
}

override toString(): string {
if (isSdsParameter(this.path)) {
return `File write to ${getQualifiedName(this.path)}`;
} else if (typeof this.path === 'string') {
return `File write to "${this.path}"`;
} else {
return 'File write to ?';
}
}
}

/**
* The function calls a callable that is given as a parameter and that might be impure.
*
* @param parameter The parameter that is called.
*/
export class PotentiallyImpureParameterCall extends ImpurityReason {
override isSideEffect = true;

constructor(readonly parameter: SdsParameter | undefined) {
super();
}

override equals(other: unknown): boolean {
return other instanceof PotentiallyImpureParameterCall && this.parameter === other.parameter;
}

override toString(): string {
if (isSdsParameter(this.parameter)) {
return `Potentially impure call of ${getQualifiedName(this.parameter)}`;
} else {
return 'Potentially impure call of ?';
}
}
}

/**
* A function is impure due to some reason that is not covered by the other impurity reasons.
*/
class OtherImpurityReasonClass extends ImpurityReason {
override isSideEffect = true;

override equals(other: unknown): boolean {
return other instanceof OtherImpurityReasonClass;
}

override toString(): string {
return 'Other';
}
}

/**
* A function is impure due to some reason that is not covered by the other impurity reasons.
*/
export const OtherImpurityReason = new OtherImpurityReasonClass();
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export const annotationCallMustNotLackArgumentList = (node: SdsAnnotationCall, a
`The annotation '${node.annotation?.$refText}' has required parameters, so an argument list must be added.`,
{
node,
property: 'annotation',
code: CODE_ANNOTATION_CALL_MISSING_ARGUMENT_LIST,
},
);
Expand Down
40 changes: 40 additions & 0 deletions packages/safe-ds-lang/src/language/validation/purity.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { type ValidationAcceptor } from 'langium';
import type { SdsFunction } from '../generated/ast.js';
import { findFirstAnnotationCallOf } from '../helpers/nodeProperties.js';
import type { SafeDsServices } from '../safe-ds-module.js';

const CODE_PURITY_IMPURE_AND_PURE = 'purity/impure-and-pure';
const CODE_PURITY_MUST_BE_SPECIFIED = 'purity/must-be-specified';

export const functionPurityMustBeSpecified = (services: SafeDsServices) => {
const annotations = services.builtins.Annotations;

return (node: SdsFunction, accept: ValidationAcceptor) => {
if (annotations.isPure(node) && annotations.isImpure(node)) {
return accept('error', "'@Impure' and '@Pure' are mutually exclusive.", {
node,
property: 'name',
code: CODE_PURITY_IMPURE_AND_PURE,
});
} else if (!annotations.isImpure(node) && !annotations.isPure(node)) {
return accept(
'error',
"The purity of a function must be specified. Call the annotation '@Pure' or '@Impure'.",
{
node,
property: 'name',
code: CODE_PURITY_MUST_BE_SPECIFIED,
},
);
}

const impureAnnotationCall = findFirstAnnotationCallOf(node, annotations.Impure);
if (impureAnnotationCall && annotations.streamImpurityReasons(node).isEmpty()) {
accept('error', 'At least one impurity reason must be specified.', {
node: impureAnnotationCall,
property: 'annotation',
code: CODE_PURITY_MUST_BE_SPECIFIED,
});
}
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ import {
unionTypeMustHaveTypes,
unionTypeShouldNotHaveDuplicateTypes,
} from './other/types/unionTypes.js';
import { functionPurityMustBeSpecified } from './purity.js';
import {
annotationCallArgumentListShouldBeNeeded,
annotationParameterListShouldNotBeEmpty,
Expand Down Expand Up @@ -247,6 +248,7 @@ export const registerValidationChecks = function (services: SafeDsServices) {
SdsFunction: [
functionMustContainUniqueNames,
functionResultListShouldNotBeEmpty,
functionPurityMustBeSpecified(services),
pythonCallMustOnlyContainValidTemplateExpressions(services),
pythonNameMustNotBeSetIfPythonCallIsSet(services),
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class Any {
/**
* Returns a string representation of the object.
*/
@Pure
@PythonCall("str($this)")
fun toString() -> s: String
}
Expand Down
36 changes: 36 additions & 0 deletions packages/safe-ds-lang/tests/helpers/testDescription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,39 @@ export class TestDescriptionError extends Error {
this.stack = uri.toString();
}
}

/**
* Tests for an `equals` method.
*/
export interface EqualsTest<T> {
/**
* Produces the first value to compare, which must not be equal to {@link unequalValueOfSameType}.
*/
value: () => T;

/**
* Produces the second node to compare, which must not be equal to {@link value}. If the type is a singleton, leave
* this field `undefined`.
*/
unequalValueOfSameType?: () => T;

/**
* Produces a value of a different type.
*/
valueOfOtherType: () => T;
}

/**
* Tests for a `toString` method
*/
export interface ToStringTest<T> {
/**
* The value to convert to a string.
*/
value: T;

/**
* The expected string representation of the value.
*/
expectedString: string;
}
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
import { beforeAll, describe, it } from 'vitest';
import { listBuiltinFiles } from '../../../src/language/builtins/fileFinder.js';
import { uriToShortenedResourceName } from '../../../src/helpers/resources.js';
import { createSafeDsServices } from '../../../src/language/safe-ds-module.js';
import { AssertionError } from 'assert';
import { URI } from 'langium';
import { NodeFileSystem } from 'langium/node';
import { beforeAll, describe, it } from 'vitest';
import { Diagnostic, DiagnosticSeverity } from 'vscode-languageserver';
import { URI } from 'langium';
import { locationToString } from '../../helpers/location.js';
import { AssertionError } from 'assert';
import { isEmpty } from '../../../src/helpers/collectionUtils.js';
import { loadDocuments } from '../../helpers/testResources.js';
import { CODE_EXPERIMENTAL_LANGUAGE_FEATURE } from '../../../src/language/validation/experimentalLanguageFeatures.js';
import { uriToShortenedResourceName } from '../../../src/helpers/resources.js';
import { listBuiltinFiles } from '../../../src/language/builtins/fileFinder.js';
import { createSafeDsServices } from '../../../src/language/index.js';
import {
CODE_EXPERIMENTAL_ASSIGNED_RESULT,
CODE_EXPERIMENTAL_CALLED_ANNOTATION,
CODE_EXPERIMENTAL_CORRESPONDING_PARAMETER,
CODE_EXPERIMENTAL_REFERENCED_DECLARATION,
} from '../../../src/language/validation/builtins/experimental.js';
import { CODE_EXPERIMENTAL_LANGUAGE_FEATURE } from '../../../src/language/validation/experimentalLanguageFeatures.js';
import { locationToString } from '../../helpers/location.js';
import { loadDocuments } from '../../helpers/testResources.js';

const services = createSafeDsServices(NodeFileSystem).SafeDs;
const builtinFiles = listBuiltinFiles();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import { afterEach, describe, expect, it } from 'vitest';
import { createSafeDsServices } from '../../../src/language/safe-ds-module.js';
import { AssertionError } from 'assert';
import { AstNode, EmptyFileSystem } from 'langium';
import { clearDocuments } from 'langium/test';
import { getNodeOfType } from '../../helpers/nodeFinder.js';
import { AssertionError } from 'assert';
import { afterEach, describe, expect, it } from 'vitest';
import {
isSdsAnnotation,
isSdsAttribute,
Expand All @@ -15,6 +13,8 @@ import {
isSdsResult,
isSdsTypeParameter,
} from '../../../src/language/generated/ast.js';
import { createSafeDsServices } from '../../../src/language/index.js';
import { getNodeOfType } from '../../helpers/nodeFinder.js';

const services = createSafeDsServices(EmptyFileSystem).SafeDs;
const commentProvider = services.documentation.CommentProvider;
Expand Down
Loading

0 comments on commit a15b0af

Please sign in to comment.