Skip to content

Commit e9def03

Browse files
gjcairogjcairo
authored
Add CompileTests for InternalImportsByDefault (apple#1709)
* Add CompileTests for InternalImportsByDefault * Update Reference and Makefile * Add missing headers * Update Makefile * Fix references * Fix Makefile * Fix Package.swift * Fix build on 5.8
1 parent beeb414 commit e9def03

File tree

12 files changed

+289
-4
lines changed

12 files changed

+289
-4
lines changed

‎.gitignore

+2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ xcbaselines
1111
mined_words.txt
1212
/CompileTests/MultiModule/.build
1313
/CompileTests/MultiModule/.swiftpm
14+
/CompileTests/InternalImportsByDefault/.build
15+
/CompileTests/InternalImportsByDefault/.swiftpm
1416
/*DescriptorTestData.bin
1517
/Package.resolved
1618
/PluginLibEditionDefaults.bin

‎CompileTests/InternalImportsByDefault/Package.swift

+56
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// swift-tools-version: 5.8
2+
3+
// Package.swift
4+
//
5+
// Copyright (c) 2024 Apple Inc. and the project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See LICENSE.txt for license information:
9+
// https://github.com/apple/swift-protobuf/blob/main/LICENSE.txt
10+
11+
import PackageDescription
12+
13+
#if compiler(>=5.9)
14+
let package = Package(
15+
name: "CompileTests",
16+
dependencies: [
17+
.package(path: "../..")
18+
],
19+
targets: [
20+
.executableTarget(
21+
name: "InternalImportsByDefault",
22+
dependencies: [
23+
.product(name: "SwiftProtobuf", package: "swift-protobuf"),
24+
],
25+
exclude: [
26+
"Protos/SomeProtoWithBytes.proto",
27+
"Protos/ServiceOnly.proto"
28+
],
29+
swiftSettings: [
30+
.enableExperimentalFeature("InternalImportsByDefault"),
31+
.enableExperimentalFeature("AccessLevelOnImport"),
32+
// Enable warnings as errors so the build fails if warnings are
33+
// present in generated code.
34+
.unsafeFlags(["-warnings-as-errors"])
35+
],
36+
plugins: [
37+
.plugin(name: "SwiftProtobufPlugin", package: "swift-protobuf")
38+
]
39+
),
40+
]
41+
)
42+
#else
43+
let package = Package(
44+
name: "CompileTests",
45+
targets: [
46+
.executableTarget(
47+
name: "InternalImportsByDefault",
48+
exclude: [
49+
"swift-protobuf-config.json",
50+
"Protos/SomeProtoWithBytes.proto",
51+
"Protos/ServiceOnly.proto"
52+
]
53+
)
54+
]
55+
)
56+
#endif

‎CompileTests/InternalImportsByDefault/README.md

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# CompileTests/InternalImportsByDefault
2+
3+
This is a test case that ensures that generated code builds correctly when
4+
`InternalImportsByDefault` is enabled and the code is generated with public
5+
visibility.
6+
7+
When support for access level modifiers on imports was first added, an issue
8+
was encountered where publicly-generated protos would generate build errors and
9+
warnings when `InternalImportsByDefault` was enabled, as some dependencies were
10+
imported without an explicit access level modifier (i.e. `Foundation`), and some
11+
where sometimes imported as `public` without actually being used in the
12+
generated code at all (i.e. `Foundation` and `SwiftProtobuf`).

‎CompileTests/InternalImportsByDefault/Sources/InternalImportsByDefault/Protos/ServiceOnly.proto

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// This proto file should generate an empty file, since the plugin will ignore
2+
// service definitions.
3+
// This is here to make sure we don't import Foundation or SwiftProtobuf when
4+
// it's not necessary.
5+
6+
service SomeService {
7+
}

‎CompileTests/InternalImportsByDefault/Sources/InternalImportsByDefault/Protos/SomeProtoWithBytes.proto

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// This proto will generate a Swift file that imports Foundation, because it
2+
// defines a bytes field.
3+
// Because InternalImportsByDefault is enabled on this module and we generate
4+
// protos with public visibility, the build will fail if the access level
5+
// modifier is missing (or wrong) since it will default the import to `internal`
6+
// and cause a conflict of access levels, since the `someBytes` property defined
7+
// on the message will be public.
8+
9+
message SomeProtoWithBytes {
10+
optional bytes someBytes = 2;
11+
optional string ext_str = 100;
12+
}

‎CompileTests/InternalImportsByDefault/Sources/InternalImportsByDefault/main.swift

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// main.swift
2+
//
3+
// Copyright (c) 2024 Apple Inc. and the project authors
4+
// Licensed under Apache License v2.0 with Runtime Library Exception
5+
//
6+
// See LICENSE.txt for license information:
7+
// https://github.com/apple/swift-protobuf/blob/main/LICENSE.txt
8+
9+
// This test only makes sense for Swift 5.9+ because 5.8 doesn't support access
10+
// level on imports.
11+
#if compiler(>=5.9)
12+
private import Foundation
13+
14+
struct InternalImportsByDefault {
15+
static func main() {
16+
let protoWithBytes = SomeProtoWithBytes.with { proto in
17+
proto.someBytes = Data()
18+
proto.extStr = ""
19+
}
20+
blackhole(protoWithBytes)
21+
}
22+
}
23+
24+
@inline(never)
25+
func blackhole<T>(_: T) {}
26+
#endif

‎CompileTests/InternalImportsByDefault/Sources/InternalImportsByDefault/swift-protobuf-config.json

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"invocations": [
3+
{
4+
"protoFiles": [
5+
"Protos/SomeProtoWithBytes.proto",
6+
"Protos/ServiceOnly.proto",
7+
],
8+
"visibility": "public",
9+
"useAccessLevelOnImports": true
10+
}
11+
]
12+
}

‎Makefile

+35-4
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,15 @@ PROTOS_DIRS=Conformance protoc-gen-swiftTests SwiftProtobuf SwiftProtobufPluginL
7979
clean \
8080
compile-tests \
8181
compile-tests-multimodule \
82+
compile-tests-internalimportsbydefault \
8283
default \
8384
docs \
8485
install \
8586
pod-lib-lint \
8687
reference \
8788
regenerate \
8889
regenerate-compiletests-multimodule-protos \
90+
copy-compiletests-internalimportsbydefault-protos \
8991
regenerate-compiletests-protos \
9092
regenerate-conformance-protos \
9193
regenerate-fuzz-protos \
@@ -194,19 +196,33 @@ test-plugin: build ${PROTOC_GEN_SWIFT}
194196
--tfiws_opt=ProtoPathModuleMappings=Protos/CompileTests/MultiModule/module_mappings.pbascii \
195197
--tfiws_out=_test/CompileTests/MultiModule \
196198
`(find Protos/CompileTests/MultiModule -type f -name "*.proto")`
199+
@mkdir -p _test/CompileTests/InternalImportsByDefault
200+
${GENERATE_SRCS} \
201+
-I Protos/CompileTests/InternalImportsByDefault \
202+
--tfiws_opt=Visibility=Public \
203+
--tfiws_opt=UseAccessLevelOnImports=true \
204+
--tfiws_out=_test/CompileTests/InternalImportsByDefault \
205+
`(find Protos/CompileTests/InternalImportsByDefault -type f -name "*.proto")`
197206
diff -ru _test Reference
198207

199208
# Test the SPM plugin.
200209
test-spm-plugin:
201210
env PROTOC_PATH=$(shell realpath ${PROTOC}) ${SWIFT} test --package-path PluginExamples
202211

203-
compile-tests: compile-tests-multimodule
212+
compile-tests: \
213+
compile-tests-multimodule \
214+
compile-tests-internalimportsbydefault
204215

205-
# Test that ensure generating public into multiple modules with `import public`
216+
# Test that ensures generating public into multiple modules with `import public`
206217
# yields buildable code.
207218
compile-tests-multimodule:
208219
${SWIFT} test --package-path CompileTests/MultiModule
209220

221+
# Test that ensures that using access level modifiers on imports yields code that's buildable
222+
# when `InternalImportsByDefault` is enabled on the module.
223+
compile-tests-internalimportsbydefault:
224+
env PROTOC_PATH=$(shell realpath ${PROTOC}) ${SWIFT} build --package-path CompileTests/InternalImportsByDefault
225+
210226

211227
# Rebuild the reference files by running the local version of protoc-gen-swift
212228
# against our menagerie of sample protos.
@@ -240,6 +256,13 @@ reference: build ${PROTOC_GEN_SWIFT}
240256
--tfiws_opt=ProtoPathModuleMappings=Protos/CompileTests/MultiModule/module_mappings.pbascii \
241257
--tfiws_out=Reference/CompileTests/MultiModule \
242258
`(find Protos/CompileTests/MultiModule -type f -name "*.proto")`
259+
@mkdir -p Reference/CompileTests/InternalImportsByDefault
260+
${GENERATE_SRCS} \
261+
-I Protos/CompileTests/InternalImportsByDefault \
262+
--tfiws_opt=Visibility=Public \
263+
--tfiws_opt=UseAccessLevelOnImports=true \
264+
--tfiws_out=Reference/CompileTests/InternalImportsByDefault \
265+
`(find Protos/CompileTests/InternalImportsByDefault -type f -name "*.proto")`
243266

244267
#
245268
# Rebuild the generated .pb.swift test files by running
@@ -494,10 +517,12 @@ regenerate-conformance-protos: build ${PROTOC_GEN_SWIFT}
494517
`find Protos/Conformance -type f -name "*.proto"`
495518

496519
# Rebuild just the protos used by the CompileTests.
497-
regenerate-compiletests-protos: regenerate-compiletests-multimodule-protos
520+
regenerate-compiletests-protos: \
521+
regenerate-compiletests-multimodule-protos \
522+
copy-compiletests-internalimportsbydefault-protos
498523

499524
# Update the CompileTests/MultiModule files.
500-
# NOTE: Any changes here must be done of the "test-plugin" target so it
525+
# NOTE: Any changes here must also be done on the "test-plugin" target so it
501526
# generates in the same way.
502527
regenerate-compiletests-multimodule-protos: build ${PROTOC_GEN_SWIFT}
503528
find CompileTests/MultiModule -name "*.pb.swift" -exec rm -f {} \;
@@ -508,6 +533,12 @@ regenerate-compiletests-multimodule-protos: build ${PROTOC_GEN_SWIFT}
508533
--tfiws_out=CompileTests/MultiModule \
509534
`(find Protos/CompileTests/MultiModule -type f -name "*.proto")`
510535

536+
# We use the plugin for the InternalImportsByDefault test, so we don't actually need to regenerate
537+
# anything. However, to keep the protos centralised in a single place (the Protos directory),
538+
# this simply copies those files to the InternalImportsByDefault package in case they change.
539+
copy-compiletests-internalimportsbydefault-protos:
540+
@cp Protos/CompileTests/InternalImportsByDefault/* CompileTests/InternalImportsByDefault/Sources/InternalImportsByDefault/Protos
541+
511542
# Helper to check if there is a protobuf checkout as expected.
512543
check-for-protobuf-checkout:
513544
@if [ ! -d "${GOOGLE_PROTOBUF_CHECKOUT}/src/google/protobuf" ]; then \

‎Protos/CompileTests/InternalImportsByDefault/ServiceOnly.proto

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// This proto file should generate an empty file, since the plugin will ignore
2+
// service definitions.
3+
// This is here to make sure we don't import Foundation or SwiftProtobuf when
4+
// it's not necessary.
5+
6+
service SomeService {
7+
}

‎Protos/CompileTests/InternalImportsByDefault/SomeProtoWithBytes.proto

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// This proto will generate a Swift file that imports Foundation, because it
2+
// defines a bytes field.
3+
// Because InternalImportsByDefault is enabled on this module and we generate
4+
// protos with public visibility, the build will fail if the access level
5+
// modifier is missing (or wrong) since it will default the import to `internal`
6+
// and cause a conflict of access levels, since the `someBytes` property defined
7+
// on the message will be public.
8+
9+
message SomeProtoWithBytes {
10+
optional bytes someBytes = 2;
11+
optional string ext_str = 100;
12+
}

‎Reference/CompileTests/InternalImportsByDefault/ServiceOnly.pb.swift

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// DO NOT EDIT.
2+
// swift-format-ignore-file
3+
// swiftlint:disable all
4+
//
5+
// Generated by the Swift generator plugin for the protocol buffer compiler.
6+
// Source: ServiceOnly.proto
7+
//
8+
// For information on using the generated types, please see the documentation:
9+
// https://github.com/apple/swift-protobuf/
10+
11+
// This file contained no messages, enums, or extensions.

‎Reference/CompileTests/InternalImportsByDefault/SomeProtoWithBytes.pb.swift

+97
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// DO NOT EDIT.
2+
// swift-format-ignore-file
3+
// swiftlint:disable all
4+
//
5+
// Generated by the Swift generator plugin for the protocol buffer compiler.
6+
// Source: SomeProtoWithBytes.proto
7+
//
8+
// For information on using the generated types, please see the documentation:
9+
// https://github.com/apple/swift-protobuf/
10+
11+
public import Foundation
12+
public import SwiftProtobuf
13+
14+
// If the compiler emits an error on this type, it is because this file
15+
// was generated by a version of the `protoc` Swift plug-in that is
16+
// incompatible with the version of SwiftProtobuf to which you are linking.
17+
// Please ensure that you are building against the same version of the API
18+
// that was used to generate this file.
19+
fileprivate struct _GeneratedWithProtocGenSwiftVersion: SwiftProtobuf.ProtobufAPIVersionCheck {
20+
struct _2: SwiftProtobuf.ProtobufAPIVersion_2 {}
21+
typealias Version = _2
22+
}
23+
24+
public struct SomeProtoWithBytes: @unchecked Sendable {
25+
// SwiftProtobuf.Message conformance is added in an extension below. See the
26+
// `Message` and `Message+*Additions` files in the SwiftProtobuf library for
27+
// methods supported on all messages.
28+
29+
public var someBytes: Data {
30+
get {return _someBytes ?? Data()}
31+
set {_someBytes = newValue}
32+
}
33+
/// Returns true if `someBytes` has been explicitly set.
34+
public var hasSomeBytes: Bool {return self._someBytes != nil}
35+
/// Clears the value of `someBytes`. Subsequent reads from it will return its default value.
36+
public mutating func clearSomeBytes() {self._someBytes = nil}
37+
38+
public var extStr: String {
39+
get {return _extStr ?? String()}
40+
set {_extStr = newValue}
41+
}
42+
/// Returns true if `extStr` has been explicitly set.
43+
public var hasExtStr: Bool {return self._extStr != nil}
44+
/// Clears the value of `extStr`. Subsequent reads from it will return its default value.
45+
public mutating func clearExtStr() {self._extStr = nil}
46+
47+
public var unknownFields = SwiftProtobuf.UnknownStorage()
48+
49+
public init() {}
50+
51+
fileprivate var _someBytes: Data? = nil
52+
fileprivate var _extStr: String? = nil
53+
}
54+
55+
// MARK: - Code below here is support for the SwiftProtobuf runtime.
56+
57+
extension SomeProtoWithBytes: SwiftProtobuf.Message, SwiftProtobuf._MessageImplementationBase, SwiftProtobuf._ProtoNameProviding {
58+
public static let protoMessageName: String = "SomeProtoWithBytes"
59+
public static let _protobuf_nameMap: SwiftProtobuf._NameMap = [
60+
2: .same(proto: "someBytes"),
61+
100: .standard(proto: "ext_str"),
62+
]
63+
64+
public mutating func decodeMessage<D: SwiftProtobuf.Decoder>(decoder: inout D) throws {
65+
while let fieldNumber = try decoder.nextFieldNumber() {
66+
// The use of inline closures is to circumvent an issue where the compiler
67+
// allocates stack space for every case branch when no optimizations are
68+
// enabled. https://github.com/apple/swift-protobuf/issues/1034
69+
switch fieldNumber {
70+
case 2: try { try decoder.decodeSingularBytesField(value: &self._someBytes) }()
71+
case 100: try { try decoder.decodeSingularStringField(value: &self._extStr) }()
72+
default: break
73+
}
74+
}
75+
}
76+
77+
public func traverse<V: SwiftProtobuf.Visitor>(visitor: inout V) throws {
78+
// The use of inline closures is to circumvent an issue where the compiler
79+
// allocates stack space for every if/case branch local when no optimizations
80+
// are enabled. https://github.com/apple/swift-protobuf/issues/1034 and
81+
// https://github.com/apple/swift-protobuf/issues/1182
82+
try { if let v = self._someBytes {
83+
try visitor.visitSingularBytesField(value: v, fieldNumber: 2)
84+
} }()
85+
try { if let v = self._extStr {
86+
try visitor.visitSingularStringField(value: v, fieldNumber: 100)
87+
} }()
88+
try unknownFields.traverse(visitor: &visitor)
89+
}
90+
91+
public static func ==(lhs: SomeProtoWithBytes, rhs: SomeProtoWithBytes) -> Bool {
92+
if lhs._someBytes != rhs._someBytes {return false}
93+
if lhs._extStr != rhs._extStr {return false}
94+
if lhs.unknownFields != rhs.unknownFields {return false}
95+
return true
96+
}
97+
}

0 commit comments

Comments
 (0)