Disallow using In/Out structs with From/As annotations

[ankit209]

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.

Fixes #1060

[JacobOaks]

I understand disallowing fx.From with fx.In and fx.Out with fx.As. But is there any reason why we should be disallowing fx.From (applies to inputs only) with a function that returns an fx.Out (is an output) and fx.As (applies to outputs only) with a function that accepts an fx.In (is an input)?

[codecov]

Codecov Report

Merging #1068 (f08a8b6) into master (195960b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1068   +/-   ##
=======================================
  Coverage   98.52%   98.53%           
=======================================
  Files          39       39           
  Lines        2992     2996    +4     
=======================================
+ Hits         2948     2952    +4     
  Misses         37       37           
  Partials        7        7           

Impacted Files	Coverage Δ
annotated.go	99.24% <100.00%> (+<0.01%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[tchung1118]

Leaving couple of comments. Also lint seems to have failed? (try running make lint)

[tchung1118]

This is actually the old behavior we had for As/From annotations with Out/In structs, which is the safest way to go before easing up on this requirement. However, I agree with Jacob that this is a little overly restrictive, considering As annotation does not affect parameters of a constructor, and From annotation does not affect results of a constructor at all. So we could probably let users use As and From annotations with In and Out structs, respectively.

I'll leave it up to you to decide on how you'd like to proceed.

[ankit209]

Makes sense.
Have removed this unintended restriction and to allow fx.Out struct with From annotation and fx.In struct with As annotation.

[ankit209]

I understand disallowing fx.From with fx.In and fx.Out with fx.As. But is there any reason why we should be disallowing fx.From (applies to inputs only) with a function that returns an fx.Out (is an output) and fx.As (applies to outputs only) with a function that accepts an fx.In (is an input)?

Agreed @JacobOaks . Thanks for calling this out, I have made the necessary edits.

[ankit209]

Leaving couple of comments. Also lint seems to have failed? (try running make lint)

Thanks @tchung1118. Have made the required edits and fixed lint errors as well.

[JacobOaks]

This LGTM but I'll wait to see if @tchung1118 has any outstanding comments.

[tchung1118]

LGTM

[ankit209]

Thanks for correcting the documentation and approving the PR @sywhang