Skip to content

Commit 8043159

Browse files
authored
refactor: replace hand-rolled helpers with the library calls that already ship (#414)
1 parent d6f023e commit 8043159

22 files changed

Lines changed: 85 additions & 119 deletions

File tree

‎.github/scripts/gen-action-docs.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
import { SECTIONS } from "../../src/sections/registry.js";
2121
import { agree } from "../../src/text.js";
2222
import { countWord } from "./lib/count-word.js";
23-
import { escapeRe, type GeneratedRegion, regenerateRegions } from "./lib/generated-regions.js";
23+
import { type GeneratedRegion, regenerateRegions } from "./lib/generated-regions.js";
2424

2525
const ROOT = join(import.meta.dir, "..", "..");
2626

@@ -305,7 +305,7 @@ function blockShape(lines: string): RegExp {
305305
}
306306

307307
function tableShape(header: string, cells: string): RegExp {
308-
return blockShape(String.raw`${escapeRe(header)}\n(?:\| ${cells} \|\n)*`);
308+
return blockShape(String.raw`${RegExp.escape(header)}\n(?:\| ${cells} \|\n)*`);
309309
}
310310

311311
/** A JSON string literal as JSON.stringify() emits it: its own escapes only, bare quotes never. */
@@ -353,7 +353,7 @@ export const GENERATED_REGIONS: Readonly<Record<string, readonly GeneratedRegion
353353
{
354354
name: "policy-count-sentence",
355355
placement: { kind: "under-heading", heading: "# The undeclared policy" },
356-
body: blockShape(String.raw`[A-Z][a-z-]*${escapeRe(COUNT_SENTENCE_LEAD)}[^\n]+\.\n`),
356+
body: blockShape(String.raw`[A-Z][a-z-]*${RegExp.escape(COUNT_SENTENCE_LEAD)}[^\n]+\.\n`),
357357
render: block(() => renderPolicyCountSentence(knobbedSections())),
358358
},
359359
{
@@ -370,7 +370,7 @@ export const GENERATED_REGIONS: Readonly<Record<string, readonly GeneratedRegion
370370
{
371371
name: "permissions-grant-sentence",
372372
placement: { kind: "under-heading", heading: "## What to grant" },
373-
body: blockShape(String.raw`${escapeRe(GRANT_SENTENCE_LEAD)}[^\n]+\.\n`),
373+
body: blockShape(String.raw`${RegExp.escape(GRANT_SENTENCE_LEAD)}[^\n]+\.\n`),
374374
render: block(() => renderGrantSentence(SECTIONS)),
375375
},
376376
{
@@ -388,7 +388,7 @@ export const GENERATED_REGIONS: Readonly<Record<string, readonly GeneratedRegion
388388
heading: "## Checking settings changes on pull requests",
389389
},
390390
body: blockShape(
391-
String.raw`${escapeRe(NO_GATED_READS)}\n|${escapeRe(GATED_READS_LEAD_IN)}\n\n(?:${GATED_READ_BULLET})+`,
391+
String.raw`${RegExp.escape(NO_GATED_READS)}\n|${RegExp.escape(GATED_READS_LEAD_IN)}\n\n(?:${GATED_READ_BULLET})+`,
392392
),
393393
render: block(() => renderCheckModeGatedReads(SECTIONS)),
394394
},

‎.github/scripts/gen-docs.ts‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,7 @@ import type { UndeclaredPolicy } from "../../src/types.js";
1515
import { readArchitecture, renderArchitectureMermaid } from "./arch-lint.js";
1616
import { COVERAGE_DATA, type CoverageData } from "./coverage-data.js";
1717
import { ENDPOINT_ANCHORS, type EndpointAnchors } from "./endpoint-docs.js";
18-
import {
19-
escapeRe,
20-
type GeneratedRegion,
21-
regenerateRegions,
22-
regionBounds,
23-
} from "./lib/generated-regions.js";
18+
import { type GeneratedRegion, regenerateRegions, regionBounds } from "./lib/generated-regions.js";
2419

2520
const ROOT = join(import.meta.dir, "..", "..");
2621
export const COVERAGE_PATH = "docs/reference/coverage.md";
@@ -428,7 +423,7 @@ function sectionsTableRegion(name: string, heading: string): GeneratedRegion {
428423
name,
429424
placement: { kind: "under-heading", heading },
430425
body: new RegExp(
431-
String.raw`^\n(?:${escapeRe(TABLE_HEADER)}\n(?:\| \x60[a-z_]+\x60 \| [^\n]* \|\n)*)?$`,
426+
String.raw`^\n(?:${RegExp.escape(TABLE_HEADER)}\n(?:\| \x60[a-z_]+\x60 \| [^\n]* \|\n)*)?$`,
432427
),
433428
render: () => `\n${renderSectionsTable(SECTIONS, DOCS)}\n`,
434429
};
@@ -439,7 +434,7 @@ function outputsListRegion(name: string, heading: string): GeneratedRegion {
439434
name,
440435
placement: { kind: "under-heading", heading },
441436
body: new RegExp(
442-
String.raw`^(?:\x60[a-z]+\x60(?: / \x60[a-z]+\x60)*${escapeRe(RESULT_TAIL)})?$`,
437+
String.raw`^(?:\x60[a-z]+\x60(?: / \x60[a-z]+\x60)*${RegExp.escape(RESULT_TAIL)})?$`,
443438
),
444439
render: () => renderOutputsList(RUN_RESULTS),
445440
};
@@ -450,7 +445,7 @@ function patUrlRegion(name: string): GeneratedRegion {
450445
return {
451446
name,
452447
placement: { kind: "tail" },
453-
body: new RegExp(String.raw`^\n(?:\[${escapeRe(PAT_FORM_LABEL)}\]: \S+\n)?$`),
448+
body: new RegExp(String.raw`^\n(?:\[${RegExp.escape(PAT_FORM_LABEL)}\]: \S+\n)?$`),
454449
render: () => `\n[${PAT_FORM_LABEL}]: ${patFormUrl()}\n`,
455450
};
456451
}
@@ -505,10 +500,10 @@ const nonBlank = (excluded: string): string =>
505500
String.raw`[ \t]*[^${excluded}\s][^${excluded}\r\n]*`;
506501
const PROSE_LINE = `${nonBlank("")}\n`;
507502
const CELL = nonBlank("|");
508-
const KEY_CELL = String.raw`\[\x60[a-z_]+\x60\]\(${escapeRe(SECTIONS_PAGE)}\)(?: \(\x60${nonBlank("|\x60")}\x60\))?`;
503+
const KEY_CELL = String.raw`\[\x60[a-z_]+\x60\]\(${RegExp.escape(SECTIONS_PAGE)}\)(?: \(\x60${nonBlank("|\x60")}\x60\))?`;
509504
const SUPPORTED_ROWS = String.raw`(?:\| ${CELL} \| ${KEY_CELL} \| ${CELL} \|\n)+`;
510505
const GAP_ROWS = String.raw`(?:\| ${CELL} \| ${CELL} \| ${CELL} \|\n)+`;
511-
const GAPS_BODY = String.raw`(?:${PROSE_LINE}\n${escapeRe(GAPS_HEADER)}\n|${escapeRe(GAPS_HEADER)}\n${GAP_ROWS})`;
506+
const GAPS_BODY = String.raw`(?:${PROSE_LINE}\n${RegExp.escape(GAPS_HEADER)}\n|${RegExp.escape(GAPS_HEADER)}\n${GAP_ROWS})`;
512507
const BULLETS = `(?:- ${PROSE_LINE})+`;
513508
const NOTE_GROUPS = String.raw`(?:\*\*${nonBlank("*")}\*\* \(\x60[a-z_]+\x60\)\n\n${BULLETS}\n)+`;
514509

@@ -519,10 +514,10 @@ const COVERAGE_REGIONS: readonly GeneratedRegion[] = [
519514
name: "coverage",
520515
placement: { kind: "tail" },
521516
body: new RegExp(
522-
String.raw`^\n(?:(?:${PROSE_LINE}\n)+${escapeRe(SUPPORTED_HEADING)}\n\n${escapeRe(SUPPORTED_HEADER)}\n` +
523-
String.raw`${SUPPORTED_ROWS}\n${escapeRe(NOTES_HEADING)}\n\n${NOTE_GROUPS}` +
524-
String.raw`${escapeRe(GAPS_HEADING)}\n\n${GAPS_BODY}\n${escapeRe(NO_API_HEADING)}\n\n` +
525-
String.raw`${PROSE_LINE}\n${BULLETS}\n${escapeRe(OUT_OF_SCOPE_HEADING)}\n\n${BULLETS})?$`,
517+
String.raw`^\n(?:(?:${PROSE_LINE}\n)+${RegExp.escape(SUPPORTED_HEADING)}\n\n${RegExp.escape(SUPPORTED_HEADER)}\n` +
518+
String.raw`${SUPPORTED_ROWS}\n${RegExp.escape(NOTES_HEADING)}\n\n${NOTE_GROUPS}` +
519+
String.raw`${RegExp.escape(GAPS_HEADING)}\n\n${GAPS_BODY}\n${RegExp.escape(NO_API_HEADING)}\n\n` +
520+
String.raw`${PROSE_LINE}\n${BULLETS}\n${RegExp.escape(OUT_OF_SCOPE_HEADING)}\n\n${BULLETS})?$`,
526521
),
527522
render: () => `\n${renderCoverage(SECTIONS, DOCS, COVERAGE_DATA, ENDPOINT_ANCHORS)}\n`,
528523
},

‎.github/scripts/lib/generated-regions.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,6 @@ import { Parser } from "yaml";
88
/** Which comment syntax a file's markers use: a complete `<!-- -->` comment, or a whole-line YAML `#` comment. */
99
export type MarkerSyntax = "html" | "yaml";
1010

11-
/** `text` as a regex source matching itself literally; body shapes splice renderer constants through it. */
12-
export function escapeRe(text: string): string {
13-
return text.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
14-
}
15-
1611
/** Character offsets of one marker: `[start, end)`. */
1712
export type MarkerSpan = readonly [start: number, end: number];
1813

‎src/discovery/central.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@
44
*/
55

66
import { existsSync, readdirSync, statSync } from "node:fs";
7-
import { join } from "node:path";
7+
import { extname, join, parse } from "node:path";
88
import { err, ok, type Result } from "neverthrow";
99
import { type SlugKey, slugKey } from "../github/slug.js";
1010
import type { CentralFileProblem, ProblemOf } from "../problem.js";
1111
import { type CentralTarget, SLUG_RE } from "./targets.js";
1212

13-
const YAML_EXT = /\.ya?ml$/;
13+
const YAML_EXTENSIONS = new Set([".yml", ".yaml"]);
1414

1515
export function resolveCentralTargets(
1616
reposDir: string,
@@ -52,13 +52,13 @@ export function resolveCentralTargets(
5252
);
5353
continue;
5454
}
55-
if (!YAML_EXT.test(inner)) {
55+
if (!YAML_EXTENSIONS.has(extname(inner))) {
5656
warnings.push(
5757
`ignoring ${innerPath}: not a .yml/.yaml file, so it defines no target repository`,
5858
);
5959
continue;
6060
}
61-
addTarget(`${owner}/${inner.replace(YAML_EXT, "")}`, innerPath);
61+
addTarget(`${owner}/${parse(inner).name}`, innerPath);
6262
}
6363
};
6464

