Skip to content

Commit

Permalink
feat: check whether type parameter bounds are acyclic (#886)
Browse files Browse the repository at this point in the history 
Closes #874

### Summary of Changes

Add a check to ensure type parameter bounds are acyclic.
  • Loading branch information
@lars-reimann
lars-reimann authored Feb 12, 2024
1 parent 2fc7fe6 commit bcf1a4b
Show file tree
Hide file tree
Showing 12 changed files with 332 additions and 68 deletions.
19 changes: 19 additions & 0 deletions packages/safe-ds-lang/src/language/validation/other/declarations/typeParameterBounds.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { isSdsDeclaration, SdsTypeParameterBound } from '../../../generated/ast.js';
import { getContainerOfType, ValidationAcceptor } from 'langium';
import { NamedType, UnknownType } from '../../../typing/model.js';
import { SafeDsServices } from '../../../safe-ds-module.js';

export const CODE_TYPE_PARAMETER_BOUND_LEFT_OPERAND = 'type-parameter-bound/left-operand';
export const CODE_TYPE_PARAMETER_BOUND_RIGHT_OPERAND = 'type-parameter-bound/right-operand';

export const typeParameterBoundLeftOperandMustBeOwnTypeParameter = (
node: SdsTypeParameterBound,
Expand All @@ -23,3 +26,19 @@ export const typeParameterBoundLeftOperandMustBeOwnTypeParameter = (
});
}
};

