unisonweb / unison

A friendly programming language from the future
https://unison-lang.org
Other
5.74k stars 267 forks source link

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

Open rlmark opened 11 months ago

rlmark commented 11 months ago

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

aryairani commented 4 months ago

@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.

rlmark commented 4 months ago

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

aryairani commented 1 month ago

Here's an updated transcript with ucm 0.5.25:

```ucm
temp/main> builtins.mergeio
unique type models.Drawing
  = { title : Text,
      author : Text,
      time : Nat,
      canvasData : Text }
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.

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:

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'
temp/main> add