Skip to content

Commit 3721557

Browse files
refactor: pretty printing combinator for lists/tuples (dfinity#668)
Gets rid of the dodgy `is_empty` function that performed full traversals of documents to tell if they were empty.
1 parent 4767ae2 commit 3721557

File tree

14 files changed

+222
-180
lines changed

14 files changed

+222
-180
lines changed

CHANGELOG.md

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010
+ The `args` field of the `candid::types::internal::Function` struct now is a `Vec<ArgType>` instead of `Vec<Type>`, to preserve argument names.
1111
+ The `TypeInner::Class` variant now takes `Vec<ArgType>` instead of `Vec<Type>` as its first parameter, to preserve argument names.
1212

13+
* [BREAKING]: Removed the `candid::pretty::concat` function
14+
+ `candid::pretty::enclose` and `candid::pretty:enclose_space` don't collapse the separators on empty documents anymore
15+
1316
* Non-breaking changes:
1417
+ Added `pp_named_args`, `pp_named_init_args` in `pretty::candid` module.
1518

@@ -409,7 +412,7 @@ The source code of this tool has been removed, as it was deprecated in [PR#405](
409412
* Bump ic-types to 0.3
410413
* `candid::utils::service_compatible` to check for upgrade compatibility of two service types
411414
412-
## 2021-12-20
415+
## 2021-12-20
413416
414417
### Rust (0.7.9)
415418
@@ -435,7 +438,7 @@ The source code of this tool has been removed, as it was deprecated in [PR#405](
435438
436439
### Rust (0.7.5 -- 0.7.7)
437440
438-
* Support import when parsing did files with `check_file` function
441+
* Support import when parsing did files with `check_file` function
439442
* Fix TypeScript binding for reference types
440443
441444
### Candid UI
@@ -454,7 +457,7 @@ The source code of this tool has been removed, as it was deprecated in [PR#405](
454457
* Add `#[candid_path("path_to_candid")]` helper attribute to the candid derive macro
455458
* Update `ic-types` to 0.2.0
456459
457-
## 2021-06-03
460+
## 2021-06-03
458461
459462
### Spec
460463
@@ -494,7 +497,7 @@ The source code of this tool has been removed, as it was deprecated in [PR#405](
494497
* Fix TypeScript binding for tuple
495498
* Rust support for Func and Service value
496499
497-
## 2021-03-17
500+
## 2021-03-17
498501
499502
### Rust (0.6.18)
500503
@@ -645,7 +648,7 @@ The source code of this tool has been removed, as it was deprecated in [PR#405](
645648
646649
* No longer requires the shortest LEB128 number in deserialization [#79](https://github.com/dfinity/candid/pull/79)
647650
648-
### Rust
651+
### Rust
649652
650653
* Parser improvements:
651654
+ Floats in fractional number, no e-notation yet

rust/candid/src/pretty/candid.rs

Lines changed: 23 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,8 @@ pub fn pp_ty_inner(ty: &TypeInner) -> RcDoc<'_> {
107107
Record(ref fs) => {
108108
let t = Type(ty.clone().into());
109109
if t.is_tuple() {
110-
let tuple = concat(fs.iter().map(|f| pp_ty(&f.ty)), ";");
111-
kwd("record").append(enclose_space("{", tuple, "}"))
110+
let fs = fs.iter().map(|f| pp_ty(&f.ty));
111+
kwd("record").append(sep_enclose_space(fs, ";", "{", "}"))
112112
} else {
113113
kwd("record").append(pp_fields(fs, false))
114114
}
@@ -151,8 +151,7 @@ pub(crate) fn pp_field(field: &Field, is_variant: bool) -> RcDoc<'_> {
151151
}
152152

153153
fn pp_fields(fs: &[Field], is_variant: bool) -> RcDoc<'_> {
154-
let fields = fs.iter().map(|f| pp_field(f, is_variant));
155-
enclose_space("{", concat(fields, ";"), "}")
154+
sep_enclose_space(fs.iter().map(|f| pp_field(f, is_variant)), ";", "{", "}")
156155
}
157156

158157
pub fn pp_function(func: &Function) -> RcDoc<'_> {
@@ -176,16 +175,14 @@ pub fn pp_named_args(args: &[ArgType]) -> RcDoc<'_> {
176175
pp_ty(&arg.typ)
177176
}
178177
});
179-
let doc = concat(args, ",");
180-
enclose("(", doc, ")")
178+
sep_enclose(args, ",", "(", ")")
181179
}
182180

183181
/// Pretty-prints arguments in the form of `(type1, type2)`.
184182
///
185183
/// To print named arguments, use [`pp_named_args`] instead.
186184
pub fn pp_args(args: &[Type]) -> RcDoc<'_> {
187-
let doc = concat(args.iter().map(pp_ty), ",");
188-
enclose("(", doc, ")")
185+
sep_enclose(args.iter().map(pp_ty), ",", "(", ")")
189186
}
190187

191188
/// Pretty-prints return types in the form of `(type1, type2)`.
@@ -205,22 +202,19 @@ pub fn pp_modes(modes: &[FuncMode]) -> RcDoc<'_> {
205202
}
206203

