Skip to content

Commit

Permalink
feat: various checks for calls (#638)
Browse files Browse the repository at this point in the history
Closes partially #543

### Summary of Changes

* Warn if results of a call are ignored implicitly in an assignment
* Error if receiver of a call is not callable
* Error if receiver of a call points to a class without a constructor

---------

Co-authored-by: megalinter-bot <129584137+megalinter-bot@users.noreply.github.com>
  • Loading branch information
lars-reimann and megalinter-bot authored Oct 12, 2023
1 parent 83936b8 commit e0fa032
Show file tree
Hide file tree
Showing 20 changed files with 491 additions and 150 deletions.
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@
],
"configurationDefaults": {
"[safe-ds]": {
"editor.wordSeparators": "`~!@#$%^&*()-=+[]{}\\|;:'\",.<>/?»«"
"editor.wordSeparators": "`~!@#$%^&*()-=+[]{}\\|;:'\",.<>/?»«",
"files.trimTrailingWhitespace": true
}
}
},
Expand Down
12 changes: 7 additions & 5 deletions src/language/builtins/safe-ds-annotations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,39 +4,41 @@ import { SafeDsModuleMembers } from './safe-ds-module-members.js';
import { resourceNameToUri } from '../../helpers/resources.js';
import { URI } from 'langium';

const CORE_ANNOTATIONS_URI = resourceNameToUri('builtins/safeds/lang/coreAnnotations.sdsstub');
const ANNOTATION_USAGE_URI = resourceNameToUri('builtins/safeds/lang/annotationUsage.sdsstub');
const IDE_INTEGRATION_URI = resourceNameToUri('builtins/safeds/lang/ideIntegration.sdsstub');
const MATURITY_URI = resourceNameToUri('builtins/safeds/lang/maturity.sdsstub');

