uber-go/fx

Disallow using In/Out structs with From/As annotations

tchung1118 opened this issue · 1 comments

In #1053, we eased up the requirement for function signature for applying annotations to allow using In/Out structs with ResultTags/ParamTags. This had an unintended change in behavior where users are now able to use In/Out structs with From/As annotations. For example, the below code used to error out:

type Foo interface { ... }
type Bar struct { ... } // Bar implements Foo
struct Res {
  fx.Out

  ABar Bar
}
fx.Annotate(
  func() Res { ... },
  fx.As(new(Foo)),
)

Currently the code above applies the As annotation to ABar field and transforms the target function's signature accordingly. However, this was an unintended change in behavior, and we should have a design discussion about whether we actually want this before cutting a new release. Since it will require a breaking change to disable this after a new release is cut, I think the safe approach is to disable this for now and enable it after having decided that this is indeed what we want.

Would love to pick up this issue. Suggested change in annotated.go.

@tchung1118 Can you please add me as a contributor to uber/fx repo ?

diff --git a/annotated.go b/annotated.go
index 3c9ecf7..80c5243 100644
--- a/annotated.go
+++ b/annotated.go
@@ -1591,8 +1591,8 @@ func (ann *annotated) cleanUpAsResults() {
 }

 // checks and returns a non-nil error if the target function:
-// - returns an fx.Out struct as a result.
-// - takes in an fx.In struct as a parameter.
+// - returns an fx.Out struct as a result and has either of ResultTags, As or From annotation
+// - takes in an fx.In struct as a parameter and has either of ParamTags, As or From annotation
 // - has an error result not as the last result.
 func (ann *annotated) typeCheckOrigFn() error {
        ft := reflect.TypeOf(ann.Target)
@@ -1608,8 +1608,11 @@ func (ann *annotated) typeCheckOrigFn() error {
                if ot.Kind() != reflect.Struct {
                        continue
                }
-               if len(ann.ResultTags) > 0 && dig.IsOut(reflect.New(ft.Out(i)).Elem().Interface()) {
-                       return errors.New("fx.Out structs cannot be annotated with fx.ResultTags")
+               if !dig.IsOut(reflect.New(ft.Out(i)).Elem().Interface()) {
+                       continue
+               }
+               if len(ann.ResultTags) > 0 || len(ann.As) > 0 || len(ann.From) > 0 {
+                       return errors.New("fx.Out structs cannot be annotated with either of fx.ResultTags, fx.As or fx.From")
                }
        }

@@ -1618,8 +1621,11 @@ func (ann *annotated) typeCheckOrigFn() error {
                if it.Kind() != reflect.Struct {
                        continue
                }
-               if len(ann.ParamTags) > 0 && dig.IsIn(reflect.New(ft.In(i)).Elem().Interface()) {
-                       return errors.New("fx.In structs cannot be annotated with fx.ParamTags")
+               if !dig.IsIn(reflect.New(ft.In(i)).Elem().Interface()) {
+                       continue
+               }
+               if len(ann.ParamTags) > 0 || len(ann.As) > 0 || len(ann.From) > 0 {
+                       return errors.New("fx.In structs cannot be annotated with either of fx.ParamTags, fx.As or fx.From")
                }
        }
        return nil