207204
fn pp_service<'a>(serv: &'a [(String, Type)], docs: Option<&'a DocComments>) -> RcDoc<'a> {
208-
let doc = concat(
209-
serv.iter().map(|(id, func)| {
210-
let doc = docs
211-
.and_then(|docs| docs.lookup_service_method(id))
212-
.map(|docs| pp_docs(docs))
213-
.unwrap_or(RcDoc::nil());
214-
let func_doc = match func.as_ref() {
215-
TypeInner::Func(ref f) => pp_function(f),
216-
TypeInner::Var(_) => pp_ty(func),
217-
_ => unreachable!(),
218-
};
219-
doc.append(pp_text(id)).append(kwd(" :")).append(func_doc)
220-
}),
221-
";",
222-
);
223-
enclose_space("{", doc, "}")
205+
let methods = serv.iter().map(|(id, func)| {
206+
let doc = docs
207+
.and_then(|docs| docs.lookup_service_method(id))
208+
.map(|docs| pp_docs(docs))
209+
.unwrap_or(RcDoc::nil());
210+
let func_doc = match func.as_ref() {
211+
TypeInner::Func(ref f) => pp_function(f),
212+
TypeInner::Var(_) => pp_ty(func),
213+
_ => unreachable!(),
214+
};
215+
doc.append(pp_text(id)).append(kwd(" :")).append(func_doc)
216+
});
217+
sep_enclose_space(methods, ";", "{", "}")
224218
}
225219

226220
fn pp_defs(env: &TypeEnv) -> RcDoc<'_> {
@@ -533,8 +527,8 @@ pub mod value {
533527
}
534528

535529
fn pp_fields(depth: usize, fields: &[IDLField]) -> RcDoc<'_> {
536-
let fs = concat(fields.iter().map(|f| pp_field(depth, f, false)), ";");
537-
enclose_space("{", fs, "}")
530+
let fs = fields.iter().map(|f| pp_field(depth, f, false));
531+
sep_enclose_space(fs, ";", "{", "}")
538532
}
539533

540534
pub fn pp_char(v: u8) -> String {
@@ -561,13 +555,13 @@ pub mod value {
561555
RcDoc::as_string(format!("{v:?}"))
562556
} else {
563557
let values = vs.iter().map(|v| pp_value(depth - 1, v));
564-
kwd("vec").append(enclose_space("{", concat(values, ";"), "}"))
558+
kwd("vec").append(sep_enclose_space(values, ";", "{", "}"))
565559
}
566560
}
567561
Record(fields) => {
568562
if is_tuple(v) {
569563
let fields = fields.iter().map(|f| pp_value(depth - 1, &f.val));
570-
kwd("record").append(enclose_space("{", concat(fields, ";"), "}"))
564+
kwd("record").append(sep_enclose_space(fields, ";", "{", "}"))
571565
} else {
572566
kwd("record").append(pp_fields(depth, fields))
573567
}
@@ -584,6 +578,6 @@ pub mod value {
584578
.args
585579
.iter()
586580
.map(|v| pp_value(MAX_ELEMENTS_FOR_PRETTY_PRINT, v));
587-
enclose("(", concat(args, ","), ")")
581+
sep_enclose(args, ",", "(", ")")
588582
}
589583
}

