Skip to content

Commit

Permalink
More descriptive function information (#7629)
Browse files Browse the repository at this point in the history
Fixes #7359 by printing more information about the function including partially applied arguments and over-saturated arguments.
  • Loading branch information
JaroslavTulach authored Aug 24, 2023
1 parent dbd6bff commit 20e18d2
Show file tree
Hide file tree
Showing 7 changed files with 245 additions and 14 deletions.
23 changes: 16 additions & 7 deletions distribution/lib/Standard/Base/0.0.0-dev/src/Errors/Common.enso
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import project.Errors.Common.Arithmetic_Error
import project.Meta
import project.Nothing.Nothing
import project.Panic.Panic
import project.Function.Function
import project.Data.Text.Extensions

from project.Data.Boolean import Boolean, False, True

Expand Down Expand Up @@ -58,16 +60,23 @@ type Type_Error
- name: The name of the argument whose type is mismatched.
Error expected actual name

## PRIVATE
Infer the type of the actual value in a human-readable format.
type_of_actual self =
tpe = Meta.type_of self.actual
if tpe.is_error then self.actual.to_display_text else tpe.to_display_text

## PRIVATE
Convert the Type_Error error to a human-readable format.
to_display_text : Text
to_display_text self = "Type error: expected `"+self.name+"` to be "+self.expected.to_display_text+", but got "+self.type_of_actual+"."
to_display_text self =
type_of_actual =
tpe = Meta.type_of self.actual
if tpe.is_error then self.actual.to_display_text else case self.actual of
fn:Function ->
fn_info = fn.to_text
missing_args = fn_info.split " " . filter (_.ends_with "=_") . map (_.split "=") . map (_.at 0)
fn_info + case missing_args.length of
0 -> ""
1 -> ". Try to apply " + (missing_args.at 0) + " argument"
_ -> ". Try to apply " + (missing_args.join ", ") + " arguments"
_ -> tpe.to_display_text

"Type error: expected `"+self.name+"` to be "+self.expected.to_display_text+", but got "+type_of_actual+"."

@Builtin_Type
type Compile_Error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class ReplTest
result.toString shouldEqual "Error in method `to_text` of [Bar 1]: Expected Text but got 42"
}
inside(executor.evaluate("C.Baz 1")) { case Right(result) =>
result.toString shouldEqual "Error in method `to_text` of [Baz 1]: Expected Text but got C.to_text[Test:18-40]"
result.toString shouldEqual "Error in method `to_text` of [Baz 1]: Expected Text but got C.to_text[Test:18:1-29]"
}
inside(executor.evaluate("Pattern.compile 'foo'")) {
case Right(result) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,10 @@ private String toString(String prefix, int depth, String suffix, Object obj) {
sb.append(suffix);
}
if (obj != null) {
var errorMessage = InteropLibrary.getUncached().toDisplayString(obj);
var errorMessage = switch (obj) {
case Function fn -> fn.toString(false);
default -> InteropLibrary.getUncached().toDisplayString(obj);
};
if (errorMessage != null) {
sb.append(errorMessage);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,14 @@ Atom instantiate(Object... arguments) throws ArityException {
@ExportMessage
@TruffleBoundary
String toDisplayString(boolean allowSideEffects) {
return "Constructor<" + getDisplayName() + ">";
var sb = new StringBuilder();
sb.append("Constructor<").append(getDisplayName()).append(">");
for (var f : getFields()) {
if (!f.hasDefaultValue()) {
sb.append(" ").append(f.getName()).append("=_");
}
}
return sb.toString();
}

/** @return the fully qualified name of this constructor. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,16 +402,53 @@ String toDisplayString(boolean sideEffects) {
}

@Override
@CompilerDirectives.TruffleBoundary
public String toString() {
return toString(true);
}

@CompilerDirectives.TruffleBoundary
public final String toString(boolean includeArguments) {
var n = callTarget.getRootNode();
var ss = n.getSourceSection();
if (ss == null) {
return super.toString();
}
var s = ss.getSource();
var iop = InteropLibrary.getUncached();
var src = ss.getSource();
var start = ss.getStartLine();
final int end = start + s.getLineCount();
return n.getName() + "[" + s.getName() + ":" + start + "-" + end + "]";
var end = ss.getEndLine();
var sb = new StringBuilder();
sb.append(n.getName());
sb.append("[").append(src.getName()).append(":").append(start);
if (end == start) {
sb.append(":").append(ss.getStartColumn()).append("-").append(ss.getEndColumn());
} else {
sb.append("-").append(end);
}
sb.append("]");
if (includeArguments) {
for (var i = 0; i < schema.getArgumentsCount(); i++) {
ArgumentDefinition info = schema.getArgumentInfos()[i];
if (info.hasDefaultValue()) {
continue;
}
var name = info.getName();
sb.append(" ").append(name).append("=");
if (preAppliedArguments != null && preAppliedArguments[i] != null) {
sb.append(iop.toDisplayString(preAppliedArguments[i], false));
} else {
sb.append("_");
}
}
if (schema.getOversaturatedArguments() != null) {
for (var i = 0; i < schema.getOversaturatedArguments().length; i++) {
if (oversaturatedArguments != null && oversaturatedArguments[i] != null) {
sb.append(" +").append(schema.getOversaturatedArguments()[i].getName()).append("=");
sb.append(iop.toDisplayString(oversaturatedArguments[i], false));
}
}
}
}
return sb.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.net.URI;
import java.net.URISyntaxException;

import org.graalvm.polyglot.Context;
import org.graalvm.polyglot.PolyglotException;
import org.graalvm.polyglot.Source;
Expand Down Expand Up @@ -353,6 +354,109 @@ public void binaryWithVec() throws Exception {
assertEquals("binary.Bin", ok3.getMetaObject().getMetaQualifiedName());
}

@Test
public void partiallyAppliedConstructor() throws Exception {
final URI uri = new URI("memory://partial.enso");
final Source src = Source.newBuilder("enso", """
from Standard.Base import Integer
type V
Val a b c
create x:V = x.a + x.b + x.c
mix a =
partial = V.Val 1 a
create partial
""", uri.getHost())
.uri(uri)
.buildLiteral();

var module = ctx.eval(src);
var mix = module.invokeMember("eval_expression", "mix");

try {
var res = mix.execute(7);
fail("No result expected: " + res);
} catch (PolyglotException ex) {
assertContains("Type error", ex.getMessage());
assertContains("expected `x` to be V", ex.getMessage());
assertContains("got V.Val[partial", ex.getMessage());
assertContains("a=1", ex.getMessage());
assertContains("b=7", ex.getMessage());
assertContains("c=_", ex.getMessage());
}
}

@Test
public void oversaturatedFunction() throws Exception {
final URI uri = new URI("memory://oversaturated.enso");
final Source src = Source.newBuilder("enso", """
from Standard.Base import Integer
fn a b c =
sum = a + b + c
add a = sum + a
add
neg x:Integer = -x
mix n = neg (fn 2 a=4 n)
""", uri.getHost())
.uri(uri)
.buildLiteral();

var module = ctx.eval(src);
var mix = module.invokeMember("eval_expression", "mix");

try {
var res = mix.execute(7);
fail("No result expected: " + res);
} catch (PolyglotException ex) {
assertContains("Type error", ex.getMessage());
assertContains("expected `x` to be Integer", ex.getMessage());
assertContains("got oversaturated.fn[", ex.getMessage());
assertContains("a=2", ex.getMessage());
assertContains("b=7", ex.getMessage());
assertContains("c=_", ex.getMessage());
assertContains("+a=4", ex.getMessage());
}
}

@Test
public void suspendedArgumentsUnappliedFunction() throws Exception {
final URI uri = new URI("memory://suspended.enso");
final Source src = Source.newBuilder("enso", """
from Standard.Base import Integer
fn ~a ~b ~c =
add x = if x == 0 then 0 else x * (a + b + c)
add
neg x:Integer = -x
mix a = neg (fn c=(2/0) b=(a/0))
""", uri.getHost())
.uri(uri)
.buildLiteral();

var module = ctx.eval(src);
var mix = module.invokeMember("eval_expression", "mix");

try {
var res = mix.execute(0);
fail("No result expected: " + res);
} catch (PolyglotException ex) {
assertContains("Type error", ex.getMessage());
assertContains("expected `x` to be Integer", ex.getMessage());
assertContains("got suspended.fn[", ex.getMessage());
assertContains("a=_", ex.getMessage());
assertContains("b=suspended.mix<arg-b>", ex.getMessage());
assertContains("c=suspended.mix<arg-c>", ex.getMessage());
assertContains("[suspended:9:28-30]", ex.getMessage());
}
}

private static void assertTypeError(String expArg, String expType, String realType, String msg) {
if (!msg.contains(expArg)) {
fail("Expecting value " + expArg + " in " + msg);
Expand All @@ -364,4 +468,10 @@ private static void assertTypeError(String expArg, String expType, String realTy
fail("Expecting value " + realType + " in " + msg);
}
}

private static void assertContains(String exp, String msg) {
if (!msg.contains(exp)) {
fail("Expecting " + msg + " to contain " + exp);
}
}
}
65 changes: 65 additions & 0 deletions test/Tests/src/Semantic/Error_Spec.enso
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from Standard.Base import all
import Standard.Base.Errors.Common.Unsupported_Argument_Types
import Standard.Base.Errors.Common.Type_Error
import Standard.Base.Errors.Illegal_Argument.Illegal_Argument
import Standard.Base.Errors.Illegal_State.Illegal_State

Expand All @@ -16,6 +17,8 @@ import Standard.Test.Extensions

type My_Type
Value foo
Multi_Value foo bar
Default_Value foo=2 bar

throw_a_bar = Error.throw "bar"
throw_a_bar_panicking = Panic.throw "bar"
Expand Down Expand Up @@ -341,4 +344,66 @@ spec =
c2 . should_equal "finalizer"
v2.to_vector . should_equal [1, 2]

Test.group "Type Errors" <|
my_func x y =
x + y

my_defaulted_func x=5 y =
x + y

neg n:Number = -n

extract x:My_Type = x.foo

Test.specify "everything is ok" <|
neg (my_func -5 -2) . should_equal 7

Test.specify "try to apply one argument" <|
r = Panic.recover Type_Error <| neg (my_func -5)
r . should_fail_with Type_Error
r.to_display_text.should_contain "Try to apply y argument."

Test.specify "try to apply two arguments" <|
r = Panic.recover Type_Error <| neg my_func
r . should_fail_with Type_Error
r.to_display_text.should_contain "Try to apply x, y arguments."

Test.specify "apply two arguments with one defaulted" <|
r = Panic.recover Type_Error <| neg my_defaulted_func
r . should_fail_with Type_Error
r.to_display_text.should_contain "Try to apply y argument."

Test.specify "report unapplied constructor nicely" <|
r = Panic.recover Type_Error <| extract My_Type.Value
r . should_fail_with Type_Error
r.to_display_text.should_contain "Try to apply foo argument."

Test.specify "report unapplied constructor with default value nicely" <|
r = Panic.recover Type_Error <| extract My_Type.Default_Value
r . should_fail_with Type_Error
r.to_display_text.should_contain "Try to apply bar argument."

Test.specify "report partially applied constructor nicely" <|
r = Panic.recover Type_Error <| extract (My_Type.Multi_Value 42)
r . should_fail_with Type_Error
r.to_display_text.should_contain "Try to apply bar argument."

Test.specify "try to apply two arguments with over-saturated" <|
r = Panic.recover Type_Error <| neg (my_func z=10)
r . should_fail_with Type_Error
r.to_display_text . should_contain "Try to apply x, y arguments"

Test.specify "types and unapplied arguments" <|
c = C.Baz C.to_text
r = Panic.recover Type_Error <| neg (c.to_num c=3)
r . should_fail_with Type_Error
r.to_display_text . should_contain "Try to apply a, b arguments"



type C
Baz x

C.to_num self a b c = a+b+c

main = Test_Suite.run_main spec

0 comments on commit 20e18d2

Please sign in to comment.