Skip to content

Commit

Permalink
feat: error if index of indexed access is invalid (#796)
Browse files Browse the repository at this point in the history
Closes #16

### Summary of Changes

* Show an error if we know that a map key does not exist.
* Show an error if we know that a list index is out of bounds.
  • Loading branch information
lars-reimann authored Nov 24, 2023
1 parent 2d1abc9 commit 5017759
Show file tree
Hide file tree
Showing 6 changed files with 209 additions and 40 deletions.
16 changes: 16 additions & 0 deletions packages/safe-ds-lang/src/language/partialEvaluation/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,13 @@ export class EvaluatedList extends EvaluatedNode {
return this.elements[index] ?? UnknownEvaluatedNode;
}

/**
* Returns the size of the list.
*/
get size(): number {
return this.elements.length;
}

override equals(other: unknown): boolean {
if (other === this) {
return true;
Expand Down Expand Up @@ -331,6 +338,15 @@ export class EvaluatedMap extends EvaluatedNode {
return this.entries.findLast((it) => it.key.equals(key))?.value ?? UnknownEvaluatedNode;
}

/**
* Returns whether the map contains the given key.
*
* @param key The key to look for.
*/
has(key: EvaluatedNode): boolean {
return this.entries.some((it) => it.key.equals(key));
}

override equals(other: unknown): boolean {
if (other === this) {
return true;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import { AstNode, AstNodeLocator, getDocument, WorkspaceCache } from 'langium';
import { AstNode, AstNodeLocator, getContainerOfType, getDocument, WorkspaceCache } from 'langium';
import { isEmpty } from '../../helpers/collectionUtils.js';
import {
isSdsArgument,
isSdsAssignee,
isSdsAssignment,
isSdsBlockLambda,
isSdsBoolean,
isSdsCall,
isSdsDeclaration,
isSdsEnumVariant,
isSdsExpression,
isSdsExpressionLambda,
Expand All @@ -16,6 +19,7 @@ import {
isSdsMap,
isSdsMemberAccess,
isSdsNull,
isSdsParameter,
isSdsParenthesizedExpression,
isSdsPrefixOperation,
isSdsReference,
Expand All @@ -25,15 +29,17 @@ import {
isSdsTemplateStringEnd,
isSdsTemplateStringInner,
isSdsTemplateStringStart,
SdsAssignee,
SdsCall,
SdsDeclaration,
SdsExpression,
SdsIndexedAccess,
SdsInfixOperation,
SdsList,
SdsMap,
SdsMemberAccess,
SdsParameter,
SdsPrefixOperation,
SdsReference,
SdsTemplateString,
} from '../generated/ast.js';
import { getArguments, getParameters } from '../helpers/nodeProperties.js';
Expand All @@ -47,6 +53,7 @@ import {
EvaluatedList,
EvaluatedMap,
EvaluatedMapEntry,
EvaluatedNamedTuple,
EvaluatedNode,
ExpressionLambdaClosure,
FloatConstant,
Expand Down Expand Up @@ -111,15 +118,60 @@ export class SafeDsPartialEvaluator {
node: AstNode | undefined,
substitutions: ParameterSubstitutions,
): EvaluatedNode {
if (isSdsExpression(node)) {
if (isSdsAssignee(node)) {
return this.evaluateAssignee(node, substitutions);
} else if (isSdsDeclaration(node)) {
return this.evaluateDeclaration(node, substitutions);
} else if (isSdsExpression(node)) {
return this.evaluateExpression(node, substitutions);
} /* c8 ignore start */ else {
return UnknownEvaluatedNode;
} /* c8 ignore stop */
}

private evaluateAssignee(node: SdsAssignee, substitutions: ParameterSubstitutions): EvaluatedNode {
const containingAssignment = getContainerOfType(node, isSdsAssignment);
if (!containingAssignment) {
/* c8 ignore next 2 */
return UnknownEvaluatedNode;
}

const evaluatedExpression = this.evaluateWithSubstitutions(containingAssignment.expression, substitutions);
const nodeIndex = node.$containerIndex ?? -1;
if (evaluatedExpression instanceof EvaluatedNamedTuple) {
/* c8 ignore next */ // TODO test
return evaluatedExpression.getSubstitutionByIndex(nodeIndex);
} else if (nodeIndex === 0) {
return evaluatedExpression;
} else {
/* c8 ignore next 2 */ // TODO test
return UnknownEvaluatedNode;
}
}

private evaluateDeclaration(node: SdsDeclaration, substitutions: ParameterSubstitutions): EvaluatedNode {
if (isSdsEnumVariant(node)) {
return new EvaluatedEnumVariant(node, undefined);
} else if (isSdsParameter(node)) {
return this.evaluateParameter(node, substitutions);
} else if (isSdsSegment(node)) {
return new NamedCallable(node);
} else {
return UnknownEvaluatedNode;
}
}

private evaluateParameter(node: SdsParameter, substitutions: ParameterSubstitutions): EvaluatedNode {
if (substitutions.has(node)) {
/* c8 ignore next */ // TODO test
return substitutions.get(node)!;
} else if (node.defaultValue) {
return this.evaluateWithSubstitutions(node.defaultValue, substitutions);
} else {
return UnknownEvaluatedNode;
}
}

private evaluateExpression(node: SdsExpression, substitutions: ParameterSubstitutions): EvaluatedNode {
// Base cases
if (isSdsBoolean(node)) {
Expand Down Expand Up @@ -164,7 +216,7 @@ export class SafeDsPartialEvaluator {
} else if (isSdsPrefixOperation(node)) {
return this.evaluatePrefixOperation(node, substitutions);
} else if (isSdsReference(node)) {
return this.evaluateReference(node, substitutions);
return this.evaluateWithSubstitutions(node.target.ref, substitutions);
} else if (isSdsTemplateString(node)) {
return this.evaluateTemplateString(node, substitutions);
} /* c8 ignore start */ else {
Expand Down Expand Up @@ -508,10 +560,10 @@ export class SafeDsPartialEvaluator {
return UnknownEvaluatedNode;
}

private evaluateMemberAccess(node: SdsMemberAccess, _substitutions: ParameterSubstitutions): EvaluatedNode {
private evaluateMemberAccess(node: SdsMemberAccess, substitutions: ParameterSubstitutions): EvaluatedNode {
const member = node.member?.target?.ref;
if (isSdsEnumVariant(member)) {
return new EvaluatedEnumVariant(member, undefined);
return this.evaluateWithSubstitutions(member, substitutions);
}

// return when (val simpleReceiver = receiver.evaluate(substitutions)) {
Expand All @@ -525,40 +577,6 @@ export class SafeDsPartialEvaluator {
return UnknownEvaluatedNode;
}

private evaluateReference(_node: SdsReference, _substitutions: ParameterSubstitutions): EvaluatedNode {
// TODO: always call evaluateWithSubstitutions so caching works
// const target = node.target.ref;

// is SdsPlaceholder -> declaration.evaluateAssignee(substitutions)
// is SdsParameter -> declaration.evaluateParameter(substitutions)
// else -> undefined
// }
return UnknownEvaluatedNode;
}

// private fun SdsAbstractAssignee.evaluateAssignee(substitutions: ParameterSubstitutions): SdsSimplifiedExpression? {
// val simpleFullAssignedExpression = closestAncestorOrNull<SdsAssignment>()
// ?.expression
// ?.evaluate(substitutions)
// ?: return undefined
//
// return when (simpleFullAssignedExpression) {
// is SdsIntermediateRecord -> simpleFullAssignedExpression.getSubstitutionByIndexOrNull(indexOrNull())
// else -> when {
// indexOrNull() == 0 -> simpleFullAssignedExpression
// else -> undefined
// }
// }
// }
//
// private fun SdsParameter.evaluateParameter(substitutions: ParameterSubstitutions): SdsSimplifiedExpression? {
// return when {
// this in substitutions -> substitutions[this]
// isOptional() -> defaultValue?.evaluate(substitutions)
// else -> undefined
// }
// }

// -----------------------------------------------------------------------------------------------------------------
// canBeValueOfConstantParameter
// -----------------------------------------------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { SdsIndexedAccess } from '../../../generated/ast.js';
import { ValidationAcceptor } from 'langium';
import { SafeDsServices } from '../../../safe-ds-module.js';
import { EvaluatedList, EvaluatedMap, IntConstant } from '../../../partialEvaluation/model.js';

export const CODE_INDEXED_ACCESS_INVALID_INDEX = 'indexed-access/invalid-index';

export const indexedAccessIndexMustBeValid = (services: SafeDsServices) => {
const coreTypes = services.types.CoreTypes;
const partialEvaluator = services.evaluation.PartialEvaluator;
const typeComputer = services.types.TypeComputer;

return (node: SdsIndexedAccess, accept: ValidationAcceptor): void => {
const indexValue = partialEvaluator.evaluate(node.index);
if (!indexValue.isFullyEvaluated) {
return;
}

const receiverValue = partialEvaluator.evaluate(node.receiver);

// Check map key
if (receiverValue instanceof EvaluatedMap) {
if (!receiverValue.has(indexValue)) {
accept('error', `Map key '${indexValue}' does not exist.`, {
node,
property: 'index',
code: CODE_INDEXED_ACCESS_INVALID_INDEX,
});
}
return;
}

// Check list index
if (!(indexValue instanceof IntConstant)) {
return;
}

if (receiverValue instanceof EvaluatedList) {
if (indexValue.value < 0 || indexValue.value >= receiverValue.size) {
accept('error', `List index '${indexValue}' is out of bounds.`, {
node,
property: 'index',
code: CODE_INDEXED_ACCESS_INVALID_INDEX,
});
}
}

const receiverType = typeComputer.computeType(node.receiver);
if (receiverType.equals(coreTypes.List)) {
if (indexValue.value < 0) {
accept('error', `List index '${indexValue}' is out of bounds.`, {
node,
property: 'index',
code: CODE_INDEXED_ACCESS_INVALID_INDEX,
});
}
}
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ import {
yieldTypeMustMatchResultType,
} from './types.js';
import { statementMustDoSomething } from './other/statements/statements.js';
import { indexedAccessIndexMustBeValid } from './other/expressions/indexedAccess.js';

/**
* Register custom validation checks.
Expand Down Expand Up @@ -270,6 +271,7 @@ export const registerValidationChecks = function (services: SafeDsServices) {
SdsImport: [importPackageMustExist(services), importPackageShouldNotBeEmpty(services)],
SdsImportedDeclaration: [importedDeclarationAliasShouldDifferFromDeclarationName(services)],
SdsIndexedAccess: [
indexedAccessIndexMustBeValid(services),
indexedAccessIndexMustHaveCorrectType(services),
indexedAccessReceiverMustBeListOrMap(services),
indexedAccessesShouldBeUsedWithCaution(services),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package tests.validation.other.indexedAccess.listIndexOutOfBounds

@Pure
fun myFunction() -> resultList: List<Int>

segment mySegment(parameterList: List<Int>, parameterIndex: Int) {
val placeholderList = [1];

// $TEST$ error "List index '-1' is out of bounds."
myFunction()[»-1«];
// $TEST$ no error r"List index '.*' is out of bounds\."
myFunction()[»0«];
// $TEST$ no error r"List index '.*' is out of bounds\."
myFunction()[»1«];
// $TEST$ no error r"List index '.*' is out of bounds\."
myFunction()[»parameterIndex«];

// $TEST$ error "List index '-1' is out of bounds."
parameterList[»-1«];
// $TEST$ no error r"List index '.*' is out of bounds\."
parameterList[»0«];
// $TEST$ no error r"List index '.*' is out of bounds\."
parameterList[»1«];
// $TEST$ no error r"List index '.*' is out of bounds\."
parameterList[»parameterIndex«];

// $TEST$ error "List index '-1' is out of bounds."
placeholderList[»-1«];
// $TEST$ no error r"List index '.*' is out of bounds\."
placeholderList[»0«];
// $TEST$ error "List index '1' is out of bounds."
placeholderList[»1«];
// $TEST$ no error r"List index '.*' is out of bounds\."
placeholderList[»parameterIndex«];

// $TEST$ no error r"List index '.*' is out of bounds\."
unresolved[»-1«];
// $TEST$ no error r"List index '.*' is out of bounds\."
placeholderList[»unresolved«];
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package tests.validation.other.indexedAccess.mapKeyDoesNotExist

@Pure
fun myFunction() -> resultMap: Map<String, Int>

segment mySegment(parameterMap: Map<String, Int>, parameterKey: String) {
val placeholderMap = {"key": 1};

// $TEST$ no error r"Map key '.*' does not exist\."
myFunction()[»"unknown"«];
// $TEST$ no error r"Map key '.*' does not exist\."
myFunction()[»"key"«];
// $TEST$ no error r"Map key '.*' does not exist\."
myFunction()[»parameterKey«];

// $TEST$ no error r"Map key '.*' does not exist\."
parameterMap[»"unknown"«];
// $TEST$ no error r"Map key '.*' does not exist\."
parameterMap[»"key"«];
// $TEST$ no error r"Map key '.*' does not exist\."
parameterMap[»parameterKey«];

// $TEST$ error "Map key '"unknown"' does not exist."
placeholderMap[»"unknown"«];
// $TEST$ no error r"Map key '.*' does not exist\."
placeholderMap[»"key"«];
// $TEST$ no error r"Map key '.*' does not exist\."
placeholderMap[»parameterKey«];

// $TEST$ no error r"Map key '.*' does not exist\."
unresolved[»"unknown"«];
// $TEST$ no error r"Map key '.*' does not exist\."
placeholderMap[»unresolved«];
}

0 comments on commit 5017759

Please sign in to comment.