Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: error if argument lists are missing #642

Merged
merged 4 commits into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/language/helpers/safe-ds-node-mapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export class SafeDsNodeMapper {
if (receiverType instanceof CallableType) {
return receiverType.sdsCallable;
} else if (receiverType instanceof StaticType) {
const declaration = receiverType.instanceType.sdsDeclaration;
const declaration = receiverType.instanceType.declaration;
if (isSdsCallable(declaration)) {
return declaration;
}
Expand Down
4 changes: 2 additions & 2 deletions src/language/scoping/safe-ds-scope-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,15 +207,15 @@ export class SafeDsScopeProvider extends DefaultScopeProvider {
let receiverType = this.typeComputer.computeType(node.receiver);

if (receiverType instanceof ClassType) {
const ownInstanceMembers = classMembersOrEmpty(receiverType.sdsClass, (it) => !isStatic(it));
const ownInstanceMembers = classMembersOrEmpty(receiverType.declaration, (it) => !isStatic(it));
// val superTypeMembers = type.sdsClass.superClassMembers()
// .filter { !it.isStatic() }
// .toList()
//
// Scopes.scopeFor(members, Scopes.scopeFor(superTypeMembers, resultScope))
return this.createScopeForNodes(ownInstanceMembers, resultScope);
} else if (receiverType instanceof EnumVariantType) {
const parameters = parametersOrEmpty(receiverType.sdsEnumVariant);
const parameters = parametersOrEmpty(receiverType.declaration);
return this.createScopeForNodes(parameters, resultScope);
}

Expand Down
42 changes: 21 additions & 21 deletions src/language/typing/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,32 +177,32 @@ export class NamedTupleEntry<T extends SdsDeclaration> {
}
}

export abstract class NamedType extends Type {
protected constructor(readonly sdsDeclaration: SdsDeclaration) {
export abstract class NamedType<T extends SdsDeclaration> extends Type {
protected constructor(readonly declaration: T) {
super();
}

abstract override copyWithNullability(isNullable: boolean): NamedType;
abstract override copyWithNullability(isNullable: boolean): NamedType<T>;

override toString(): string {
if (this.isNullable) {
return `${this.sdsDeclaration.name}?`;
return `${this.declaration.name}?`;
} else {
return this.sdsDeclaration.name;
return this.declaration.name;
}
}
}

export class ClassType extends NamedType {
export class ClassType extends NamedType<SdsClass> {
constructor(
readonly sdsClass: SdsClass,
declaration: SdsClass,
override readonly isNullable: boolean,
) {
super(sdsClass);
super(declaration);
}

override copyWithNullability(isNullable: boolean): ClassType {
return new ClassType(this.sdsClass, isNullable);
return new ClassType(this.declaration, isNullable);
}

override equals(other: Type): boolean {
Expand All @@ -214,20 +214,20 @@ export class ClassType extends NamedType {
return false;
}

return other.sdsClass === this.sdsClass && other.isNullable === this.isNullable;
return other.declaration === this.declaration && other.isNullable === this.isNullable;
}
}

export class EnumType extends NamedType {
export class EnumType extends NamedType<SdsEnum> {
constructor(
readonly sdsEnum: SdsEnum,
declaration: SdsEnum,
override readonly isNullable: boolean,
) {
super(sdsEnum);
super(declaration);
}

override copyWithNullability(isNullable: boolean): EnumType {
return new EnumType(this.sdsEnum, isNullable);
return new EnumType(this.declaration, isNullable);
}

override equals(other: Type): boolean {
Expand All @@ -239,20 +239,20 @@ export class EnumType extends NamedType {
return false;
}

return other.sdsEnum === this.sdsEnum && other.isNullable === this.isNullable;
return other.declaration === this.declaration && other.isNullable === this.isNullable;
}
}

export class EnumVariantType extends NamedType {
export class EnumVariantType extends NamedType<SdsEnumVariant> {
constructor(
readonly sdsEnumVariant: SdsEnumVariant,
declaration: SdsEnumVariant,
override readonly isNullable: boolean,
) {
super(sdsEnumVariant);
super(declaration);
}

override copyWithNullability(isNullable: boolean): EnumVariantType {
return new EnumVariantType(this.sdsEnumVariant, isNullable);
return new EnumVariantType(this.declaration, isNullable);
}

override equals(other: Type): boolean {
Expand All @@ -264,7 +264,7 @@ export class EnumVariantType extends NamedType {
return false;
}

return other.sdsEnumVariant === this.sdsEnumVariant && other.isNullable === this.isNullable;
return other.declaration === this.declaration && other.isNullable === this.isNullable;
}
}

Expand All @@ -274,7 +274,7 @@ export class EnumVariantType extends NamedType {
export class StaticType extends Type {
override readonly isNullable = false;

constructor(readonly instanceType: NamedType) {
constructor(readonly instanceType: NamedType<SdsDeclaration>) {
super();
}

Expand Down
2 changes: 1 addition & 1 deletion src/language/typing/safe-ds-class-hierarchy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export class SafeDsClassHierarchy {
const [firstParentType] = parentTypesOrEmpty(node);
const computedType = this.typeComputer.computeType(firstParentType);
if (computedType instanceof ClassType) {
return computedType.sdsClass;
return computedType.declaration;
}

return undefined;
Expand Down
22 changes: 19 additions & 3 deletions src/language/typing/safe-ds-type-computer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ import {
SdsFunction,
SdsIndexedAccess,
SdsInfixOperation,
SdsMemberAccess,
SdsParameter,
SdsPrefixOperation,
SdsReference,
Expand All @@ -83,6 +84,7 @@ import {
resultsOrEmpty,
typeArgumentsOrEmpty,
} from '../helpers/nodeProperties.js';
import { isEmpty } from 'radash';

export class SafeDsTypeComputer {
private readonly astNodeLocator: AstNodeLocator;
Expand Down Expand Up @@ -335,8 +337,7 @@ export class SafeDsTypeComputer {
return UnknownType;
}
} else if (isSdsMemberAccess(node)) {
const memberType = this.computeType(node.member);
return memberType.copyWithNullability(node.isNullSafe || memberType.isNullable);
return this.computeTypeOfMemberAccess(node);
} else if (isSdsParenthesizedExpression(node)) {
return this.computeType(node.expression);
} else if (isSdsPrefixOperation(node)) {
Expand Down Expand Up @@ -367,7 +368,7 @@ export class SafeDsTypeComputer {
}
} else if (receiverType instanceof StaticType) {
const instanceType = receiverType.instanceType;
if (isSdsCallable(instanceType.sdsDeclaration)) {
if (isSdsCallable(instanceType.declaration)) {
return instanceType;
}
}
Expand Down Expand Up @@ -406,6 +407,21 @@ export class SafeDsTypeComputer {
}
}

private computeTypeOfMemberAccess(node: SdsMemberAccess) {
const memberType = this.computeType(node.member);

// A member access of an enum variant without parameters always yields an instance, even if it is not in a call
if (memberType instanceof StaticType && !isSdsCall(node.$container)) {
const instanceType = memberType.instanceType;

if (instanceType instanceof EnumVariantType && isEmpty(parametersOrEmpty(instanceType.declaration))) {
return instanceType;
}
}

return memberType.copyWithNullability(node.isNullSafe || memberType.isNullable);
}

private computeTypeOfArithmeticPrefixOperation(node: SdsPrefixOperation): Type {
const leftOperandType = this.computeType(node.operand);

Expand Down
28 changes: 27 additions & 1 deletion src/language/validation/other/declarations/annotationCalls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,42 @@ import {
isSdsCallable,
isSdsCallableType,
isSdsLambda,
SdsAnnotationCall,
SdsCallableType,
SdsLambda,
SdsParameter,
} from '../../../generated/ast.js';
import { getContainerOfType, ValidationAcceptor } from 'langium';
import { annotationCallsOrEmpty, parametersOrEmpty, resultsOrEmpty } from '../../../helpers/nodeProperties.js';
import {
annotationCallsOrEmpty,
isRequiredParameter,
parametersOrEmpty,
resultsOrEmpty,
} from '../../../helpers/nodeProperties.js';
import { isEmpty } from 'radash';

export const CODE_ANNOTATION_CALL_MISSING_ARGUMENT_LIST = 'annotation-call/missing-argument-list';
export const CODE_ANNOTATION_CALL_TARGET_PARAMETER = 'annotation-call/target-parameter';
export const CODE_ANNOTATION_CALL_TARGET_RESULT = 'annotation-call/target-result';

export const annotationCallMustNotLackArgumentList = (node: SdsAnnotationCall, accept: ValidationAcceptor) => {
if (node.argumentList) {
return;
}

const requiredParameters = parametersOrEmpty(node.annotation.ref).filter(isRequiredParameter);
if (!isEmpty(requiredParameters)) {
accept(
'error',
`The annotation '${node.annotation.$refText}' has required parameters, so an argument list must be added.`,
{
node,
code: CODE_ANNOTATION_CALL_MISSING_ARGUMENT_LIST,
},
);
}
};

export const callableTypeParametersMustNotBeAnnotated = (node: SdsCallableType, accept: ValidationAcceptor) => {
for (const parameter of parametersOrEmpty(node)) {
for (const annotationCall of annotationCallsOrEmpty(parameter)) {
Expand Down
23 changes: 22 additions & 1 deletion src/language/validation/other/expressions/memberAccesses.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,31 @@
import { SafeDsServices } from '../../../safe-ds-module.js';
import { SdsMemberAccess } from '../../../generated/ast.js';
import { isSdsCall, isSdsEnumVariant, SdsMemberAccess } from '../../../generated/ast.js';
import { ValidationAcceptor } from 'langium';
import { UnknownType } from '../../../typing/model.js';
import { isEmpty } from 'radash';
import { parametersOrEmpty } from '../../../helpers/nodeProperties.js';

export const CODE_MEMBER_ACCESS_MISSING_ENUM_VARIANT_INSTANTIATION = 'member-access/missing-enum-variant-instantiation';
export const CODE_MEMBER_ACCESS_MISSING_NULL_SAFETY = 'member-access/missing-null-safety';

export const memberAccessOfEnumVariantMustNotLackInstantiation = (
node: SdsMemberAccess,
accept: ValidationAcceptor,
): void => {
const declaration = node.member.target.ref;
if (!isSdsEnumVariant(declaration)) {
return;
}

if (!isSdsCall(node.$container) && !isEmpty(parametersOrEmpty(declaration))) {
accept('error', `The enum variant '${declaration.name}' has parameters, so an argument list must be added.`, {
node,
property: 'member',
code: CODE_MEMBER_ACCESS_MISSING_ENUM_VARIANT_INSTANTIATION,
});
}
};

export const memberAccessMustBeNullSafeIfReceiverIsNullable =
(services: SafeDsServices) =>
(node: SdsMemberAccess, accept: ValidationAcceptor): void => {
Expand Down
8 changes: 7 additions & 1 deletion src/language/validation/safe-ds-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,15 @@ import { lambdaParameterMustNotHaveConstModifier } from './other/expressions/lam
import { indexedAccessesShouldBeUsedWithCaution } from './experimentalLanguageFeatures.js';
import { requiredParameterMustNotBeExpert } from './builtins/expert.js';
import {
annotationCallMustNotLackArgumentList,
callableTypeParametersMustNotBeAnnotated,
callableTypeResultsMustNotBeAnnotated,
lambdaParametersMustNotBeAnnotated,
} from './other/declarations/annotationCalls.js';
import { memberAccessMustBeNullSafeIfReceiverIsNullable } from './other/expressions/memberAccesses.js';
import {
memberAccessMustBeNullSafeIfReceiverIsNullable,
memberAccessOfEnumVariantMustNotLackInstantiation,
} from './other/expressions/memberAccesses.js';
import { importPackageMustExist, importPackageShouldNotBeEmpty } from './other/imports.js';
import { singleUseAnnotationsMustNotBeRepeated } from './builtins/repeatable.js';
import {
Expand Down Expand Up @@ -123,6 +127,7 @@ export const registerValidationChecks = function (services: SafeDsServices) {
annotationCallAnnotationShouldNotBeDeprecated(services),
annotationCallAnnotationShouldNotBeExperimental(services),
annotationCallArgumentListShouldBeNeeded,
annotationCallMustNotLackArgumentList,
],
SdsArgument: [
argumentCorrespondingParameterShouldNotBeDeprecated(services),
Expand Down Expand Up @@ -164,6 +169,7 @@ export const registerValidationChecks = function (services: SafeDsServices) {
SdsMemberAccess: [
memberAccessMustBeNullSafeIfReceiverIsNullable(services),
memberAccessNullSafetyShouldBeNeeded(services),
memberAccessOfEnumVariantMustNotLackInstantiation,
],
SdsModule: [
moduleDeclarationsMustMatchFileKind,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ describe('SafeDsNodeMapper', () => {
expect(nodeMapper.callToCallableOrUndefined(call)?.$type).toBe('SdsEnumVariant');
});

it('should return the called enum variant (aliased)', async () => {
it('should return undefined (aliased enum variant without parameter)', async () => {
const code = `
enum MyEnum {
MyEnumVariant
Expand All @@ -191,6 +191,22 @@ describe('SafeDsNodeMapper', () => {
}
`;

const call = await getNodeOfType(services, code, isSdsAbstractCall);
expect(nodeMapper.callToCallableOrUndefined(call)).toBeUndefined();
});

it('should return the called enum variant (aliased with parameter)', async () => {
const code = `
enum MyEnum {
MyEnumVariant(p: Int)
}

pipeline myPipeline {
val alias = MyEnum.MyEnumVariant;
alias(1);
}
`;

const call = await getNodeOfType(services, code, isSdsAbstractCall);
expect(nodeMapper.callToCallableOrUndefined(call)?.$type).toBe('SdsEnumVariant');
});
Expand Down
Loading