Produce more helpful output when module output is overwritten

[HT154]

Evaluating this module:

output: String = "abc"

Now produces

–– Pkl Error ––
Expected `output` of module `file:///$snippetsDir/input/modules/invalidOutput1.pkl` to be of type `ModuleOutput`, but got type `String`.

x | output: String = "abc"
                     ^^^^^
at output (file:///$snippetsDir/input/modules/invalidOutput1.pkl)

Resolves #709

[translatenix]

ideally this problem would be caught right here (override with different type), but I guess that's impossible for now

[bioball]

We'd like to make Pkl stricter here and enforce that subclasses are also subtypes. Maybe something to do before we cut a 1.0 release.

[bioball]

Thanks for the PR! Generally, this is the right approach. See comments for some minor issues here.

[bioball]

Move this and invliadOutput1 to input/errors.

I know we have some other error tests in input/modules, but those should probably be moved too. The intention is to put all testing around error messages in the errors/ dir.

[bioball]

We'd like to make Pkl stricter here and enforce that subclasses are also subtypes. Maybe something to do before we cut a 1.0 release.

[bioball]

There's a fair bit of logic here that's bespoke to reading output.value, so it's not too appropriate to be re-purposed. For example, it show different error messages based on whether output.value was explicitly assigned or not.

I think we should just copy some of the logic here that's useful, and inline that into readModuleOutput.

[translatenix]

We'd like to make Pkl stricter here and enforce that subclasses are also subtypes.

Just a note that covariant property types aren't sound:

open class Foo { name: Any }
class Bar extends Foo { name: String } // covariant property type

bar: Bar = new { name = "bar" }
foo: Foo = bar
foo2: Foo = (foo) {
  name = 123 // static type checker is happy
}
name = foo2.name // boom

[bioball]

Yeah; unless they are marked either fixed or const (property becomes only an output and not an input)