test(scanner): confirm --strict-types closes #16, transitive factory wrapping

internal/scanner/strict_test.go

@@ -71,6 +71,71 @@ func TestScanStrict_positiveInterfaceSatisfaction(t *testing.T) {
 	}
 }
 
+func TestScanStrict_positiveTransitiveFactoryWrapping(t *testing.T) {
+	// Issue #16's exact repro: a cross-package factory function
+	// (wrapper.NewClientWrapper) whose declared return type isn't
+	// *ld.LDClient directly but a wrapper struct with a client field,
+	// called from a different package than the one that constructs it.
+	// Turns out this needed no new resolution logic at all — found while
+	// building this regression test, not before: resolveByStaticType
+	// (identity.go, added for issue #15's interface-satisfaction fix)
+	// checks *any* expression's real static type against the SDK client
+	// type, not just an assignment's RHS — so `w.Inner` in
+	// `w.Inner.BoolVariation(...)` resolves correctly as a byproduct,
+	// since go/types reports a field selector's type exactly like any
+	// other expression's, regardless of how the struct it's read from was
+	// itself constructed.
+	dir := "testdata/strict/transitive_factory"
+	cfg := config.Config{
+		Include:  []string{"**/*.go"},
+		Exclude:  []string{"**/vendor/**", "**/.git/**"},
+		Provider: "launchdarkly",
+	}
+
+	phase1, err := Scan(dir, cfg)
+	if err != nil {
+		t.Fatalf("Scan error = %v", err)
+	}
+	if len(phase1.Usages) != 0 {
+		t.Fatalf("Scan (Phase 1) found %d usage(s), want 0 — transitive factory wrapping is exactly what Phase 1 cannot see: %+v", len(phase1.Usages), phase1.Usages)
+	}
+
+	strict, err := ScanStrict(dir, cfg)
+	if err != nil {
+		t.Fatalf("ScanStrict error = %v", err)
+	}
+	if len(strict.Warnings) != 0 {
+		t.Fatalf("ScanStrict warnings = %+v, want none — the fixture module builds cleanly", strict.Warnings)
+	}
+	// 2: the one-hop repro (w.Inner.BoolVariation) and a two-hop variant
+	// (o.Middle.Inner.BoolVariation) — proving resolveByStaticType
+	// generalizes past a single field-selector hop for free, since it
+	// queries go/types for the whole expression's real type directly
+	// rather than manually walking one hop at a time.
+	if len(strict.Usages) != 2 {
+		t.Fatalf("ScanStrict found %d usage(s), want 2: %+v", len(strict.Usages), strict.Usages)
+	}
+	wantFlagKeys := map[string]bool{"transitive-factory-flag": false, "two-hop-transitive-factory-flag": false}
+	for _, got := range strict.Usages {
+		if _, ok := wantFlagKeys[got.FlagKey]; !ok {
+			t.Errorf("unexpected usage %+v", got)
+			continue
+		}
+		wantFlagKeys[got.FlagKey] = true
+		if got.DetectedBy != "strict-types" {
+			t.Errorf("usage %q DetectedBy = %q, want strict-types", got.FlagKey, got.DetectedBy)
+		}
+		if got.SDK != "go-server-sdk-v7" {
+			t.Errorf("usage %q SDK = %q, want go-server-sdk-v7", got.FlagKey, got.SDK)
+		}
+	}
+	for k, found := range wantFlagKeys {
+		if !found {
+			t.Errorf("missing expected usage for flag key %q", k)
+		}
+	}
+}
+
 func TestScanStrict_failedLoadFallsBackToPhase1(t *testing.T) {
 	// A directory with no go.mod at all (or above it) fails to load as a
 	// module — ScanStrict must still return Phase 1's result plus a

internal/scanner/testdata/strict/transitive_factory/consumer/.gitignore

@@ -0,0 +1 @@
+/consumer

internal/scanner/testdata/strict/transitive_factory/consumer/main.go

@@ -0,0 +1,27 @@
+package main
+
+import "flaglint-strict-fixture-transitive-factory/wrapper"
+
+// Reproduces issue #16 exactly: w.Inner.BoolVariation(...) is not
+// detected by Phase 1, since wrapper.NewClientWrapper isn't a registered
+// factory function (its return type is *ClientWrapper, not *ld.LDClient
+// directly) — the composite-literal binding establishing
+// ClientWrapper.Inner also lives in a different package than this call
+// site, so Phase 1's same-package composite-literal resolution doesn't
+// reach it either.
+func run() {
+	w := wrapper.NewClientWrapper()
+	_, _ = w.Inner.BoolVariation("transitive-factory-flag", nil, false)
+}
+
+// runTwoHop proves the resolution generalizes past a single field-selector
+// hop — o.Middle.Inner, not just w.Inner.
+func runTwoHop() {
+	o := wrapper.NewOuterWrapper()
+	_, _ = o.Middle.Inner.BoolVariation("two-hop-transitive-factory-flag", nil, false)
+}
+
+func main() {
+	run()
+	runTwoHop()
+}

internal/scanner/testdata/strict/transitive_factory/go.mod

@@ -0,0 +1,7 @@
+module flaglint-strict-fixture-transitive-factory
+
+go 1.22
+
+require github.com/launchdarkly/go-server-sdk/v7 v7.0.0
+
+replace github.com/launchdarkly/go-server-sdk/v7 => ./stubsdk

internal/scanner/testdata/strict/transitive_factory/stubsdk/client.go

@@ -0,0 +1,16 @@
+// Package ldclient is a minimal local stand-in for the real
+// github.com/launchdarkly/go-server-sdk/v7 — see the sibling
+// interface_satisfaction fixture's stubsdk for why.
+package ldclient
+
+import "time"
+
+type LDClient struct{}
+
+func MakeClient(sdkKey string, waitFor time.Duration) (*LDClient, error) {
+	return &LDClient{}, nil
+}
+
+func (c *LDClient) BoolVariation(key string, ctx interface{}, defaultVal bool) (bool, error) {
+	return defaultVal, nil
+}

internal/scanner/testdata/strict/transitive_factory/stubsdk/go.mod

@@ -0,0 +1,3 @@
+module github.com/launchdarkly/go-server-sdk/v7
+
+go 1.22

internal/scanner/testdata/strict/transitive_factory/wrapper/wrapper.go

@@ -0,0 +1,37 @@
+package wrapper
+
+import (
+	"time"
+
+	ld "github.com/launchdarkly/go-server-sdk/v7"
+)
+
+// ClientWrapper is issue #16's exact repro shape: a factory function
+// (NewClientWrapper) whose *declared return type* is not *ld.LDClient
+// directly, but a wrapper struct that itself has a client field.
+type ClientWrapper struct {
+	Inner *ld.LDClient
+}
+
+// NewClientWrapper is NOT a registered Phase 1 factory function (its
+// return type is *ClientWrapper, not *ld.LDClient) — closing this
+// generally requires real go/types information (--strict-types), per
+// issue #16 and ADR 005.
+func NewClientWrapper() *ClientWrapper {
+	client, _ := ld.MakeClient("sdk-key", 5*time.Second)
+	return &ClientWrapper{Inner: client}
+}
+
+// OuterWrapper adds a second hop (Middle.Inner, not just Inner) — proving
+// resolveByStaticType's go/types.Info.TypeOf(rhs) generalizes to an
+// arbitrarily deep field-selector chain for free, since it queries the
+// expression's real type directly rather than manually walking one hop at
+// a time the way Phase 1's syntax-only structFieldTypes chain resolution
+// does.
+type OuterWrapper struct {
+	Middle *ClientWrapper
+}
+
+func NewOuterWrapper() *OuterWrapper {
+	return &OuterWrapper{Middle: NewClientWrapper()}
+}