Skip to content

Commit 98d229d

Browse files
samchonclaude
andcommitted
fix(paths): commit resolution to the best pattern and hoist the walk closure
Self-review follow-ups on the same rewriter, both aligning it further with its oracles: - resolveSource fell through to weaker patterns when the best-precedence match's targets named no program source, rewriting the import at a module tsc never resolved. tsc's tryLoadModuleUsingPaths commits to the single matched pattern -- when its substitutions fail, paths resolution fails -- so the rewriter now stops there and leaves the specifier untouched. - visitModuleSpecifiers handed ForEachChild a fresh closure at every node, one allocation per AST node per apply pass across the whole program. The recursion now runs through one closure per walk, the same discipline linthost's walkDescendants documents for exactly this reason. Refs #310. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AivFfPoUxoU26Dko5Jbm21
1 parent ef53992 commit 98d229d

3 files changed

Lines changed: 124 additions & 64 deletions

File tree

packages/paths/driver/paths.go

Lines changed: 44 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -122,40 +122,49 @@ func (r *rewriter) apply(file *shimast.SourceFile) {
122122
// visit for every string-literal module specifier it finds. Covered nodes
123123
// include import/export declarations, require() calls, dynamic import()
124124
// expressions, import-equals declarations, and import-type nodes.
125+
//
126+
// The recursion runs through one closure created per walk, not one per node:
127+
// handing ForEachChild a fresh closure at every node would allocate once per
128+
// AST node across the whole program on every apply pass.
125129
func visitModuleSpecifiers(node *shimast.Node, visit func(*shimast.Node)) {
126130
if node == nil {
127131
return
128132
}
129-
switch node.Kind {
130-
case shimast.KindImportDeclaration:
131-
visit(node.AsImportDeclaration().ModuleSpecifier)
132-
case shimast.KindExportDeclaration:
133-
visit(node.AsExportDeclaration().ModuleSpecifier)
134-
case shimast.KindImportEqualsDeclaration:
135-
ref := node.AsImportEqualsDeclaration().ModuleReference
136-
if ref != nil && ref.Kind == shimast.KindExternalModuleReference {
137-
visit(ref.AsExternalModuleReference().Expression)
133+
var walk func(node *shimast.Node) bool
134+
walk = func(node *shimast.Node) bool {
135+
if node == nil {
136+
return false
138137
}
139-
case shimast.KindImportType:
140-
arg := node.AsImportTypeNode().Argument
141-
if arg != nil && arg.Kind == shimast.KindLiteralType {
142-
visit(arg.AsLiteralTypeNode().Literal)
143-
}
144-
case shimast.KindModuleDeclaration:
145-
decl := node.AsModuleDeclaration()
146-
if decl != nil {
147-
visit(decl.Name())
148-
}
149-
case shimast.KindCallExpression:
150-
call := node.AsCallExpression()
151-
if isModuleSpecifierCall(call) && call.Arguments != nil && len(call.Arguments.Nodes) > 0 {
152-
visit(call.Arguments.Nodes[0])
138+
switch node.Kind {
139+
case shimast.KindImportDeclaration:
140+
visit(node.AsImportDeclaration().ModuleSpecifier)
141+
case shimast.KindExportDeclaration:
142+
visit(node.AsExportDeclaration().ModuleSpecifier)
143+
case shimast.KindImportEqualsDeclaration:
144+
ref := node.AsImportEqualsDeclaration().ModuleReference
145+
if ref != nil && ref.Kind == shimast.KindExternalModuleReference {
146+
visit(ref.AsExternalModuleReference().Expression)
147+
}
148+
case shimast.KindImportType:
149+
arg := node.AsImportTypeNode().Argument
150+
if arg != nil && arg.Kind == shimast.KindLiteralType {
151+
visit(arg.AsLiteralTypeNode().Literal)
152+
}
153+
case shimast.KindModuleDeclaration:
154+
decl := node.AsModuleDeclaration()
155+
if decl != nil {
156+
visit(decl.Name())
157+
}
158+
case shimast.KindCallExpression:
159+
call := node.AsCallExpression()
160+
if isModuleSpecifierCall(call) && call.Arguments != nil && len(call.Arguments.Nodes) > 0 {
161+
visit(call.Arguments.Nodes[0])
162+
}
153163
}
154-
}
155-
node.ForEachChild(func(child *shimast.Node) bool {
156-
visitModuleSpecifiers(child, visit)
164+
node.ForEachChild(walk)
157165
return false
158-
})
166+
}
167+
walk(node)
159168
}
160169

161170
// isModuleSpecifierCall reports whether call is a dynamic import() or a
@@ -198,9 +207,13 @@ func (r *rewriter) rewrite(fromSource string, specifier string) (string, bool) {
198207
return rel, true
199208
}
200209

201-
// resolveSource finds the source file that a tsconfig paths specifier resolves to.
202-
// It iterates over sorted patterns and, for each match, tries all substitution
203-
// targets (with and without known extensions) including index files.
210+
// resolveSource finds the source file that a tsconfig paths specifier
211+
// resolves to. Patterns are pre-sorted into tsc's precedence order, and like
212+
// tsc's tryLoadModuleUsingPaths the resolution commits to the first (best)
213+
// matching pattern: only that pattern's substitution targets are tried, in
214+
// order, with extension and index fallbacks. When none of them names a
215+
// program source the specifier stays unrewritten — falling through to a
216+
// weaker pattern would rewrite at a module the type checker never resolved.
204217
func (r *rewriter) resolveSource(specifier string) (string, bool) {
205218
for _, pattern := range r.patterns {
206219
star, ok := matchPattern(pattern.pattern, specifier)
@@ -214,6 +227,7 @@ func (r *rewriter) resolveSource(specifier string) (string, bool) {
214227
return source, true
215228
}
216229
}
230+
return "", false
217231
}
218232
return "", false
219233
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package paths_test
2+
3+
import "testing"
4+
5+
// TestRewriterResolveSourceCommitsToBestPattern verifies no fall-through past the matched pattern.
6+
//
7+
// Locks `paths.go::resolveSource` to tsc's tryLoadModuleUsingPaths contract:
8+
// resolution commits to the single best-precedence matching pattern and tries
9+
// only that pattern's substitution targets. When none of them names a program
10+
// source, tsc's paths lookup fails — it never consults a weaker pattern — so
11+
// falling through here would rewrite the import at a module the type checker
12+
// never resolved.
13+
//
14+
// 1. Configure a long-prefix pattern whose target is missing and a catch-all whose target exists.
15+
// 2. Resolve a specifier that matches both.
16+
// 3. Assert resolution fails instead of landing on the catch-all's source.
17+
func TestRewriterResolveSourceCommitsToBestPattern(t *testing.T) {
18+
root := "/repo"
19+
patterns := []pathsPathPattern{
20+
{pattern: "*", targets: []string{"src/anywhere/*"}},
21+
{pattern: "@app/*", targets: []string{"src/app/*"}},
22+
}
23+
pathsOrderPatterns(patterns)
24+
rewriter := &pathsRewriter{
25+
basePath: root,
26+
patterns: patterns,
27+
sourceFiles: map[string]string{
28+
root + "/src/anywhere/@app/widget.ts": root + "/src/anywhere/@app/widget.ts",
29+
root + "/src/anywhere/other.ts": root + "/src/anywhere/other.ts",
30+
},
31+
}
32+
if source, ok := pathsResolveSource(rewriter, "@app/widget"); ok {
33+
t.Fatalf("resolution fell through past the best pattern to %q", source)
34+
}
35+
if source, ok := pathsResolveSource(rewriter, "other"); !ok || source != root+"/src/anywhere/other.ts" {
36+
t.Fatalf("catch-all pattern mismatch: source=%q ok=%v", source, ok)
37+
}
38+
}

website/src/content/docs/development/walkthroughs/paths.mdx

Lines changed: 42 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -218,36 +218,41 @@ func visitModuleSpecifiers(node *shimast.Node, visit func(*shimast.Node)) {
218218
if node == nil {
219219
return
220220
}
221-
switch node.Kind {
222-
case shimast.KindImportDeclaration:
223-
visit(node.AsImportDeclaration().ModuleSpecifier)
224-
case shimast.KindExportDeclaration:
225-
visit(node.AsExportDeclaration().ModuleSpecifier)
226-
case shimast.KindImportEqualsDeclaration:
227-
ref := node.AsImportEqualsDeclaration().ModuleReference
228-
if ref != nil && ref.Kind == shimast.KindExternalModuleReference {
229-
visit(ref.AsExternalModuleReference().Expression)
221+
var walk func(node *shimast.Node) bool
222+
walk = func(node *shimast.Node) bool {
223+
if node == nil {
224+
return false
230225
}
231-
case shimast.KindImportType:
232-
arg := node.AsImportTypeNode().Argument
233-
if arg != nil && arg.Kind == shimast.KindLiteralType {
234-
visit(arg.AsLiteralTypeNode().Literal)
235-
}
236-
case shimast.KindModuleDeclaration:
237-
decl := node.AsModuleDeclaration()
238-
if decl != nil {
239-
visit(decl.Name())
240-
}
241-
case shimast.KindCallExpression:
242-
call := node.AsCallExpression()
243-
if isModuleSpecifierCall(call) && call.Arguments != nil && len(call.Arguments.Nodes) > 0 {
244-
visit(call.Arguments.Nodes[0])
226+
switch node.Kind {
227+
case shimast.KindImportDeclaration:
228+
visit(node.AsImportDeclaration().ModuleSpecifier)
229+
case shimast.KindExportDeclaration:
230+
visit(node.AsExportDeclaration().ModuleSpecifier)
231+
case shimast.KindImportEqualsDeclaration:
232+
ref := node.AsImportEqualsDeclaration().ModuleReference
233+
if ref != nil && ref.Kind == shimast.KindExternalModuleReference {
234+
visit(ref.AsExternalModuleReference().Expression)
235+
}
236+
case shimast.KindImportType:
237+
arg := node.AsImportTypeNode().Argument
238+
if arg != nil && arg.Kind == shimast.KindLiteralType {
239+
visit(arg.AsLiteralTypeNode().Literal)
240+
}
241+
case shimast.KindModuleDeclaration:
242+
decl := node.AsModuleDeclaration()
243+
if decl != nil {
244+
visit(decl.Name())
245+
}
246+
case shimast.KindCallExpression:
247+
call := node.AsCallExpression()
248+
if isModuleSpecifierCall(call) && call.Arguments != nil && len(call.Arguments.Nodes) > 0 {
249+
visit(call.Arguments.Nodes[0])
250+
}
245251
}
246-
}
247-
node.ForEachChild(func(child *shimast.Node) bool {
248-
visitModuleSpecifiers(child, visit)
252+
node.ForEachChild(walk)
249253
return false
250-
})
254+
}
255+
walk(node)
251256
}
252257

253258
func isModuleSpecifierCall(call *shimast.CallExpression) bool {
@@ -283,17 +288,17 @@ After the switch, `node.ForEachChild` recurses into every child. This is what ma
283288
The closure-based visitor is the simplest way to walk an AST in Go without an explicit interface. The shape:
284289

285290
```go
286-
func walk(node *X, visit func(*X)) {
287-
if node == nil { return }
291+
var walk func(node *X) bool
292+
walk = func(node *X) bool {
293+
if node == nil { return false }
288294
visit(node)
289-
node.ForEachChild(func(child *X) bool {
290-
walk(child, visit)
291-
return false
292-
})
295+
node.ForEachChild(walk)
296+
return false
293297
}
298+
walk(root)
294299
```
295300

296-
returning `false` keeps the iteration alive. Returning `true` would short-circuit, useful for "find the first node matching predicate," not useful when you want to visit every node.
301+
returning `false` keeps the iteration alive. Returning `true` would short-circuit, useful for "find the first node matching predicate," not useful when you want to visit every node. Declare the recursion closure once per walk and hand the same value to every `ForEachChild` call — building a fresh closure at each node allocates once per AST node, which a program-wide pass pays millions of times.
297302

298303
### 4. Resolving a specifier
299304

@@ -348,6 +353,7 @@ func (r *rewriter) resolveSource(specifier string) (string, bool) {
348353
return source, true
349354
}
350355
}
356+
return "", false
351357
}
352358
return "", false
353359
}
@@ -377,6 +383,8 @@ For pattern `@lib/*` and specifier `@lib/greet`:
377383

378384
The two guards mirror tsc: a pattern with more than one `*` is not a pattern at all (`TryParsePattern` discards it), and a specifier shorter than the literal halves combined must not match — it can still satisfy both the prefix and suffix probes (`"@lib/x"` against `"@lib/x*x"`), and slicing the star capture out of it would panic on inverted bounds.
379385

386+
Resolution also _commits_ to the first (best-precedence) matching pattern, exactly like tsc's `tryLoadModuleUsingPaths`: only that pattern's targets are tried, and when none of them names a program source the specifier stays unrewritten. Falling through to a weaker pattern would rewrite the import at a module the type checker never resolved.
387+
380388
The captured `*` substitutes into each target: `./src/modules/*``./src/modules/greet`. The candidate is then joined against `basePath` (the tsconfig directory) and looked up in `r.sourceFiles`.
381389

382390
`lookupSource` walks three stages, falling through on each miss: literal match in the `sourceFiles` map → stem with each of `.ts` / `.tsx` / `.mts` / `.cts` / `.js` / `.jsx` / `.mjs` / `.cjs` appended → stem + `/index.<ext>` for directory-style imports. The exact source is in [`driver/paths.go::lookupSource`](https://github.com/samchon/ttsc/tree/master/packages/paths/driver/paths.go).

0 commit comments

Comments
 (0)