Skip to content

Commit 294e853

Browse files
committed
fix(konsistent): match variable-depth ** alongside {placeholder} and excludeFiles globs
Path matching required the pattern and matched path to have the same number of segments when extracting placeholder values. This made any `**` segment collapse to matching exactly one directory level whenever the pattern also contained a `{placeholder}`, silently dropping candidates at every other depth (#56). `excludeFiles` entries hit the same equal-segment-count check, so a `**/`-prefixed entry (as shown in docs/reference/configuration.md) never matched and the file stayed in scope (#57). Path segment matching now backtracks through candidate consumption counts for `**`, so it can reconcile a variable-depth wildcard with placeholder extraction and with plain excludeFiles glob patterns.
1 parent c199e74 commit 294e853

6 files changed

Lines changed: 262 additions & 24 deletions

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"@konsistent/convention": patch
3+
"konsistent": patch
4+
---
5+
6+
fix(konsistent): match variable-depth `**` alongside `{placeholder}` segments and glob-style `excludeFiles` patterns
7+
8+
`**` in a `paths` pattern next to a `{placeholder}` segment previously only matched a single intermediate directory level instead of every depth, because placeholder extraction required the pattern and path to have the same number of segments. `excludeFiles` entries prefixed with `**/` were silently ignored for the same reason. Path matching now backtracks through variable-depth `**` segments when extracting placeholders and when matching `excludeFiles` glob patterns, so both now behave as documented.

e2e/fixtures/exclude-files/konsistent.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
"must": [
1717
{
1818
"for": { "files": "{testFile}.spec.ts" },
19-
"excludeFiles": ["plugins/auth/helpers.spec.ts"],
19+
"excludeFiles": ["**/helpers.spec.ts"],
2020
"must": {
2121
"export": ["describe"]
2222
}

packages/konsistent/src/core/path-matcher.test.ts

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import { describe, expect, it } from "vitest";
22
import type { FileSystem } from "./filesystem.js";
3-
import { hasPlaceholders, matchPaths, patternToGlob } from "./path-matcher.js";
3+
import {
4+
hasPlaceholders,
5+
matchesPathPattern,
6+
matchPaths,
7+
patternToGlob,
8+
} from "./path-matcher.js";
49

510
function createMockFileSystem(opts: {
611
globResults?: Map<string, string[]>;
@@ -376,4 +381,111 @@ describe("matchPaths", () => {
376381
});
377382
expect(results).toHaveLength(2);
378383
});
384+
385+
it("plain ** without a placeholder matches every depth (baseline, no regression)", async () => {
386+
const fs = createMockFileSystem({
387+
globResults: new Map([
388+
["deep/**/*.ts", ["deep/top.ts", "deep/a/mid.ts", "deep/a/b/leaf.ts"]],
389+
]),
390+
});
391+
const results = await matchPaths({
392+
patterns: ["deep/**/*.ts"],
393+
fileSystem: fs,
394+
});
395+
expect(results).toHaveLength(3);
396+
expect(results.every((r) => Object.keys(r.placeholders).length === 0)).toBe(
397+
true
398+
);
399+
});
400+
401+
it("`**` adjacent to a `{placeholder}` matches every depth (#56 regression)", async () => {
402+
const fs = createMockFileSystem({
403+
globResults: new Map([
404+
["deep/**/*.ts", ["deep/top.ts", "deep/a/mid.ts", "deep/a/b/leaf.ts"]],
405+
]),
406+
});
407+
const results = await matchPaths({
408+
patterns: ["deep/**/{name}.ts"],
409+
fileSystem: fs,
410+
});
411+
expect(results).toHaveLength(3);
412+
413+
const byPath = new Map(
414+
results.map((r) => [r.path, r.placeholders.name.toString()])
415+
);
416+
expect(byPath.get("deep/top.ts")).toBe("top");
417+
expect(byPath.get("deep/a/mid.ts")).toBe("mid");
418+
expect(byPath.get("deep/a/b/leaf.ts")).toBe("leaf");
419+
});
420+
421+
it("`**` before a placeholder still enforces multi-placeholder consistency", async () => {
422+
const fs = createMockFileSystem({
423+
globResults: new Map([
424+
["deep/**/*/*.ts", ["deep/a/b/auth/auth.ts", "deep/a/b/auth/other.ts"]],
425+
]),
426+
});
427+
const results = await matchPaths({
428+
patterns: ["deep/**/{name}/{name}.ts"],
429+
fileSystem: fs,
430+
});
431+
expect(results).toHaveLength(1);
432+
expect(results[0].path).toBe("deep/a/b/auth/auth.ts");
433+
expect(results[0].placeholders.name.toString()).toBe("auth");
434+
});
435+
});
436+
437+
describe("matchesPathPattern", () => {
438+
it("matches a **/ prefixed pattern against a nested file", () => {
439+
expect(
440+
matchesPathPattern({
441+
pattern: "**/__test-env-tdd-state.ts",
442+
filePath: "foo/__test-env-tdd-state.ts",
443+
})
444+
).toBe(true);
445+
});
446+
447+
it("matches a **/ prefixed pattern regardless of nesting depth", () => {
448+
expect(
449+
matchesPathPattern({
450+
pattern: "**/__test-env-tdd-state.ts",
451+
filePath: "foo/bar/baz/__test-env-tdd-state.ts",
452+
})
453+
).toBe(true);
454+
});
455+
456+
it("matches a **/ prefixed pattern at the root (zero intermediate segments)", () => {
457+
expect(
458+
matchesPathPattern({
459+
pattern: "**/*.test.ts",
460+
filePath: "foo.test.ts",
461+
})
462+
).toBe(true);
463+
});
464+
465+
it("matches a trailing ** pattern against any nested file", () => {
466+
expect(
467+
matchesPathPattern({
468+
pattern: "src/internal/**",
469+
filePath: "src/internal/deep/file.ts",
470+
})
471+
).toBe(true);
472+
});
473+
474+
it("does not match unrelated paths", () => {
475+
expect(
476+
matchesPathPattern({
477+
pattern: "**/__test-env-tdd-state.ts",
478+
filePath: "foo/other.ts",
479+
})
480+
).toBe(false);
481+
});
482+
483+
it("still matches exact literal paths without wildcards", () => {
484+
expect(
485+
matchesPathPattern({
486+
pattern: "src/internal.ts",
487+
filePath: "src/internal.ts",
488+
})
489+
).toBe(true);
490+
});
379491
});

packages/konsistent/src/core/path-matcher.ts

Lines changed: 113 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -179,41 +179,133 @@ function mergeExtracted(opts: {
179179
return true;
180180
}
181181

182+
/*
183+
* Matches pattern segments against path segments, extracting placeholder
184+
* values along the way. Unlike a simple index-by-index walk, this supports a
185+
* `**` pattern segment consuming a variable number of path segments (zero or
186+
* more), backtracking through candidate consumption counts until the rest of
187+
* the pattern also matches. This is what lets `**` coexist with `{placeholder}`
188+
* segments elsewhere in the same pattern — e.g. `deep/**\/{name}.ts` matching
189+
* `deep/top.ts`, `deep/a/mid.ts`, and `deep/a/b/leaf.ts` alike.
190+
*/
191+
function matchPatternSegments(opts: {
192+
patternSegments: string[];
193+
pathSegments: string[];
194+
patternIndex: number;
195+
pathIndex: number;
196+
}): ExtractedValues | null {
197+
const { patternSegments, pathSegments, patternIndex, pathIndex } = opts;
198+
199+
if (patternIndex === patternSegments.length) {
200+
return pathIndex === pathSegments.length
201+
? { values: {}, constraints: {} }
202+
: null;
203+
}
204+
205+
const segment = patternSegments[patternIndex];
206+
207+
if (segment === "**") {
208+
for (
209+
let consumedUpTo = pathIndex;
210+
consumedUpTo <= pathSegments.length;
211+
consumedUpTo++
212+
) {
213+
const rest = matchPatternSegments({
214+
patternSegments,
215+
pathSegments,
216+
patternIndex: patternIndex + 1,
217+
pathIndex: consumedUpTo,
218+
});
219+
if (rest !== null) {
220+
return rest;
221+
}
222+
}
223+
return null;
224+
}
225+
226+
if (pathIndex >= pathSegments.length) {
227+
return null;
228+
}
229+
230+
const segmentResult = extractValueFromSegment({
231+
patternSegment: segment,
232+
pathSegment: pathSegments[pathIndex],
233+
});
234+
if (segmentResult === null) {
235+
return null;
236+
}
237+
238+
const rest = matchPatternSegments({
239+
patternSegments,
240+
pathSegments,
241+
patternIndex: patternIndex + 1,
242+
pathIndex: pathIndex + 1,
243+
});
244+
if (rest === null) {
245+
return null;
246+
}
247+
248+
const values = { ...segmentResult.values };
249+
if (!mergeExtracted({ existing: values, incoming: rest.values })) {
250+
return null;
251+
}
252+
253+
return {
254+
values,
255+
constraints: { ...segmentResult.constraints, ...rest.constraints },
256+
};
257+
}
258+
182259
function tryExtractPlaceholders(opts: {
183260
pattern: string;
184261
pathSegments: string[];
185262
}): Record<string, string> | null {
186263
const { pattern, pathSegments } = opts;
187264
const patternSegments = pattern.split("/");
188-
if (patternSegments.length !== pathSegments.length) {
189-
return null;
190-
}
191265

192-
const extracted: Record<string, string> = {};
193-
const allConstraints: Record<string, string> = {};
194-
for (let i = 0; i < patternSegments.length; i++) {
195-
const segmentResult = extractValueFromSegment({
196-
patternSegment: patternSegments[i],
197-
pathSegment: pathSegments[i],
198-
});
199-
if (segmentResult === null) {
200-
return null;
201-
}
202-
if (
203-
!mergeExtracted({ existing: extracted, incoming: segmentResult.values })
204-
) {
205-
return null;
206-
}
207-
Object.assign(allConstraints, segmentResult.constraints);
266+
const matched = matchPatternSegments({
267+
patternSegments,
268+
pathSegments,
269+
patternIndex: 0,
270+
pathIndex: 0,
271+
});
272+
if (matched === null) {
273+
return null;
208274
}
209275

210276
if (
211-
!satisfiesConstraints({ values: extracted, constraints: allConstraints })
277+
!satisfiesConstraints({
278+
values: matched.values,
279+
constraints: matched.constraints,
280+
})
212281
) {
213282
return null;
214283
}
215284

216-
return extracted;
285+
return matched.values;
286+
}
287+
288+
/*
289+
* Tests whether a single file path matches a path pattern, supporting the
290+
* same glob syntax as `paths` (`*`, `?`, and variable-depth `**`), without
291+
* extracting placeholder values. Used for `excludeFiles` patterns, which are
292+
* plain glob patterns rather than placeholder patterns.
293+
*/
294+
export function matchesPathPattern(opts: {
295+
pattern: string;
296+
filePath: string;
297+
}): boolean {
298+
const { pattern, filePath } = opts;
299+
const patternSegments = pattern.split("/");
300+
const pathSegments = filePath.split("/");
301+
return (
302+
matchPatternSegments({
303+
patternSegments,
304+
pathSegments,
305+
patternIndex: 0,
306+
pathIndex: 0,
307+
}) !== null
308+
);
217309
}
218310

219311
function toPlaceholderMap(opts: {

packages/konsistent/src/core/runner.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -974,6 +974,29 @@ describe("excludeFiles", () => {
974974
expect(diagnostics).toEqual([]);
975975
});
976976

977+
it("convention-level excludeFiles with a **/ prefixed pattern skips matching file (#57 regression)", async () => {
978+
const config: ConfigV1 = {
979+
version: "v1",
980+
conventions: [
981+
{
982+
name: "source-files",
983+
paths: "foo/**/*.ts",
984+
excludeFiles: ["**/__test-env-tdd-state.ts"],
985+
must: { haveType: "file" },
986+
},
987+
],
988+
};
989+
const fs = createMockFileSystem({
990+
globResults: new Map([
991+
["foo/**/*.ts", ["foo/__test-env-tdd-state.ts", "foo/utils.ts"]],
992+
]),
993+
directories: new Set(["foo/__test-env-tdd-state.ts"]),
994+
files: new Set(["foo/utils.ts"]),
995+
});
996+
const { diagnostics } = await run({ config, fileSystem: fs });
997+
expect(diagnostics).toEqual([]);
998+
});
999+
9771000
it("convention-level excludeFiles does not skip non-matching file", async () => {
9781001
const config: ConfigV1 = {
9791002
version: "v1",

packages/konsistent/src/core/runner.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import type { Diagnostic, DiagnosticSeverity } from "./diagnostics.js";
3232
import { createDiagnostic } from "./diagnostics.js";
3333
import type { FileSystem } from "./filesystem.js";
3434
import type { MatchedPath } from "./path-matcher.js";
35-
import { matchPaths } from "./path-matcher.js";
35+
import { matchesPathPattern, matchPaths } from "./path-matcher.js";
3636
import { PlaceholderValue } from "./placeholder.js";
3737
import {
3838
parsePlaceholderConstraint,
@@ -191,6 +191,9 @@ function isFileExcluded(opts: {
191191
if (filePath === resolved || basename(filePath) === resolved) {
192192
return true;
193193
}
194+
if (matchesPathPattern({ pattern: resolved, filePath })) {
195+
return true;
196+
}
194197
}
195198
return false;
196199
}

0 commit comments

Comments
 (0)