rust/candid/src/pretty/utils.rs

Lines changed: 124 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,26 @@
1-
use pretty::RcDoc;
1+
use pretty::{RcAllocator, RcDoc};
22

33
pub const INDENT_SPACE: isize = 2;
44
pub const LINE_WIDTH: usize = 80;
55

6-
fn is_empty(doc: &RcDoc) -> bool {
7-
use pretty::Doc::*;
8-
match &**doc {
9-
Nil => true,
10-
FlatAlt(t1, t2) => is_empty(t2) || is_empty(t1),
11-
Union(t1, t2) => is_empty(t1) && is_empty(t2),
12-
Group(t) | Nest(_, t) | Annotated((), t) => is_empty(t),
13-
_ => false,
14-
}
15-
}
16-
176
pub fn enclose<'a>(left: &'a str, doc: RcDoc<'a>, right: &'a str) -> RcDoc<'a> {
18-
if is_empty(&doc) {
19-
RcDoc::text(left).append(right)
20-
} else {
21-
RcDoc::text(left)
22-
.append(RcDoc::line_())
23-
.append(doc)
24-
.nest(INDENT_SPACE)
25-
.append(RcDoc::line_())
26-
.append(right)
27-
.group()
28-
}
7+
RcDoc::text(left)
8+
.append(RcDoc::line_())
9+
.append(doc)
10+
.nest(INDENT_SPACE)
11+
.append(RcDoc::line_())
12+
.append(right)
13+
.group()
2914
}
3015

3116
pub fn enclose_space<'a>(left: &'a str, doc: RcDoc<'a>, right: &'a str) -> RcDoc<'a> {
32-
if is_empty(&doc) {
33-
RcDoc::text(left).append(right)
34-
} else {
35-
RcDoc::text(left)
36-
.append(RcDoc::line())
37-
.append(doc)
38-
.nest(INDENT_SPACE)
39-
.append(RcDoc::line())
40-
.append(right)
41-
.group()
42-
}
17+
RcDoc::text(left)
18+
.append(RcDoc::line())
19+
.append(doc)
20+
.nest(INDENT_SPACE)
21+
.append(RcDoc::line())
22+
.append(right)
23+
.group()
4324
}
4425

4526
/// Intersperse the separator between each item in `docs`.
@@ -50,20 +31,6 @@ where
5031
RcDoc::intersperse(docs, RcDoc::text(sep).append(RcDoc::line()))
5132
}
5233

53-
/// Append the separator to each item in `docs`. If it is displayed in a single line, omit the last separator.
54-
pub fn concat<'a, D>(docs: D, sep: &'a str) -> RcDoc<'a>
55-
where
56-
D: Iterator<Item = RcDoc<'a>>,
57-
{
58-
let mut docs = docs.peekable();
59-
if docs.peek().is_none() {
60-
return RcDoc::nil();
61-
}
62-
let singleline = RcDoc::intersperse(docs, RcDoc::text(sep).append(RcDoc::line()));
63-
let multiline = singleline.clone().append(sep);
64-
multiline.flat_alt(singleline)
65-
}
66-
6734
pub fn lines<'a, D>(docs: D) -> RcDoc<'a>
6835
where
6936
D: Iterator<Item = RcDoc<'a>>,
@@ -90,15 +57,120 @@ pub fn quote_ident(id: &str) -> RcDoc<'_> {
9057
.append(RcDoc::space())
9158
}
9259