export const typeParameterBoundRightOperandMustBeNamedType = (services: SafeDsServices) => {
const typeComputer = services.types.TypeComputer;

return (node: SdsTypeParameterBound, accept: ValidationAcceptor) => {
const boundType = typeComputer.computeType(node.rightOperand);

if (boundType !== UnknownType && !(boundType instanceof NamedType)) {
accept('error', 'Bounds of type parameters must be named types.', {
node,
property: 'rightOperand',
code: CODE_TYPE_PARAMETER_BOUND_RIGHT_OPERAND,
});
}
};
};
139 changes: 97 additions & 42 deletions packages/safe-ds-lang/src/language/validation/other/declarations/typeParameters.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ import {
isSdsClass,
isSdsClassMember,
isSdsDeclaration,
isSdsNamedType,
isSdsNamedTypeDeclaration,
isSdsParameter,
isSdsParameterList,
isSdsTypeArgument,
isSdsTypeParameter,
isSdsUnionType,
SdsClass,
SdsDeclaration,
Expand All @@ -17,16 +19,93 @@ import {
import { isStatic, TypeParameter } from '../../../helpers/nodeProperties.js';
import { SafeDsServices } from '../../../safe-ds-module.js';
import { SafeDsNodeMapper } from '../../../helpers/safe-ds-node-mapper.js';
import { NamedType, UnknownType } from '../../../typing/model.js';
import { SafeDsTypeComputer } from '../../../typing/safe-ds-type-computer.js';
import { UnknownType } from '../../../typing/model.js';

export const CODE_TYPE_PARAMETER_CYCLIC_BOUND = 'type-parameter/cyclic-bound';
export const CODE_TYPE_PARAMETER_INCOMPATIBLE_BOUNDS = 'type-parameter/incompatible-bounds';
export const CODE_TYPE_PARAMETER_INSUFFICIENT_CONTEXT = 'type-parameter/insufficient-context';
export const CODE_TYPE_PARAMETER_INVALID_BOUND = 'type-parameter/invalid-bound';
export const CODE_TYPE_PARAMETER_MULTIPLE_BOUNDS = 'type-parameter/multiple-bounds';
export const CODE_TYPE_PARAMETER_USAGE = 'type-parameter/usage';
export const CODE_TYPE_PARAMETER_VARIANCE = 'type-parameter/variance';

export const typeParameterBoundMustBeAcyclic = (node: SdsTypeParameter, accept: ValidationAcceptor) => {
const lowerBound = TypeParameter.getLowerBounds(node)[0];
if (lowerBound && !lowerTypeParameterBoundIsAcyclic(lowerBound)) {
accept('error', 'Bounds of type parameters must be acyclic.', {
node: lowerBound,
code: CODE_TYPE_PARAMETER_CYCLIC_BOUND,
});
}

const upperBound = TypeParameter.getUpperBounds(node)[0];
if (upperBound && !upperTypeParameterBoundIsAcyclic(upperBound)) {
accept('error', 'Bounds of type parameters must be acyclic.', {
node: upperBound,
code: CODE_TYPE_PARAMETER_CYCLIC_BOUND,
});
}
};

const lowerTypeParameterBoundIsAcyclic = (node: SdsTypeParameterBound): boolean => {
const visited = new Set<SdsTypeParameter>();
let current: SdsTypeParameterBound | undefined = node;

while (current) {
const typeParameter = getBoundingTypeParameter(current, 'sub');
if (!typeParameter) {
return true;
} else if (visited.has(typeParameter)) {
return false;
}

visited.add(typeParameter);
current = TypeParameter.getLowerBounds(typeParameter)[0];
}

return true;
};

const upperTypeParameterBoundIsAcyclic = (node: SdsTypeParameterBound): boolean => {
const visited = new Set<SdsTypeParameter>();
let current: SdsTypeParameterBound | undefined = node;

while (current) {
const typeParameter = getBoundingTypeParameter(current, 'super');
if (!typeParameter) {
return true;
} else if (visited.has(typeParameter)) {
return false;
}

visited.add(typeParameter);
current = TypeParameter.getUpperBounds(typeParameter)[0];
}

return true;
};

/**
* Returns the next type parameter to be visited when checking for cycles.
*
* @param node
* The current type parameter bound.
*
* @param invertedOperator
* The operator for the inverted bound direction ('sub' for lower bounds, 'super' for upper bounds).
*/
const getBoundingTypeParameter = (
node: SdsTypeParameterBound,
invertedOperator: string,
): SdsTypeParameter | undefined => {
if (node.operator === invertedOperator) {
return node.leftOperand?.ref;
} else if (isSdsNamedType(node.rightOperand) && isSdsTypeParameter(node.rightOperand.declaration?.ref)) {
return node.rightOperand.declaration?.ref;
} else {
return undefined;
}
};

export const typeParameterBoundsMustBeCompatible = (services: SafeDsServices) => {
const typeChecker = services.types.TypeChecker;
const typeComputer = services.types.TypeComputer;
Expand Down Expand Up @@ -88,48 +167,24 @@ export const typeParameterMustHaveSufficientContext = (node: SdsTypeParameter, a
}
};

export const typeParameterMustHaveOneValidLowerAndUpperBound = (services: SafeDsServices) => {
const typeComputer = services.types.TypeComputer;

return (node: SdsTypeParameter, accept: ValidationAcceptor) => {
TypeParameter.getLowerBounds(node).forEach((it, index) => {
if (index === 0) {
checkIfBoundIsValid(it, typeComputer, accept);
} else {
accept('error', `The type parameter '${node.name}' can only have a single lower bound.`, {
node: it,
code: CODE_TYPE_PARAMETER_MULTIPLE_BOUNDS,
});
}
});

TypeParameter.getUpperBounds(node).forEach((it, index) => {
if (index === 0) {
checkIfBoundIsValid(it, typeComputer, accept);
} else {
accept('error', `The type parameter '${node.name}' can only have a single upper bound.`, {
node: it,
code: CODE_TYPE_PARAMETER_MULTIPLE_BOUNDS,
});
}
export const typeParameterMustNotHaveMultipleBounds = (node: SdsTypeParameter, accept: ValidationAcceptor) => {
TypeParameter.getLowerBounds(node)
.slice(1)
.forEach((it) => {
accept('error', `The type parameter '${node.name}' can only have a single lower bound.`, {
node: it,
code: CODE_TYPE_PARAMETER_MULTIPLE_BOUNDS,
});
});
};
};

const checkIfBoundIsValid = (
node: SdsTypeParameterBound,
typeComputer: SafeDsTypeComputer,
accept: ValidationAcceptor,
) => {
const boundType = typeComputer.computeType(node.rightOperand);

if (boundType !== UnknownType && !(boundType instanceof NamedType)) {
accept('error', 'Bounds of type parameters must be named types.', {
node,
property: 'rightOperand',
code: CODE_TYPE_PARAMETER_INVALID_BOUND,
TypeParameter.getUpperBounds(node)
.slice(1)
.forEach((it) => {
accept('error', `The type parameter '${node.name}' can only have a single upper bound.`, {
node: it,
code: CODE_TYPE_PARAMETER_MULTIPLE_BOUNDS,
});
});
}
};

export const typeParameterMustBeUsedInCorrectPosition = (services: SafeDsServices) => {
Expand Down
16 changes: 12 additions & 4 deletions packages/safe-ds-lang/src/language/validation/safe-ds-validator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,16 @@ import {
segmentResultMustBeAssignedExactlyOnce,
segmentShouldBeUsed,
} from './other/declarations/segments.js';
import { typeParameterBoundLeftOperandMustBeOwnTypeParameter } from './other/declarations/typeParameterBounds.js';
import {
typeParameterBoundLeftOperandMustBeOwnTypeParameter,
typeParameterBoundRightOperandMustBeNamedType,
} from './other/declarations/typeParameterBounds.js';
import {
typeParameterBoundMustBeAcyclic,
typeParameterBoundsMustBeCompatible,
typeParameterMustBeUsedInCorrectPosition,
typeParameterMustHaveOneValidLowerAndUpperBound,
typeParameterMustHaveSufficientContext,
typeParameterMustNotHaveMultipleBounds,
typeParameterMustOnlyBeVariantOnClass,
} from './other/declarations/typeParameters.js';
import { callArgumentMustBeConstantIfParameterIsConstant, callMustNotBeRecursive } from './other/expressions/calls.js';
Expand Down Expand Up @@ -349,13 +353,17 @@ export const registerValidationChecks = function (services: SafeDsServices) {
SdsTemplateString: [templateStringMustHaveExpressionBetweenTwoStringParts],
SdsTypeCast: [typeCastExpressionMustHaveUnknownType(services)],
SdsTypeParameter: [
typeParameterBoundMustBeAcyclic,
typeParameterBoundsMustBeCompatible(services),
typeParameterMustBeUsedInCorrectPosition(services),
typeParameterMustHaveSufficientContext,
typeParameterMustHaveOneValidLowerAndUpperBound(services),
typeParameterMustNotHaveMultipleBounds,
typeParameterMustOnlyBeVariantOnClass,
],
SdsTypeParameterBound: [typeParameterBoundLeftOperandMustBeOwnTypeParameter],
SdsTypeParameterBound: [
typeParameterBoundLeftOperandMustBeOwnTypeParameter,
typeParameterBoundRightOperandMustBeNamedType(services),
],
SdsTypeParameterList: [
typeParameterListMustNotHaveRequiredTypeParametersAfterOptionalTypeParameters,
typeParameterListShouldNotBeEmpty(services),
Expand Down
12 changes: 12 additions & 0 deletions packages/safe-ds-lang/tests/helpers/testAssertions.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { Type } from '../../src/language/typing/model.js';
import { AssertionError } from 'assert';

export const expectEqualTypes = (actual: Type, expected: Type) => {
if (!actual.equals(expected)) {
throw new AssertionError({
message: `Expected type '${actual.toString()}' to equal type '${expected.toString()}'.`,
actual: actual.toString(),
expected: expected.toString(),
});
}
};
22 changes: 15 additions & 7 deletions packages/safe-ds-lang/tests/language/typing/model.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
} from '../../../src/language/typing/model.js';
import { getNodeOfType } from '../../helpers/nodeFinder.js';
import type { EqualsTest, ToStringTest } from '../../helpers/testDescription.js';
import { expectEqualTypes } from '../../helpers/testAssertions.js';

const services = (await createSafeDsServicesWithBuiltins(NodeFileSystem)).SafeDs;
const code = `
Expand Down Expand Up @@ -327,11 +328,13 @@ describe('type model', async () => {
];
describe.each(substituteTypeParametersTest)('substituteTypeParameters', ({ type, substitutions, expectedType }) => {
it(`should return the expected value (${type.constructor.name} -- ${type})`, () => {
expect(type.substituteTypeParameters(substitutions).equals(expectedType)).toBeTruthy();
const actual = type.substituteTypeParameters(substitutions);
expectEqualTypes(actual, expectedType);
});

it(`should not change if no substitutions are passed (${type.constructor.name} -- ${type})`, () => {
expect(type.substituteTypeParameters(new Map()).equals(type)).toBeTruthy();
const actual = type.substituteTypeParameters(new Map());
expectEqualTypes(actual, type);
});
});

Expand Down Expand Up @@ -427,7 +430,8 @@ describe('type model', async () => {
];
describe.each(unwrapTests)('unwrap', ({ type, expectedType }) => {
it(`should remove any unnecessary containers (${type.constructor.name} -- ${type})`, () => {
expect(type.unwrap()).toSatisfy((actualType: Type) => actualType.equals(expectedType));
const actual = type.unwrap();
expectEqualTypes(actual, expectedType);
});
});

Expand Down Expand Up @@ -571,7 +575,8 @@ describe('type model', async () => {
];
describe.each(updateNullabilityTests)('updateNullability', ({ type, isNullable, expectedType }) => {
it(`should return the expected value (${type.constructor.name} -- ${type})`, () => {
expect(type.updateNullability(isNullable).equals(expectedType)).toBeTruthy();
const actual = type.updateNullability(isNullable);
expectEqualTypes(actual, expectedType);
});
});

Expand All @@ -596,7 +601,8 @@ describe('type model', async () => {
expectedType: new ClassType(class1, new Map(), false),
},
])('should return the type of the parameter at the given index (%#)', ({ type, index, expectedType }) => {
expect(type.getParameterTypeByIndex(index).equals(expectedType)).toBeTruthy();
const actual = type.getParameterTypeByIndex(index);
expectEqualTypes(actual, expectedType);
});
});
});
Expand All @@ -615,7 +621,8 @@ describe('type model', async () => {
expectedType: new UnionType(),
},
])('should return the type of the parameter at the given index (%#)', ({ type, index, expectedType }) => {
expect(type.getTypeParameterTypeByIndex(index).equals(expectedType)).toBeTruthy();
const actual = type.getTypeParameterTypeByIndex(index);
expectEqualTypes(actual, expectedType);
});
});
});
Expand All @@ -636,7 +643,8 @@ describe('type model', async () => {
expectedType: new ClassType(class1, new Map(), false),
},
])('should return the entry at the given index (%#)', ({ type, index, expectedType }) => {
expect(type.getTypeOfEntryByIndex(index).equals(expectedType)).toBeTruthy();
const actual = type.getTypeOfEntryByIndex(index);
expectEqualTypes(actual, expectedType);
});
});
});
Expand Down
3 changes: 1 addition & 2 deletions packages/safe-ds-lang/tests/language/typing/safe-ds-class-hierarchy.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ import {
SdsClass,
type SdsClassMember,
} from '../../../src/language/generated/ast.js';
import { getClassMembers } from '../../../src/language/helpers/nodeProperties.js';
import { createSafeDsServicesWithBuiltins } from '../../../src/language/index.js';
import { createSafeDsServicesWithBuiltins, getClassMembers } from '../../../src/language/index.js';
import { getNodeOfType } from '../../helpers/nodeFinder.js';

const services = (await createSafeDsServicesWithBuiltins(NodeFileSystem)).SafeDs;
Expand Down
Loading

0 comments on commit bcf1a4b

Please sign in to comment.