Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 91 additions & 0 deletions contrib/examples/issue-302-unused-fixture-audit/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# Unused Fixture Audit

Self-contained reference for issue [#302](https://github.com/Vellar-Wallet/vellar-sdk/issues/302): confirming which exports of `src/x402-test-fixtures.ts` are still referenced, and which imports of it are dead.

## Run tests

```bash
npx vitest run contrib/examples/issue-302-unused-fixture-audit
```

## Outcome: there are no unused fixtures to remove

Issue #302 asks to remove "several fixtures that are no longer referenced by any current test file". Its first requirement is to **confirm each fixture has no remaining references** — and on this branch, that confirmation comes back negative: **every export in `src/x402-test-fixtures.ts` is still referenced by a live test.** There is nothing to delete.

The audit below is produced by `formatAuditReport()` in this module, run against the real files:

| Export | Kind | References | Used by |
| --- | --- | ---: | --- |
| `C_ADDRESS` | const | 2 | src/x402-client.test.ts, contrib/x402-client-fallback.test.ts |
| `TOKEN` | const | 13 | src/x402-client.test.ts, src/x402-guards.test.ts, contrib/x402-guards-boundary.test.ts |
| `PAYTO` | const | 4 | src/x402-client.test.ts |
| `SIM_SOURCE` | const | 13 | src/x402-client.test.ts, contrib/x402-client-fallback.test.ts |
| `CAIP2_TESTNET` | const | 31 | src/x402-guards.test.ts, contrib/x402-guards-boundary.test.ts |
| `b64` | function | 11 | src/x402-guards.test.ts, contrib/x402-guards-boundary.test.ts |
| `requirements` | function | 56 | src/x402-client.test.ts, src/x402-guards.test.ts, contrib/x402-client-fallback.test.ts, contrib/x402-guards-boundary.test.ts |
| `decoded` | function | 34 | src/x402-guards.test.ts, contrib/x402-guards-boundary.test.ts |
| `response402` | function | 13 | src/x402-client.test.ts, src/x402-guards.test.ts, contrib/x402-client-fallback.test.ts, contrib/x402-guards-boundary.test.ts |

Reference counts **exclude the import line itself**, so every number above is a genuine use in a test body.

`PAYTO` is worth calling out: it is the one export used by a single consumer, and on `main` it genuinely was imported by nobody. Tests added to `dev` since then use it, which is precisely why the audit is worth re-running rather than trusting a stale answer.

### What the audit did find

One real cleanup, applied in this PR:

```
Dead imports in contrib/x402-client-fallback.test.ts: `requirements`, `response402`
```

Both names were imported and never used. That is a dead *import*, not an unused *export* — the fixtures themselves are used elsewhere — so the fix is to shorten the import, not to delete the fixtures. Done in [`contrib/x402-client-fallback.test.ts`](../../x402-client-fallback.test.ts).

## Why a tool instead of a grep

A bare `grep -c NAME` gives the wrong answer here, in three separate ways:

1. **It counts the import line.** A fixture that is imported and never used still looks used. This is exactly the `contrib/x402-client-fallback.test.ts` case above — grep reports `requirements` as referenced when it isn't.
2. **It counts unrelated local declarations.** `PAYTO` is independently declared in `src/x402-auth-entry.test.ts`, `packages/mcp-x402-payer/test/helpers.ts`, and others. Grepping the repo for `PAYTO` returns 13 hits that have nothing to do with the fixture module.
3. **It cannot distinguish an unused export from a dead import.** The two have different fixes — delete the export vs. shorten the import — and conflating them leads to deleting a fixture that another file still needs.

`auditFixtures()` resolves all three by parsing each consumer's fixture import statement, then counting whole-word references in that consumer's body with the fixture import line removed.

## Usage

```ts
import { readFileSync } from "node:fs";
import { auditFixtures, formatAuditReport } from "./fixture-audit";

const result = auditFixtures({
fixtureSource: readFileSync("src/x402-test-fixtures.ts", "utf8"),
moduleId: "x402-test-fixtures",
consumers: ["src/x402-client.test.ts", "src/x402-guards.test.ts"].map((file) => ({
file,
source: readFileSync(file, "utf8"),
})),
});

result.unusedExports; // [] — safe to delete, were there any
result.deadImports; // [{ file, names }] — imported but never referenced
console.log(formatAuditReport(result));
```

## Semantics

| Case | Result |
|------|--------|
| Export imported and referenced by a consumer | Used |
| Export imported by a consumer but never referenced | Unused export **and** a dead import for that file |
| Export referenced only inside the fixture module itself | Unused — internal use doesn't justify an `export` |
| Export referenced by at least one of several consumers | Used; still a dead import in the files that don't use it |
| A name declared locally in a file that doesn't import the module | Ignored — the file isn't a consumer |
| Longer identifier containing the name (`TOKEN` vs `TOKEN_LIST`) | Not counted; matching is whole-word |
| Aliased import (`TOKEN as ASSET`) | Counts references to the local name (`ASSET`) |

## Guarding against regression

The final `describe` block in the test file runs the audit against the real repository files on every `npm test`. If a future change orphans a fixture, `reports NO unused exports` fails and names it — turning "is this still used?" from a manual grep into a standing check, and flagging the moment the removal #302 asked for actually becomes correct.

## Limits

Line-oriented parsing, no TypeScript AST. It handles the syntax these fixtures actually use — single-line named imports, `as` aliases, inline `type` modifiers — and deliberately does not attempt multi-line import blocks for the *fixture* module, `import * as ns`, or re-exports. A name referenced only inside a comment or string literal counts as a reference. For this fixture module and its four consumers that is sufficient; for a general-purpose dead-code pass, use a tool built on the compiler API such as `knip` or `ts-prune`.
271 changes: 271 additions & 0 deletions contrib/examples/issue-302-unused-fixture-audit/fixture-audit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,271 @@
import { readFileSync } from "node:fs";
import { dirname, join, relative, resolve } from "node:path";
import { fileURLToPath } from "node:url";
import { describe, expect, it } from "vitest";
import {
auditFixtures,
countReferences,
formatAuditReport,
parseFixtureExports,
parseFixtureImports,
} from "./fixture-audit";

const MODULE_ID = "x402-test-fixtures";
const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), "../../..");