60+
/// Separate each item in `docs` with the separator `sep`, and enclose the result in `open` and `close`.
61+
/// When placed on multiple lines, the last element gets a trailing separator.
62+
pub fn sep_enclose<'a, D, S, O, C>(docs: D, sep: S, open: O, close: C) -> RcDoc<'a>
63+
where
64+
D: IntoIterator<Item = RcDoc<'a>>,
65+
S: pretty::Pretty<'a, RcAllocator>,
66+
O: pretty::Pretty<'a, RcAllocator>,
67+
C: pretty::Pretty<'a, RcAllocator>,
68+
{
69+
let sep = sep.pretty(&RcAllocator);
70+
let elems = RcDoc::intersperse(docs, sep.clone().append(RcDoc::line()));
71+
open.pretty(&RcAllocator)
72+
.into_doc()
73+
.append(RcDoc::line_())
74+
.append(elems)
75+
.append(sep.flat_alt(RcDoc::nil()))
76+
.nest(INDENT_SPACE)
77+
.append(RcDoc::line_())
78+
.append(close.pretty(&RcAllocator))
79+
.group()
80+
}
81+
82+
/// Like `sep_enclose`, but inserts a space between the opening delimiter and the first element,
83+
/// and between the last element and the closing delimiter when placed on a single line.
84+
pub fn sep_enclose_space<'a, D, S, O, C>(docs: D, sep: S, open: O, close: C) -> RcDoc<'a>
85+
where
86+
D: IntoIterator<Item = RcDoc<'a>>,
87+
S: pretty::Pretty<'a, RcAllocator>,
88+
O: pretty::Pretty<'a, RcAllocator>,
89+
C: pretty::Pretty<'a, RcAllocator>,
90+
{
91+
let mut docs = docs.into_iter().peekable();
92+
if docs.peek().is_none() {
93+
return open.pretty(&RcAllocator).append(close).into_doc();
94+
}
95+
let open = open
96+
.pretty(&RcAllocator)
97+
.append(RcDoc::nil().flat_alt(RcDoc::space()));
98+
let close = RcDoc::nil().flat_alt(RcDoc::space()).append(close);
99+
sep_enclose(docs, sep, open, close)
100+
}
101+
93102
#[cfg(test)]
94-
mod test {
103+
mod tests {
95104
use super::*;
96105

97106
#[test]
98-
fn concat_empty() {
99-
let t = concat(vec![].into_iter(), ",")
107+
fn enclose_empty() {
108+
let t = sep_enclose(vec![], ",", "(", ")")
100109
.pretty(LINE_WIDTH)
101110
.to_string();
102-
assert_eq!(t, "");
111+
assert_eq!(t, "()");
112+
}
113+
114+
#[test]
115+
fn enclose_single_line() {
116+
let printed = sep_enclose(vec![str("a"), str("b")], ",", "(", ")")
117+
.pretty(LINE_WIDTH)
118+
.to_string();
119+
assert_eq!(printed, "(a, b)");
120+
}
121+
122+
#[test]
123+
fn enclose_multiline() {
124+
let docs: Vec<RcDoc> = vec![
125+
str("Very long line to make sure we get a multiline document"),
126+
str("Very long line to make sure we get a multiline document"),
127+
];
128+
let printed = sep_enclose(docs, ",", "(", ")").pretty(20).to_string();
129+
assert_eq!(
130+
printed,
131+
"
132+
(
133+
Very long line to make sure we get a multiline document,
134+
Very long line to make sure we get a multiline document,
135+
)
136+
"
137+
.trim()
138+
);
139+
}
140+
141+
#[test]
142+
fn enclose_empty_space() {
143+
let t = sep_enclose_space(vec![], ",", "(", ")")
144+
.pretty(LINE_WIDTH)
145+
.to_string();
146+
assert_eq!(t, "()");
147+
}
148+
149+
#[test]
150+
fn enclose_single_line_space() {
151+
let printed = sep_enclose_space(vec![str("a"), str("b")], ",", "(", ")")
152+
.pretty(LINE_WIDTH)
153+
.to_string();
154+
assert_eq!(printed, "( a, b )");
155+
}
156+
157+
#[test]
158+
fn enclose_multiline_space() {
159+
let docs: Vec<RcDoc> = vec![
160+
str("Very long line to make sure we get a multiline document"),
161+
str("Very long line to make sure we get a multiline document"),
162+
];
163+
let printed = sep_enclose_space(docs, ",", "(", ")")
164+
.pretty(20)
165+
.to_string();
166+
assert_eq!(
167+
printed,
168+
"
169+
(
170+
Very long line to make sure we get a multiline document,
171+
Very long line to make sure we get a multiline document,
172+
)"
173+
.trim()
174+
);
103175
}
104176
}

0 commit comments

Comments
 (0)