tweag / nickel

Better configuration for less
https://nickel-lang.org/
MIT License
2.35k stars 89 forks source link

Lazily attaching functional contracts to record contracts #1619

Open YorikSar opened 1 year ago

YorikSar commented 1 year ago

Is your feature request related to a problem? Please describe.

I want to define a record contract in a library and force 2 fields to be mutually exclusive. Right now I can't achieve that, because functional contracts are applied to partially defined record instead of the final one:

nickel> let C = { f1, f2 } | (std.contract.from_predicate (fun v => v.f1 != null && v.f2 == null || v.f1 == null && v.f2 != null)) in { f1 = 1} | C
error: missing definition for `f1`
  ┌─ <repl-input-26:1:63
  │
1 │ let C = { f1, f2 } | (std.contract.from_predicate (fun v => v.f1 != null && v.f2 == null || v.f1 == null && v.f2 != null)) in { f1 = 1} | C
  │         ----------                                          --^^
  │         │                                                   │ │
  │         │                                                   │ required here
  │         │                                                   accessed here
  │         in this record

Describe the solution you'd like

I'd like to be able to somehow attach contract to the record without breaking its laziness, allowing the field introspection for LSP and defining fields later in the code.

Describe alternatives you've considered

The only alternative that I see currently is to add relevant condition to the places where these fields are used and call fail_with whenever they are broken.

Additional context

This surfaced when I was developing builders for (organist)[https://github.com/nickel-lang/organist]. As an example, my builder requires either text or file to get the text from, so I have contract like this and want to make text and file mutually exclusive:

      {
        name,
        version | default = "0.0.0",
        text | NullOr NixString | default = null,
        file | NullOr NixString | default = null,
        ...
      }

See also this thread in Matrix

yannham commented 1 year ago

I think there is a class of things that are fundamentally incompatible with lazyness. Record contract are lazy because their contracts only apply to subfields in isolation, but you try to express a whole-record property. Deep down, this is an operation which requires to evaluate the fields in question, and I don't really see how you could implement it as a lazy operation: when would the check fire? For record contracts it's obvious: the check for a subfield fires when the subfield is requested by evaluation. For "global" property like yours, it's less clear.

All of that being said, I think you can still achieve what you want today by cheating a bit, and transforming this "global" or "mutual property" to a property of each subfiled that also happen to depends on another subfield. It's a bit abstract, so let me show you an example instead:

let MutExclusiveWith = fun other_field => std.contract.from_predicate (
  fun value => (value == null && other_field != null) || (value != null && other_field == null) in
{
  c1 | MutExclusiveWith c2,
  c2 | MutExclusiveWith c1,
}

Indeed, the contract attached to a field is morally a part of the field as well, as such, it can also depend recursively and lazily on other fields, exactly as the value of a field. We have to write the contract two times, which is a bit more annoying, but it's not too bad, I think. Here the lazyness structure is explicit: MutExclusiveWith will fire whenever one of c1 or c2 is requested. Would that work for your use-case?

YorikSar commented 1 year ago

Maybe "lazy" is a wrong term here, but rather "deferred application". I want in

let LibContract = { a } | OtherRecordContract |' FunContract in
let result = { a = 1 } | LibContract in
result

|' to mean that FunContract is applied to { a = 1} | OtherRecordContract when result is being evaluated, not to { a } | OtherRecordContract when LibContract is being evaluated. Of course, |' is not a good syntax for it, maybe it should be | std.contract.deferred FunContract or smth like that.

YorikSar commented 1 year ago

Oh, and thanks for the hack, it would work for me indeed.

yannham commented 1 year ago

Wait, my example doesn't work currently, let me fix it.

yannham commented 1 year ago

Ah, in fact I can't do that, because contract are applied to the value, but to compute the contract for c1 it needs the contract for c2, and so on...so we get infinite recursion even when c1 and c2 have values. However, this works:

let MutExclusiveWith = fun other_field => std.contract.from_predicate (
  fun value => (value == null && other_field != null) || (value != null &&
other_field == null)) in
let MutExclusiveWith = fun other_field => std.contract.from_predicate (
  fun value => (value == null && other_field != null) || (value != null &&
other_field == null)) in
({
  c1 | MutExclusiveWith c2,
  c2,
} & {c1 = null, c2 = null})
yannham commented 1 year ago

Tangentially, note that being undefined doesn't equate to being null. Both your desired |' and my example would still fail if one of the field actually doesn't have a definition.

YorikSar commented 1 year ago

infinite recursion

Oh, I didn't notice that part. Makes me yearn for Prolog style facts :)

Both your desired |' and my example would still fail if one of the field actually doesn't have a definition.

Well, FunContract could check if values are defined with has_field, I guess... But really having default null values would be good enough, I think.

yannham commented 12 months ago

Well, FunContract could check if values are defined with has_field, I guess... But really having default null values would be good enough, I think.

I don't think that's the case:

nickel> %has_field% "foo" {}
false

nickel> %has_field% "foo" {foo}
true

nickel> %has_field% "foo" {foo = 1}
true

A field without a definition is still present, for has_field (I had to use the primop because for some subtle contract reasons std.record.has_field actually requires that all fields are defined - maybe we should ship an alternative)

yannham commented 12 months ago

Currently there's way to test if a field exist but is not defined. That is totally possible to add, though.