Skip to content

Commit

Permalink
feat: error if type parameter has multiple lower/upper bounds (#870)
Browse files Browse the repository at this point in the history
Closes #867

### Summary of Changes

Just like we don't have multiple inheritance or intersection types, we
only want to deal with a single lower and a single upper bound for type
parameters. We currently don't have a use-case for multiple bounds, so
we may as well simplify the implementation of the DSL. This PR adds a
check to ensure that type parameters have a single lower and a single
upper bound.
  • Loading branch information
lars-reimann authored Feb 10, 2024
1 parent 9bf5fec commit 6035b76
Show file tree
Hide file tree
Showing 15 changed files with 193 additions and 39 deletions.
10 changes: 5 additions & 5 deletions packages/safe-ds-lang/src/language/grammar/safe-ds.langium
Original file line number Diff line number Diff line change
Expand Up @@ -378,22 +378,22 @@ SdsConstraintList returns SdsConstraintList:
interface SdsConstraint extends SdsObject {}

SdsConstraint returns SdsConstraint:
SdsTypeParameterConstraint
SdsTypeParameterBound
;

interface SdsTypeParameterConstraint extends SdsConstraint {
interface SdsTypeParameterBound extends SdsConstraint {
leftOperand?: @SdsTypeParameter
operator: string
rightOperand: SdsType
}

SdsTypeParameterConstraint returns SdsTypeParameterConstraint:
SdsTypeParameterBound returns SdsTypeParameterBound:
leftOperand=[SdsTypeParameter:ID]
operator=SdsTypeParameterConstraintOperator
operator=SdsTypeParameterBoundOperator
rightOperand=SdsType
;

SdsTypeParameterConstraintOperator returns string:
SdsTypeParameterBoundOperator returns string:
'sub' | 'super'
;

Expand Down
28 changes: 28 additions & 0 deletions packages/safe-ds-lang/src/language/helpers/nodeProperties.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
isSdsClass,
isSdsDeclaration,
isSdsEnum,
isSdsEnumVariant,
isSdsFunction,
isSdsLambda,
isSdsModule,
Expand All @@ -23,6 +24,7 @@ import {
isSdsSegment,
isSdsTypeArgumentList,
isSdsTypeParameter,
isSdsTypeParameterBound,
isSdsTypeParameterList,
SdsAbstractCall,
SdsAbstractResult,
Expand All @@ -40,6 +42,7 @@ import {
SdsClass,
SdsClassMember,
SdsColumn,
SdsConstraint,
SdsDeclaration,
SdsEnum,
SdsEnumVariant,
Expand All @@ -62,6 +65,7 @@ import {
SdsTypeArgument,
SdsTypeArgumentList,
SdsTypeParameter,
SdsTypeParameterBound,
SdsTypeParameterList,
} from '../generated/ast.js';

Expand Down Expand Up @@ -169,6 +173,13 @@ export namespace TypeParameter {
export const isInvariant = (node: SdsTypeParameter | undefined): boolean => {
return isSdsTypeParameter(node) && !node.variance;
};

export const getBounds = (node: SdsTypeParameter | undefined): SdsTypeParameterBound[] => {
const declarationContainingTypeParameter = getContainerOfType(node?.$container, isSdsDeclaration);
return getConstraints(declarationContainingTypeParameter).filter(
(it) => isSdsTypeParameterBound(it) && it.leftOperand?.ref === node,
) as SdsTypeParameterBound[];
};
}

// -------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -252,6 +263,23 @@ export const getColumns = (node: SdsSchema | undefined): SdsColumn[] => {
return node?.columnList?.columns ?? [];
};

export const getConstraints = (node: SdsDeclaration | undefined): SdsConstraint[] => {
if (isSdsAnnotation(node)) {
/* c8 ignore next 2 */
return node.constraintList?.constraints ?? [];
} else if (isSdsClass(node)) {
return node.constraintList?.constraints ?? [];
} else if (isSdsEnumVariant(node)) {
/* c8 ignore next 2 */
return node.constraintList?.constraints ?? [];
} else if (isSdsFunction(node)) {
return node.constraintList?.constraints ?? [];
} else {
/* c8 ignore next 2 */
return [];
}
};

export const getEnumVariants = (node: SdsEnum | undefined): SdsEnumVariant[] => {
return node?.body?.variants ?? [];
};
Expand Down
6 changes: 3 additions & 3 deletions packages/safe-ds-lang/src/language/lsp/safe-ds-formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ export class SafeDsFormatter extends AbstractFormatter {
// -----------------------------------------------------------------------------
else if (ast.isSdsConstraintList(node)) {
this.formatSdsConstraintList(node);
} else if (ast.isSdsTypeParameterConstraint(node)) {
this.formatSdsTypeParameterConstraint(node);
} else if (ast.isSdsTypeParameterBound(node)) {
this.formatSdsTypeParameterBound(node);
}

// -----------------------------------------------------------------------------
Expand Down Expand Up @@ -529,7 +529,7 @@ export class SafeDsFormatter extends AbstractFormatter {
}
}

private formatSdsTypeParameterConstraint(node: ast.SdsTypeParameterConstraint) {
private formatSdsTypeParameterBound(node: ast.SdsTypeParameterBound) {
const formatter = this.getNodeFormatter(node);

formatter.property('operator').surround(oneSpace());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
isSdsSegment,
isSdsTypeArgument,
isSdsTypeParameter,
isSdsTypeParameterConstraint,
isSdsTypeParameterBound,
isSdsYield,
} from '../generated/ast.js';
import { SafeDsServices } from '../safe-ds-module.js';
Expand Down Expand Up @@ -108,7 +108,7 @@ export class SafeDsSemanticTokenProvider extends AbstractSemanticTokenProvider {
type: SemanticTokenTypes.typeParameter,
});
}
} else if (isSdsTypeParameterConstraint(node)) {
} else if (isSdsTypeParameterBound(node)) {
acceptor({
node,
property: 'leftOperand',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { isSdsDeclaration, SdsTypeParameterConstraint } from '../../../generated/ast.js';
import { isSdsDeclaration, SdsTypeParameterBound } from '../../../generated/ast.js';
import { getContainerOfType, ValidationAcceptor } from 'langium';

export const CODE_TYPE_PARAMETER_CONSTRAINT_LEFT_OPERAND = 'type-parameter-constraint/left-operand';
export const CODE_TYPE_PARAMETER_BOUND_LEFT_OPERAND = 'type-parameter-bound/left-operand';

export const typeParameterConstraintLeftOperandMustBeOwnTypeParameter = (
node: SdsTypeParameterConstraint,
export const typeParameterBoundLeftOperandMustBeOwnTypeParameter = (
node: SdsTypeParameterBound,
accept: ValidationAcceptor,
) => {
const typeParameter = node.leftOperand?.ref;
Expand All @@ -16,10 +16,10 @@ export const typeParameterConstraintLeftOperandMustBeOwnTypeParameter = (
const declarationWithTypeParameters = getContainerOfType(typeParameter.$container, isSdsDeclaration);

if (declarationWithConstraint !== declarationWithTypeParameters) {
accept('error', 'The left operand must refer to a type parameter of the declaration with the constraint.', {
accept('error', 'The left operand must refer to a type parameter of the declaration with the bound.', {
node,
property: 'leftOperand',
code: CODE_TYPE_PARAMETER_CONSTRAINT_LEFT_OPERAND,
code: CODE_TYPE_PARAMETER_BOUND_LEFT_OPERAND,
});
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { SafeDsServices } from '../../../safe-ds-module.js';
import { SafeDsNodeMapper } from '../../../helpers/safe-ds-node-mapper.js';

export const CODE_TYPE_PARAMETER_INSUFFICIENT_CONTEXT = 'type-parameter/insufficient-context';
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';

Expand Down Expand Up @@ -58,6 +59,35 @@ export const typeParameterMustHaveSufficientContext = (node: SdsTypeParameter, a
}
};

export const typeParameterMustNotHaveMultipleBounds = (node: SdsTypeParameter, accept: ValidationAcceptor) => {
const bounds = TypeParameter.getBounds(node);

let foundLowerBound = false;
let foundUpperBound = false;

for (const bound of bounds) {
if (bound.operator === 'super') {
if (foundLowerBound) {
accept('error', 'A type parameter can only have a single lower bound.', {
node: bound,
code: CODE_TYPE_PARAMETER_MULTIPLE_BOUNDS,
});
}

foundLowerBound = true;
} else if (bound.operator === 'sub') {
if (foundUpperBound) {
accept('error', 'A type parameter can only have a single upper bound.', {
node: bound,
code: CODE_TYPE_PARAMETER_MULTIPLE_BOUNDS,
});
}

foundUpperBound = true;
}
}
};

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,11 @@ import {
segmentResultMustBeAssignedExactlyOnce,
segmentShouldBeUsed,
} from './other/declarations/segments.js';
import { typeParameterConstraintLeftOperandMustBeOwnTypeParameter } from './other/declarations/typeParameterConstraints.js';
import { typeParameterBoundLeftOperandMustBeOwnTypeParameter } from './other/declarations/typeParameterBounds.js';
import {
typeParameterMustBeUsedInCorrectPosition,
typeParameterMustHaveSufficientContext,
typeParameterMustNotHaveMultipleBounds,
typeParameterMustOnlyBeVariantOnClass,
} from './other/declarations/typeParameters.js';
import { callArgumentMustBeConstantIfParameterIsConstant, callMustNotBeRecursive } from './other/expressions/calls.js';
Expand Down Expand Up @@ -351,9 +352,10 @@ export const registerValidationChecks = function (services: SafeDsServices) {
SdsTypeParameter: [
typeParameterMustHaveSufficientContext,
typeParameterMustBeUsedInCorrectPosition(services),
typeParameterMustNotHaveMultipleBounds,
typeParameterMustOnlyBeVariantOnClass,
],
SdsTypeParameterConstraint: [typeParameterConstraintLeftOperandMustBeOwnTypeParameter],
SdsTypeParameterBound: [typeParameterBoundLeftOperandMustBeOwnTypeParameter],
SdsTypeParameterList: [
typeParameterListMustNotHaveRequiredTypeParametersAfterOptionalTypeParameters,
typeParameterListShouldNotBeEmpty(services),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package tests.scoping.typeParameterConstraints.inAnnotation
package tests.scoping.typeParameterBounds.inAnnotation

fun myFunction1<Before>()

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package tests.scoping.typeParameterConstraints.inEnumVariantInNestedEnum
package tests.scoping.typeParameterBounds.inEnumVariantInNestedEnum

// $TEST$ target container
class MyClass<»Container«> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package tests.scoping.typeParameterConstraints.inGlobalClass
package tests.scoping.typeParameterBounds.inGlobalClass

fun myFunction1<Before>()

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package tests.scoping.typeParameterConstraints.inGlobalFunction
package tests.scoping.typeParameterBounds.inGlobalFunction

fun myFunction1<Before>()

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package tests.scoping.typeParameterConstraints.inMethod
package tests.scoping.typeParameterBounds.inMethod

fun myFunction1<BeforeGlobal>()

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package tests.scoping.typeParameterConstraints.inNestedClass
package tests.scoping.typeParameterBounds.inNestedClass

fun myFunction1<BeforeGlobal>()

Expand Down
Original file line number Diff line number Diff line change
@@ -1,61 +1,61 @@
package tests.validation.other.declarations.constraints.typeParameterConstraints.typeParameterOnContainer
package tests.validation.other.declarations.constraints.typeParameterBounds.typeParameterOnContainer

annotation MyAnnotation where {
// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the constraint."
// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the bound."
»Unresolved« sub MyGlobalClass,
}

class MyGlobalClass<T1> where {
// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the constraint."
// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the bound."
»T1« sub MyGlobalClass,

// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the constraint."
// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the bound."
»Unresolved« sub MyGlobalClass
} {
class MyNestedClass<T2> where {
// $TEST$ error "The left operand must refer to a type parameter of the declaration with the constraint."
// $TEST$ error "The left operand must refer to a type parameter of the declaration with the bound."
»T1« sub MyGlobalClass,

// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the constraint."
// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the bound."
»T2« sub MyGlobalClass,

// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the constraint."
// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the bound."
»Unresolved« sub MyGlobalClass,
}

enum MyNestedEnum {
MyEnumVariant where {
// $TEST$ error "The left operand must refer to a type parameter of the declaration with the constraint."
// $TEST$ error "The left operand must refer to a type parameter of the declaration with the bound."
»T1« sub MyGlobalClass,

// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the constraint."
// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the bound."
»Unresolved« sub MyGlobalClass,
}
}

fun myMethod<T2>() where {
// $TEST$ error "The left operand must refer to a type parameter of the declaration with the constraint."
// $TEST$ error "The left operand must refer to a type parameter of the declaration with the bound."
»T1« sub MyGlobalClass,

// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the constraint."
// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the bound."
»T2« sub MyGlobalClass,

// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the constraint."
// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the bound."
»Unresolved« sub MyGlobalClass,
}
}

enum MyGlobalEnum {
MyEnumVariant where {
// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the constraint."
// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the bound."
»Unresolved« sub MyGlobalClass,
}
}

fun myGlobalFunction<T1>() where {
// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the constraint."
// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the bound."
»T1« sub MyGlobalClass,

// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the constraint."
// $TEST$ no error "The left operand must refer to a type parameter of the declaration with the bound."
»Unresolved« sub MyGlobalClass,
}
Loading

0 comments on commit 6035b76

Please sign in to comment.