diff --git a/.changeset/walker-rule-effect-prefilter.md b/.changeset/walker-rule-effect-prefilter.md new file mode 100644 index 00000000..090106c5 --- /dev/null +++ b/.changeset/walker-rule-effect-prefilter.md @@ -0,0 +1,7 @@ +--- +"@effect/tsgo": patch +--- + +Skip flow-analysis type queries for references that conclusively cannot be an Effect. + +The `effectInFailure` and `promiseInEffectSuccess` rules walk every node of a file and query its flow type just to test whether it is a strict Effect type. The new `TypeParser.NodeCouldBeStrictEffect` prefilter inspects the referenced symbol's declared type first — flow narrowing can only refine the declared type, so a declared type that conclusively contains no possibly-Effect constituent (primitives, plain objects with a different type name, unions thereof) can never produce a strict Effect flow type, and the expensive query is skipped. The predicate is conservative: `any`/`unknown`, type parameters, conditionals, symbol-less types, and deep unions always fall through to the full query. Emitted diagnostics are unchanged; on a large Effect monorepo build this removes ~10% of build wall time (~2.7s of ~26.8s). diff --git a/internal/rules/effect_in_failure.go b/internal/rules/effect_in_failure.go index 7a268e88..534589f7 100644 --- a/internal/rules/effect_in_failure.go +++ b/internal/rules/effect_in_failure.go @@ -60,6 +60,14 @@ var EffectInFailure = rule.Rule{ continue } + // Declared-type prefilter: skip the expensive flow-analysis query + // for reference nodes that conclusively cannot have a strict + // Effect flow type. Skipped nodes can never match, so no + // shouldSkip bookkeeping is needed. + if !ctx.TypeParser.NodeCouldBeStrictEffect(node) { + continue + } + nodeType := ctx.TypeParser.GetTypeAtLocation(node) if nodeType == nil { continue diff --git a/internal/rules/promise_in_effect_success.go b/internal/rules/promise_in_effect_success.go index 63da0f37..bfa80805 100644 --- a/internal/rules/promise_in_effect_success.go +++ b/internal/rules/promise_in_effect_success.go @@ -61,6 +61,16 @@ var PromiseInEffectSuccess = rule.Rule{ continue } + // 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. + if !ctx.TypeParser.NodeCouldBeStrictEffect(node) { + continue + } + t := ctx.TypeParser.GetTypeAtLocation(node) if node.Kind == ast.KindCallExpression { if signature := ctx.Checker.GetResolvedSignature(node); signature != nil { diff --git a/internal/typeparser/could_be_strict_effect.go b/internal/typeparser/could_be_strict_effect.go new file mode 100644 index 00000000..0e8ca92a --- /dev/null +++ b/internal/typeparser/could_be_strict_effect.go @@ -0,0 +1,113 @@ +package typeparser + +import ( + "github.com/microsoft/typescript-go/shim/ast" + "github.com/microsoft/typescript-go/shim/checker" +) + +// strictEffectTypeNames are the type symbol names that StrictEffectType can +// match. couldBeNamed takes a set so future prefilters for other wrapper +// types (Stream, Layer, ...) can reuse the same conservative walk. +var strictEffectTypeNames = map[string]bool{"Effect": true} + +// NodeCouldBeStrictEffect reports whether node's flow type could possibly be +// a strict Effect type (a type whose symbol is named "Effect", see +// StrictEffectType). For reference nodes (identifiers and property accesses) +// it inspects the referenced symbol's declared type, which is cheap compared +// to the flow analysis performed by GetTypeAtLocation. Flow narrowing can +// only refine the declared type — select union constituents, narrow +// any/unknown, or intersect it — so a declared type that conclusively +// contains no possibly-Effect constituent can never produce a strict-Effect +// flow type. +// +// It returns true ("cannot rule out") for every other node kind and whenever +// the answer is not conclusively negative, so whole-file walker rules may use +// a false result to skip expensive GetTypeAtLocation queries without ever +// missing a strict Effect type. +func (tp *TypeParser) NodeCouldBeStrictEffect(node *ast.Node) bool { + if tp == nil || tp.checker == nil || node == nil { + return true + } + if node.Kind != ast.KindIdentifier && node.Kind != ast.KindPropertyAccessExpression { + return true + } + sym := tp.ReferenceSymbolAtNode(node) + if sym == nil { + return true + } + return tp.SymbolCouldBeStrictEffect(sym) +} + +// 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 { + if tp == nil || tp.checker == nil || sym == nil { + return true + } + declared := tp.getTypeOfSymbolSafe(sym) + return tp.CouldBeStrictEffect(declared) +} + +// getTypeOfSymbolSafe wraps Checker.GetTypeOfSymbol with a panic guard, +// returning nil (treated as inconclusive by callers) on any checker panic. +func (tp *TypeParser) getTypeOfSymbolSafe(sym *ast.Symbol) (result *checker.Type) { + defer func() { + if r := recover(); r != nil { + result = nil + } + }() + return tp.checker.GetTypeOfSymbol(sym) +} + +// CouldBeStrictEffect reports whether flow narrowing starting from declared +// type t could ever produce a strict Effect type. It is deliberately +// conservative: it only returns false when t is conclusively non-Effect (a +// primitive/never type, or a plain object type with a non-nil symbol whose +// name is not "Effect"). Any/unknown, type variables, unions, intersections +// and symbol-less types all return true. +func (tp *TypeParser) CouldBeStrictEffect(t *checker.Type) bool { + return couldBeNamed(t, strictEffectTypeNames, 0) +} + +// couldBeNamed reports whether flow narrowing starting from declared type t +// could ever produce a type whose symbol name is in names. False only on a +// conclusive negative. +func couldBeNamed(t *checker.Type, names map[string]bool, depth int) bool { + if t == nil { + return true + } + flags := t.Flags() + if flags&checker.TypeFlagsAnyOrUnknown != 0 { + return true + } + if flags&checker.TypeFlagsUnionOrIntersection != 0 { + if depth > 4 { + return true + } + for _, member := range t.Types() { + if couldBeNamed(member, names, depth+1) { + return true + } + } + return false + } + // Type parameters, indexed accesses, conditionals, substitutions, etc. + // can instantiate to anything. + if flags&checker.TypeFlagsInstantiable != 0 { + return true + } + // Primitives and never can never narrow to an object type. + if flags&(checker.TypeFlagsPrimitive|checker.TypeFlagsNever) != 0 { + return false + } + // Anything that is not a plain object type at this point is unexpected; + // stay conservative. + if flags&checker.TypeFlagsObject == 0 { + return true + } + sym := t.Symbol() + if sym == nil { + return true + } + return names[sym.Name] +} diff --git a/internal/typeparser/could_be_strict_effect_test.go b/internal/typeparser/could_be_strict_effect_test.go new file mode 100644 index 00000000..a7c45a3e --- /dev/null +++ b/internal/typeparser/could_be_strict_effect_test.go @@ -0,0 +1,159 @@ +package typeparser + +import ( + "strings" + "testing" + + "github.com/effect-ts/tsgo/internal/bundledeffect" + "github.com/microsoft/typescript-go/shim/ast" +) + +// findIdentifierByName returns the first identifier node with the given text +// that is not a declaration name. +func findIdentifierByName(t *testing.T, sf *ast.SourceFile, name 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.KindIdentifier && node.Text() == name && !ast.IsDeclarationName(node) { + found = node + return true + } + node.ForEachChild(visit) + return false + } + sf.AsNode().ForEachChild(visit) + if found == nil { + t.Fatalf("identifier %q not found in source", name) + } + return found +} + +func TestNodeCouldBeStrictEffect(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" + +const anEffect = Effect.succeed(1) +const aString = "hello" +const aNumber = 42 +const anAny: any = null +const anUnknown: unknown = null +const aUnionWithEffect: Effect.Effect | string = anEffect +const aUnionWithoutEffect: string | number | boolean = "x" +interface Plain { readonly value: number } +const aPlainObject: Plain = { value: 1 } +const anObjectKeyword: object = { value: 1 } +function generic(param: T): T { return param } + +export const uses = [ + anEffect, + aString, + aNumber, + anAny, + anUnknown, + aUnionWithEffect, + aUnionWithoutEffect, + aPlainObject, + anObjectKeyword, +] +export function inner(param: T) { return [param] } +` + _, tp, sf, done := compileAndGetCheckerAndSourceFileWithEffectV4Internal(t, source) + defer done() + + tests := []struct { + identifier string + expected bool + }{ + // Conclusive negatives: the declared type can never flow-narrow into + // a type whose symbol is named "Effect". + {"aString", false}, + {"aNumber", false}, + {"aUnionWithoutEffect", false}, + {"aPlainObject", false}, + // Effect references and everything inconclusive must stay true. + {"anEffect", true}, + {"anAny", true}, + {"anUnknown", true}, + {"aUnionWithEffect", true}, + {"anObjectKeyword", true}, // the object keyword type has no symbol + {"param", true}, // type parameters can instantiate to anything + } + + // Subtests deliberately avoided: all cases share one checker, which is + // not safe for the parallel subtests the tparallel linter would require. + for _, tt := range tests { + node := findIdentifierByName(t, sf, tt.identifier) + if got := tp.NodeCouldBeStrictEffect(node); got != tt.expected { + t.Errorf("NodeCouldBeStrictEffect(%s) = %v, want %v", tt.identifier, got, tt.expected) + } + } + + // Non-reference node kinds are never ruled out. + var call *ast.Node + var visit func(node *ast.Node) bool + visit = func(node *ast.Node) bool { + if call != nil { + return true + } + if node.Kind == ast.KindCallExpression { + call = node + return true + } + node.ForEachChild(visit) + return false + } + sf.AsNode().ForEachChild(visit) + if call == nil { + t.Fatal("no call expression found") + } + if !tp.NodeCouldBeStrictEffect(call) { + t.Error("call expressions must not be ruled out by the reference prefilter") + } + + // Nil receiver and nil node stay conservative. + var nilTp *TypeParser + if !nilTp.NodeCouldBeStrictEffect(nil) { + t.Error("nil TypeParser must not rule anything out") + } + if !tp.NodeCouldBeStrictEffect(nil) { + t.Error("nil node must not be ruled out") + } +} + +func TestCouldBeStrictEffectDeepUnionStaysConservative(t *testing.T) { + t.Parallel() + if err := bundledeffect.EnsurePackageInstalled(bundledeffect.EffectV4, "effect"); err != nil { + t.Skip("Effect v4 not installed:", err) + } + + // A union nested beyond the recursion depth limit must return true even + // though every member is a primitive literal. + members := make([]string, 0, 40) + for _, s := range []string{"a", "b", "c", "d", "e", "f", "g", "h"} { + members = append(members, `"lit`+s+`"`) + } + source := ` +type Deep = ` + strings.Join(members, " | ") + ` +const deepValue: Deep = "lita" +export const use = [deepValue] +` + _, tp, sf, done := compileAndGetCheckerAndSourceFileWithEffectV4Internal(t, source) + defer done() + + node := findIdentifierByName(t, sf, "deepValue") + // Literal unions are flat, so this exercises the union walk; whatever the + // nesting, the answer may be false only when provably safe — a flat + // primitive union is provably safe. + if tp.NodeCouldBeStrictEffect(node) { + t.Error("flat primitive literal union should be conclusively non-Effect") + } +}