From c16a3400005d2cdffce860e8fc33d03c9734ac6d Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Sun, 19 Apr 2026 13:20:05 -0400 Subject: [PATCH] Add generic status envelopes rule --- README.md | 1 + src/default-registry.ts | 2 + src/rules/generic-status-envelopes/README.md | 50 +++++++ src/rules/generic-status-envelopes/index.ts | 138 +++++++++++++++++++ tests/generic-status-envelopes.test.ts | 108 +++++++++++++++ tests/heuristics.test.ts | 5 + 6 files changed, 304 insertions(+) create mode 100644 src/rules/generic-status-envelopes/README.md create mode 100644 src/rules/generic-status-envelopes/index.ts create mode 100644 tests/generic-status-envelopes.test.ts diff --git a/README.md b/README.md index ea10b3e..0b7fd40 100644 --- a/README.md +++ b/README.md @@ -135,6 +135,7 @@ Current checks focus on patterns that often show up in unreviewed generated code - [error-obscuring catch blocks](src/rules/error-obscuring/README.md) (default-return or generic replacement error) - [empty catch blocks](src/rules/empty-catch/README.md) - [promise `.catch()` default fallbacks](src/rules/promise-default-fallbacks/README.md) +- [generic status envelopes](src/rules/generic-status-envelopes/README.md) - [stringified unknown errors](src/rules/stringified-unknown-errors/README.md) - [async wrapper / `return await` noise](src/rules/async-noise/README.md) - [pass-through wrappers](src/rules/pass-through-wrappers/README.md) diff --git a/src/default-registry.ts b/src/default-registry.ts index a68ccd4..a7771e6 100644 --- a/src/default-registry.ts +++ b/src/default-registry.ts @@ -18,6 +18,7 @@ import { emptyCatchRule } from "./rules/empty-catch"; import { errorObscuringRule } from "./rules/error-obscuring"; import { errorSwallowingRule } from "./rules/error-swallowing"; import { promiseDefaultFallbacksRule } from "./rules/promise-default-fallbacks"; +import { genericStatusEnvelopesRule } from "./rules/generic-status-envelopes"; import { stringifiedUnknownErrorsRule } from "./rules/stringified-unknown-errors"; import { barrelDensityRule } from "./rules/barrel-density"; import { directoryFanoutHotspotRule } from "./rules/directory-fanout-hotspot"; @@ -46,6 +47,7 @@ export function createDefaultRegistry(): Registry { registry.registerRule(errorObscuringRule); registry.registerRule(emptyCatchRule); registry.registerRule(promiseDefaultFallbacksRule); + registry.registerRule(genericStatusEnvelopesRule); registry.registerRule(stringifiedUnknownErrorsRule); registry.registerRule(barrelDensityRule); registry.registerRule(passThroughWrappersRule); diff --git a/src/rules/generic-status-envelopes/README.md b/src/rules/generic-status-envelopes/README.md new file mode 100644 index 0000000..310df06 --- /dev/null +++ b/src/rules/generic-status-envelopes/README.md @@ -0,0 +1,50 @@ +# api.generic-status-envelopes + +Flags shallow boolean result wrappers like `{ ok: true, data }` and `{ success: false, error: ... }`. + +- **Family:** `api` +- **Severity:** `strong` +- **Scope:** `file` +- **Requires:** `file.ast` + +## How it works + +The rule looks for object literals that combine: + +- a boolean status field: `success` or `ok` +- a generic payload field such as `message`, `error`, `data`, `rows`, `present`, or `artifactId` + +This pattern is common in generated app/service glue that wraps every operation in a shallow result envelope. It is not always wrong, but repeated use often signals generic boolean-and-payload plumbing instead of richer domain-shaped APIs. + +To avoid obvious vendored noise, the rule skips very large bundled/generated files over `5000` logical lines. + +## Flagged examples + +```ts +return { success: false, error: "Unauthorized" }; +return { success: true, message: "Repository created successfully" }; +return { ok: true, rows }; +return { ok: true, artifactId: String(foundId) }; +``` + +## Usually ignored + +```ts +return { ok: true }; +return { success: true, user }; +return { error: "missing" }; +``` + +## Scoring + +Each generic status envelope adds `2` points. +The file total is capped at `8`. + +## Benchmark signal + +Full pinned benchmark against the exact `known-ai-vs-solid-oss` cohort: + +- Signal score: **0.93 / 1.00** +- Best separating metric: **findings / file (0.93)** +- Hit rate: **8/9 AI repos** vs **2/9 mature OSS repos** +- Full results: [experimental rule report](../../../reports/autoresearch-candidate-rule.md#apigeneric-status-envelopes) diff --git a/src/rules/generic-status-envelopes/index.ts b/src/rules/generic-status-envelopes/index.ts new file mode 100644 index 0000000..65a8111 --- /dev/null +++ b/src/rules/generic-status-envelopes/index.ts @@ -0,0 +1,138 @@ +import * as ts from "typescript"; +import type { RulePlugin } from "../../core/types"; +import { getLineNumber, unwrapExpression, walk } from "../../facts/ts-helpers"; +import { delta } from "../../rule-delta"; + +const MAX_LOGICAL_LINES = 5000; +const STATUS_KEYS = new Set(["success", "ok"]); +const GENERIC_PAYLOAD_KEYS = new Set(["message", "error", "data", "rows", "present", "artifactId"]); + +function isBooleanLiteral(expression: ts.Expression): boolean { + const unwrapped = unwrapExpression(expression); + return ( + unwrapped.kind === ts.SyntaxKind.TrueKeyword || unwrapped.kind === ts.SyntaxKind.FalseKeyword + ); +} + +function isGenericPayloadProperty(property: ts.PropertyAssignment): boolean { + return ts.isIdentifier(property.name) && GENERIC_PAYLOAD_KEYS.has(property.name.text); +} + +type MatchKind = + | "returned-generic-status-envelope" + | "json-generic-status-envelope" + | "assigned-generic-status-envelope"; + +type GenericStatusEnvelopeMatch = { + line: number; + kind: MatchKind; +}; + +function isGenericStatusEnvelope(node: ts.ObjectLiteralExpression): boolean { + let hasStatus = false; + let hasPayload = false; + + for (const property of node.properties) { + if (!ts.isPropertyAssignment(property) || !ts.isIdentifier(property.name)) { + continue; + } + + if (STATUS_KEYS.has(property.name.text) && isBooleanLiteral(property.initializer)) { + hasStatus = true; + } + + if (isGenericPayloadProperty(property)) { + hasPayload = true; + } + } + + return hasStatus && hasPayload; +} + +function summarizeEnvelope( + node: ts.ObjectLiteralExpression, + sourceFile: ts.SourceFile, +): GenericStatusEnvelopeMatch | null { + if (!isGenericStatusEnvelope(node)) { + return null; + } + + const parent = node.parent; + let kind: MatchKind = "assigned-generic-status-envelope"; + + if (ts.isReturnStatement(parent)) { + kind = "returned-generic-status-envelope"; + } else if ( + ts.isCallExpression(parent) && + ts.isPropertyAccessExpression(parent.expression) && + parent.expression.name.text === "json" + ) { + kind = "json-generic-status-envelope"; + } + + return { + line: getLineNumber(sourceFile, node.getStart(sourceFile)), + kind, + }; +} + +function findGenericStatusEnvelopes(sourceFile: ts.SourceFile): GenericStatusEnvelopeMatch[] { + const matches: GenericStatusEnvelopeMatch[] = []; + + walk(sourceFile, (node) => { + if (!ts.isObjectLiteralExpression(node)) { + return; + } + + const match = summarizeEnvelope(node, sourceFile); + if (match) { + matches.push(match); + } + }); + + return matches; +} + +export const genericStatusEnvelopesRule: RulePlugin = { + id: "api.generic-status-envelopes", + family: "api", + severity: "strong", + scope: "file", + requires: ["file.ast"], + delta: delta.byLocations(), + supports(context) { + return context.scope === "file" && Boolean(context.file); + }, + evaluate(context) { + if (context.file!.logicalLineCount > MAX_LOGICAL_LINES) { + return []; + } + + const sourceFile = context.runtime.store.getFileFact( + context.file!.path, + "file.ast", + ); + if (!sourceFile) { + return []; + } + + const matches = findGenericStatusEnvelopes(sourceFile); + if (matches.length === 0) { + return []; + } + + return [ + { + ruleId: "api.generic-status-envelopes", + family: "api", + severity: "strong", + scope: "file", + path: context.file!.path, + message: `Found ${matches.length} generic status envelope${matches.length === 1 ? "" : "s"} that package boolean success into shallow payload wrappers`, + evidence: matches.map((match) => `line ${match.line}: ${match.kind}`), + score: Math.min(8, matches.length * 2), + locations: matches.map((match) => ({ path: context.file!.path, line: match.line })), + }, + ]; + }, +}; diff --git a/tests/generic-status-envelopes.test.ts b/tests/generic-status-envelopes.test.ts new file mode 100644 index 0000000..3e3ea5e --- /dev/null +++ b/tests/generic-status-envelopes.test.ts @@ -0,0 +1,108 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { DEFAULT_CONFIG } from "../src/config"; +import { analyzeRepository } from "../src/core/engine"; +import { Registry } from "../src/core/registry"; +import { createDefaultRegistry } from "../src/default-registry"; +import { genericStatusEnvelopesRule } from "../src/rules/generic-status-envelopes"; + +const tempDirs: string[] = []; + +afterEach(async () => { + await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))); +}); + +async function writeRepoFiles(rootDir: string, files: Record): Promise { + for (const [relativePath, content] of Object.entries(files)) { + const absolutePath = path.join(rootDir, relativePath); + await mkdir(path.dirname(absolutePath), { recursive: true }); + await writeFile(absolutePath, content); + } +} + +async function createTempRepo(files: Record): Promise { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "slop-scan-generic-status-envelopes-")); + tempDirs.push(rootDir); + await writeRepoFiles(rootDir, files); + return rootDir; +} + +function createCandidateRegistry(): Registry { + const baseRegistry = createDefaultRegistry(); + const registry = new Registry(); + + for (const language of baseRegistry.getLanguages()) { + registry.registerLanguage(language); + } + + for (const provider of baseRegistry.getFactProviders()) { + registry.registerFactProvider(provider); + } + + registry.registerRule(genericStatusEnvelopesRule); + return registry; +} + +describe("generic-status-envelopes rule", () => { + test("flags shallow boolean-and-payload result wrappers", async () => { + const rootDir = await createTempRepo({ + "src/slop.ts": [ + "export function createResult() {", + " return { success: false, error: 'Unauthorized' };", + "}", + "", + "export function listRows(rows: string[]) {", + " return { ok: false, rows };", + "}", + "", + "export function respond(id: string) {", + " return Response.json({ ok: true, artifactId: id });", + "}", + ].join("\n"), + "src/legit.ts": [ + "export function createResult(user: object) {", + " return { success: true, user };", + "}", + "", + "export function ping() {", + " return { ok: true };", + "}", + ].join("\n"), + }); + + const result = await analyzeRepository(rootDir, DEFAULT_CONFIG, createCandidateRegistry()); + const finding = result.findings.find( + (nextFinding) => nextFinding.ruleId === "api.generic-status-envelopes", + ); + + expect(finding).toBeDefined(); + expect(finding?.path).toBe("src/slop.ts"); + expect(finding?.evidence).toEqual([ + "line 2: returned-generic-status-envelope", + "line 10: json-generic-status-envelope", + ]); + expect(finding?.locations).toEqual([ + { path: "src/slop.ts", line: 2 }, + { path: "src/slop.ts", line: 10 }, + ]); + expect(result.findings).toHaveLength(1); + }); + + test("ignores giant bundled files that would otherwise create vendor noise", async () => { + const hugeFile = [ + ...Array.from({ length: 5001 }, (_, index) => `export const filler${index} = ${index};`), + "export const response = { ok: true, rows: [] };", + "", + ].join("\n"); + + const rootDir = await createTempRepo({ + "src/bundle.ts": hugeFile, + }); + + const result = await analyzeRepository(rootDir, DEFAULT_CONFIG, createCandidateRegistry()); + + expect(result.findings).toHaveLength(0); + }); +}); diff --git a/tests/heuristics.test.ts b/tests/heuristics.test.ts index df126b7..5abd41a 100644 --- a/tests/heuristics.test.ts +++ b/tests/heuristics.test.ts @@ -44,6 +44,10 @@ describe("heuristic rule pack", () => { " }", "}", "", + "export function createEnvelope() {", + " return { success: false, error: 'Unauthorized' };", + "}", + "", "export function getErrorMessage(error: unknown) {", " const message = error instanceof Error ? error.message : String(error);", " return { message };", @@ -85,6 +89,7 @@ describe("heuristic rule pack", () => { expect(ruleIds.has("comments.placeholder-comments")).toBe(true); expect(ruleIds.has("defensive.error-obscuring")).toBe(true); expect(ruleIds.has("defensive.promise-default-fallbacks")).toBe(true); + expect(ruleIds.has("api.generic-status-envelopes")).toBe(true); expect(ruleIds.has("defensive.stringified-unknown-errors")).toBe(true); expect(ruleIds.has("defensive.async-noise")).toBe(true); expect(ruleIds.has("structure.pass-through-wrappers")).toBe(true);