Skip to content

Commit 99b453e

Browse files
sergeyshmakovclaude
andcommitted
fix: .match() returns 'covers' (not 'parent') for deep template vs concrete
When `this` contained DEEP_WILDCARD, matchesPrefix could succeed against a longer `other` by collapsing `**`, and the old code interpreted that as "this is a literal prefix of other" → "parent". It also returned `null` for `a.**.b vs a.b` because patternMatches required equal lengths. Two fixes: - patternMatches now supports `**` collapsing zero-or-more segments (was equal-length-only). This is the relation the docs describe as "covers". - match() checks wildcard coverage BEFORE literal parent/child, and gates the parent/child branches on the shorter side being wildcard-free. A wildcard prefix is not a literal prefix. Tests: - deep template vs longer concrete → covers / covered-by (was parent/child) - deep template vs shorter concrete (** collapses to zero) → covers (was null) - single-* template vs equal-length concrete unchanged Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c29b397 commit 99b453e

3 files changed

Lines changed: 87 additions & 11 deletions

File tree

src/impl/base-path-impl.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type {
77
TemplatePath,
88
} from "../types.js";
99
import {
10+
hasWildcardSegment,
1011
matchesPrefix,
1112
patternMatches,
1213
resolveSegments,
@@ -56,19 +57,30 @@ export abstract class AbstractPathImpl<T = unknown, V = unknown> {
5657
match(other: ResolvablePath<T>): MatchResult | null {
5758
const otherSegs = resolveSegments(other);
5859
if (segmentsEqual(this.segments, otherSegs)) return { relation: "equals" };
60+
61+
// Check wildcard coverage BEFORE literal prefix. patternMatches handles
62+
// `**` collapsing, so a deep template can fully cover a concrete path of
63+
// any length — that's a "covers", not "parent". Order matters: a literal
64+
// prefix check that's wildcard-aware (via matchesPrefix) would otherwise
65+
// misclassify "a.**.b covers a.x.y.b" as "parent".
66+
if (patternMatches(this.segments, otherSegs)) return { relation: "covers" };
67+
if (patternMatches(otherSegs, this.segments))
68+
return { relation: "covered-by" };
69+
70+
// Literal parent/child: the shorter side must be wildcard-free, otherwise
71+
// it's not a literal prefix.
5972
if (
73+
!hasWildcardSegment(otherSegs) &&
6074
matchesPrefix(this.segments, otherSegs) &&
6175
this.segments.length > otherSegs.length
6276
)
6377
return { relation: "child" };
6478
if (
79+
!hasWildcardSegment(this.segments) &&
6580
matchesPrefix(otherSegs, this.segments) &&
6681
otherSegs.length > this.segments.length
6782
)
6883
return { relation: "parent" };
69-
if (patternMatches(this.segments, otherSegs)) return { relation: "covers" };
70-
if (patternMatches(otherSegs, this.segments))
71-
return { relation: "covered-by" };
7284
return null;
7385
}
7486

src/tests/interactions.spec.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,40 @@ describe("Path interactions", () => {
120120
expect(a.match(b)?.relation).toBe("equals");
121121
});
122122

123+
it("deep template returns 'covers' against a longer concrete (** collapse)", () => {
124+
interface Node {
125+
value: string;
126+
children: Node[];
127+
}
128+
interface Root {
129+
tree: Node;
130+
}
131+
const deep = path((p: Root) => p.tree).deep((n) => n.value);
132+
const deeper = path((p: Root) => p.tree.children[0].value);
133+
// Pre-fix bug: returned "parent" because matchesPrefix(other, this)
134+
// was true via ** expansion AND otherSegs.length > this.segments.length.
135+
expect(deep.match(deeper)?.relation).toBe("covers");
136+
expect(deeper.match(deep)?.relation).toBe("covered-by");
137+
});
138+
139+
it("deep template returns 'covers' against a shorter concrete (** collapses to zero)", () => {
140+
interface Data {
141+
a: { b: string };
142+
}
143+
const deep = path((p: Data) => p.a).deep((n) => n.b);
144+
const shorter = path((p: Data) => p.a.b);
145+
// Pre-fix bug: returned null because patternMatches required equal lengths.
146+
expect(deep.match(shorter)?.relation).toBe("covers");
147+
expect(shorter.match(deep)?.relation).toBe("covered-by");
148+
});
149+
150+
it("single-* template returns 'covers' for an equal-length concrete (unchanged behaviour)", () => {
151+
const tmpl = path((p: R) => p.items).each((i) => i.name);
152+
const concrete = path((p: R) => p.items[0].name);
153+
expect(tmpl.match(concrete)?.relation).toBe("covers");
154+
expect(concrete.match(tmpl)?.relation).toBe("covered-by");
155+
});
156+
123157
it("immutability: does not mutate original path", () => {
124158
const concrete = path((p: R) => p.items[0].name);
125159
const template = path((p: R) => p.items).each(

src/utils.ts

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -85,18 +85,48 @@ export function matchesPrefix(
8585
return p === prefix.length;
8686
}
8787

88+
/**
89+
* Returns `true` iff `pattern` matches `concrete` exactly when wildcards
90+
* are expanded:
91+
* - `WILDCARD` (`*`) consumes exactly one segment.
92+
* - `DEEP_WILDCARD` (`**`) consumes zero or more segments.
93+
* - all other segments must be `===` to the corresponding concrete segment.
94+
*
95+
* Lengths need not match: a single `**` lets the pattern collapse to a
96+
* shorter concrete or stretch to a longer one. This is the "covers"
97+
* relation used by `.match()` and is broader than `matchesPrefix`, which
98+
* only requires the pattern to match a leading slice.
99+
*/
88100
export function patternMatches(
89101
pattern: readonly Segment[],
90102
concrete: readonly Segment[],
91103
): boolean {
92-
if (pattern.length !== concrete.length) return false;
93-
for (let i = 0; i < pattern.length; i++) {
94-
if (
95-
pattern[i] !== WILDCARD &&
96-
pattern[i] !== DEEP_WILDCARD &&
97-
pattern[i] !== concrete[i]
98-
)
104+
function walk(pi: number, ci: number): boolean {
105+
if (pi === pattern.length) return ci === concrete.length;
106+
const seg = pattern[pi];
107+
if (seg === DEEP_WILDCARD) {
108+
for (let skip = 0; ci + skip <= concrete.length; skip++) {
109+
if (walk(pi + 1, ci + skip)) return true;
110+
}
99111
return false;
112+
}
113+
if (ci === concrete.length) return false;
114+
if (seg === WILDCARD) return walk(pi + 1, ci + 1);
115+
if (seg !== concrete[ci]) return false;
116+
return walk(pi + 1, ci + 1);
117+
}
118+
return walk(0, 0);
119+
}
120+
121+
function hasWildcardSegment(segments: readonly Segment[]): boolean {
122+
for (const s of segments) {
123+
if (s === WILDCARD || s === DEEP_WILDCARD) return true;
100124
}
101-
return true;
125+
return false;
102126
}
127+
128+
/**
129+
* Returns `true` iff `segments` contains a wildcard sentinel
130+
* (`WILDCARD` or `DEEP_WILDCARD`).
131+
*/
132+
export { hasWildcardSegment };

0 commit comments

Comments
 (0)