types(ses): move comments where typedoc can see them (#2205)

closes: #XXXX
refs: #XXXX

## Description

Before this PR, there was a lot of redundancy between
`ses/src/error/types.js` and `ses/types.d.ts` in typing the
`ses/src/error/assert.js` module. This cased several problems:
- `yarn docs` was picking up only the doc-comments in the
`ses/types.d.ts` file, dropping all the useful doc-comments in the
`ses/src/error/types.js` file.
- redundant expression of the same meaning in different syntaxes is a
horrible maintenance burden. They will get out of sync. (This happened
to me at first in a recent PR when I added the `sanitize` option.)
- The types as exported by the `ses/src/error/types.js` are ambient,
which we're trying to reduce and eventually eliminate. The types as
exported by `ses/types.d.ts` are not ambient, requiring them to be
explicitly imported. Which is finally pleasant with `@import`!
- By importing types with `@import` rather than `@typedef {import('...).
...} ...`, the documentation attached to the types is visible in the IDE
(at least vscode) and more usage locations.

This PR should be a pure refactoring from a runtime semantics POV. No
behavior of any code should be any different. All the changes are only
to static information: types and doc-comments.

### Security Considerations

As mentioned above, redundant expression of the same meaning in
different syntaxes ... will get out of sync. This makes code more
confusing to review reliably, so this PR helps security.

### Scaling Considerations

none
### Documentation Considerations

Most of the point!

After this PR, `yarn docs` produces much better documentation for the
types related to the assert.js module. It is thus also an attempt to get
one's feet wet at making this kind of improvement. However, even after
this PR, the output of `yarn docs` still leaves plenty of room for
further improvement ;)

### Testing Considerations

Probably none. Conceivably more type tests could be relevant, but if so
it is not apparent to me.

### Compatibility Considerations

Because this PR (allegedly) changes no runtime behavior, it cannot cause
any incompat runtime behavior.

However, client code is also a client of the exported types. Changing
types from ambient to non-ambient risks breaking whether old clients
continue to pass static type checking.

Which brings us back to the oft-repeated question for reviewers:

***Do I need to mark this PR with a `!`?***

### Upgrade Considerations

none.

- [ ] Includes `*BREAKING*:` in the commit message with migration
instructions for any breaking change.
- [ ] Updates `NEWS.md` for user-facing changes.

Author: erights

packages/ses/src/error/assert.js

@@ -46,6 +46,17 @@ import './types.js';
 import './internal-types.js';
 import { makeNoteLogArgsArrayKit } from './note-log-args.js';
 
+/**
+ * @import {BaseAssert,
+ *          Assert,
+ *          AssertionFunctions,
+ *          AssertionUtilities,
+ *          StringablePayload,
+ *          DetailsToken,
+ *          MakeAssert,
+ * } from '../../types.js'
+ */
+
 // For our internal debugging purposes, uncomment
 // const internalDebugConsole = console;
 
@@ -54,7 +65,7 @@ import { makeNoteLogArgsArrayKit } from './note-log-args.js';
 /** @type {WeakMap<StringablePayload, any>} */
 const declassifiers = new WeakMap();
 
