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
109 changes: 109 additions & 0 deletions src/drift/__tests__/path-false-positives.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import { describe, it, expect } from "vitest";
import { execFileSync } from "node:child_process";
import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { extractClaims } from "../claims.js";
import { checkPaths } from "../checkers/path.js";

/**
* Cases from issue #143 (MISSING_PATH false positives) plus the
* version-shaped-ref case found on 0.8.x. Each scaffold is its own git
* repository so `git check-ignore` resolves against the fixture rather than
* whatever repository the temp directory happens to sit inside.
*/
function runScaffold(markdown: string, extraFiles: Record<string, string> = {}) {
const projectRoot = mkdtempSync(join(tmpdir(), "mex-drift-"));
execFileSync("git", ["init", "-q"], { cwd: projectRoot });

const scaffoldRoot = join(projectRoot, ".mex");
mkdirSync(scaffoldRoot, { recursive: true });

for (const [relative, contents] of Object.entries(extraFiles)) {
const target = join(projectRoot, relative);
mkdirSync(join(target, ".."), { recursive: true });
writeFileSync(target, contents);
}

const docPath = join(scaffoldRoot, "ROUTER.md");
writeFileSync(docPath, markdown);

const claims = extractClaims(docPath, ".mex/ROUTER.md");
return checkPaths(claims, projectRoot, scaffoldRoot);
}

const missingPaths = (markdown: string, extraFiles?: Record<string, string>) =>
runScaffold(markdown, extraFiles)
.filter((issue) => issue.code === "MISSING_PATH")
.map((issue) => issue.claim?.value);

describe("MISSING_PATH false positives", () => {
it("does not claim a file the document says was deleted (#143 defect 1)", () => {
expect(
missingPaths("# Changelog\n\n- Deleted orphaned files: `check_commands.js`\n")
).toEqual([]);
});

it("does not claim a bare trailing directory (#143 defect 2)", () => {
expect(missingPaths("# Layout\n\n- Screenshots live in `screenshots/`\n")).toEqual([]);
});

it("does not claim a numeric delta (#143 defect 2)", () => {
expect(missingPaths("# Rating\n\n- Net movement was `+12/-5` this week\n")).toEqual([]);
});

it("finds a file inside a dot-directory (#143 defect 3)", () => {
expect(
missingPaths("# CI\n\n- The workflow is `deploy.yml`\n", {
".github/workflows/deploy.yml": "name: deploy\n",
})
).toEqual([]);
});

// Not part of #143. A trailing version number reads as a file extension to
// the guard above, so `release/2.1.0` never reaches the unrooted check and is
// reported as a missing path. Tracked separately; unskip with the fix.
it.skip("does not claim a version-shaped branch name", () => {
expect(
missingPaths("# Release\n\n- Work landed on `release/2.1.0` and `python/3.11`\n")
).toEqual([]);
});

it("does not claim a pseudo-path pair such as overall/overall", () => {
expect(missingPaths("# Stats\n\n- Rating shown as `overall/overall`\n")).toEqual([]);
});

it("does not claim middot-separated pseudo-paths", () => {
expect(missingPaths("# Modes\n\n- Queues: `8ball/pro` · `9ball/pro`\n")).toEqual([]);
});

it("does not claim a glob", () => {
expect(
missingPaths("# Routes\n\n- Admin routes live in `web/routes/admin/*.js`\n")
).toEqual([]);
});

// Markdown wraps prose freely, so negation has to be scoped to the enclosing
// paragraph. Scoping it to the raw line missed both of these.
it("honours negation when the reference is on a continuation line", () => {
expect(
missingPaths(
"# Cleanup\n\n- Deleted orphaned files during the sweep:\n `check_commands.js`\n"
)
).toEqual([]);
});

it("honours negation across a wrapped sentence", () => {
expect(
missingPaths("# Cleanup\n\n- The helper was removed, so\n `legacy_client.py` is gone\n")
).toEqual([]);
});

it("still checks a directory reference rooted at a directory that exists", () => {
// The trailing-separator change must not turn every documented directory
// into prose: `.mex/local/` roots at a real directory and stays a claim.
expect(missingPaths("# State\n\n- Local state lives in `.mex/local/`\n")).toEqual([
".mex/local/",
]);
});
});
27 changes: 20 additions & 7 deletions src/drift/checkers/path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,20 +75,28 @@ export function checkPaths(
}

