Spec 014's checks: the holes the review of 20 Oct found

scripts/check-baseline-totals.mjs

@@ -8,8 +8,10 @@
  * reaches goes up, and the per-module checks read the baseline in the tree.
  * So a move could carry a rise inside it unseen. This compares each
  * baseline's total with the base branch's: a move keeps the totals, typing
- * lowers them, nothing may raise them. lint-baseline.json's total and the
- * dead-code total are compared too.
+ * lowers them, nothing may raise them. A module the base already counts may
+ * not rise either (moves create their modules), so a rise cannot hide behind
+ * a fall elsewhere. lint-baseline.json's total and the dead-code total are
+ * compared too.
  *
  *   node scripts/check-baseline-totals.mjs [base]    # base defaults to origin/main
  */
@@ -63,6 +65,22 @@ for (const [name, file, total] of BASELINES) {
   if (before !== undefined && now > before) rose.push(`${name} rose: ${now}, base ${before}`);
 }
 
+// A module the base already counts may not rise, unless it is one the split
+// creates to receive moved code (spec 014: the shared modules, the families,
+// the registry and the server), which a later move may add to. Code is moved
+// out of service.ts into those, never into the services' other modules, so
+// a rise in any other is new code, whatever fell elsewhere to keep the
+// total. The split's modules are held by the total.
+const RECEIVES_MOVES =
+  /^tools\/content-node\/(java-text|body|spring|answers|objects|reference-entities|search-params|release|registry|server|[\w-]+-endpoints)\.ts$/;
+for (const [name, file] of BASELINES.slice(0, 2)) {
+  const now = JSON.parse(readFileSync(file, 'utf8'));
+  const was = atBase(file) ?? {};
+  for (const [module, count] of Object.entries(now))
+    if (module in was && count > was[module] && !RECEIVES_MOVES.test(module))
+      rose.push(`${name} in ${module} rose: ${count}, base ${was[module]}`);
+}
+
 // Lint's counts are per rule and global, so no move changes them; a rise in
 // one rule must not hide behind a fall in another (check-lint.mjs's own
 // reason for counting per rule).

scripts/check-lint.mjs

@@ -90,6 +90,13 @@ export function countLoose(report, root = ROOT) {
   return modules;
 }
 
+/** Modules whose loose count went down, not yet locked in. */
+export const looseDecreases = (counts, baseline) =>
+  Object.entries(baseline)
+    .filter(([module, n]) => (counts[module] ?? 0) < n)
+    .map(([module, n]) => `${module}: ${n} -> ${counts[module] ?? 0}`)
+    .sort();
+
 /** Modules whose loose count went up; one the baseline does not list is held to none. */
 export const looseIncreases = (counts, baseline) =>
   Object.entries(counts)
@@ -191,6 +198,14 @@ async function main() {
     console.error('\nType them: `npx eslint <module>` lists them (spec 013).');
     process.exit(1);
   }
