From dae244edd5016b6f73f4d5aa2d4aec0c293e8946 Mon Sep 17 00:00:00 2001 From: Mattia Manzati Date: Sun, 9 Aug 2026 19:34:52 +0200 Subject: [PATCH] perf: extend the walker-rule prefilter to call expressions A call expression's type is its resolved signature's return type, and both the signature and that return type are cached from the main check phase. NodeCouldBeStrictEffect now consults them for KindCallExpression nodes and rules the call out when the declared return type conclusively cannot be a strict Effect, sparing the expensive re-check that GetTypeAtLocation performs on calls. Resolved signatures are instantiated, so generic calls whose instantiation is conclusively non-Effect are ruled out precisely; signature-less calls, optional chains and every inconclusive return type stay conservative. promiseInEffectSuccess also stops computing GetTypeAtLocation for call nodes only to discard it: it now goes straight to the resolved signature's return type and falls back to the location query only when no signature resolves. Diagnostics are unchanged (verified byte-identical on a full Effect monorepo build); wall time for that build drops a further ~4.6% on top of the reference prefilter. Co-Authored-By: Claude Fable 5 --- .changeset/call-signature-effect-prefilter.md | 7 ++ internal/rules/promise_in_effect_success.go | 13 ++- internal/typeparser/could_be_strict_effect.go | 24 +++++ .../typeparser/could_be_strict_effect_test.go | 101 ++++++++++++++++-- 4 files changed, 131 insertions(+), 14 deletions(-) create mode 100644 .changeset/call-signature-effect-prefilter.md diff --git a/.changeset/call-signature-effect-prefilter.md b/.changeset/call-signature-effect-prefilter.md new file mode 100644 index 00000000..6f13de31 --- /dev/null +++ b/.changeset/call-signature-effect-prefilter.md @@ -0,0 +1,7 @@ +--- +"@effect/tsgo": patch +--- + +Extend the walker-rule prefilter to call expressions. + +A call expression's type is its resolved signature's return type, and both the signature and that return type are already cached from the main check phase. `NodeCouldBeStrictEffect` now consults them for call nodes and skips the expensive flow-analysis re-check when the declared return type conclusively cannot be a strict Effect. Signature-less calls, optional chains, and every inconclusive return type stay conservative, and `promiseInEffectSuccess` no longer computes a location type for calls only to discard it. Emitted diagnostics are unchanged; on a large Effect monorepo build this removes a further ~4.6% of wall time on top of the reference-node prefilter, bringing the total Effect diagnostics overhead versus a pristine tsgo build of the same commit down to ~17%. diff --git a/internal/rules/promise_in_effect_success.go b/internal/rules/promise_in_effect_success.go index bfa80805..6b14105f 100644 --- a/internal/rules/promise_in_effect_success.go +++ b/internal/rules/promise_in_effect_success.go @@ -63,20 +63,23 @@ var PromiseInEffectSuccess = rule.Rule{ // Declared-type prefilter: a diagnostic requires a strict Effect // flow type on the node, which reference nodes with a - // conclusively non-Effect declared type can never have. Skipped - // nodes can never match, so no matched-map bookkeeping is needed. - // Call expressions are unaffected: the prefilter only rules out - // identifiers and property accesses. + // conclusively non-Effect declared type — and calls whose + // resolved signature conclusively cannot return one — can never + // have. Skipped nodes can never match, so no matched-map + // bookkeeping is needed. if !ctx.TypeParser.NodeCouldBeStrictEffect(node) { continue } - t := ctx.TypeParser.GetTypeAtLocation(node) + var t *checker.Type if node.Kind == ast.KindCallExpression { if signature := ctx.Checker.GetResolvedSignature(node); signature != nil { t = ctx.Checker.GetReturnTypeOfSignature(signature) } } + if t == nil { + t = ctx.TypeParser.GetTypeAtLocation(node) + } effect := ctx.TypeParser.StrictEffectType(t, node) if effect == nil || !typeContainsPromise(ctx.TypeParser, effect.A) { continue diff --git a/internal/typeparser/could_be_strict_effect.go b/internal/typeparser/could_be_strict_effect.go index 0e8ca92a..9f6b22b0 100644 --- a/internal/typeparser/could_be_strict_effect.go +++ b/internal/typeparser/could_be_strict_effect.go @@ -28,6 +28,9 @@ func (tp *TypeParser) NodeCouldBeStrictEffect(node *ast.Node) bool { if tp == nil || tp.checker == nil || node == nil { return true } + if node.Kind == ast.KindCallExpression { + return tp.callCouldReturnStrictEffect(node) + } if node.Kind != ast.KindIdentifier && node.Kind != ast.KindPropertyAccessExpression { return true } @@ -38,6 +41,27 @@ func (tp *TypeParser) NodeCouldBeStrictEffect(node *ast.Node) bool { return tp.SymbolCouldBeStrictEffect(sym) } +// callCouldReturnStrictEffect reports whether a call expression's type could +// possibly be a strict Effect type, based on the return type of its resolved +// signature. The signature and its return type are cached from the main check +// phase, so consulting them is cheap compared to re-checking the call via +// GetTypeAtLocation. A call expression's type is its resolved signature's +// return type (union-widened with undefined for optional chains, which the +// conservative union walk handles), so a conclusively non-Effect declared +// return type rules the node out. +func (tp *TypeParser) callCouldReturnStrictEffect(node *ast.Node) (result bool) { + defer func() { + if r := recover(); r != nil { + result = true + } + }() + signature := tp.checker.GetResolvedSignature(node) + if signature == nil { + return true + } + return couldBeNamed(tp.checker.GetReturnTypeOfSignature(signature), strictEffectTypeNames, 0) +} + // SymbolCouldBeStrictEffect reports whether a reference to sym could possibly // have a strict-Effect flow type, based on the symbol's declared type only. func (tp *TypeParser) SymbolCouldBeStrictEffect(sym *ast.Symbol) bool { diff --git a/internal/typeparser/could_be_strict_effect_test.go b/internal/typeparser/could_be_strict_effect_test.go index a7c45a3e..9a140bca 100644 --- a/internal/typeparser/could_be_strict_effect_test.go +++ b/internal/typeparser/could_be_strict_effect_test.go @@ -6,6 +6,7 @@ import ( "github.com/effect-ts/tsgo/internal/bundledeffect" "github.com/microsoft/typescript-go/shim/ast" + "github.com/microsoft/typescript-go/shim/scanner" ) // findIdentifierByName returns the first identifier node with the given text @@ -97,26 +98,27 @@ export function inner(param: T) { return [param] } } } - // Non-reference node kinds are never ruled out. - var call *ast.Node + // Other non-reference node kinds (e.g. binary expressions) are never + // ruled out. + var binary *ast.Node var visit func(node *ast.Node) bool visit = func(node *ast.Node) bool { - if call != nil { + if binary != nil { return true } - if node.Kind == ast.KindCallExpression { - call = node + if node.Kind == ast.KindArrayLiteralExpression { + binary = node return true } node.ForEachChild(visit) return false } sf.AsNode().ForEachChild(visit) - if call == nil { - t.Fatal("no call expression found") + if binary == nil { + t.Fatal("no array literal expression found") } - if !tp.NodeCouldBeStrictEffect(call) { - t.Error("call expressions must not be ruled out by the reference prefilter") + if !tp.NodeCouldBeStrictEffect(binary) { + t.Error("non-reference, non-call node kinds must not be ruled out") } // Nil receiver and nil node stay conservative. @@ -157,3 +159,84 @@ export const use = [deepValue] t.Error("flat primitive literal union should be conclusively non-Effect") } } + +// findCallByCalleeName returns the first call expression whose callee text +// contains the given substring. +func findCallByCalleeName(t *testing.T, sf *ast.SourceFile, callee string) *ast.Node { + t.Helper() + var found *ast.Node + var visit func(node *ast.Node) bool + visit = func(node *ast.Node) bool { + if found != nil { + return true + } + if node.Kind == ast.KindCallExpression { + expr := node.AsCallExpression().Expression + if expr != nil && strings.Contains(scanner.GetTextOfNode(expr), callee) { + found = node + return true + } + } + node.ForEachChild(visit) + return false + } + sf.AsNode().ForEachChild(visit) + if found == nil { + t.Fatalf("call to %q not found in source", callee) + } + return found +} + +func TestCallCouldReturnStrictEffect(t *testing.T) { + t.Parallel() + if err := bundledeffect.EnsurePackageInstalled(bundledeffect.EffectV4, "effect"); err != nil { + t.Skip("Effect v4 not installed:", err) + } + + source := ` +import { Effect } from "effect" + +declare function makesEffect(): Effect.Effect +declare function makesString(): string +declare function makesUnion(flag: boolean): Effect.Effect | undefined +declare function makesAny(): any +declare function generic(value: T): T +declare const maybe: { makesEffect(): Effect.Effect } | undefined + +export const uses = [ + makesEffect(), + makesString(), + makesUnion(true), + makesAny(), + generic("x"), + maybe?.makesEffect(), +] +` + _, tp, sf, done := compileAndGetCheckerAndSourceFileWithEffectV4Internal(t, source) + defer done() + + // Subtests deliberately avoided: all cases share one checker, which is + // not safe for the parallel subtests the tparallel linter would require. + tests := []struct { + callee string + expected bool + }{ + // Conclusive negative: the declared return type can never be Effect. + {"makesString", false}, + // Effect-returning calls and every inconclusive case stay true. + {"makesEffect", true}, + {"makesUnion", true}, // union containing Effect + {"makesAny", true}, // any return + // The resolved signature is instantiated, so generic("x") conclusively + // returns the primitive literal "x" and is ruled out. + {"generic", false}, + {"maybe?.makesEffect", true}, // optional chain: Effect | undefined union + } + + for _, tt := range tests { + call := findCallByCalleeName(t, sf, tt.callee) + if got := tp.NodeCouldBeStrictEffect(call); got != tt.expected { + t.Errorf("NodeCouldBeStrictEffect(call %s) = %v, want %v", tt.callee, got, tt.expected) + } + } +}