Skip to content

Commit

Permalink
Use the original unevaluated type and contract annotation in `nickel …
Browse files Browse the repository at this point in the history
…doc` (tweag#1529)

* Use the original unevaluated type and contract annotation in `nickel doc`

With this change we no longer print evaluated types or contracts in
field annotations for `nickel doc`. Ever since `nickel doc` started
evaluating terms to address tweag#1462 contract and type annotations would be
evaluated before reaching the documentation extraction stage. This means
they would be affected by program transformations and the result would
most likely be meaningless to the user.

Incidentally, `nickel query` already correctly used the original
unevaluated type for contract annotations but not for type annotations.
If a type annotation contains a `Term` as a `TypeF::Flat` variant, it
was possible to trigger the same undesirable behaviour:

```
❯ cargo run --bin nickel -- query foo <<<'{ foo : { x | Dyn, y } = {x = 1, y = 2} | { x | Dyn, y } }'
    Finished dev [unoptimized + debuginfo] target(s) in 0.12s
     Running `target/debug/nickel query foo`
• type: let %182 = $dyn in { x | Dyn, y, }

Available fields
• x
• y
```

Fixes tweag#1519

* Add a snapshot test against regressions
  • Loading branch information
vkleen authored Aug 14, 2023
1 parent 3b5e151 commit 3df0131
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 3 deletions.
5 changes: 5 additions & 0 deletions cli/tests/snapshot/inputs/docs/evaluation.ncl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# capture = 'stdout'
# command = ['doc', '--stdout', '--format=json']
{
foo | { x | Dyn, y }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: cli/tests/snapshot/main.rs
expression: out
---
{"foo":{"fields":null,"type":null,"contracts":["{ x | Dyn, y, }"],"documentation":null}}
8 changes: 6 additions & 2 deletions core/src/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -498,19 +498,23 @@ mod doc {
.map(|(ident, field)| {
let fields = field.value.as_ref().and_then(Self::extract_from_term);

// We use the original user-written type stored
// in the label. Using `lt.typ` instead is often
// unreadable, since we evaluate terms to a record
// spine before extracting documentation
let typ = field
.metadata
.annotation
.typ
.as_ref()
.map(|lt| lt.typ.to_string());
.map(|lt| lt.label.typ.to_string());

let contracts = field
.metadata
.annotation
.contracts
.iter()
.map(|lt| lt.typ.to_string())
.map(|lt| lt.label.typ.to_string())
.collect();

let documentation = field.metadata.doc.clone();
Expand Down
10 changes: 9 additions & 1 deletion core/src/repl/query_print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,15 @@ fn render_query_result<R: QueryPrinter>(
renderer.write_metadata(
out,
"type",
&metadata.annotation.typ.as_ref().unwrap().typ.to_string(),
&metadata
.annotation
.typ
.as_ref()
.unwrap()
// We use the original type here, as well.
.label
.typ
.to_string(),
)?;
found = true;
}
Expand Down

0 comments on commit 3df0131

Please sign in to comment.