describe("parseFixtureExports", () => {
it("picks up both const and function exports, in declaration order", () => {
const source = [
"import type { Foo } from './types';",
"export const TOKEN = 'C...';",
"export const PAYTO = 'G...';",
"export function b64(o: unknown): string { return ''; }",
].join("\n");

expect(parseFixtureExports(source)).toEqual([
{ name: "TOKEN", kind: "const" },
{ name: "PAYTO", kind: "const" },
{ name: "b64", kind: "function" },
]);
});

it("ignores non-exported declarations and type-only exports", () => {
const source = [
"const INTERNAL = 1;",
"function helper() {}",
"export type Thing = { a: number };",
"export interface Other { b: string }",
"export const REAL = 2;",
].join("\n");

expect(parseFixtureExports(source)).toEqual([{ name: "REAL", kind: "const" }]);
});
});

describe("parseFixtureImports", () => {
it("reads the named bindings off the fixture import line", () => {
const source = [
"import { describe, it } from 'vitest';",
"import { TOKEN, b64, requirements } from './x402-test-fixtures';",
].join("\n");

expect(parseFixtureImports(source, MODULE_ID)).toEqual(["TOKEN", "b64", "requirements"]);
});

it("returns the local name for aliased imports, since that is what the body uses", () => {
const source = "import { TOKEN as ASSET, b64 } from '../src/x402-test-fixtures';";
expect(parseFixtureImports(source, MODULE_ID)).toEqual(["ASSET", "b64"]);
});

it("strips inline `type` modifiers", () => {
const source = "import { type Requirements, TOKEN } from './x402-test-fixtures';";
expect(parseFixtureImports(source, MODULE_ID)).toEqual(["Requirements", "TOKEN"]);
});

it("returns nothing when the file does not import the fixture module", () => {
const source = "import { describe, it } from 'vitest';\nconst TOKEN = 'local';";
expect(parseFixtureImports(source, MODULE_ID)).toEqual([]);
});

it("is not fooled by a longer import list appearing earlier in the file", () => {
// Regression guard: a lazy `[\s\S]*?` regex spans from the FIRST import in
// the file to the fixture specifier, capturing every intervening binding.
const source = [
"import { describe, expect, it, vi } from 'vitest';",
"import {",
" DisallowedAssetError,",
" InvalidRequirementsError,",
"} from './x402-types';",
"import { TOKEN, PAYTO } from './x402-test-fixtures';",
].join("\n");

expect(parseFixtureImports(source, MODULE_ID)).toEqual(["TOKEN", "PAYTO"]);
});
});