@@ -71,7 +71,7 @@ export function resolveCentralTargets(
7171
scanOwnerDir(entryPath, entry);
7272
continue;
7373
}
74-
if (!YAML_EXT.test(entry)) {
74+
if (!YAML_EXTENSIONS.has(extname(entry))) {
7575
warnings.push(
7676
`ignoring ${entryPath}: not a .yml/.yaml file, so it defines no target repository`,
7777
);
@@ -81,7 +81,7 @@ export function resolveCentralTargets(
8181
ownerlessFiles.push(entryPath);
8282
continue;
8383
}
84-
addTarget(`${adminOwner}/${entry.replace(YAML_EXT, "")}`, entryPath);
84+
addTarget(`${adminOwner}/${parse(entry).name}`, entryPath);
8585
}
8686
if (ownerlessFiles.length > 0) {
8787
errors.push({ kind: "ownerless", files: ownerlessFiles });

‎src/engine/validate.ts‎

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/** Shape validation against each section's loose zod shape; the parsed output, not the input, is what the engine applies. */
22

33
import { err, ok, type Result } from "neverthrow";
4+
import { z } from "zod";
45
import { nonPlainKind } from "../plain-data.js";
56
import type { ProblemOf } from "../problem.js";
67
import { LIST_SECTIONS, type ListSection, SECTION_KEYS, type SettingsFile } from "../schema.js";
@@ -10,6 +11,7 @@ import {
1011
type DeclaredSecretValue,
1112
} from "../sections/contract/module.js";
1213
import { listLayering, sectionModule, sectionShape } from "../sections/registry.js";
14+
import { valueAt } from "../sections/shared/list-section.js";
1315
import { agree, countNoun } from "../text.js";
1416
import { type SettingsSource, validateSecretRef } from "./secret-refs.js";
1517

@@ -19,18 +21,6 @@ function isListSection(key: string): key is ListSection {
1921
return LIST_KEYS.has(key);
2022
}
2123

22-
/** The value at an issue's path, so a null the author wrote can be told from a type the author got wrong. */
23-
function valueAt(root: unknown, path: readonly PropertyKey[]): unknown {
24-
let node: unknown = root;
25-
for (const step of path) {
26-
if (typeof node !== "object" || node === null) {
27-
return undefined;
28-
}
29-
node = (node as Record<PropertyKey, unknown>)[step];
30-
}
31-
return node;
32-
}
33-
3424
/** zod's issue fields this module reads; `legal` is this action's own, set where a shape refuses null itself. */
3525
interface NullIssue {
3626
code: string;
@@ -230,18 +220,14 @@ export function validateSectionShapes(
230220
if (!parsed.success) {
231221
const issues = parsed.error.issues;
232222
for (const issue of issues.slice(0, 5)) {
233-
const path = issue.path
234-
.map((p) => (typeof p === "number" ? `[${p}]` : `.${String(p)}`))
235-
.join("");
223+
const path = z.core.toDotPath([key, ...issue.path]);
236224
// A null the shape refused is the author saying "empty" where GitHub has no empty state: name the values that
237225
// exist. A shape's own diagnostic (a custom issue) already names the fix, unless it supplies the legal values itself.
238226
if (valueAt(declared, issue.path) === null && rewritesForNull(issue as NullIssue)) {
239-
problems.push(
240-
`${key}${path} has no empty state; write ${legalValues(issue as NullIssue)}`,
241-
);
227+
problems.push(`${path} has no empty state; write ${legalValues(issue as NullIssue)}`);
242228
continue;
243229
}
244-
problems.push(`${key}${path}: ${issue.message}`);
230+
problems.push(`${path}: ${issue.message}`);
245231
}
246232
if (issues.length > 5) {
247233
// A silently truncated list costs one fix-and-rerun cycle per hidden offender.

‎src/sections/contract/endpoints.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -243,10 +243,7 @@ export function expand(
243243
);
244244
}
245245
if (query && Object.keys(query).length > 0) {
246-
const qs = Object.entries(query)
247-
.map(([key, value]) => `${encodeURIComponent(key)}=${encodeURIComponent(value)}`)
248-
.join("&");
249-
return `${path}?${qs}`;
246+
return `${path}?${new URLSearchParams(query)}`;
250247
}
251248
return path;
252249
}

‎src/sections/contract/live.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*/
66

77
import { err, ok, type Result } from "neverthrow";
8-
import type { z } from "zod";
8+
import { z } from "zod";
99
import { countNoun } from "../../text.js";
1010
import { endpointMethod, endpointPath } from "./endpoints.js";
1111
import type { SectionFailure } from "./errors.js";
@@ -85,10 +85,7 @@ export function parseLive<T>(
8585
}
8686
const issues = parsed.error.issues;
8787
const shown = issues.slice(0, 3).map((issue) => {
88-
const path = issue.path
89-
.map((part) => (typeof part === "number" ? `[${part}]` : `.${String(part)}`))
90-
.join("");
91-
return `${path.replace(/^\./, "") || "(body)"}: ${issue.message}`;
88+
return `${z.core.toDotPath(issue.path) || "(body)"}: ${issue.message}`;
9289
});
9390
const more =
9491
issues.length > 3 ? `; and ${countNoun(issues.length - 3, "more issue", "more issues")}` : "";

‎src/sections/contract/permissions.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,7 @@ export function samePermission(
3535
}
3636
const resources = new Set(a.repo);
3737
const others = new Set(b.repo);
38-
return (
39-
a.org === b.org &&
40-
resources.size === others.size &&
41-
[...resources].every((resource) => others.has(resource))
42-
);
38+
return a.org === b.org && resources.size === others.size && resources.isSubsetOf(others);
4339
}
4440

4541
/** Human-facing label for each PAT resource, as shown in the token UI. */

‎src/sections/custom_properties/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ function sameValue(a: WireValue, b: WireValue): boolean {
5454
if (Array.isArray(a) && Array.isArray(b)) {
5555
const setA = new Set(a);
5656
const setB = new Set(b);
57-
return setA.size === setB.size && [...setA].every((element) => setB.has(element));
57+
return setA.size === setB.size && setA.isSubsetOf(setB);
5858
}
5959
return a === b;
6060
}

‎src/sections/environments/branch-policies.ts‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { err, ok, Result, safeTry } from "neverthrow";
77
import { z } from "zod";
88
import { subsetDiff } from "../../engine/diff.js";
99
import type { UndeclaredPolicy } from "../../types.js";
10-
import { type SectionFailure, sectionFailure } from "../contract/errors.js";
10+
import type { SectionFailure } from "../contract/errors.js";
1111
import { liveByIdentity, liveIdentity } from "../contract/live.js";
1212
import {
1313
type DeclaredIssue,
@@ -18,7 +18,11 @@ import {
1818
undeclaredNote,
1919
} from "../contract/module.js";
2020
import { hasDrift, plainData, type Read } from "../contract/plan.js";
21-
import type { EnvironmentRestOp, EnvironmentsRestContext } from "./endpoints.js";
21+
import {
22+
type EnvironmentRestOp,
23+
type EnvironmentsRestContext,
24+
unreconcilable,
25+
} from "./endpoints.js";
2226
import type { LiveEnvironmentBody } from "./index.js";
2327
import type { NestedPlan } from "./nested.js";
2428
import type { DeploymentBranchPolicyConfig } from "./schema.js";
@@ -46,23 +50,18 @@ function livePolicyType(policy: LiveBranchPolicy): string {
4650
return typeof policy.type === "string" ? policy.type : "branch";
4751
}
4852

49-
function unreconcilable(envName: string, what: string): SectionFailure {
50-
return sectionFailure(
51-
"live-shape",
52-
`environments: the deployment branch-policy list for environment "${envName}" returned a policy without ${what}, so it cannot be reconciled. Check the "api-version" input against the GitHub REST docs for this endpoint`,
53-
);
54-
}
53+
const POLICY = { list: "deployment branch-policy", entry: "policy" };
5554

5655
function livePolicyId(policy: LiveBranchPolicy, envName: string): Result<string, SectionFailure> {
5756
if (policy.id === undefined) {
58-
return err(unreconcilable(envName, "an id"));
57+
return err(unreconcilable(POLICY, envName, "an id"));
5958
}
6059
return ok(String(policy.id));
6160
}
6261

6362
function livePolicyName(policy: LiveBranchPolicy, envName: string): Result<string, SectionFailure> {
6463
if (typeof policy.name !== "string") {
65-
return err(unreconcilable(envName, "a name"));
64+
return err(unreconcilable(POLICY, envName, "a name"));
6665
}
6766
return ok(policy.name);
6867
}

0 commit comments

Comments
 (0)