Skip to content

Commit

Permalink
feat: optional error messages for constraints (#1275)
Browse files Browse the repository at this point in the history
Closes #1082

### Summary of Changes

Constraints can now have custom error messages. These can be static
strings or template strings that references constant parameter, e.g. in

```
@pure fun f(
    const p1: Int,
    const p2: Int = -2,
) where {
    p1 >= 0 else "This parameter must be non-negative.",
    p2 > p1 else "p2 must be greater than p1, but p2 was {{ p2 }} and p1 was {{ p1 }}.",
}
```
  • Loading branch information
lars-reimann authored Nov 29, 2024
1 parent d30edbc commit fce761c
Show file tree
Hide file tree
Showing 29 changed files with 295 additions and 66 deletions.
1 change: 1 addition & 0 deletions docs/src/lexer/safe_ds_lexer/_safe_ds_lexer.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

keywords_generic = (
"as",
"else",
"const",
"import",
"in",
Expand Down
5 changes: 4 additions & 1 deletion packages/safe-ds-lang/src/language/grammar/safe-ds.langium
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,9 @@ SdsConstraintList returns SdsConstraintList:
'}'
;

interface SdsConstraint extends SdsObject {}
interface SdsConstraint extends SdsObject {
message?: SdsString | SdsTemplateString
}

SdsConstraint returns SdsConstraint:
SdsParameterBound
Expand All @@ -400,6 +402,7 @@ SdsParameterBound returns SdsParameterBound:
leftOperand=[SdsParameter:ID]
operator=SdsComparisonOperator
rightOperand=SdsExpression
('else' message=(SdsString | SdsTemplateString))?
;


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ export class SafeDsFormatter extends AbstractFormatter {
const formatter = this.getNodeFormatter(node);

formatter.property('operator').surround(oneSpace());
formatter.keyword('else').surround(oneSpace());
}

// -----------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { isSdsParameter, isSdsReference, isSdsString, SdsConstraint } from '../../../generated/ast.js';
import { AstUtils, ValidationAcceptor } from 'langium';
import { Parameter } from '../../../helpers/nodeProperties.js';

export const CODE_CONSTRAINT_MESSAGE = 'constraint/message';

export const messageOfConstraintsMustOnlyReferenceConstantParameters = (
node: SdsConstraint,
accept: ValidationAcceptor,
) => {
if (!node.message || isSdsString(node.message)) {
return;
}

for (const expression of node.message.expressions) {
const isInvalid = AstUtils.streamAst(expression)
.filter(isSdsReference)
.map((reference) => reference.target.ref)
.some((target) => {
return target && (!isSdsParameter(target) || !Parameter.isConstant(target));
});

if (isInvalid) {
accept('error', 'The message of a constraint must only reference constant parameters.', {
node: expression,
property: 'target',
code: CODE_CONSTRAINT_MESSAGE,
});
}
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@ import {
} from '../../../generated/ast.js';
import { AstUtils, ValidationAcceptor } from 'langium';
import { getArguments, getParameters, Parameter } from '../../../helpers/nodeProperties.js';
import { Constant, EvaluatedNode, FloatConstant, IntConstant } from '../../../partialEvaluation/model.js';
import {
Constant,
EvaluatedNode,
FloatConstant,
IntConstant,
StringConstant,
} from '../../../partialEvaluation/model.js';

export const CODE_PARAMETER_BOUND_INVALID_VALUE = 'parameter-bound/invalid-value';
export const CODE_PARAMETER_BOUND_PARAMETER = 'parameter-bound/parameter';
Expand All @@ -34,10 +40,13 @@ export const callArgumentMustRespectParameterBounds = (services: SafeDsServices)

for (const bound of Parameter.getBounds(parameter)) {
const rightOperand = partialEvaluator.evaluate(bound.rightOperand, substitutions);
const errorMessage = checkBound(parameter.name, value, bound.operator, rightOperand);
const messageEvaluatedNode = partialEvaluator.evaluate(bound.message, substitutions);
const customMessage =
messageEvaluatedNode instanceof StringConstant ? messageEvaluatedNode.value : undefined;

if (errorMessage) {
accept('error', errorMessage, {
const error = checkBound(parameter.name, value, bound.operator, rightOperand, customMessage);
if (error) {
accept('error', error, {
node: argument,
property: 'value',
code: CODE_PARAMETER_BOUND_INVALID_VALUE,
Expand Down Expand Up @@ -74,12 +83,18 @@ export const parameterDefaultValueMustRespectParameterBounds = (services: SafeDs
}
}

const substitutions = new Map([[node, value]]);

// Error if the default value violates some bounds
for (const bound of Parameter.getBounds(node)) {
const rightOperand = partialEvaluator.evaluate(bound.rightOperand);
const errorMessage = checkBound(node.name, value, bound.operator, rightOperand);
if (errorMessage) {
accept('error', errorMessage, {
const messageEvaluatedNode = partialEvaluator.evaluate(bound.message, substitutions);
const customMessage =
messageEvaluatedNode instanceof StringConstant ? messageEvaluatedNode.value : undefined;

const error = checkBound(node.name, value, bound.operator, rightOperand, customMessage);
if (error) {
accept('error', error, {
node,
property: 'defaultValue',
code: CODE_PARAMETER_BOUND_INVALID_VALUE,
Expand All @@ -94,18 +109,23 @@ const checkBound = (
leftOperand: EvaluatedNode,
operator: string,
rightOperand: EvaluatedNode,
customMessage?: string,
): string | undefined => {
// Arguments must be valid
if (
(!(leftOperand instanceof FloatConstant) && !(leftOperand instanceof IntConstant)) ||
!isSdsComparisonOperator(operator) ||
(!(rightOperand instanceof FloatConstant) && !(rightOperand instanceof IntConstant))
) {
return;
return undefined;
}

const createMessage = (relation: string) => {
return `The value of '${parameterName}' must be ${relation} ${rightOperand.toString()} but was ${leftOperand.toString()}.`;
if (customMessage) {
return customMessage;
} else {
return `The value of '${parameterName}' must be ${relation} ${rightOperand.toString()} but was ${leftOperand.toString()}.`;
}
};

if (operator === '<') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ import {
outputStatementMustHaveValue,
outputStatementMustOnlyBeUsedInPipeline,
} from './other/statements/outputStatements.js';
import { messageOfConstraintsMustOnlyReferenceConstantParameters } from './other/declarations/constraints.js';

/**
* Register custom validation checks.
Expand Down Expand Up @@ -267,6 +268,7 @@ export const registerValidationChecks = function (services: SafeDsServices) {
classMemberMustMatchOverriddenMemberAndShouldBeNeeded(services),
overridingMemberPythonNameMustMatchOverriddenMember(services),
],
SdsConstraint: [messageOfConstraintsMustOnlyReferenceConstantParameters],
SdsConstraintList: [constraintListsShouldBeUsedWithCaution(services), constraintListShouldNotBeEmpty(services)],
SdsDeclaration: [
nameMustNotOccurOnCoreDeclaration(services),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
annotation MyAnnotation where { p > 0 else "" }

// -----------------------------------------------------------------------------

annotation MyAnnotation where {
p > 0 else ""
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
annotation MyAnnotation where { p >= 0 else "" }

// -----------------------------------------------------------------------------

annotation MyAnnotation where {
p >= 0 else ""
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
annotation MyAnnotation where { p < 0 else "" }

// -----------------------------------------------------------------------------

annotation MyAnnotation where {
p < 0 else ""
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
annotation MyAnnotation where { p <= 0 else "" }

// -----------------------------------------------------------------------------

annotation MyAnnotation where {
p <= 0 else ""
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
annotation MyAnnotation where { p < 0 , q > 1 }
annotation MyAnnotation where { p < 0 , q > 1 else "" }

// -----------------------------------------------------------------------------

annotation MyAnnotation where {
p < 0,
q > 1
q > 1 else ""
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// $TEST$ no_syntax_error

annotation MyAnnotation where {
p > 0 else "p must be positive but was {{ p }}"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// $TEST$ no_syntax_error

annotation MyAnnotation where {
p >= 0 else "p must be non-negative but was {{ p }}"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// $TEST$ no_syntax_error

annotation MyAnnotation where {
p < 0 else "p must be negative"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// $TEST$ no_syntax_error

annotation MyAnnotation where {
p <= 0 else "p must be non-positive"
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

annotation MyAnnotation where {
p < 0,
q > 0
q > 0 else "q must be positive"
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,27 @@ annotation MyAnnotation(
»own«: Int
) where {
// $TEST$ references own
»own« < 0,
// $TEST$ references own
»own« < 0 else "{{ »own« }}",

// $TEST$ unresolved
»before« < 0,
// $TEST$ unresolved
»before« < 0 else "{{ »before« }}",

// $TEST$ unresolved
»after« < 0,
// $TEST$ unresolved
»after« < 0 else "{{ »after« }}",

// $TEST$ unresolved
»notAParameter« < 0,
// $TEST$ references notAParameter
»notAParameter« < 0 else "{{ »notAParameter« }}",

// $TEST$ unresolved
»unresolved« < 0
// $TEST$ unresolved
»unresolved« < 0 else "{{ »unresolved« }}"
}

fun myFunction2(after: Int)

class notAParameter
// $TEST$ target notAParameter
class »notAParameter«
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,33 @@ class MyClass(container: Int) {
»own«: Int
) where {
// $TEST$ references own
»own« < 0,
// $TEST$ references own
»own« < 0 else "{{ »own« }}",

// $TEST$ unresolved
»container« < 0,
// $TEST$ unresolved
»container« < 0 else "{{ »container« }}",

// $TEST$ unresolved
»beforeEnum« < 0,
// $TEST$ unresolved
»beforeEnum« < 0 else "{{ »beforeEnum« }}",

// $TEST$ unresolved
»afterEnum« < 0,
// $TEST$ unresolved
»afterEnum« < 0 else "{{ »afterEnum« }}",

// $TEST$ unresolved
»notAParameter« < 0,
// $TEST$ references notAParameter
»notAParameter« < 0 else "{{ »notAParameter« }}",

// $TEST$ unresolved
»unresolved« < 0
// $TEST$ unresolved
»unresolved« < 0 else "{{ »unresolved« }}"
}
}

fun myFunction2(afterEnum: Int)
}

class notAParameter
// $TEST$ target notAParameter
class »notAParameter«
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,27 @@ class MyClass(
»own«: Int
) where {
// $TEST$ references own
»own« < 0,
// $TEST$ references own
»own« < 0 else "{{ »own« }}",

// $TEST$ unresolved
»before« < 0,
// $TEST$ unresolved
»before« < 0 else "{{ »before« }}",

// $TEST$ unresolved
»after« < 0,
// $TEST$ unresolved
»after« < 0 else "{{ »after« }}",

// $TEST$ unresolved
»notAParameter« < 0,
// $TEST$ references notAParameter
»notAParameter« < 0 else "{{ »notAParameter« }}",

// $TEST$ unresolved
»unresolved« < 0
// $TEST$ unresolved
»unresolved« < 0 else "{{ »unresolved« }}"
}

fun myFunction2(after: Int)

class notAParameter
// $TEST$ target notAParameter
class »notAParameter«
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,27 @@ fun myFunction2(
»own«: Int
) where {
// $TEST$ references own
»own« < 0,
// $TEST$ references own
»own« < 0 else "{{ »own« }}",

// $TEST$ unresolved
»before« < 0,
// $TEST$ unresolved
»before« < 0 else "{{ »before« }}",

// $TEST$ unresolved
»after« < 0,
// $TEST$ unresolved
»after« < 0 else "{{ »after« }}",

// $TEST$ unresolved
»notAParameter« < 0,
// $TEST$ references notAParameter
»notAParameter« < 0 else "{{ »notAParameter« }}",

// $TEST$ unresolved
»unresolved« < 0
// $TEST$ unresolved
»unresolved« < 0 else "{{ »unresolved« }}"
}

fun myFunction3(after: Int)

class notAParameter
// $TEST$ target notAParameter
class »notAParameter«
Loading

0 comments on commit fce761c

Please sign in to comment.