describe("countReferences", () => {
it("does not count the fixture import line itself", () => {
const source = ["import { TOKEN } from './x402-test-fixtures';", "const other = 1;"].join("\n");
expect(countReferences(source, "TOKEN", MODULE_ID)).toBe(0);
});

it("counts real uses in the body", () => {
const source = [
"import { TOKEN } from './x402-test-fixtures';",
"it('works', () => {",
" expect(TOKEN).toBe(TOKEN);",
"});",
].join("\n");

expect(countReferences(source, "TOKEN", MODULE_ID)).toBe(2);
});

it("matches whole words only, so a longer identifier does not count", () => {
const source = ["const TOKEN_LIST = [];", "const MY_TOKEN = 1;", "use(TOKEN);"].join("\n");
expect(countReferences(source, "TOKEN", MODULE_ID)).toBe(1);
});
});

describe("auditFixtures", () => {
const fixtureSource = [
"export const USED = 'a';",
"export const DEAD = 'b';",
"export function helper() { return DEAD; }",
].join("\n");

it("flags an export that nobody imports", () => {
const result = auditFixtures({
fixtureSource,
moduleId: MODULE_ID,
consumers: [
{
file: "a.test.ts",
source: "import { USED, helper } from './x402-test-fixtures';\nuse(USED); helper();",
},
],
});

expect(result.unusedExports).toEqual(["DEAD"]);
});

it("does NOT count a fixture's use inside the fixture module itself as a reference", () => {
// `helper()` references DEAD, but that is internal to the fixture module.
// An export used only internally still shouldn't be exported.
const result = auditFixtures({
fixtureSource,
moduleId: MODULE_ID,
consumers: [{ file: "a.test.ts", source: "import { USED } from './x402-test-fixtures';\nuse(USED);" }],
});

expect(result.unusedExports).toContain("DEAD");
});

it("treats an imported-but-unreferenced export as unused, not used", () => {
const result = auditFixtures({
fixtureSource,
moduleId: MODULE_ID,
consumers: [
// DEAD is imported but never referenced — the exact case a bare grep
// gets wrong.
{ file: "a.test.ts", source: "import { USED, DEAD } from './x402-test-fixtures';\nuse(USED);" },
],
});

// `helper` is unused too — no consumer imports it at all.
expect(result.unusedExports).toEqual(["DEAD", "helper"]);
expect(result.deadImports).toEqual([{ file: "a.test.ts", names: ["DEAD"] }]);
});

it("counts an export as used when ANY consumer references it", () => {
const result = auditFixtures({
fixtureSource,
moduleId: MODULE_ID,
consumers: [
{ file: "a.test.ts", source: "import { DEAD } from './x402-test-fixtures';" },
{ file: "b.test.ts", source: "import { DEAD } from './x402-test-fixtures';\nuse(DEAD);" },
],
});

expect(result.unusedExports).not.toContain("DEAD");
// Still reported as a dead import in the file that doesn't use it.
expect(result.deadImports).toEqual([{ file: "a.test.ts", names: ["DEAD"] }]);
});

it("ignores files that never import the fixture module, even if names collide", () => {
const result = auditFixtures({
fixtureSource,
moduleId: MODULE_ID,
consumers: [
{ file: "a.test.ts", source: "import { USED } from './x402-test-fixtures';\nuse(USED);" },
// Declares its own DEAD; must not make the fixture's DEAD look used.
{ file: "unrelated.test.ts", source: "const DEAD = 'local';\nuse(DEAD); use(DEAD);" },
],
});

// The unrelated file's local `DEAD` must not rescue the fixture's `DEAD`.
expect(result.unusedExports).toEqual(["DEAD", "helper"]);
expect(result.consumers.map((c) => c.file)).toEqual(["a.test.ts"]);
});

it("sums references across consumers and records who imports what", () => {
const result = auditFixtures({
fixtureSource,
moduleId: MODULE_ID,
consumers: [
{ file: "a.test.ts", source: "import { USED } from './x402-test-fixtures';\nuse(USED); use(USED);" },
{ file: "b.test.ts", source: "import { USED } from './x402-test-fixtures';\nuse(USED);" },
],
});

const used = result.verdicts.find((v) => v.name === "USED");
expect(used?.totalReferences).toBe(3);
expect(used?.importedBy).toEqual(["a.test.ts", "b.test.ts"]);
});
});

