Skip to content

Commit

Permalink
Overhaul and improve testing of the pretty printer (tweag#1532)
Browse files Browse the repository at this point in the history
* Move `Type` pretty printer test into `core/src/pretty.rs`

* Revamp pretty printer for `TypeF::Array`

* Start overhauling and testing the Type pretty printer

* Simplify pretty printer integration test

* Expand pretty printer integration tests to include stdlib

* continue overhauling the pretty printer

* Add tests for "let" pretty printing

* Ignore empty annotations in TypeAnnotation::attach_term

* Get almost all pretty printer tests to pass, missing `pass/strings/symbolic_strings.ncl`

* Use `Term::StrChunks` when desugaring symbolic strings

* Fix missing newlines in multiline strings

* Handle record patterns in let bindings

* `fun` pretty printing

* Update snapshot tests

* Fix pretty-printer dependent test cases

* Remove TODO comment

* Improve formatting for record fields with metadata

* Remove a redundant clone and some redundant calls to `pretty`

* Consistently use `%<stuff>` for internal values

* Address comments from code review

* Derive `Eq` for `OpPos`

* format other binary operators the same way as `&&` and `||`

* Add a comment about special unary operators

* Adjust grouping for field metadata annotations

* Simplify `Array` pretty printing code

* Document the lack of `indoc!` in `pretty_multiline_strings`
  • Loading branch information
vkleen authored Aug 25, 2023
1 parent 85fdb7e commit 6b44916
Show file tree
Hide file tree
Showing 20 changed files with 1,311 additions and 667 deletions.
8 changes: 4 additions & 4 deletions cli/tests/snapshot/inputs/pretty/simple_record.ncl
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# capture = 'stdout'
# command = ['pprint-ast']
{
a | Number | default = 1,
b : String | force = "some long string that goes past the 80 character line limit for pretty printing",
c : { x : Number, y: Number } = { x = 999.8979, y = 500 },
d | Array std.string.NonEmpty = ["a", "list", "of", "non", "empty", "strings"],
aaaaa | Number | default = 1,
bbbbb : String | force = "some long string that goes past the 80 character line limit for pretty printing",
ccccc : { x : Number, y: Number } = { x = 999.8979, y = 500 },
ddddd | Array std.string.NonEmpty = ["a", "list", "of", "non", "empty", "strings"],
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ error: type error: missing row `b`
4let { a, b } = { a = 1, c = 2 } in
^^^^^^^^^^^^^^^^ this expression
= Expected an expression of type `{ b: _a, a: _b }`, which contains the field `b`
= Found an expression of type `{ c: _c, a: _d }`, which does not contain the field `b`
= Expected an expression of type `{ b : _a, a : _b }`, which contains the field `b`
= Found an expression of type `{ c : _c, a : _d }`, which does not contain the field `b`


Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
source: cli/tests/snapshot/main.rs
expression: err
---
error: values of type `[| 'z ; _erows_a |]` are not guaranteed to be compatible with polymorphic enum tail `[| ; r |]`
error: values of type `[| 'z; _erows_a |]` are not guaranteed to be compatible with polymorphic enum tail `[| ; r |]`
┌─ [INPUTS_PATH]/errors/enum_forall_parametricity_violation.ncl:4:26
4 │ match { 'x => 'y, _ => 'z }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
source: tests/snapshot/main.rs
source: cli/tests/snapshot/main.rs
expression: err
---
error: multiple rows declaration
Expand All @@ -8,7 +8,7 @@ error: multiple rows declaration
3let f | forall r. { ; r } -> { x: Number; r } = fun r => %record_insert% "x" r 1 in (f { x = 0 } : _)
^^^^^^^^^ this expression
= Found an expression of a record type `{ x: _a }` with the row `x`
= Found an expression of a record type `{ x : _a }` with the row `x`
= But this type appears inside another row type, which already has a declaration for the field `x`
= A type cannot have two conflicting declarations for the same row

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
source: cli/tests/snapshot/main.rs
expression: err
---
error: values of type `{ }` are not guaranteed to be compatible with polymorphic record tail `{ ; r }`
error: values of type `{ }` are not guaranteed to be compatible with polymorphic record tail `{ ; r }`
┌─ [INPUTS_PATH]/errors/record_forall_parametricity_violation.ncl:4:12
4fun x => x
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
source: cli/tests/snapshot/main.rs
expression: out
---
let rec fib =
fun n => if (n == 0) || (n == 1) then 1 else ((fib (n - 1)) + (fib (n - 2)))
in

let rec fib
= fun n => if (n == 0) || (n == 1) then 1 else (fib (n - 1)) + (fib (n - 2))
in
fib 5
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,18 @@ expression: out
{
field
| doc m%%"
Contract to enforce the value is a string that represents a boolean literal. Additionally casts "True" to "true"
and "False" to "false". This shouldn't interpolate: %{null}

For example:
```nickel
("True" | BoolLiteral) =>
"true"
("hello" | BoolLiteral) =>
error
(true | BoolLiteral) =>
error
```
Contract to enforce the value is a string that represents a boolean literal. Additionally casts "True" to "true"
and "False" to "false". This shouldn't interpolate: %{null}
For example:
```nickel
("True" | BoolLiteral) =>
"true"
("hello" | BoolLiteral) =>
error
(true | BoolLiteral) =>
error
```
"%%
= 1,
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ source: cli/tests/snapshot/main.rs
expression: out
---
{
a | Number | default
= 1,
b : String | force
=
"some long string that goes past the 80 character line limit for pretty printing",
c : { x: Number, y: Number }
= { x = 999.8979, y = 500, },
d | Array std.string.NonEmpty
aaaaa | Number | default = 1,
bbbbb
: String
| force
= "some long string that goes past the 80 character line limit for pretty printing",
ccccc : { x : Number, y : Number } = { x = 999.8979, y = 500, },
ddddd
| Array std.string.NonEmpty
= [ "a", "list", "of", "non", "empty", "strings" ],
}
2 changes: 1 addition & 1 deletion core/src/parser/grammar.lalrpop
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ StrChunks: RichTerm = {

if let StringStartDelimiter::Symbolic(prefix) = start {
let terms = chunks.into_iter().map(|chunk| match chunk {
StrChunk::Literal(l) => Term::Str(l.into()).into(),
StrChunk::Literal(_) => Term::StrChunks(vec![chunk]).into(),
StrChunk::Expr(e, _) => e,
}).collect();

Expand Down
2 changes: 1 addition & 1 deletion core/src/parser/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ fn mk_symbolic_single_chunk(prefix: &str, s: &str) -> RichTerm {
(
FieldPathElem::Ident("fragments".into()),
Field::from(RichTerm::from(Array(
Array::new(Rc::new([Str(s.into()).into()])),
Array::new(Rc::new([mk_single_chunk(s)])),
Default::default(),
))),
),
Expand Down
4 changes: 4 additions & 0 deletions core/src/parser/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,10 @@ impl Annot for TypeAnnotation {

impl AttachTerm<RichTerm> for TypeAnnotation {
fn attach_term(self, rt: RichTerm) -> RichTerm {
if self.is_empty() {
return rt;
}

let pos = rt.pos;
RichTerm::new(Term::Annotated(self, rt), pos)
}
Expand Down
Loading

0 comments on commit 6b44916

Please sign in to comment.