unison icon indicating copy to clipboard operation
unison copied to clipboard

unhelpful error message on an accidentally recursive call to a name suffix

Open rlmark opened this issue 2 years ago • 3 comments

version: release/M5f

Today when trying to write a function named

test.gen.Drawing : '{Random} Drawing

With a record type in my codebase sharing the name suffix (but not the path)

unique type models.Drawing = {blah: Text...}

I found that this is an invalid name that cannot be added to the UCM because it gets interpreted as one of the record type data constructors.

The easy workaround is to add additional paths to the term name following the type name:

test.generators.Drawing.gen : '{Random} Drawing

A transcript replicating the issue is attached recordTypeIssue.md

rlmark avatar Sep 26 '23 19:09 rlmark

@rlmark Sorry for the delay:

I think what's happening here—today, at least, which may be different from what happened back when the bug was first submitted—is:

test.gen.Drawing = do
  title' : Text
  title' = "ksdf"
  author' : Text
  author' = "skdfj"
  time' : Nat
  time' = 34324
  canvasData' : Text
  canvasData' = "badfslkdfjsldf"
  Drawing title' author' time' canvasData'

Drawing on the last line is being interpreted as a recursive call to the function, not as the constructor test.gen.Drawing.Drawing as you meant it. This is "working as intended" although the error message isn't helpful.

Calling it with a unique suffix works, e.g.:

Drawing.Drawing title' author' time' canvasData'
gen.Drawing.Drawing title' author' time' canvasData'
test.gen.Drawing.Drawing title' author' time' canvasData'

I'm going to reclassify this as an "error message" improvement; let me know what you think.

aryairani avatar May 01 '24 18:05 aryairani

That sounds great @aryairani. That seems like what it's doing.

rlmark avatar May 01 '24 18:05 rlmark

Here's an updated transcript with ucm 0.5.25:

```ucm
temp/main> builtins.mergeio
```


```unison
unique type models.Drawing
  = { title : Text,
      author : Text,
      time : Nat,
      canvasData : Text }
```


```ucm
temp/main> add
```

This doesn't work, it thinks `Drawing` is a recursive call. It talks about having found a value of type `o` (the unknown (o)utput type of `test.gen.Drawing`) where a value of type `Text ->{𝕖2} Nat ->{𝕖1} Text ->{𝕖} o` is expected.  
```unison:error
test.gen.Drawing = do
  title' : Text
  title' = "ksdf"
  author' : Text
  author' = "skdfj"
  time' : Nat
  time' = 34324
  canvasData' : Text
  canvasData' = "badfslkdfjsldf"
  Drawing title' author' time' canvasData'
```
It would be nice if it mention `Drawing` is not a unique suffix (even though it is unique within the file), that it's assuming you meant `test.gen.Drawing` "which is a recursive call" (if that's easy to do, otherwise skip), but that some other possible matches include
models.Drawing.Drawing, "which also has the right type" (if that's easy to do, otherwise skip).

So, assuming everything was easy:

> I found a value of type: `o` (highlighting `Drawing`)
> where I expected to find: `Text ->{𝕖2} Nat ->{𝕖1} Text ->{𝕖} o`
>
> [source code]
>
> I assumed that `Drawing` means `test.gen.Drawing`, which is a recursive call, but some other possible matches include:
> 
>   models.Drawing.Drawing : Text -> Nat -> Text -> models.Drawing (which has a compatible type)

This one is okay, I've qualified the constructor name:
```unison
test.gen.Drawing = do
  title' : Text
  title' = "ksdf"
  author' : Text
  author' = "skdfj"
  time' : Nat
  time' = 34324
  canvasData' : Text
  canvasData' = "badfslkdfjsldf"
  models.Drawing.Drawing title' author' time' canvasData'
```

```ucm
temp/main> add
```

aryairani avatar Jul 23 '24 20:07 aryairani