describe("formatAuditReport", () => {
it("states that nothing can be removed when every export is used", () => {
const report = formatAuditReport(
auditFixtures({
fixtureSource: "export const USED = 'a';",
moduleId: MODULE_ID,
consumers: [{ file: "a.test.ts", source: "import { USED } from './x402-test-fixtures';\nuse(USED);" }],
}),
);

expect(report).toContain("nothing to remove");
expect(report).toContain("| `USED` | const | 1 | a.test.ts |");
});

it("names the removable exports when there are any", () => {
const report = formatAuditReport(
auditFixtures({
fixtureSource: "export const USED = 'a';\nexport const DEAD = 'b';",
moduleId: MODULE_ID,
consumers: [{ file: "a.test.ts", source: "import { USED } from './x402-test-fixtures';\nuse(USED);" }],
}),
);

expect(report).toContain("Unused exports (safe to remove): `DEAD`");
});
});

// The audit that answers issue #302 for the tree this test runs against.
describe("src/x402-test-fixtures.ts (live audit)", () => {
const fixturePath = join(repoRoot, "src/x402-test-fixtures.ts");
// Every file known to import the fixtures module, per a repo-wide search.
const consumerPaths = [
"src/x402-client.test.ts",
"src/x402-guards.test.ts",
"contrib/x402-client-fallback.test.ts",
"contrib/x402-guards-boundary.test.ts",
];

const result = auditFixtures({
fixtureSource: readFileSync(fixturePath, "utf8"),
moduleId: MODULE_ID,
consumers: consumerPaths.map((p) => ({
file: relative(repoRoot, join(repoRoot, p)).replace(/\\/g, "/"),
source: readFileSync(join(repoRoot, p), "utf8"),
})),
});

it("finds every consumer of the fixture module", () => {
expect(result.consumers).toHaveLength(consumerPaths.length);
});

it("reports NO unused exports — the premise of #302 does not hold on this branch", () => {
// If a future change orphans a fixture, this fails and names it, which is
// exactly when the removal #302 asked for becomes correct.
expect(result.unusedExports).toEqual([]);
});

it("confirms each individual export is referenced by at least one live test", () => {
for (const verdict of result.verdicts) {
expect(
verdict.totalReferences,
`${verdict.name} is exported but never referenced`,
).toBeGreaterThan(0);
}
});
});
Loading
Loading