Skip to content

Commit c680b92

Browse files
authored
docs: Miscellaneous commentary cleanup (#6751)
1 parent 229708b commit c680b92

File tree

2 files changed

+50
-71
lines changed

2 files changed

+50
-71
lines changed

packages/assert/src/types.js

Lines changed: 49 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
/// <reference types="ses"/>
33

44
// Based on
5-
// https://github.com/Agoric/SES-shim/blob/HEAD/packages/ses/src/error/types.js
5+
// https://github.com/endojs/endo/blob/HEAD/packages/ses/src/error/types.js
66
// Coordinate edits until we refactor to avoid this duplication
77
// At https://github.com/Agoric/agoric-sdk/issues/2774
88
// is a record of a failed attempt to remove this duplication.
@@ -41,8 +41,9 @@
4141
*
4242
* The `assert.fail` method.
4343
*
44-
* Fail an assertion, recording details to the console and
45-
* raising an exception with just type information.
44+
* Fail an assertion, recording full details to the console and
45+
* raising an exception with a message in which `details` substitution values
46+
* have been masked.
4647
*
4748
* The optional `optDetails` can be a string for backwards compatibility
4849
* with the nodejs assertion library.
@@ -148,14 +149,15 @@
148149
* Assert an expected typeof result.
149150
* @param {any} specimen The value to get the typeof
150151
* @param {Details=} optDetails The details to throw
152+
* @returns {asserts specimen is string}
151153
*/
152154

153155
/**
154156
* @callback AssertNote
155157
* The `assert.note` method.
156158
*
157-
* Annotate this error with these details, potentially to be used by an
158-
* augmented console, like the causal console of `console.js`, to
159+
* Annotate an error with details, potentially to be used by an
160+
* augmented console such as the causal console of `console.js`, to
159161
* provide extra information associated with logged errors.
160162
*
161163
* @param {Error} error
@@ -185,36 +187,22 @@
185187

186188
/**
187189
* To "declassify" and quote a substitution value used in a
188-
* details`...` template literal, enclose that substitution expression
189-
* in a call to `quote`. This states that the argument should appear quoted
190-
* (as if with `JSON.stringify`), in the error message of the thrown error. The
190+
* ``` details`...` ``` template literal, enclose that substitution expression
191+
* in a call to `quote`. This makes the value appear quoted
192+
* (as if with `JSON.stringify`) in the message of the thrown error. The
191193
* payload itself is still passed unquoted to the console as it would be
192194
* without `quote`.
193195
*
194-
* Starting from the example in the `details` comment, say instead that the
195-
* color the sky is supposed to be is also computed. Say that we still don't
196-
* want to reveal the sky's actual color, but we do want the thrown error's
197-
* message to reveal what color the sky was supposed to be:
196+
* For example, the following will reveal the expected sky color, but not the
197+
* actual incorrect sky color, in the thrown error's message:
198198
* ```js
199-
* assert.equal(
200-
* sky.color,
201-
* color,
202-
* details`${sky.color} should be ${quote(color)}`,
203-
* );
199+
* sky.color === expectedColor || Fail`${sky.color} should be ${quote(expectedColor)}`;
204200
* ```
205201
*
206-
* The normal convention is to locally rename `quote` to `q` and
207-
* `details` to `X`
208-
* ```js
209-
* const { details: X, quote: q } = assert;
210-
* ```
211-
* so the above example would then be
202+
* The normal convention is to locally rename `details` to `X` and `quote` to `q`
203+
* like `const { details: X, quote: q } = assert;`, so the above example would then be
212204
* ```js
213-
* assert.equal(
214-
* sky.color,
215-
* color,
216-
* X`${sky.color} should be ${q(color)}`,
217-
* );
205+
* sky.color === expectedColor || Fail`${sky.color} should be ${q(expectedColor)}`;
218206
* ```
219207
*
220208
* @callback AssertQuote
@@ -266,75 +254,66 @@
266254
* ```js
267255
* assert(sky.isBlue(), details`${sky.color} should be "blue"`);
268256
* ```
257+
* or following the normal convention to locally rename `details` to `X`
258+
* and `quote` to `q` like `const { details: X, quote: q } = assert;`:
259+
* ```js
260+
* assert(sky.isBlue(), X`${sky.color} should be "blue"`);
261+
* ```
262+
* However, note that in most cases it is preferable to instead use the `Fail`
263+
* template literal tag (which has the same input signature as `details`
264+
* but automatically creates and throws an error):
265+
* ```js
266+
* sky.isBlue() || Fail`${sky.color} should be "blue"`;
267+
* ```
268+
*
269269
* The details template tag returns a `DetailsToken` object that can print
270270
* itself with the formatted message in two ways.
271-
* It will report the real details to
272-
* the console but include only the typeof information in the thrown error
271+
* It will report full details to the console, but
272+
* mask embedded substitution values with their typeof information in the thrown error
273273
* to prevent revealing secrets up the exceptional path. In the example
274274
* above, the thrown error may reveal only that `sky.color` is a string,
275275
* whereas the same diagnostic printed to the console reveals that the
276-
* sky was green.
276+
* sky was green. This masking can be disabled for an individual substitution value
277+
* using `quote`.
277278
*
278-
* The `raw` member of a `template` is ignored, so a simple
279-
* `string[]` can also be used as a template.
279+
* The `raw` property of an input template array is ignored, so a simple
280+
* array of strings may be provided directly.
280281
*/
281282

282283
/**
283284
* @typedef {(template: TemplateStringsArray | string[], ...args: any) => never} FailTag
284-
* The `Fail` tamplate tag supports replacing patterns like
285-
* ```js
286-
* assert(cond, X`...complaint...`);
287-
* ```
288-
* or
289-
* ```js
290-
* cond || assert.fail(X`...complaint...`);
291-
* ```
292-
* with patterns like
293-
* ```js
294-
* cond || Fail`...complaint...`;
295-
* ```
296285
*
297-
* However, due to [weakness in current
298-
* TypeScript](https://github.com/microsoft/TypeScript/issues/51426), the `||`
299-
* patterns are not as powerful as the `assert(...)` call at enabling static
300-
* reasoning. Of the `||`, again due to weaknesses in current TypeScript,
301-
* the
302-
* ```js
303-
* cond || Fail`...complaint...`
304-
* ```
305-
* pattern is not as powerful as the
286+
* Use the `Fail` function as a template literal tag to efficiently
287+
* create and throw a `details`-style error only when a condition is not satisfied.
306288
* ```js
307-
* cond || assert.fail(X`...complaint...`);
289+
* condition || Fail`...complaint...`;
308290
* ```
309-
* at enabling static resoning. Despite these problems, we do not want to
310-
* return to the
291+
* This avoids the overhead of creating usually-unnecessary errors like
311292
* ```js
312-
* assert(cond, X`...complaint...`)
293+
* assert(condition, details`...complaint...`);
313294
* ```
314-
* style because of the substantial overhead in
315-
* evaluating the `X` template in the typical `true` case where it is not
316-
* needed. And we do not want to return to the
295+
* while improving readability over alternatives like
317296
* ```js
318-
* assert.fail(X`...complaint...`)`
297+
* condition || assert.fail(details`...complaint...`);
319298
* ```
320-
* because of the verbosity and loss of readability. Instead, until/unless
321-
* https://github.com/microsoft/TypeScript/issues/51426 is fixed, for those
322-
* new-style assertions where this loss of static reasoning is a problem,
299+
*
300+
* However, due to current weakness in TypeScript, static reasoning
301+
* is less powerful with the `||` patterns than with an `assert` call.
302+
* Until/unless https://github.com/microsoft/TypeScript/issues/51426 is fixed,
303+
* for `||`-style assertions where this loss of static reasoning is a problem,
323304
* instead express the assertion as
324305
* ```js
325-
* if (!cond) {
306+
* if (!condition) {
326307
* Fail`...complaint...`;
327308
* }
328309
* ```
329310
* or, if needed,
330311
* ```js
331-
* if (!cond) {
332-
* // `throw` is noop since `Fail` throws. But linter confused
312+
* if (!condition) {
313+
* // `throw` is noop since `Fail` throws, but it improves static analysis
333314
* throw Fail`...complaint...`;
334315
* }
335316
* ```
336-
* This avoid the TypeScript bugs that cause the loss of static reasoning,
337-
* but with no loss of efficiency and little loss of readability.
338317
*/
339318

340319
/**

packages/vat-data/src/types.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ type DefineKindOptions<C> = {
8484
/**
8585
* Intended for internal use only.
8686
* If an `interfaceGuard` is provided, then the raw methods passed alongside
87-
* it wrapped by a function that first checks that this method's guard
87+
* it are wrapped by a function that first checks that this method's guard
8888
* pattern is satisfied before calling the raw method.
8989
*
9090
* In `defineDurableKind` and its siblings, this defaults to off.

0 commit comments

Comments
 (0)