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) + } + } +}