Skip to content

Commit

Permalink
feat: error if argument lists are missing (#642)
Browse files Browse the repository at this point in the history
Closes partially #543

### Summary of Changes

Show an error if
* an annotation call is missing an argument list,
* a reference to an enum variant is missing an argument list.

---------

Co-authored-by: megalinter-bot <129584137+megalinter-bot@users.noreply.github.com>
  • Loading branch information
lars-reimann and megalinter-bot authored Oct 16, 2023
1 parent 5a9dcbb commit f5ee1bd
Show file tree
Hide file tree
Showing 13 changed files with 228 additions and 44 deletions.
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

0 comments on commit f5ee1bd

Please sign in to comment.