Trying to set Object property causes unclear error and in specific situations causes wrong error location

[seancrowe]

Summary

We have two related issues:

1. The error message from setting the property of ReScript Object with no @set decorator set is unclear, not helpful, and probably a bug. See example

Instead of

This expression has type hasName
  It has no method name#=
Hint: Did you mean name?

We would expect

The object field name is not mutable
Hint: Did you mean to add the @set decorator

2. If the type is inlined into the function like (p: {.. "name":string}) the error message will not show up until a value is passed into the function, and then the error is located at the wrong location. See example

We would expect the error message to be shown at the correct location at function creation.

Detailed Discussion of Issues

Issue 1

We have a type hasName which is has a property "name" that is of type string

type hasName = {
  "name": string,
}

When that type is used in a function where the property tries to be set, we get an error.

let printNameE = (p:hasName) => {
  p["name"] = "changed"
  Js.log(p["name"])
}

The error reads:

This expression has type hasName
  It has no method name#=
Hint: Did you mean name?

The error will be located on the p scoped variable.

See example

Getting an error makes sense as we cannot modify the property of a ReScript Object unless the @set decorator is used. However, from an end user perspective, the error message makes no sense. My guess is that it is a semi-bug. The hint is also not helpful, because it suggests what you are doing, using "name".

The error instead should be something like this:

The object field name is not mutable
Hint: Did you mean to add the @set decorator

Issue 2 - The Bigger Bug 🐛

However, the situation gets worse, because in a real-world use case the hasName type would not be used and instead the type would be inlined like so

let printName = (p: {.. "name":string}) => {
  p["name"] = "changed"
  Js.log(p["name"])
}

When you inline the type, then you get no errors.

See example

The error will not show up until you try and pass a value into the function. Even then, the error location is at the value passed into the function call, not the p scoped variable.

See example

In addition, the error is similar, but worse as it adds more details that are not really relevant to the situation. Someone new to JavaScript and ReScript would be very confused on how to fix this situation.

So beyond getting an even less clear error, the error is located in the wrong place, which could be very hard to track down especially if this function is used in a completely different file.

Note

If you remove the .. syntax, the unclear error shows up again in the correct location. See example. However,

However, having the .. syntax or not should not confused the compiler. The .. syntax, just means "and bring everything else". Since we defined "name" in the type signature it is not possible another "name" of a different type (not string) can exist.

Also keep in mind that type hasName is not equal to the inline {"name":string} but more equal to the inline {.. "name":string}

Another Consequence 🐛

So, another unfortunate consequence of this incorrect error placement is that it will consume (hide) any errors about passing the wrong Object shape into the function printName(). So, if we try to pass in an Object where "name" is defined as an int value, the error message will not be

You can convert int to string with Belt.Int.toString.

but instead, it will be no method name#= error message discussed above. See example

This is even worse, as someone new to JavaScript or ReScript would be even more confused and probably not be able to solve the situation from the error message alone.

Use Case

This is not an arbitrary use case. It is part of Data Oriented Programming, and more generically FP, to pass in data that can be transformed over a series of function calls. These function calls do not care about the complete shape of the data, but only parts of the data that the function is interested in transforming (or causing a side effect). This is a common practice in Clojure and semi-common in JavaScript

To be type safe, (which is a benefit of ReScript), you will want to define what the function requires from the Object, but not want the complete shape of the Object.

A real-world example is that the company I work for has client data. Each client, and their data, is represented by an Object. (Not a Record because the data will transform as it travels through the system.) The client Object contains a lot of properties. However, a function called getClientSLA() only cares about an Object of shape {.. sla:sla} where type sla is another Object shape:

type sla = {
  "p1": int,
  "p2": int,
  "p3": int
}

getClientSLA() does not care about anything else in the Object passed, nor should it.

Since many people are coming from an OOP to FP paradigm, it is not uncommon for someone to try and set an Object property. Thus, this unclear error situation can at best be confusing and at worse go unnoticed until much later.

Tested

Tested on Windows, using ReScript 9.1.4 and 10.0.0-rc.1

[mununki]

I'm not going to say that the error msg is kind or informative enough, but the object is not allowed to be updated basically. https://rescript-lang.org/docs/manual/latest/object#update The usage of @set is only for the binding to javascript object. In this regard, the error ~~msg~~ of issue 1 is correct, because the arg p has annotated that it has a type hasAName. The error msg of issue 2 is not wrong either. The type checker has no idea that name in the inlined type annotation and name to be updated are the same. Therefore, the type checker can't throw the error until it is evaluated.

[seancrowe]

@mattdamon108 yeah issue 1 I was thinking it may just be an improvement.

Issue 2, being expected does not make sense to me. I am naive of the compiler innerworkings, but that also makes me a good person to report the developer experience.

So, the reason issue 2 does not make sense to me is that the only difference in behavior is due to the .. syntax. See example

To your point:

The type checker has no idea that name in the inlined type annotation and name to be updated are the same

From my naive experience, the .. shouldn't suddenly confuse the compiler that the property does or doesn't exist, when it is clearly defined in the inline. I would expect the behavior with or without .. to be the same in this regard.

Especially as mentioned in my original post, these two types appear to me to be about the same type - or at least that is what the compiler hints tell me.

This also weird from a developer experience, because after doing some more testing, the .. syntax behaves as expected in other scenarios, such as trying to use the wrong types: example. I haven't tested every scenario FYI.

[mununki]

First of all, the inlined annotated type {.. "name": string} or {"name": string} is not inferred to the type withName. Those are not bounded to each other at all. As the doc of the object says, the object doesn't need the type declaration explicitly. Its type is inferred by its values. So, type withName is not unified to the inlined object type annotation{.. "name": string} or `{"name": string} in function arg in type inference, unless you annotate it directly.

From my naive experience, the .. shouldn't suddenly confuse the compiler that the property does or doesn't exist, when it is clearly defined in the inline.

Actually, .. is causing a different result by the compiler. One is open object, and the other is closed object. The different outputs by compiler are caused by one more reason which is the object is not updated basically. I agree that a better error msg will bring a better developer experience.

[cristianoc]

The fundamental difficulty here are that certain aspects of objects are not native. They're implemented by adding methods, and the implementation details leak into the error messages. The way to improve on this would be making objects completely native.

[cristianoc]

If one could make them also more similar to the recently introduced records with optional fields, that would also help to have a more cohesive user experience.

[seancrowe]

First of all, the inlined annotated type {.. "name": string} or {"name": string} is not inferred to the type withName

After more testing, I see these types are not as similar as I first tested