/**
* True when a slash-separated value names no file type, does not end in a
* directory separator, and its first segment does not exist at either root.
* API routes and placeholders take this shape; a real relative path almost
* always starts from a directory that is actually there.
* True when a value names no file type and its first segment does not exist at
* either root. API routes and placeholders take this shape; a real relative
* path almost always starts from a directory that is actually there.
*
* A trailing separator marks a directory reference rather than disqualifying
* the value. `screenshots/` in a layout note is prose when nothing by that name
* is there, while `.mex/local/` still roots at a directory that exists and so
* remains a claim the checker is entitled to test.
*/
function isUnrootedReference(
value: string,
projectRoot: string,
scaffoldRoot: string
): boolean {
if (!value.includes("/") || value.startsWith("/") || value.endsWith("/")) return false;
if (/\.[A-Za-z0-9]+$/.test(value)) return false;
if (value.startsWith("/")) return false;

const first = value.split("/")[0];
const trimmed = value.replace(/\/+$/, "");
const isDirectoryRef = trimmed !== value;
if (!trimmed.includes("/") && !isDirectoryRef) return false;
if (/\.[A-Za-z0-9]+$/.test(trimmed)) return false;

const first = trimmed.split("/")[0];
if (!first || first.startsWith("@") || first === "." || first === "..") return false;

if (existsSync(resolve(projectRoot, first))) return false;
Expand Down Expand Up @@ -234,10 +242,13 @@ function pathExists(

// Bare filenames: search recursively — the file may exist in a subdirectory
if (!value.includes("/")) {
// `dot: true` so a file that lives in a hidden directory is found:
// a backticked `deploy.yml` normally sits in `.github/workflows/`.
const matches = globSync(`**/${value}`, {
cwd: projectRoot,
ignore: ["node_modules/**", ".mex/**", "dist/**", ".git/**"],
maxDepth: 5,
dot: true,
});
if (matches.length > 0) return true;

Expand All @@ -249,6 +260,7 @@ function pathExists(
cwd: scaffoldRoot,
ignore: ["node_modules/**"],
maxDepth: 5,
dot: true,
});
if (inScaffold.length > 0) return true;
}
Expand All @@ -265,6 +277,7 @@ function pathExists(
cwd: projectRoot,
ignore: ["node_modules/**", "dist/**", ".git/**"],
maxDepth: 6,
dot: true,
});
if (matches.length > 0) return true;
}
Expand Down
70 changes: 67 additions & 3 deletions src/drift/claims.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import { readFileSync } from "node:fs";
import { visit } from "unist-util-visit";
import { parseMarkdown, getHeadingAtLine, isNegatedSection } from "../markdown.js";
import {
parseMarkdown,
getHeadingAtLine,
isNegatedSection,
isNegatedText,
} from "../markdown.js";
import type { Claim } from "../types.js";
import type { Root, Code, InlineCode, ListItem, Strong, Text } from "mdast";
import type { Root, Code, Content, InlineCode, ListItem, Strong, Text } from "mdast";

