soto-project / soto

Swift SDK for AWS that works on Linux, macOS and iOS
https://soto.codes
Apache License 2.0
877 stars 83 forks source link

Prefix all Service Libraries with `AWS` #218

Closed fabianfett closed 4 years ago

fabianfett commented 4 years ago

Original title: Namespaces don't work if Facade struct has same name as Library

Origin of the issue: https://github.com/fabianfett/swift-lambda-runtime/pull/25

The code that breaks

import NIO
import DynamoDB
import AWSSDKSwiftCore
import LambdaRuntime

func configureDynamoDB(group: EventLoopGroup, environment: Environment) -> DynamoDB {
    return DynamoDB(
         accessKeyId: environment.accessKeyId,
         secretAccessKey: environment.secretAccessKey,
         sessionToken: environment.sessionToken,
         region: Region(rawValue: environment.region),
         eventLoopGroupProvider: .shared(group)
    )
}

LambdaRuntime must be imported because of the Environment type. Since LambdaRuntime also holds a DynamoDB type, the compiler emits an error:

'DynamoDB' is ambiguous for type lookup in this context

There are two issues here:

LambdaRuntime Issue

Since the package LambdaRuntime has the same name as the class LambdaRuntime.DynamoDB the compiler can not differentiate between the namespace and the class. Trying to access: LambdaRuntime.DynamoDB leads to the following error:

'DynamoDB' is not a member type of 'LambdaRuntime'

For that reason, I will rename LambdaRuntime the class to Runtime. With this LambdaRuntime always refers to the package/namespace and not a struct. This is already in progress with:

https://github.com/fabianfett/swift-lambda-runtime/pull/26

With this fix done, we can now access LambdaRuntime.DynamoDB without problems.

AWSSDKSwift Issue

Even though we can specify LambdaRuntime.DynamoDB we still can not specify DynamoDB.DynamoDB since we still can't access the namespace. The whole Swift namespace is broken as long as a type has the same name as the library. IMHO we should start a rename here to prevent those naming collisions where one can not fallback on the namespaces. I know this is breaking. But it's also a great feature to not fall apart in the moment of naming collisions.

import NIO
import DynamoDB
import AWSSDKSwiftCore
import LambdaRuntime

func configureDynamoDB(group: EventLoopGroup, environment: Environment) -> DynamoDB {
    return DynamoDB(
         accessKeyId: environment.accessKeyId,
         secretAccessKey: environment.secretAccessKey,
         sessionToken: environment.sessionToken,
         region: Region(rawValue: environment.region),
         eventLoopGroupProvider: .shared(group)
    )
}

For easy reproduction this is the Package.swift I used.

// swift-tools-version:5.1
// The swift-tools-version declares the minimum version of Swift required to build this package.

import PackageDescription

let package = Package(
    name: "LambdaEventsTest",
    products: [
      .executable(name: "LambdaEventsTest", targets: ["LambdaEventsTest"])
    ],
    dependencies: [
        .package(url: "https://github.com/apple/swift-nio.git", .upToNextMajor(from: "2.9.0")),
        .package(url: "https://github.com/swift-aws/aws-sdk-swift.git", .upToNextMajor(from: "4.0.0")),
        .package(url: "https://github.com/fabianfett/swift-lambda-runtime", .upToNextMajor(from: "0.4.0")),
    ],
    targets: [
        .target(
            name: "LambdaEventsTest",
            dependencies: ["DynamoDB", "NIO", "LambdaRuntime"]),
        .testTarget(
            name: "LambdaEventsTestTests",
            dependencies: ["LambdaEventsTest"]),
    ]
)

This issue went undiscovered since I haven't used both frameworks in a single file yet.

fabianfett commented 4 years ago

Verified the issue with @weissi on vapor discord chat:

Yes That’s why in the sswg we try to make sure that module and type names never clash It’s unfortunate though :sob: Maybe you want to file a Swift bug, there might already be one but I don’t think so

fabianfett commented 4 years ago

Also this resolves name clashes with: https://github.com/LiveUI/S3

adam-fowler commented 4 years ago

fixed in #225