diff --git a/PluginExamples/Package.swift b/PluginExamples/Package.swift index 46e7bc6ac..82327bbbf 100644 --- a/PluginExamples/Package.swift +++ b/PluginExamples/Package.swift @@ -66,6 +66,15 @@ let package = Package( .plugin(name: "SwiftProtobufPlugin", package: "swift-protobuf") ] ), + .target( + name: "PathToUnderscores", + dependencies: [ + .product(name: "SwiftProtobuf", package: "swift-protobuf") + ], + plugins: [ + .plugin(name: "SwiftProtobufPlugin", package: "swift-protobuf") + ] + ), .testTarget( name: "ExampleTests", dependencies: [ @@ -76,6 +85,7 @@ let package = Package( .target(name: "CustomProtoPath"), .product(name: "Nonexhaustive", package: "Subpackage"), .target(name: "UsesWKTs"), + .target(name: "PathToUnderscores"), ] ), ], diff --git a/PluginExamples/Package@swift-6.1.swift b/PluginExamples/Package@swift-6.1.swift index 92d4b26e5..9693f6eb5 100644 --- a/PluginExamples/Package@swift-6.1.swift +++ b/PluginExamples/Package@swift-6.1.swift @@ -65,6 +65,15 @@ let package = Package( .plugin(name: "SwiftProtobufPlugin", package: "swift-protobuf") ] ), + .target( + name: "PathToUnderscores", + dependencies: [ + .product(name: "SwiftProtobuf", package: "swift-protobuf") + ], + plugins: [ + .plugin(name: "SwiftProtobufPlugin", package: "swift-protobuf") + ] + ), .testTarget( name: "ExampleTests", dependencies: [ @@ -74,6 +83,7 @@ let package = Package( .target(name: "AccessLevelOnImport"), .target(name: "CustomProtoPath"), .target(name: "UsesWKTs"), + .target(name: "PathToUnderscores"), ] ), ], diff --git a/PluginExamples/Sources/ExampleTests/ExampleTests.swift b/PluginExamples/Sources/ExampleTests/ExampleTests.swift index 257197989..341f1304c 100644 --- a/PluginExamples/Sources/ExampleTests/ExampleTests.swift +++ b/PluginExamples/Sources/ExampleTests/ExampleTests.swift @@ -1,6 +1,7 @@ import CustomProtoPath import Import import Nested +import PathToUnderscores import Simple import UsesWKTs import XCTest @@ -61,4 +62,15 @@ final class ExampleTests: XCTestCase { XCTAssertEqual(usesWKTs.name, "UsesWKTs") XCTAssertEqual(usesWKTs.aTimestamp.seconds, 2) } + + // The two protos in this target share the file name Duplicate.proto in different + // subdirectories. The build only succeeds because the plugin generates them with + // PathToUnderscores naming, so they land in distinctly named files instead of + // colliding. This is the case reported in issue 1761. + func testPathToUnderscores() { + let foo = FooDuplicate.with { $0.name = "Foo" } + let bar = BarDuplicate.with { $0.name = "Bar" } + XCTAssertEqual(foo.name, "Foo") + XCTAssertEqual(bar.name, "Bar") + } } diff --git a/PluginExamples/Sources/PathToUnderscores/Proto/bar/Duplicate.proto b/PluginExamples/Sources/PathToUnderscores/Proto/bar/Duplicate.proto new file mode 100644 index 000000000..1c9cf72f4 --- /dev/null +++ b/PluginExamples/Sources/PathToUnderscores/Proto/bar/Duplicate.proto @@ -0,0 +1,5 @@ +syntax = "proto3"; + +message BarDuplicate { + string name = 1; +} diff --git a/PluginExamples/Sources/PathToUnderscores/Proto/foo/Duplicate.proto b/PluginExamples/Sources/PathToUnderscores/Proto/foo/Duplicate.proto new file mode 100644 index 000000000..105f664aa --- /dev/null +++ b/PluginExamples/Sources/PathToUnderscores/Proto/foo/Duplicate.proto @@ -0,0 +1,5 @@ +syntax = "proto3"; + +message FooDuplicate { + string name = 1; +} diff --git a/PluginExamples/Sources/PathToUnderscores/empty.swift b/PluginExamples/Sources/PathToUnderscores/empty.swift new file mode 100644 index 000000000..d4445d2a5 --- /dev/null +++ b/PluginExamples/Sources/PathToUnderscores/empty.swift @@ -0,0 +1,3 @@ +/// DO NOT DELETE. +/// +/// We need to keep this file otherwise the plugin is not running. diff --git a/PluginExamples/Sources/PathToUnderscores/swift-protobuf-config.json b/PluginExamples/Sources/PathToUnderscores/swift-protobuf-config.json new file mode 100644 index 000000000..4a6fc432e --- /dev/null +++ b/PluginExamples/Sources/PathToUnderscores/swift-protobuf-config.json @@ -0,0 +1,11 @@ +{ + "invocations": [ + { + "protoFiles": [ + "Proto/foo/Duplicate.proto", + "Proto/bar/Duplicate.proto", + ], + "visibility": "public" + } + ] +} diff --git a/Plugins/SwiftProtobufPlugin/plugin.swift b/Plugins/SwiftProtobufPlugin/plugin.swift index b0e5a8484..fc1f3c273 100644 --- a/Plugins/SwiftProtobufPlugin/plugin.swift +++ b/Plugins/SwiftProtobufPlugin/plugin.swift @@ -106,6 +106,12 @@ struct SwiftProtobufPlugin { /// The visibility of the generated files. var visibility: Visibility? /// The file naming strategy to use. + /// + /// The build plugin always generates files with `PathToUnderscores` naming. The + /// generated files live in the build directory, so the file name on disk is never + /// observed and there is no benefit to giving users a choice here. This field is kept + /// for backwards compatibility: it may be omitted or set to `PathToUnderscores`. Any + /// other value is rejected so an explicit setting is never silently ignored. var fileNaming: FileNaming? /// Whether internal imports should be annotated as `@_implementationOnly`. var implementationOnlyImports: Bool? @@ -222,11 +228,25 @@ struct SwiftProtobufPlugin { protocArgs.append("--swift_opt=Visibility=\(visibility.rawValue)") } - // Add the file naming if it was set - if let fileNaming = invocation.fileNaming { - protocArgs.append("--swift_opt=FileNaming=\(fileNaming.rawValue)") + // The build plugin always uses PathToUnderscores naming. Warn once per invocation on any + // other explicit value so the setting is never silently ignored, without breaking existing + // builds. + if let fileNaming = invocation.fileNaming, fileNaming != .pathToUnderscores { + Diagnostics.warning( + """ + The 'fileNaming' option '\(fileNaming.rawValue)' is ignored by the build plugin. The build \ + plugin always generates files using the 'PathToUnderscores' naming because the \ + generated files go into the build directory and the name is never observed. + """ + ) } + // Always generate with PathToUnderscores naming. The declared output paths below are + // derived the same way, so the names the build system expects can never drift from the + // names protoc-gen-swift actually writes. The configured fileNaming, if any, only triggers + // the warning above and does not change the naming used. + protocArgs.append("--swift_opt=FileNaming=PathToUnderscores") + // Add the implementation only imports flag if it was set if let implementationOnlyImports = invocation.implementationOnlyImports { protocArgs.append("--swift_opt=ImplementationOnlyImports=\(implementationOnlyImports)") @@ -244,17 +264,19 @@ struct SwiftProtobufPlugin { var inputFiles = [URL]() var outputFiles = [URL]() - for var file in invocation.protoFiles { + for file in invocation.protoFiles { // Append the file to the protoc args so that it is used for generating protocArgs.append(file) inputFiles.append(protoDirectory.appending(path: file)) - // The name of the output file is based on the name of the input file. - // We validated in the beginning that every file has the suffix of .proto - // This means we can just drop the last 5 elements and append the new suffix - file.removeLast(5) - file.append("pb.swift") - let protobufOutputPath = outputDirectory.appending(path: file) + // The output file name has to match exactly what protoc-gen-swift writes, otherwise + // the build system looks for a file that does not exist and the build fails. We always + // generate with PathToUnderscores naming, so the relative proto path becomes a single + // file name with the directory separators replaced by underscores. We validated up + // front that every file has the .proto suffix, which is dropped for .pb.swift. + let outputName = String(file.dropLast(".proto".count)) + .replacingOccurrences(of: "/", with: "_") + ".pb.swift" + let protobufOutputPath = outputDirectory.appending(path: outputName) // Add the outputPath as an output file outputFiles.append(protobufOutputPath)