const KNOWN_EXTENSIONS = /\.(ts|js|tsx|jsx|py|go|rs|rb|java|json|yaml|yml|toml|md|css|scss|html|vue|svelte|sh)$/;
const COMMAND_PREFIXES = /^(npm|yarn|pnpm|bun|make|cargo|python|pip|go|node|npx|tsx)\s/;
Expand Down Expand Up @@ -95,6 +100,14 @@ export function extractClaims(filePath: string, source: string): Claim[] {
const tree = parseMarkdown(content);
const claims: Claim[] = [];

// Negation is not always heading-scoped: a bullet that records a deletion
// sits under an ordinary heading. Nor is a raw line the right unit, because
// markdown wraps prose freely and the word and the reference it governs
// routinely land on different lines. The sentence is the unit that actually
// governs: a paragraph describing an unrelated removal elsewhere must not
// silence every path it happens to mention.
const negatedByContext = collectNegatedReferences(tree);

// A stack doc declares a dependency as `- **name** — description`, so only
// bold that opens a list item is a declaration. Collected up front because
// both the inline-code pass and the bold pass need to know which nodes are
Expand All @@ -121,7 +134,7 @@ export function extractClaims(filePath: string, source: string): Claim[] {
visit(tree, "inlineCode", (node: InlineCode) => {
const line = node.position?.start.line ?? 0;
const heading = getHeadingAtLine(tree, line);
const negated = isNegatedSection(heading);
const negated = isNegatedSection(heading) || negatedByContext.has(node);

// A package named inside a dependency entry is not a file. `youtubei.js`
// ends in a known extension, so without this it was reported as a
Expand Down Expand Up @@ -249,6 +262,57 @@ export function extractClaims(filePath: string, source: string): Claim[] {
return claims;
}

/**
* Inline-code nodes whose own sentence describes them as deleted or absent.
*
* The paragraph text is rebuilt in reading order while recording where each
* reference sits inside it, so a cue can be attributed to the sentence that
* contains it. A sentence ends at `.`, `!` or `?` followed by whitespace,
* which leaves version numbers such as `0.8.1` intact and keeps a colon inside
* its sentence -- "Deleted orphaned files: `check_commands.js`" is one clause.
*/
function collectNegatedReferences(tree: Root): Set<InlineCode> {
const negated = new Set<InlineCode>();

visit(tree, "paragraph", (paragraph) => {
let text = "";
const positions: Array<{ node: InlineCode; at: number }> = [];

const walk = (node: Content): void => {
if (node.type === "inlineCode") {
positions.push({ node, at: text.length });
text += node.value;
return;
}
if ("value" in node && typeof node.value === "string") {
text += node.value;
return;
}
if ("children" in node) (node.children as Content[]).forEach(walk);
};
(paragraph.children as Content[]).forEach(walk);
if (positions.length === 0) return;

const spans: Array<[number, number]> = [];
let start = 0;
const boundary = /[.!?](?=\s|$)/g;
let match: RegExpExecArray | null;
while ((match = boundary.exec(text)) !== null) {
spans.push([start, match.index + 1]);
start = match.index + 1;
}
spans.push([start, text.length]);

for (const { node, at } of positions) {
const span = spans.find(([from, to]) => at >= from && at < to);
const sentence = span ? text.slice(span[0], span[1]) : text;
if (isNegatedText(sentence)) negated.add(node);
}
});

return negated;
}

function getStrongText(node: Strong): string | null {
const text = node.children
.filter((c): c is Text => c.type === "text")
Expand Down
20 changes: 20 additions & 0 deletions src/markdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,26 @@ export function getTextContent(node: Content | Root): string {
return "";
}

/**
* Words that mark surrounding prose as describing something that is gone or
* deliberately absent. A heading is not always the right scope: a changelog
* bullet naming a deleted file sits under an ordinary heading yet still refers
* to something that should not exist on disk.
*/
const NEGATED_TEXT =
/\b(?:deleted|removed|dropped|retired|orphaned|unreferenced|absent|no longer|does not exist|never created)\b/i;

/**
* True when this passage describes a path as deleted or deliberately absent.
* Callers pass a whole block rather than one line: markdown wraps prose freely,
* so the word and the reference it governs routinely sit on different lines of
* the same sentence.
*/
export function isNegatedText(text: string | undefined): boolean {
if (!text) return false;
return NEGATED_TEXT.test(text);
}

/** Check if a heading or its ancestors suggest negation */
export function isNegatedSection(heading: string | null): boolean {
if (!heading) return false;
Expand Down
Loading