export class SafeDsAnnotations extends SafeDsModuleMembers<SdsAnnotation> {
isDeprecated(node: SdsAnnotatedObject | undefined): boolean {
return this.hasAnnotationCallOf(node, this.Deprecated);
}

private get Deprecated(): SdsAnnotation | undefined {
return this.getAnnotation(CORE_ANNOTATIONS_URI, 'Deprecated');
return this.getAnnotation(MATURITY_URI, 'Deprecated');
}

isExperimental(node: SdsAnnotatedObject | undefined): boolean {
return this.hasAnnotationCallOf(node, this.Experimental);
}

private get Experimental(): SdsAnnotation | undefined {
return this.getAnnotation(CORE_ANNOTATIONS_URI, 'Experimental');
return this.getAnnotation(MATURITY_URI, 'Experimental');
}

isExpert(node: SdsParameter | undefined): boolean {
return this.hasAnnotationCallOf(node, this.Expert);
}

private get Expert(): SdsAnnotation | undefined {
return this.getAnnotation(CORE_ANNOTATIONS_URI, 'Expert');
return this.getAnnotation(IDE_INTEGRATION_URI, 'Expert');
}

isRepeatable(node: SdsAnnotation | undefined): boolean {
return this.hasAnnotationCallOf(node, this.Repeatable);
}

private get Repeatable(): SdsAnnotation | undefined {
return this.getAnnotation(CORE_ANNOTATIONS_URI, 'Repeatable');
return this.getAnnotation(ANNOTATION_USAGE_URI, 'Repeatable');
}

private hasAnnotationCallOf(node: SdsAnnotatedObject | undefined, expected: SdsAnnotation | undefined): boolean {
Expand Down
13 changes: 12 additions & 1 deletion src/language/helpers/safe-ds-node-mapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {
isSdsBlock,
isSdsCall,
isSdsCallable,
isSdsClass,
isSdsEnumVariant,
isSdsNamedType,
isSdsReference,
isSdsSegment,
Expand Down Expand Up @@ -108,8 +110,17 @@ export class SafeDsNodeMapper {
}
}

// If the RHS is a call, the assignee gets the corresponding result
// If the RHS instantiates a class or enum variant, the first assignee gets the entire RHS
const callable = this.callToCallableOrUndefined(expression);
if (isSdsClass(callable) || isSdsEnumVariant(callable)) {
if (assigneePosition === 0) {
return expression;
} else {
return undefined;
}
}

// Otherwise, the assignee gets the result at the same position
const abstractResults = abstractResultsOrEmpty(callable);
return abstractResults[assigneePosition];
}
Expand Down
19 changes: 11 additions & 8 deletions src/language/typing/model.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import {
isSdsNull,
SdsAbstractResult,
SdsCallable,
SdsClass,
SdsDeclaration,
SdsEnum,
SdsEnumVariant,
SdsLiteral,
SdsParameter,
} from '../generated/ast.js';

/* c8 ignore start */
Expand All @@ -28,8 +30,8 @@ export class CallableType extends Type {

constructor(
readonly sdsCallable: SdsCallable,
readonly inputType: NamedTupleType,
readonly outputType: NamedTupleType,
readonly inputType: NamedTupleType<SdsParameter>,
readonly outputType: NamedTupleType<SdsAbstractResult>,
) {
super();
}
Expand Down Expand Up @@ -99,10 +101,10 @@ export class LiteralType extends Type {
}
}

export class NamedTupleType extends Type {
export class NamedTupleType<T extends SdsDeclaration> extends Type {
override readonly isNullable = false;

constructor(readonly entries: NamedTupleEntry[]) {
constructor(readonly entries: NamedTupleEntry<T>[]) {
super();
}

Expand All @@ -125,7 +127,7 @@ export class NamedTupleType extends Type {
return this;
}

override copyWithNullability(_isNullable: boolean): NamedTupleType {
override copyWithNullability(_isNullable: boolean): NamedTupleType<T> {
return this;
}

Expand Down Expand Up @@ -159,8 +161,9 @@ export class NamedTupleType extends Type {
}
}

export class NamedTupleEntry {
export class NamedTupleEntry<T extends SdsDeclaration> {
constructor(
readonly sdsDeclaration: T | undefined,
readonly name: string,
readonly type: Type,
) {}
Expand All @@ -169,8 +172,8 @@ export class NamedTupleEntry {
return `${this.name}: ${this.type}`;
}

equals(other: NamedTupleEntry): boolean {
return this.name === other.name && this.type.equals(other.type);
equals(other: NamedTupleEntry<SdsDeclaration>): boolean {
return this.sdsDeclaration === other.sdsDeclaration && this.name === other.name && this.type.equals(other.type);
}
}

Expand Down
17 changes: 10 additions & 7 deletions src/language/typing/safe-ds-type-computer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import {
isSdsTypeProjection,
isSdsUnionType,
isSdsYield,
SdsAbstractResult,
SdsAssignee,
SdsCall,
SdsCallableType,
Expand Down Expand Up @@ -164,7 +165,7 @@ export class SafeDsTypeComputer {
private computeTypeOfDeclaration(node: SdsDeclaration): Type {
if (isSdsAnnotation(node)) {
const parameterEntries = parametersOrEmpty(node).map(
(it) => new NamedTupleEntry(it.name, this.computeType(it.type)),
(it) => new NamedTupleEntry(it, it.name, this.computeType(it.type)),
);

return new CallableType(node, new NamedTupleType(parameterEntries), new NamedTupleType([]));
Expand Down Expand Up @@ -193,10 +194,10 @@ export class SafeDsTypeComputer {

private computeTypeOfCallableWithManifestTypes(node: SdsFunction | SdsSegment | SdsCallableType): Type {
const parameterEntries = parametersOrEmpty(node).map(
(it) => new NamedTupleEntry(it.name, this.computeType(it.type)),
(it) => new NamedTupleEntry(it, it.name, this.computeType(it.type)),
);
const resultEntries = resultsOrEmpty(node.resultList).map(
(it) => new NamedTupleEntry(it.name, this.computeType(it.type)),
(it) => new NamedTupleEntry(it, it.name, this.computeType(it.type)),
);

return new CallableType(node, new NamedTupleType(parameterEntries), new NamedTupleType(resultEntries));
Expand Down Expand Up @@ -278,18 +279,20 @@ export class SafeDsTypeComputer {
return this.computeTypeOfCall(node);
} else if (isSdsBlockLambda(node)) {
const parameterEntries = parametersOrEmpty(node).map(
(it) => new NamedTupleEntry(it.name, this.computeType(it)),
(it) => new NamedTupleEntry(it, it.name, this.computeType(it)),
);
const resultEntries = blockLambdaResultsOrEmpty(node).map(
(it) => new NamedTupleEntry(it.name, this.computeType(it)),
(it) => new NamedTupleEntry(it, it.name, this.computeType(it)),
);

return new CallableType(node, new NamedTupleType(parameterEntries), new NamedTupleType(resultEntries));
} else if (isSdsExpressionLambda(node)) {
const parameterEntries = parametersOrEmpty(node).map(
(it) => new NamedTupleEntry(it.name, this.computeType(it)),
(it) => new NamedTupleEntry(it, it.name, this.computeType(it)),
);
const resultEntries = [new NamedTupleEntry('result', this.computeType(node.result))];
const resultEntries = [
new NamedTupleEntry<SdsAbstractResult>(undefined, 'result', this.computeType(node.result)),
];

return new CallableType(node, new NamedTupleType(parameterEntries), new NamedTupleType(resultEntries));
} else if (isSdsIndexedAccess(node)) {
Expand Down
34 changes: 32 additions & 2 deletions src/language/validation/other/statements/assignments.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { isSdsPipeline, SdsAssignment, SdsYield } from '../../../generated/ast.js';
import { isSdsCall, isSdsPipeline, SdsAssignment, SdsYield } from '../../../generated/ast.js';
import { getContainerOfType, ValidationAcceptor } from 'langium';
import { SafeDsServices } from '../../../safe-ds-module.js';
import { assigneesOrEmpty } from '../../../helpers/nodeProperties.js';
import { abstractResultsOrEmpty, assigneesOrEmpty } from '../../../helpers/nodeProperties.js';
import { pluralize } from '../../../helpers/stringUtils.js';

export const CODE_ASSIGNMENT_IMPLICITLY_IGNORED_RESULT = 'assignment/implicitly-ignored-result';
export const CODE_ASSIGMENT_NOTHING_ASSIGNED = 'assignment/nothing-assigned';
export const CODE_ASSIGMENT_YIELD_FORBIDDEN_IN_PIPELINE = 'assignment/yield-forbidden-in-pipeline';

Expand All @@ -19,6 +21,34 @@ export const assignmentAssigneeMustGetValue =
}
};

export const assignmentShouldNotImplicitlyIgnoreResult = (services: SafeDsServices) => {
const nodeMapper = services.helpers.NodeMapper;

return (node: SdsAssignment, accept: ValidationAcceptor): void => {
const expression = node.expression;
if (!isSdsCall(expression)) {
return;
}

const assignees = assigneesOrEmpty(node);
const callable = nodeMapper.callToCallableOrUndefined(expression);
const results = abstractResultsOrEmpty(callable);

if (results.length > assignees.length) {
const kind = pluralize(results.length - assignees.length, 'result');
const names = results
.slice(assignees.length)
.map((result) => `'${result.name}'`)
.join(', ');

accept('warning', `The assignment implicitly ignores the ${kind} ${names}.`, {
node,
code: CODE_ASSIGNMENT_IMPLICITLY_IGNORED_RESULT,
});
}
};
};

export const yieldMustNotBeUsedInPipeline = (node: SdsYield, accept: ValidationAcceptor): void => {
const containingPipeline = getContainerOfType(node, isSdsPipeline);

Expand Down
15 changes: 12 additions & 3 deletions src/language/validation/safe-ds-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,14 @@ import {
unionTypeShouldNotHaveASingularTypeArgument,
} from './style.js';
import { templateStringMustHaveExpressionBetweenTwoStringParts } from './other/expressions/templateStrings.js';
import { assignmentAssigneeMustGetValue, yieldMustNotBeUsedInPipeline } from './other/statements/assignments.js';
import {
assignmentAssigneeMustGetValue,
assignmentShouldNotImplicitlyIgnoreResult,
yieldMustNotBeUsedInPipeline,
} from './other/statements/assignments.js';
import {
attributeMustHaveTypeHint,
callReceiverMustBeCallable,
namedTypeMustSetAllTypeParameters,
parameterMustHaveTypeHint,
resultMustHaveTypeHint,
Expand Down Expand Up @@ -99,7 +104,11 @@ export const registerValidationChecks = function (services: SafeDsServices) {
assigneeAssignedResultShouldNotBeDeprecated(services),
assigneeAssignedResultShouldNotBeExperimental(services),
],
SdsAssignment: [assignmentAssigneeMustGetValue(services), assignmentShouldHaveMoreThanWildcardsAsAssignees],
SdsAssignment: [
assignmentAssigneeMustGetValue(services),
assignmentShouldNotImplicitlyIgnoreResult(services),
assignmentShouldHaveMoreThanWildcardsAsAssignees,
],
SdsAnnotation: [
annotationMustContainUniqueNames,
annotationParameterListShouldNotBeEmpty,
Expand All @@ -118,7 +127,7 @@ export const registerValidationChecks = function (services: SafeDsServices) {
SdsArgumentList: [argumentListMustNotHavePositionalArgumentsAfterNamedArguments],
SdsAttribute: [attributeMustHaveTypeHint],
SdsBlockLambda: [blockLambdaMustContainUniqueNames],
SdsCall: [callArgumentListShouldBeNeeded(services)],
SdsCall: [callArgumentListShouldBeNeeded(services), callReceiverMustBeCallable(services)],
SdsCallableType: [
callableTypeMustContainUniqueNames,
callableTypeMustNotHaveOptionalParameters,
Expand Down
54 changes: 53 additions & 1 deletion src/language/validation/types.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,65 @@
import { getContainerOfType, ValidationAcceptor } from 'langium';
import { isSdsCallable, isSdsLambda, SdsAttribute, SdsNamedType, SdsParameter, SdsResult } from '../generated/ast.js';
import {
isSdsAnnotation,
isSdsCallable,
isSdsClass,
isSdsLambda,
isSdsMemberAccess,
isSdsPipeline,
isSdsReference,
isSdsSchema,
SdsAttribute,
SdsCall,
SdsNamedType,
SdsParameter,
SdsResult,
} from '../generated/ast.js';
import { typeArgumentsOrEmpty, typeParametersOrEmpty } from '../helpers/nodeProperties.js';
import { isEmpty } from 'radash';
import { SafeDsServices } from '../safe-ds-module.js';
import { pluralize } from '../helpers/stringUtils.js';

export const CODE_TYPE_CALLABLE_RECEIVER = 'type/callable-receiver';
export const CODE_TYPE_MISSING_TYPE_ARGUMENTS = 'type/missing-type-arguments';
export const CODE_TYPE_MISSING_TYPE_HINT = 'type/missing-type-hint';

// -----------------------------------------------------------------------------
// Type checking
// -----------------------------------------------------------------------------

export const callReceiverMustBeCallable = (services: SafeDsServices) => {
const nodeMapper = services.helpers.NodeMapper;

return (node: SdsCall, accept: ValidationAcceptor): void => {
let receiver = node.receiver;
if (isSdsMemberAccess(receiver)) {
receiver = receiver.member;
}

if (isSdsReference(receiver)) {
const target = receiver.target.ref;

// We already report other errors at this position in those cases
if (!target || isSdsAnnotation(target) || isSdsPipeline(target) || isSdsSchema(target)) {
return;
}
}

const callable = nodeMapper.callToCallableOrUndefined(node);
if (!callable) {
accept('error', 'This expression is not callable.', {
node: node.receiver,
code: CODE_TYPE_CALLABLE_RECEIVER,
});
} else if (isSdsClass(callable) && !callable.parameterList) {
accept('error', 'Cannot instantiate a class that has no constructor.', {
node: node.receiver,
code: CODE_TYPE_CALLABLE_RECEIVER,
});
}
};
};

// -----------------------------------------------------------------------------
// Missing type arguments
// -----------------------------------------------------------------------------
Expand Down
Loading

0 comments on commit e0fa032

Please sign in to comment.