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: check context of union types #677

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
24 changes: 22 additions & 2 deletions src/language/validation/experimentalLanguageFeatures.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,21 @@
import { SdsIndexedAccess, SdsLiteralType, SdsMap, SdsUnionType } from '../generated/ast.js';
import { ValidationAcceptor } from 'langium';
import {
isSdsIndexedAccess,
isSdsMap,
isSdsUnionType,
SdsIndexedAccess,
SdsLiteralType,
SdsMap,
SdsUnionType,
} from '../generated/ast.js';
import { hasContainerOfType, ValidationAcceptor } from 'langium';

export const CODE_EXPERIMENTAL_LANGUAGE_FEATURE = 'experimental/language-feature';

export const indexedAccessesShouldBeUsedWithCaution = (node: SdsIndexedAccess, accept: ValidationAcceptor): void => {
if (hasContainerOfType(node.$container, isSdsIndexedAccess)) {
return;
}

accept('warning', 'Indexed accesses are experimental and may change without prior notice.', {
node,
code: CODE_EXPERIMENTAL_LANGUAGE_FEATURE,
Expand All @@ -18,13 +30,21 @@ export const literalTypesShouldBeUsedWithCaution = (node: SdsLiteralType, accept
};

export const mapsShouldBeUsedWithCaution = (node: SdsMap, accept: ValidationAcceptor): void => {
if (hasContainerOfType(node.$container, isSdsMap)) {
return;
}

accept('warning', 'Map literals are experimental and may change without prior notice.', {
node,
code: CODE_EXPERIMENTAL_LANGUAGE_FEATURE,
});
};

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,
code: CODE_EXPERIMENTAL_LANGUAGE_FEATURE,
Expand Down
16 changes: 10 additions & 6 deletions src/language/validation/names.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,19 @@ import {
} from '../generated/ast.js';
import { getDocument, ValidationAcceptor } from 'langium';
import {
streamBlockLambdaResults,
getMatchingClassMembers,
getColumns,
getEnumVariants,
getImportedDeclarations,
getImports,
isStatic,
getMatchingClassMembers,
getModuleMembers,
getPackageName,
getParameters,
streamPlaceholders,
getResults,
getTypeParameters,
isStatic,
streamBlockLambdaResults,
streamPlaceholders,
} from '../helpers/nodeProperties.js';
import { duplicatesBy } from '../../helpers/collectionUtils.js';
import { isInPipelineFile, isInStubFile, isInTestFile } from '../helpers/fileExtensions.js';
Expand Down Expand Up @@ -217,16 +217,20 @@ export const moduleMemberMustHaveNameThatIsUniqueInPackage = (services: SafeDsSe
const builtinUris = new Set(listBuiltinFiles().map((it) => it.toString()));

return (node: SdsModule, accept: ValidationAcceptor): void => {
const moduleUri = getDocument(node).uri?.toString();
if (builtinUris.has(moduleUri)) {
return;
}

for (const member of getModuleMembers(node)) {
const packageName = getPackageName(member) ?? '';
const declarationsInPackage = packageManager.getDeclarationsInPackage(packageName);
const memberUri = getDocument(member).uri?.toString();

if (
declarationsInPackage.some(
(it) =>
it.name === member.name &&
it.documentUri.toString() !== memberUri &&
it.documentUri.toString() !== moduleUri &&
!builtinUris.has(it.documentUri.toString()),
)
) {
Expand Down
41 changes: 39 additions & 2 deletions src/language/validation/other/types/unionTypes.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,50 @@
import { SdsUnionType } from '../../../generated/ast.js';
import { ValidationAcceptor } from 'langium';
import {
isSdsAnnotation,
isSdsCallable,
isSdsClass,
isSdsFunction,
isSdsParameter,
isSdsUnionType,
SdsUnionType,
} from '../../../generated/ast.js';
import { getContainerOfType, hasContainerOfType, ValidationAcceptor } from 'langium';
import { getTypeArguments } from '../../../helpers/nodeProperties.js';
import { SafeDsServices } from '../../../safe-ds-module.js';
import { Type } from '../../../typing/model.js';
import { isEmpty } from '../../../../helpers/collectionUtils.js';

export const CODE_UNION_TYPE_CONTEXT = 'union-type/context';
export const CODE_UNION_TYPE_DUPLICATE_TYPE = 'union-type/duplicate-type';
export const CODE_UNION_TYPE_MISSING_TYPES = 'union-type/missing-types';

export const unionTypeMustBeUsedInCorrectContext = (node: SdsUnionType, accept: ValidationAcceptor): void => {
if (!contextIsCorrect(node)) {
accept('error', 'Union types must only be used for parameters of annotations, classes, and functions.', {
node,
code: CODE_UNION_TYPE_CONTEXT,
});
}
};

const contextIsCorrect = (node: SdsUnionType): boolean => {
if (hasContainerOfType(node.$container, isSdsUnionType)) {
return true;
}

const container = node.$container;
if (!isSdsParameter(container)) {
return false;
}

const containingCallable = getContainerOfType(container, isSdsCallable);
return (
!containingCallable ||
isSdsAnnotation(containingCallable) ||
isSdsClass(containingCallable) ||
isSdsFunction(containingCallable)
);
};

export const unionTypeMustHaveTypes = (node: SdsUnionType, accept: ValidationAcceptor): void => {
if (isEmpty(getTypeArguments(node.typeArgumentList))) {
accept('error', 'A union type must have at least one type.', {
Expand Down
7 changes: 6 additions & 1 deletion src/language/validation/safe-ds-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ import {
} from './other/modules.js';
import { typeParameterConstraintLeftOperandMustBeOwnTypeParameter } from './other/declarations/typeParameterConstraints.js';
import { parameterListMustNotHaveRequiredParametersAfterOptionalParameters } from './other/declarations/parameterLists.js';
import { unionTypeMustHaveTypes, unionTypeShouldNotHaveDuplicateTypes } from './other/types/unionTypes.js';
import {
unionTypeMustBeUsedInCorrectContext,
unionTypeMustHaveTypes,
unionTypeShouldNotHaveDuplicateTypes,
} from './other/types/unionTypes.js';
import {
callableTypeMustNotHaveOptionalParameters,
callableTypeParameterMustNotHaveConstModifier,
Expand Down Expand Up @@ -273,6 +277,7 @@ export const registerValidationChecks = function (services: SafeDsServices) {
SdsTypeParameterConstraint: [typeParameterConstraintLeftOperandMustBeOwnTypeParameter],
SdsTypeParameterList: [typeParameterListShouldNotBeEmpty],
SdsUnionType: [
unionTypeMustBeUsedInCorrectContext,
unionTypeMustHaveTypes,
unionTypesShouldBeUsedWithCaution,
unionTypeShouldNotHaveDuplicateTypes(services),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,10 @@ pipeline myPipeline {

// $TEST$ warning "Indexed accesses are experimental and may change without prior notice."
»{"a": "b"}["a"]«;

// $TEST$ no warning "Indexed accesses are experimental and may change without prior notice."
[1, 2][»[1, 2][1]«];

// $TEST$ no warning "Indexed accesses are experimental and may change without prior notice."
{"a": "b"}[»{"a": "b"}["a"]«];
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,7 @@ package tests.validation.experimentalLanguageFeature.maps
pipeline myPipeline {
// $TEST$ warning "Map literals are experimental and may change without prior notice."
»{"a": "b"}«;

// $TEST$ no warning "Map literals are experimental and may change without prior notice."
{"a": »{}«};
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,12 @@ package tests.validation.experimentalLanguageFeature.unionTypes

fun myFunction(
// $TEST$ warning "Union types are experimental and may change without prior notice."
p: »union<>«
p: »union<Int, Float>«,

// $TEST$ no warning "Union types are experimental and may change without prior notice."
q: union<»union<Int, Float>«, Int>,

// $TEST$ no warning "Union types are experimental and may change without prior notice."
// $TEST$ no warning "Union types are experimental and may change without prior notice."
r: union<(p: »union<Int, Float>«) -> (r: »union<Int, Float>«), Int>,
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package tests.validation.other.types.unionTypes.context

annotation MyAnnotation(
// $TEST$ no error "Union types must only be used for parameters of annotations, classes, and functions."
p: »union<Int>«,
)

class MyClass<T>(
// $TEST$ no error "Union types must only be used for parameters of annotations, classes, and functions."
p: »union<Int>«,
) {
// $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
attr a: »union<Int>«
}

enum MyEnum {
MyEnumVariant<T>(
// $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
p: »union<Int>«,
)
}

fun myFunction(
// $TEST$ no error "Union types must only be used for parameters of annotations, classes, and functions."
p: »union<Int>«,
) -> (
// $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
r: »union<Int>«,
)

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

segment mySegment2(
// $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
// $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
c: (p: »union<Int>«) -> (r: »union<Int>«),
) {
// $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
(
p: »union<Int>«,
) {};

// $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
(
p: »union<Int>«,
) -> 1;
}

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

// $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
p2: MyEnum.MyEnumVariant<»union<Int>«>,
) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package tests.validation.other.types.unionTypes.context

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

class MyClass1 {
// $TEST$ no error "Union types must only be used for parameters of annotations, classes, and functions."
attr a: union<Int, »union<Int>«>
}

class MyClass2 {
// $TEST$ no error "Union types must only be used for parameters of annotations, classes, and functions."
// $TEST$ no error "Union types must only be used for parameters of annotations, classes, and functions."
attr a: union<Int, (p: »union<Int>«) -> (r: »union<Int>«)>
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ package tests.validation.other.types.unionTypes.duplicateTypes

// $TEST$ no warning r"The type .* was already listed."

segment mySegment1(
fun myFunction1(
p: union<>
) {}
)
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package tests.validation.other.types.unionTypes.duplicateTypes

segment mySegment(
fun myFunction2(
// $TEST$ no warning r"The type .* was already listed."
p: union<»Int«>,
q: union<
Expand All @@ -11,4 +11,4 @@ segment mySegment(
// $TEST$ warning r"The type 'Int' was already listed."
»Int«,
>,
) {}
)
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
package tests.validation.other.types.unionTypes.mustHaveTypes

// $TEST$ error "A union type must have at least one type."
segment mySegment1(
fun myFunction1(
p: union»<>«
) {}
)

// $TEST$ no error "A union type must have at least one type."
segment mySegment2(
fun myFunction2(
p: union»<Int>«
) {}
)

// $TEST$ no error "A union type must have at least one type."
segment mySegment3(
fun myFunction3(
p: union»<Int, Float>«
) {}
)