-/** @type {AssertQuote} */
+/** @type {AssertionUtilities['quote']} */
 const quote = (payload, spaces = undefined) => {
   const result = freeze({
     toString: freeze(() => bestEffortStringify(payload, spaces)),
@@ -67,19 +78,7 @@ freeze(quote);
 const canBeBare = freeze(/^[\w:-]( ?[\w:-])*$/);
 
 /**
- * Embed a string directly into error details without wrapping punctuation.
- * To avoid injection attacks that exploit quoting confusion, this must NEVER
- * be used with data that is possibly attacker-controlled.
- * As a further safeguard, we fall back to quoting any input that is not a
- * string of sufficiently word-like parts separated by isolated spaces (rather
- * than throwing an exception, which could hide the original problem for which
- * explanatory details are being constructed---i.e., ``` assert.details`...` ```
- * should never be the source of a new exception, nor should an attempt to
- * render its output, although we _could_ instead decide to handle the latter
- * by inline replacement similar to that of `bestEffortStringify` for producing
- * rendered messages like `(an object) was tagged "[Unsafe bare string]"`).
- *
- * @type {AssertQuote}
+ * @type {AssertionUtilities['bare']}
  */
 const bare = (payload, spaces = undefined) => {
   if (typeof payload !== 'string' || !regexpTest(canBeBare, payload)) {
@@ -165,7 +164,7 @@ freeze(DetailsTokenProto.toString);
  * of them should be uses where the template literal has no redacted
  * substitution values. In those cases, the two are equivalent.
  *
- * @type {DetailsTag}
+ * @type {AssertionUtilities['details']}
  */
 const redactedDetails = (template, ...args) => {
   // Keep in mind that the vast majority of calls to `details` creates
@@ -174,7 +173,7 @@ const redactedDetails = (template, ...args) => {
   // all the work to happen only if needed, for example, if an assertion fails.
   const detailsToken = freeze({ __proto__: DetailsTokenProto });
   weakmapSet(hiddenDetailsMap, detailsToken, { template, args });
-  return detailsToken;
+  return /** @type {DetailsToken} */ (/** @type {unknown} */ (detailsToken));
 };
 freeze(redactedDetails);
 
@@ -189,7 +188,7 @@ freeze(redactedDetails);
  * of safety. `unredactedDetails` also sacrifices the speed of `details`,
  * which is usually fine in debugging and testing.
  *
- * @type {DetailsTag}
+ * @type {AssertionUtilities['details']}
  */
 const unredactedDetails = (template, ...args) => {
   args = arrayMap(args, arg =>
@@ -286,7 +285,7 @@ const tagError = (err, optErrorName = err.name) => {
  *
  * @param {Error} error
  */
-const sanitizeError = error => {
+export const sanitizeError = error => {
   const descs = getOwnPropertyDescriptors(error);
   const {
     name: _nameDesc,
@@ -322,7 +321,7 @@ const sanitizeError = error => {
 };
 
 /**
- * @type {AssertMakeError}
+ * @type {AssertionUtilities['error']}
  */
 const makeError = (
   optDetails = redactedDetails`Assert failed`,
@@ -397,7 +396,7 @@ const { addLogArgs, takeLogArgsArray } = makeNoteLogArgsArrayKit();
  */
 const hiddenNoteCallbackArrays = new WeakMap();
 
-/** @type {AssertNote} */
+/** @type {AssertionUtilities['note']} */
 const note = (error, detailsNote) => {
   if (typeof detailsNote === 'string') {
     // If it is a string, use it as the literal part of the template so
@@ -478,21 +477,22 @@ const makeAssert = (optRaise = undefined, unredacted = false) => {
   const details = unredacted ? unredactedDetails : redactedDetails;
   const assertFailedDetails = details`Check failed`;
 
-  /** @type {AssertFail} */
+  /** @type {AssertionFunctions['fail']} */
   const fail = (
     optDetails = assertFailedDetails,
     errConstructor = undefined,
     options = undefined,
   ) => {
     const reason = makeError(optDetails, errConstructor, options);
     if (optRaise !== undefined) {
+      // @ts-ignore returns `never` doesn't mean it isn't callable
       optRaise(reason);
     }
     throw reason;
   };
   freeze(fail);
 
-  /** @type {FailTag} */
+  /** @type {AssertionUtilities['Fail']} */
   const Fail = (template, ...args) => fail(details(template, ...args));
 
   // Don't freeze or export `baseAssert` until we add methods.
@@ -508,7 +508,7 @@ const makeAssert = (optRaise = undefined, unredacted = false) => {
     flag || fail(optDetails, errConstructor, options);
   }
 
-  /** @type {AssertEqual} */
+  /** @type {AssertionFunctions['equal']} */
   const equal = (
     actual,
     expected,
@@ -525,7 +525,7 @@ const makeAssert = (optRaise = undefined, unredacted = false) => {
   };
   freeze(equal);
 
-  /** @type {AssertTypeof} */
+  /** @type {AssertionFunctions['typeof']} */
   const assertTypeof = (specimen, typename, optDetails) => {
     // This will safely fall through if typename is not a string,
     // which is what we want.
@@ -544,7 +544,7 @@ const makeAssert = (optRaise = undefined, unredacted = false) => {
   };
   freeze(assertTypeof);
 
-  /** @type {AssertString} */
+  /** @type {AssertionFunctions['string']} */
   const assertString = (specimen, optDetails = undefined) =>
     assertTypeof(specimen, 'string', optDetails);
 

packages/ses/src/error/fatal-assert.js

@@ -5,6 +5,8 @@ import { freeze } from '../commons.js';
 import { makeAssert } from './assert.js';
 import './types.js';
 
+/** @import {Assert} from '../../types.js' */
+
 let abandon;
 // Sniff for host-provided functions for terminating the enclosing UOPT (see
 // below). Currently it only checks for the `process.abort` or `process.exit`

packages/ses/src/error/stringify-utils.js

@@ -21,6 +21,8 @@ import {
   toStringTagSymbol,
 } from '../commons.js';
 
+/** @import {StringablePayload} from '../../types.js' */
+
 /**
  * Joins English terms with commas and an optional conjunction.
  *