uber-go/fx

When combining fx.As to annotate return type as interface with optional parameters nil value check is not working as expected

Opened this issue · 1 comments

Describe the bug
When using an annotated return type of the function along with optional tag, the tagged parameter is not nil in a if b.Client == nil comparison

To Reproduce
An example program:

package main

import (
	"context"
	"fmt"
	"log"
	"net/http"
	"reflect"

	"go.uber.org/fx"
)

func NewAProvide() (*http.Client, error) {
	return nil, nil
}

type B struct {
	Client HTTPClient
}

type BParams struct {
	fx.In

	Client HTTPClient `optional:"true"`
}

type HTTPClient interface {
	Do(req *http.Request) (*http.Response, error)
}

func NewBProvide(in BParams) (*B, error) {
	return &B{Client: in.Client}, nil
}

func main() {
	app := fx.New(
		fx.Provide(fx.Annotate(NewAProvide, fx.As(new(HTTPClient))), NewBProvide),
		//fx.Provide(NewAProvide, NewBProvide),
		fx.Invoke(func(b *B) {
			fmt.Printf("reflect.ValueOf(b.Client).IsNil() == %v\n", reflect.ValueOf(b.Client).IsNil())
			if b.Client == nil {
				log.Fatalf("B client is nil: %v (%v)", b.Client, reflect.TypeOf(b.Client))
			} else {
				log.Fatalf("B client is not nil: %v (%v)", b.Client, reflect.TypeOf(b.Client))
			}
		}))
	err := app.Start(context.Background())
	if err != nil {
		log.Fatalf("Error starting app: %v", err)
	}
}

Expected behavior
I'd expect to see a message similar to:

B client is nil: <nil> (<nil>)

instead of

B client is not nil: <nil> (*http.Client)

Additional context
Add any other context about the problem here.

Hello! This is working as expected.

For background, the concept of typed nils comes from Go.

var v *http.Client = nil
var i HTTPClient = v
fmt.Println(v == nil) // true
fmt.Println(i == nil) // false

// https://go.dev/play/p/ZZS2T9XhMnb

It's a common gotcha, but it's completely valid to have a nil implementation in a non-nil interface.

type myClient struct{}

func (c *myClient) Do(req *http.Request) (*http.Response, error) {
	if c == nil {
		return http.DefaultClient.Do(req)
	}

	panic("TODO: custom logic here")
}

// Same as before:
var c *myClient = nil
var i HTTPClient = c
fmt.Println(i == nil) // false

// Can still send requests:
res, err := i.Do(req)
// ...

By providing a function that returns a nil *http.Client, and casting it to an HTTPClient, we're asking for a non-nil HTTPClient with a nil *http.Client inside it.

From Fx's point of view, this is behaving as expected.
Fx should not be trying to glean the meaning of a typed nil.
It should not discard my nil-safe implementation of an interface.