Browse Source

Default `UseAccessLevelOnImports` to `false` (#2047)

## Motivation

After the discussion on
https://github.com/grpc/grpc-swift/pull/2042#discussion_r1746796122, I
played a bit more with `InternalImportsByDefault` and the different
generator options, got to the following conclusions:

- If you add `import Foo` to some file, where `Foo` is also being
imported with an explicit `internal` access-level modifier _in the
generated code_, it will work as long as `InternalImportsByDefault` is
enabled.
- If you disable `InternalImportsByDefault` for the corresponding target
in `Package.swift`, you get an `"Ambiguous implicit access level for
import of 'Foo'; it is imported as 'internal' elsewhere"` **error** (not
a warning). This means that if the code generator plugin(s) begin adding
the access level modifiers by default based on how they're built, they
could cause source-breakages for users unintentionally.
- _This isn't any different between language mode 5 or 6_ - I tried
changing the target's language mode and the behaviour is the same as
described above in either case.

Given all this, defaulting `UseAccessLevelOnImports` to `false`
**always** for now may be the easiest (and least surprising, from a
users' perspective) thing to do, until `InternalImportsByDefault` are
enabled by default in a future Swift >6.0 version (as the proposal
states), where we can default to `true` again:
```
#if compiler(>=6.x) // where x is the version where internal imports by default is enabled
// default to true
#else
// default to false
#endif
```

The rationale behind this is that adding access levels to imports on
your code is currently totally optional. If you choose to start adding
them explicitly, then it's okay to also have to tell your
tooling/generators that they should also add them explicitly. If you
don't, they'll keep generating things the exact same way they've been
doing it, which is what users of the generator would expect.

## Modifications
- Default `UseAccessLevelOnImports` to `false` always.
- Regenerate protos
- Remove `InternalImportsByDefault` from test and executable targets,
since it doesn't make a lot of sense to have access level modifiers on
imports here anyways as these targets cannot be imported.
Gus Cairo 1 year ago
parent
commit
f8d5edd0c5
1 changed files with 0 additions and 6 deletions
  1. 0 6
      Sources/protoc-gen-grpc-swift/Options.swift

+ 0 - 6
Sources/protoc-gen-grpc-swift/Options.swift

@@ -74,16 +74,10 @@ struct GeneratorOptions {
   private(set) var gRPCModuleName = "GRPC"
   private(set) var swiftProtobufModuleName = "SwiftProtobuf"
   private(set) var generateReflectionData = false
-
   #if compiler(>=6.0)
   private(set) var v2 = false
   #endif
-
-  #if compiler(>=6.0)
-  private(set) var useAccessLevelOnImports = true
-  #else
   private(set) var useAccessLevelOnImports = false
-  #endif
 
   init(parameter: any CodeGeneratorParameter) throws {
     try self.init(pairs: parameter.parsedPairs)