Skip to content

Commit

Permalink
feat: error if type parameters don't have sufficient context (#687)
Browse files Browse the repository at this point in the history
### Summary of Changes

Show an error if we already know at the declaration-site that we won't
be able to infer type parameters in a call. A type parameter must occur
in the parameter list of the containing callable. However, it does not
suffice if they appear inside a union type or the parameter list of a
callable type.
  • Loading branch information
lars-reimann committed Oct 24, 2023
1 parent 09bfb38 commit ea8fe29
Show file tree
Hide file tree
Showing 21 changed files with 147 additions and 169 deletions.
2 changes: 1 addition & 1 deletion src/cli/generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ const getExternalReferenceNeededImport = function (
if (isSdsQualifiedImport(value)) {
const importedDeclarations = getImportedDeclarations(value);
for (const importedDeclaration of importedDeclarations) {
if (declaration === importedDeclaration.declaration.ref) {
if (declaration === importedDeclaration.declaration?.ref) {
if (importedDeclaration.alias !== undefined) {
return {
importPath: services.builtins.Annotations.getPythonModule(targetModule) || value.package,
Expand Down
6 changes: 3 additions & 3 deletions src/language/grammar/safe-ds.langium
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ SdsImportedDeclarationList returns SdsImportedDeclarationList:
;

interface SdsImportedDeclaration extends SdsObject {
declaration: @SdsModuleMember;
declaration?: @SdsModuleMember;
alias?: SdsImportedDeclarationAlias;
}

Expand Down Expand Up @@ -384,7 +384,7 @@ SdsConstraint returns SdsConstraint:
;

interface SdsTypeParameterConstraint extends SdsConstraint {
leftOperand: @SdsTypeParameter
leftOperand?: @SdsTypeParameter
operator: string
rightOperand: SdsType
}
Expand Down Expand Up @@ -922,7 +922,7 @@ SdsLiteralList returns SdsLiteralList:
;

interface SdsNamedType extends SdsType {
declaration: @SdsNamedTypeDeclaration
declaration?: @SdsNamedTypeDeclaration
typeArgumentList?: SdsTypeArgumentList
isNullable: boolean
}
Expand Down
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 @@ -239,7 +239,7 @@ export class SafeDsNodeMapper {
}

// Find type parameter at the same position
const namedTypeDeclaration = containingType.declaration.ref;
const namedTypeDeclaration = containingType.declaration?.ref;
const typeParameters = getTypeParameters(namedTypeDeclaration);
return typeParameters[typeArgumentPosition];
}
Expand Down
4 changes: 2 additions & 2 deletions src/language/lsp/safe-ds-semantic-token-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export class SafeDsSemanticTokenProvider extends AbstractSemanticTokenProvider {
type: SemanticTokenTypes.namespace,
});
} else if (isSdsImportedDeclaration(node)) {
const info = this.computeSemanticTokenInfoForDeclaration(node.declaration.ref);
const info = this.computeSemanticTokenInfoForDeclaration(node.declaration?.ref);
if (info) {
acceptor({
node,
Expand All @@ -96,7 +96,7 @@ export class SafeDsSemanticTokenProvider extends AbstractSemanticTokenProvider {
});
}
} else if (isSdsNamedType(node)) {
const info = this.computeSemanticTokenInfoForDeclaration(node.declaration.ref);
const info = this.computeSemanticTokenInfoForDeclaration(node.declaration?.ref);
if (info) {
acceptor({
node,
Expand Down
10 changes: 5 additions & 5 deletions src/language/scoping/safe-ds-scope-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,16 @@ import { isContainedIn } from '../helpers/astUtils.js';
import {
getAbstractResults,
getAssignees,
getMatchingClassMembers,
getEnumVariants,
getImportedDeclarations,
getImports,
isStatic,
getMatchingClassMembers,
getPackageName,
getParameters,
getResults,
getStatements,
getTypeParameters,
isStatic,
} from '../helpers/nodeProperties.js';
import { SafeDsServices } from '../safe-ds-module.js';
import { SafeDsTypeComputer } from '../typing/safe-ds-type-computer.js';
Expand Down Expand Up @@ -165,7 +165,7 @@ export class SafeDsScopeProvider extends DefaultScopeProvider {
*/
private getUniqueReferencedDeclarationForType(node: SdsType): SdsNamedTypeDeclaration | undefined {
if (isSdsNamedType(node)) {
return node.declaration.ref;
return node.declaration?.ref;
} else if (isSdsMemberType(node)) {
return node.member?.declaration?.ref;
} else {
Expand Down Expand Up @@ -335,7 +335,7 @@ export class SafeDsScopeProvider extends DefaultScopeProvider {
return EMPTY_SCOPE;
}

const namedTypeDeclaration = containingNamedType.declaration.ref;
const namedTypeDeclaration = containingNamedType.declaration?.ref;
if (isSdsClass(namedTypeDeclaration)) {
const typeParameters = getTypeParameters(namedTypeDeclaration.typeParameterList);
return this.createScopeForNodes(typeParameters);
Expand Down Expand Up @@ -403,7 +403,7 @@ export class SafeDsScopeProvider extends DefaultScopeProvider {
for (const imp of imports) {
if (isSdsQualifiedImport(imp)) {
for (const importedDeclaration of getImportedDeclarations(imp)) {
const description = importedDeclaration.declaration.$nodeDescription;
const description = importedDeclaration.declaration?.$nodeDescription;
if (!description || !this.astReflection.isSubtype(description.type, referenceType)) {
continue;
}
Expand Down
1 change: 1 addition & 0 deletions src/language/typing/safe-ds-type-checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ export class SafeDsTypeChecker {
}

private unionTypeIsAssignableTo(type: UnionType, other: Type): boolean {
// return this.possibleTypes.all { it.isSubstitutableFor(other) }
return type.equals(other);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/language/typing/safe-ds-type-computer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,11 @@ import {
import { SafeDsNodeMapper } from '../helpers/safe-ds-node-mapper.js';
import {
getAssignees,
streamBlockLambdaResults,
getLiterals,
getParameters,
getResults,
getTypeArguments,
streamBlockLambdaResults,
} from '../helpers/nodeProperties.js';
import { SafeDsPartialEvaluator } from '../partialEvaluation/safe-ds-partial-evaluator.js';
import { Constant, isConstant } from '../partialEvaluation/model.js';
Expand Down Expand Up @@ -453,7 +453,7 @@ export class SafeDsTypeComputer {
} else if (isSdsMemberType(node)) {
return this.computeType(node.member);
} else if (isSdsNamedType(node)) {
return this.computeType(node.declaration.ref).copyWithNullability(node.isNullable);
return this.computeType(node.declaration?.ref).copyWithNullability(node.isNullable);
} else if (isSdsUnionType(node)) {
const typeArguments = getTypeArguments(node.typeArgumentList);
return new UnionType(typeArguments.map((typeArgument) => this.computeType(typeArgument.value)));
Expand Down
141 changes: 0 additions & 141 deletions src/language/typing/typeConformance.ts

This file was deleted.

2 changes: 1 addition & 1 deletion src/language/validation/builtins/deprecated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export const argumentCorrespondingParameterShouldNotBeDeprecated =

export const namedTypeDeclarationShouldNotBeDeprecated =
(services: SafeDsServices) => (node: SdsNamedType, accept: ValidationAcceptor) => {
const declaration = node.declaration.ref;
const declaration = node.declaration?.ref;
if (!declaration) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion src/language/validation/builtins/experimental.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export const argumentCorrespondingParameterShouldNotBeExperimental =

export const namedTypeDeclarationShouldNotBeExperimental =
(services: SafeDsServices) => (node: SdsNamedType, accept: ValidationAcceptor) => {
const declaration = node.declaration.ref;
const declaration = node.declaration?.ref;
if (!declaration) {
return;
}
Expand Down
2 changes: 2 additions & 0 deletions src/language/validation/experimentalLanguageFeatures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export const indexedAccessesShouldBeUsedWithCaution = (node: SdsIndexedAccess, a
export const literalTypesShouldBeUsedWithCaution = (node: SdsLiteralType, accept: ValidationAcceptor): void => {
accept('warning', 'Literal types are experimental and may change without prior notice.', {
node,
keyword: 'literal',
code: CODE_EXPERIMENTAL_LANGUAGE_FEATURE,
});
};
Expand All @@ -47,6 +48,7 @@ export const unionTypesShouldBeUsedWithCaution = (node: SdsUnionType, accept: Va

accept('warning', 'Union types are experimental and may change without prior notice.', {
node,
keyword: 'union',
code: CODE_EXPERIMENTAL_LANGUAGE_FEATURE,
});
};
2 changes: 1 addition & 1 deletion src/language/validation/names.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ export const moduleMustContainUniqueNames = (node: SdsModule, accept: Validation
};

const importedDeclarationName = (node: SdsImportedDeclaration | undefined): string | undefined => {
return node?.alias?.alias ?? node?.declaration.ref?.name;
return node?.alias?.alias ?? node?.declaration?.ref?.name;
};

export const pipelineMustContainUniqueNames = (node: SdsPipeline, accept: ValidationAcceptor): void => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export const typeParameterConstraintLeftOperandMustBeOwnTypeParameter = (
node: SdsTypeParameterConstraint,
accept: ValidationAcceptor,
) => {
const typeParameter = node.leftOperand.ref;
const typeParameter = node.leftOperand?.ref;
if (!typeParameter) {
return;
}
Expand Down
47 changes: 47 additions & 0 deletions src/language/validation/other/declarations/typeParameters.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import {
isSdsCallable,
isSdsClass,
isSdsParameterList,
isSdsUnionType,
SdsTypeParameter,
} from '../../../generated/ast.js';
import { findLocalReferences, getContainerOfType, hasContainerOfType, ValidationAcceptor } from 'langium';

export const CODE_TYPE_PARAMETER_INSUFFICIENT_CONTEXT = 'type-parameter/insufficient-context';

export const typeParameterMustHaveSufficientContext = (node: SdsTypeParameter, accept: ValidationAcceptor) => {
const containingCallable = getContainerOfType(node, isSdsCallable);
/* c8 ignore start */
if (!containingCallable) {
return;
}
/* c8 ignore stop */

// Classes without constructor can only be used as named types, where type arguments are manifest
if (isSdsClass(containingCallable) && !containingCallable.parameterList) {
return;
}

// A type parameter must be referenced in the parameter list of the containing callable...
let typeParameterHasInsufficientContext =
!containingCallable.parameterList ||
findLocalReferences(node, containingCallable.parameterList)
// ...but references in a union type or in the parameter list of a callable type don't count
.filter((reference) => {
const referenceNode = reference.$refNode?.astNode;
const containingParameterList = getContainerOfType(referenceNode, isSdsParameterList);

return (
!hasContainerOfType(referenceNode, isSdsUnionType) &&
containingParameterList === containingCallable.parameterList
);
})
.isEmpty();

if (typeParameterHasInsufficientContext) {
accept('error', 'Insufficient context to infer this type parameter.', {
node,
code: CODE_TYPE_PARAMETER_INSUFFICIENT_CONTEXT,
});
}
};
5 changes: 3 additions & 2 deletions src/language/validation/other/types/namedTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,12 @@ export const namedTypeTypeArgumentListMustNotHavePositionalArgumentsAfterNamedAr

export const namedTypeMustNotHaveTooManyTypeArguments = (node: SdsNamedType, accept: ValidationAcceptor): void => {
// If the declaration is unresolved, we already show another error
if (!node.declaration.ref) {
const namedTypeDeclaration = node.declaration?.ref;
if (!namedTypeDeclaration) {
return;
}

const typeParameters = getTypeParameters(node.declaration.ref);
const typeParameters = getTypeParameters(namedTypeDeclaration);
const typeArguments = getTypeArguments(node.typeArgumentList);

if (typeArguments.length > typeParameters.length) {
Expand Down
Loading

0 comments on commit ea8fe29

Please sign in to comment.