Add option to annotate imports with @_implementationOnly

[gjcairo]

This PR adds a new option to the protoc generator that annotates imports as @_implementationOnly when the visibility of the protos is set as internal. This is useful when framework owners using protobuf don't want to expose internal-only types transitively imported via protos to clients.

[thomasvl]

There was past discussion in #1157, I haven't looked in detail at this, but I'm guessing it is the same idea.

[thomasvl]

So taking a quick skim at this PR and #1157 it looks like this slightly don't align. So within the @_implementationOnly I think there are a few ideas:

1. Make the import SwiftProtobuf in the generated files @_implementationOnly.
2. Make imports of other Proto types needed for a given generated file @_implementationOnly.

I believe (1) is what #1157 was originally after and would allow the generated types to be then exposed without exposing the fact that they are proto based objects.

But I'm not as clear how (2) would work in all cases, if the generated files are marked as internal visibility, does that even still make sense since the imported proto types are only being imported for fields to make use of, so aren't they still exposed in the internal interface and not really @_implementationOnly? And if a developer were to turn on public visibility with @_implementationOnly, what would happen? Is that an flat out error that the generator to should trap and message rather than leaving a developer to figure out there error via the compiler problems?

[FranzBusch]

So taking a quick skim at this PR and #1157 it looks like this slightly don't align. So within the @_implementationOnly I think there are a few ideas:

1. Make the import SwiftProtobuf in the generated files @_implementationOnly.
2. Make imports of other Proto types needed for a given generated file @_implementationOnly.

I believe (1) is what #1157 was originally after and would allow the generated types to be then exposed without exposing the fact that they are proto based objects.

But I'm not as clear how (2) would work in all cases, if the generated files are marked as internal visibility, does that even still make sense since the imported proto types are only being imported for fields to make use of, so aren't they still exposed in the internal interface and not really ``@_implementationOnly? And if a developer were to turn on public` visibility with `@_implementationOnly`, what would happen? Is that an flat out error that the generator to should trap and message rather than leaving a developer to figure out there error via the compiler problems?

The one use-case where @_implementationOnly makes sense is when you are shipping an ABI stable module that contains generated proto files and need to hide SwiftProtobuf. In this case, you have to make sure all the imports of SwiftProtobuf are prefixed with the @_implementationOnly. It is definitely an error trying to generate your code using @_implementationOnly and setting the visibility to public. That cannot work and we should make sure to error.

The second problem is interesting but I think should just work by making sure that any other import that we add via ModulePathMappings also gets the @_implementationOnly annotation since they would be pulling in SwiftProtobuf as well which we are trying to hide. We should definitely try to get a sample project setup here that exercises this and tries to build with library evolution. @gjcairo could you see that we get something like this added?

[gjcairo]

• I've made changes to show an error when both the implementationOnly option is enabled, and visibility is set to public.
• I've also created an xcframework built with library evolution enabled that had proto module mappings for other proto files.
  • With the implementationOnly option disabled, I was getting errors when building, because SwiftProtobuf and the other internal module being used by the proto file were not available but the imports showed up on the framework interface.
  • When enabling it and building the xcframework again, the project using the framework built fine.

[thomasvl]

Should this option also be expose in the SwiftPM plugin support?

[FranzBusch]

@thomasvl yes it should be exposed. I can see scenarios where projects use the SPM plug-in while shipping frameworks with library evolution enabled.

[gjcairo]

We can expose it, although I should mention that the module mappings option isn't exposed, and I'm not actually sure we can get it to work properly/in all scenarios given the sandboxing of the plugin. This may mean that in scenarios where the module mappings option is used, users are probably not using the SPM plugin anyways. But yes, I can add support for this new option in the plugin.

[FranzBusch]

Yes this is expected. IMO we need to come up with a different strategy for module dependencies with the SPM plug-in. This requires more thinking and experimenting and isn’t in scope of this work.
however there is value in the implementation only option for the plug-in without module dependencies