+  // A fall not locked in is room a later change could fill unseen.
+  const tighter = looseDecreases(loose, looseBaseline);
+  if (tighter.length > 0) {
+    console.error('Loosely typed values in the services went down; lock it in:\n');
+    for (const line of tighter) console.error(`  ${line}`);
+    console.error('\nnpm run check:lint -- --update');
+    process.exit(1);
+  }
 
   const { increased, decreased, cleared } = compare(counts, baseline);
   if (increased.length > 0) {

scripts/check-move.mjs

@@ -16,7 +16,10 @@
  *    none of the base branch's modules -- a new `javaTrim` beside a moved
  *    caller would quietly replace the one it called;
  *  - rebound: a name a moved declaration imports from a module whose
- *    declaration of it is not the one the base branch's module had.
+ *    declaration of it is not the one the base branch's module had; or any
+ *    name a moved declaration uses that is bound to something else where it
+ *    is now -- a receiving module's own declaration of that name, or a
+ *    global where the original had a declaration or an import.
  *
  *   node scripts/check-move.mjs [base]        # base defaults to origin/main
  *
@@ -128,10 +131,14 @@ function pieces(file, text) {
   const effects = []; // specifiers imported for their side effects
   const reexports = new Map(); // exported name -> [specifier, name there]
   const stars = []; // specifiers of `export * from`
+  const uses = new Map(); // declaration or endpoint key -> the names it uses
   const add = (map, key, node) => {
     const list = map.get(key) ?? [];
     list.push(canonical(sf, node));
     map.set(key, list);
+    const used = uses.get(key) ?? new Set();
+    for (const name of namesUsed(node)) used.add(name);
+    uses.set(key, used);
   };
   const endpointsIn = (list) => {
     for (const element of list.elements) {
@@ -206,7 +213,74 @@ function pieces(file, text) {
       );
     }
   }
-  return { declarations, endpoints, imports, effects, reexports, stars };
+  return { declarations, endpoints, imports, effects, reexports, stars, uses };
+}
+
+/**
+ * The names a node refers to from outside itself: every identifier but a
+ * property's name after a dot, a key in an object or class, and any name
+ * the node declares within it (a local, a parameter: those bind the same
+ * wherever the node is). The node's own name is left to the caller.
+ */
+function namesUsed(node) {
+  const names = new Set();
+  const declared = new Set();
+  const visit = (n) => {
+    if (ts.isIdentifier(n)) {
+      const p = n.parent;
+      const isNameOf =
+        (ts.isPropertyAccessExpression(p) && p.name === n) ||
+        (ts.isQualifiedName(p) && p.right === n) ||
+        ((ts.isPropertyAssignment(p) ||
+          ts.isMethodDeclaration(p) ||
+          ts.isPropertyDeclaration(p) ||
+          ts.isPropertySignature(p) ||
+          ts.isMethodSignature(p) ||
+          ts.isGetAccessorDeclaration(p) ||
+          ts.isSetAccessorDeclaration(p) ||
+          ts.isEnumMember(p)) &&
+          p.name === n) ||
+        (ts.isBindingElement(p) && p.propertyName === n) ||
+        ((ts.isVariableDeclaration(p) ||
+          ts.isParameter(p) ||
+          ts.isFunctionDeclaration(p) ||
+          ts.isClassDeclaration(p) ||
+          ts.isInterfaceDeclaration(p) ||
+          ts.isTypeAliasDeclaration(p) ||
+          ts.isBindingElement(p)) &&
+          p.name === n);
+      if (!isNameOf) names.add(n.text);
+      else if (
+        n !== node.name &&
+        (ts.isVariableDeclaration(p) ||
+          ts.isParameter(p) ||
+          ts.isFunctionDeclaration(p) ||
+          ts.isClassDeclaration(p) ||
+          ts.isBindingElement(p))
+      )
+        declared.add(n.text);
+    }
+    ts.forEachChild(n, visit);
+  };
+  visit(node);
+  // A top-level `const x = …` declares its own name, which is no local.
+  if (ts.isVariableStatement(node))
+    for (const d of node.declarationList.declarations)
+      if (ts.isIdentifier(d.name)) declared.delete(d.name.text);
+  return [...names].filter((name) => !declared.has(name));
+}
+
+/**
+ * What a name is bound to at the top of a module: its declaration there,
+ * what it imports (followed to a declaration), or a global.
+ */
+function bindingOf(modules, file, name) {
+  const module = modules.get(file);
+  if (module?.declarations.has(name))
+    return `declared: ${module.declarations.get(name).join('\n')}`;
+  if (module?.imports.has(name))
+    return `imported: ${identity(modules, ...module.imports.get(name))}`;
+  return 'global';
 }
 
 /**
@@ -339,6 +413,36 @@ for (const file of receivers) {
       );
 }
 
+// Every name a declaration or endpoint uses must be bound now as it was:
+// a moved one's, as in the module it came from -- a receiving module's own
+// `javaTrim` would quietly replace the one a moved caller called, and a
+// helper left behind unimported would fall through to a global of the same
+// name (`close`) -- and one that stayed, as where it is, so its caller is
+// not pointed at another module's `javaTrim` in the same change.
+const stayed = [];
+for (const [file, was] of before)
+  for (const kind of ['declarations', 'endpoints'])
+    for (const key of was[kind].keys())
+      if (after.get(file)?.[kind].has(key)) stayed.push([`${file}#${key}`, file]);
+for (const [where, from] of [...origins, ...stayed]) {
+  const at = where.indexOf('#');
+  const to = where.slice(0, at);
+  const key = where.slice(at + 1);
+  const kind = before.get(from).declarations.has(key) ? 'declarations' : 'endpoints';
+  for (const name of after.get(to).uses.get(key) ?? []) {
+    const was = bindingOf(before, from, name);
+    const is = bindingOf(after, to, name);
+    if (was === is) continue;
+    // What it imported may have moved too: the same declaration, now here.
+    if (was.startsWith('imported: ') && is === `declared: ${was.slice(10)}`) continue;
+    if (was.startsWith('declared: ') && is === `imported: ${was.slice(10)}`) continue;
+    if (kind === 'declarations' && name === key) continue; // its own name
+    problems.push(
+      `rebound: ${name}, used by ${key} in ${to}, is ${is.split(':')[0]} there; in ${from} at ${base} it was ${was.split(':')[0]}${was.split(':')[0] === is.split(':')[0] ? ' as something else' : ''}`
+    );
+  }
+}
+
 const count = (prefix) => problems.filter((p) => p.startsWith(prefix)).length;
 console.log(
   `moved ${moved.declarations} declarations and ${moved.endpoints} endpoints, ` +

scripts/check-types.mjs

@@ -111,6 +111,11 @@ if (up.length) {
 }
 const total = Object.values(counts).reduce((sum, n) => sum + n, 0);
 console.log(`  tools/ with noImplicitAny: ${total} left, none new`);
-if (down.length)
+// A fall not locked in is room a later change could fill unseen: the
+// baseline would not move, and nothing would say the count rose again.
+if (down.length) {
   console.log(`  ${down.length} module(s) went down; lock it in: npm run check:types -- --update`);
+  for (const [file, n] of down) console.log(`    ${file}: ${n} -> ${counts[file] ?? 0}`);
+  process.exit(1);
+}
 process.exit(0);

specs/014-content-service-modules/contracts/boundaries.md

@@ -4,18 +4,20 @@ The service's HTTP answers are not a contract this feature changes. Every one st
 
 The contract here is between the modules:
 
-| Module                                                                                                                                                                     | May import                                                      |
-| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------- |
-| `java-text`                                                                                                                                                                | the services' existing leaves (`java-hashmap`, `schema`, …)     |
-| `body`                                                                                                                                                                     | `java-text`                                                     |
-| `spring`                                                                                                                                                                   | `java-text`, `body`                                             |
-| `answers`                                                                                                                                                                  | `java-text`, `spring`                                           |
-| `objects`                                                                                                                                                                  | the above, `graph`, `domain`                                    |
-| `reference-entities`, `search-params`, `release`                                                                                                                           | the above, `solr`                                               |
-| a family (`<family>-endpoints`: data-query, pathways-events, content, person, entities, mapping-species, schema, interactors, exporters, citation, search, search-diagram) | any shared module; **never another family, never the registry** |
-| `registry`                                                                                                                                                                 | the families                                                    |
-| `server`                                                                                                                                                                   | the registry, the shared modules                                |
+| Module                                                                                                                                                                     | May import                                                                |
+| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------- |
+| `java-text`                                                                                                                                                                | the services' existing leaves (`java-hashmap`, `schema`, …)               |
+| `body`                                                                                                                                                                     | `java-text`                                                               |
+| `spring`                                                                                                                                                                   | `java-text`, `body`                                                       |
+| `answers`                                                                                                                                                                  | `java-text`, `spring`                                                     |
+| `objects`                                                                                                                                                                  | the above, `graph`, `domain`                                              |
+| `reference-entities`, `search-params`, `release`                                                                                                                           | the above, `solr`                                                         |
+| a family (`<family>-endpoints`: data-query, pathways-events, content, person, entities, mapping-species, schema, interactors, exporters, citation, search, search-diagram) | any shared module, any leaf; **never another family, never the registry** |
+| `registry`                                                                                                                                                                 | the families                                                              |
+| `server`                                                                                                                                                                   | the registry, the shared modules                                          |
 
 A test reads each module's imports and fails on any import this table does not allow, naming the module and the import. It is shown failing first: make one family import another.
 
+A **leaf** is any module of the service not in the table -- `graph`, `solr`, `domain`, `schema`, `settings`, `endpoint` (the `Endpoint` types) and the like, and any module added without a row -- and imports only leaves. Every shared module may import leaves besides the shared modules its row names: `spring` reads `domain`'s views, for one. So a family, or `answers`, declares `Endpoint[]` or returns a `Handler` from `endpoint.ts`, never from the registry. The two tools that read the endpoint list, `diff` and `invalid-inputs`, may import anything, and nothing in the service imports them.
+
 Every family module is named `<family>-endpoints.ts`, because several family names are taken already: `schema.ts` (the model's class table) and `citation.ts` (the citation formatter) stay as they are.

specs/014-content-service-modules/contracts/checks.md

@@ -16,9 +16,12 @@ It fails with a line for each difference:
 changed: searchEntry (now in search-endpoints.ts) differs from service.ts at <base>
 missing: facetMapping (in service.ts at <base>, nowhere now)
 added: javaTrim (in body-endpoints.ts: not there at <base>, nor moved in)
-rebound: javaTrim in body.ts is jsoup-text.ts's, not the one service.ts had at <base>
+rebound: javaTrim in body.ts is not the javaTrim service.ts had at <base>
+rebound: javaJson, used by pageText in paging.ts, is global there; in service.ts at <base> it was declared
 ```
 
+A `rebound` line covers both an import that resolves to something else and any name a moved declaration uses that is bound differently where it now is: the receiving module's own declaration of that name, or a global where the original module had a declaration or an import. Names the moved code declares itself (locals, parameters) are not compared. The same holds for code that stays where it was: a caller left in `service.ts` that now reaches another module's helper of the same name is `rebound` too.
+
 ## Baseline totals (`node scripts/check-baseline-totals.mjs [base]`)
 
 It passes with:
@@ -30,9 +33,14 @@ loose <n> (base <n>) · implicit any <m> (base <m>) · lint <k> (base <k>)
 It fails with:
 
 ```text
-loose rose: <n+1>, base <n> — a move keeps totals; typing lowers them
+loose rose: <n+1>, base <n> -- a move keeps totals; typing lowers them
+loose in tools/content-node/a.ts rose: 6, base 5 -- a move keeps totals; typing lowers them
 ```
 
+A module the base branch already counts may not rise, even if another falls, unless it is one the split creates to receive moved code (the shared modules, the families, the registry, the server), which a later move may add to. Code never moves into the services' other modules, so a rise in one of those is new code. The split's modules are held by the total.
+
+The per-module checks (`check:types` for implicit `any`, `check:lint` for loose values) also fail on a fall that is not locked in with `--update`, so room left by typing cannot be spent later without the baseline moving.
+
 ## Route order (a spec over the registry)
 
 It fails when the list differs from the recorded one, or when one of the 15 named pairs is out of order:

specs/014-content-service-modules/plan.md

@@ -77,7 +77,8 @@ specs/014-content-service-modules/
 tools/content-node/
 ├── service.ts           # shrinks to nothing as families move; then removed (the image's CMD runs server.ts)
 ├── server.ts            # start-up, routing, path rules, download guard, failure answers, /health
-├── registry.ts          # Endpoint type; the families' lists, in today's order
+├── endpoint.ts          # the Endpoint, Handler, Answer and Request types (a leaf)
+├── registry.ts          # the families' lists, in today's order
 ├── release.ts           # the release, answered once (R2)
 ├── java-text.ts  body.ts  spring.ts  answers.ts  objects.ts      # shared, layered (R1)
 ├── reference-entities.ts  search-params.ts                        # shared by two families
@@ -108,7 +109,7 @@ deploy/content-node/Dockerfile # CMD → server.ts, once service.ts is gone
    Each is seen to fail.
 
 2. **The release without the list**: the one logic edit; `release.ts` itself comes with layer 6. Verified on beta like a move.
-3. **The `Endpoint` type and `registry.ts`**, in the same pull request as step 2, with `service.ts`'s list typed as `Endpoint[]` in place. The casts in the tools go.
+3. **The `Endpoint` type** (`endpoint.ts`, a leaf, so families and `answers` can use it without importing the registry), in the same pull request as step 2, with `service.ts`'s list typed as `Endpoint[]` in place. The casts in the tools go.
 4. **Shared modules**, bottom layer first: `java-text`, `body`, `spring`, `answers`, `objects`, then the pair-shared two. Each is a move pull request.
 5. **Families**, least coupled first, one pull request each:
    1. citation