Skip to content

Commit

Permalink
feat: error if callable type is used in wrong context (#763)
Browse files Browse the repository at this point in the history
Closes #713

### Summary of Changes

Callables are only supposed to be used as arguments to provide a clean
graphical view. Likewise, we now show an error if a callable type is
used for anything but a parameter.
  • Loading branch information
lars-reimann authored Nov 11, 2023
1 parent 8cb2120 commit 9b1522f
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import {
isSdsMap,
isSdsTypeArgumentList,
isSdsUnionType,
SdsConstraintList,
SdsIndexedAccess,
SdsLiteralType,
SdsMap,
type SdsConstraintList,
type SdsIndexedAccess,
type SdsLiteralType,
type SdsMap,
type SdsTypeArgumentList,
type SdsTypeParameterList,
SdsUnionType,
type SdsUnionType,
} from '../generated/ast.js';

export const CODE_EXPERIMENTAL_LANGUAGE_FEATURE = 'experimental/language-feature';
Expand All @@ -24,6 +24,7 @@ export const constraintListsShouldBeUsedWithCaution = (node: SdsConstraintList,
};

export const indexedAccessesShouldBeUsedWithCaution = (node: SdsIndexedAccess, accept: ValidationAcceptor): void => {
// There's already a warning on the container
if (hasContainerOfType(node.$container, isSdsIndexedAccess)) {
return;
}
Expand All @@ -43,6 +44,7 @@ export const literalTypesShouldBeUsedWithCaution = (node: SdsLiteralType, accept
};

export const mapsShouldBeUsedWithCaution = (node: SdsMap, accept: ValidationAcceptor): void => {
// There's already a warning on the container
if (hasContainerOfType(node.$container, isSdsMap)) {
return;
}
Expand All @@ -53,23 +55,15 @@ export const mapsShouldBeUsedWithCaution = (node: SdsMap, accept: ValidationAcce
});
};

export const unionTypesShouldBeUsedWithCaution = (node: SdsUnionType, accept: ValidationAcceptor): void => {
if (hasContainerOfType(node.$container, isSdsUnionType)) {
return;
}

accept('warning', 'Union types are experimental and may change without prior notice.', {
node,
keyword: 'union',
code: CODE_EXPERIMENTAL_LANGUAGE_FEATURE,
});
};

export const typeArgumentListsShouldBeUsedWithCaution = (
node: SdsTypeArgumentList,
accept: ValidationAcceptor,
): void => {
if (hasContainerOfType(node.$container, isSdsTypeArgumentList)) {
// There's already a warning on the container
if (
hasContainerOfType(node.$container, isSdsTypeArgumentList) ||
hasContainerOfType(node.$container, isSdsUnionType)
) {
return;
}

Expand All @@ -88,3 +82,16 @@ export const typeParameterListsShouldBeUsedWithCaution = (
code: CODE_EXPERIMENTAL_LANGUAGE_FEATURE,
});
};

export const unionTypesShouldBeUsedWithCaution = (node: SdsUnionType, accept: ValidationAcceptor): void => {
// There's already a warning on the container
if (hasContainerOfType(node.$container, isSdsUnionType)) {
return;
}

accept('warning', 'Union types are experimental and may change without prior notice.', {
node,
keyword: 'union',
code: CODE_EXPERIMENTAL_LANGUAGE_FEATURE,
});
};
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { ValidationAcceptor } from 'langium';
import { SdsCallableType } from '../../../generated/ast.js';
import { getContainerOfType, ValidationAcceptor } from 'langium';
import { isSdsCallableType, isSdsParameter, SdsCallableType } from '../../../generated/ast.js';

import { getParameters, Parameter } from '../../../helpers/nodeProperties.js';

export const CODE_CALLABLE_TYPE_CONST_MODIFIER = 'callable-type/const-modifier';
export const CODE_CALLABLE_TYPE_CONTEXT = 'callable-type/context';
export const CODE_CALLABLE_TYPE_NO_OPTIONAL_PARAMETERS = 'callable-type/no-optional-parameters';

export const callableTypeParameterMustNotHaveConstModifier = (
Expand All @@ -21,6 +22,32 @@ export const callableTypeParameterMustNotHaveConstModifier = (
}
};

export const callableTypeMustBeUsedInCorrectContext = (node: SdsCallableType, accept: ValidationAcceptor): void => {
if (!contextIsCorrect(node)) {
accept('error', 'Callable types must only be used for parameters.', {
node,
code: CODE_CALLABLE_TYPE_CONTEXT,
});
}
};

const contextIsCorrect = (node: SdsCallableType): boolean => {
if (isSdsParameter(node.$container)) {
return true;
}

// Check whether we already show an error on a containing callable type
let containingCallableType = getContainerOfType(node.$container, isSdsCallableType);
while (containingCallableType) {
if (!isSdsParameter(containingCallableType.$container)) {
return true; // Container already has wrong context
}
containingCallableType = getContainerOfType(containingCallableType.$container, isSdsCallableType);
}

return false;
};

export const callableTypeMustNotHaveOptionalParameters = (node: SdsCallableType, accept: ValidationAcceptor): void => {
for (const parameter of getParameters(node)) {
if (Parameter.isOptional(parameter)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ import {
yieldMustNotBeUsedInPipeline,
} from './other/statements/assignments.js';
import {
callableTypeMustBeUsedInCorrectContext,
callableTypeMustNotHaveOptionalParameters,
callableTypeParameterMustNotHaveConstModifier,
} from './other/types/callableTypes.js';
Expand Down Expand Up @@ -217,6 +218,7 @@ export const registerValidationChecks = function (services: SafeDsServices) {
callReceiverMustBeCallable(services),
],
SdsCallableType: [
callableTypeMustBeUsedInCorrectContext,
callableTypeMustContainUniqueNames,
callableTypeMustNotHaveOptionalParameters,
callableTypeParametersMustNotBeAnnotated,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@ class MyClass1(p: MyClass1»<>«)

// $TEST$ no warning "Type argument lists & type arguments are experimental and may change without prior notice."
class MyClass2<T>(p: MyClass2<MyClass1»<>«>)

// $TEST$ no warning "Type argument lists & type arguments are experimental and may change without prior notice."
class MyClass2<T>(p: union»<>«)
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package tests.validation.other.types.callableTypes.context

annotation MyAnnotation(
// $TEST$ no error "Callable types must only be used for parameters."
p: »() -> ()«,
)

class MyClass<T>(
// $TEST$ no error "Callable types must only be used for parameters."
p: »() -> ()«,
) {
// $TEST$ error "Callable types must only be used for parameters."
attr a: »() -> ()«
}

enum MyEnum {
MyEnumVariant<T>(
// $TEST$ no error "Callable types must only be used for parameters."
p: »() -> ()«,
)
}

fun myFunction(
// $TEST$ no error "Callable types must only be used for parameters."
p: »() -> ()«,
) -> (
// $TEST$ error "Callable types must only be used for parameters."
r: »() -> ()«,
)

segment mySegment1(
// $TEST$ no error "Callable types must only be used for parameters."
p: »() -> ()«,
) -> (
// $TEST$ error "Callable types must only be used for parameters."
r: »() -> ()«,
) {}

segment mySegment2(
// $TEST$ no error "Callable types must only be used for parameters."
// $TEST$ error "Callable types must only be used for parameters."
c: (p: »() -> ()«) -> (r: »() -> ()«),
) {
// $TEST$ no error "Callable types must only be used for parameters."
(
p: »() -> ()«,
) {};

// $TEST$ no error "Callable types must only be used for parameters."
(
p: »() -> ()«,
) -> 1;
}

segment mySegment3(
// $TEST$ error "Callable types must only be used for parameters."
p1: MyClass<»() -> ()«>,

// $TEST$ error "Callable types must only be used for parameters."
p2: MyEnum.MyEnumVariant<»() -> ()«>,
) {}

segment mySegment4(
// $TEST$ error "Callable types must only be used for parameters."
p1: »() -> ()«.MyClass
) {}

segment mySegment5(
// $TEST$ error "Callable types must only be used for parameters."
p1: union<»() -> ()«>
) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package tests.validation.other.types.callableTypes.context

/*
* We already show an error for the outer callable type, if it's used in the wrong context.
*/

class MyClass1 {
// $TEST$ no error "Callable types must only be used for parameters."
attr a: () -> (r: »() -> ()«)
}

class MyClass2 {
// $TEST$ no error "Callable types must only be used for parameters."
// $TEST$ no error "Callable types must only be used for parameters."
attr a: () -> (r: (p: »() -> ()«) -> (r: »() -> ()«))
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,8 @@ segment mySegment3(
// $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
p2: MyEnum.MyEnumVariant<»union<Int>«>,
) {}

segment mySegment4(
// $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
p1: »union<Int>«.MyClass
) {}

0 comments on commit 9b1522f

Please sign in to comment.