Fix exception thrown when two classes with the same name (but not the same fully-qualified name) are registered in a Schema

[abidon]

EDIT: There is no crash, only an exception thrown

Description

This fix aims to mitigate exception thrown when registering two types that have the same name but not the same fully qualified name.

enum UserView {
  struct Public {
    // ...
  }

  struct Private {
    // ...
  }
}

enum OrganizationView {
  struct Public {
    // ...
  }
}

Since the actual implementation of AnyType.hashValue uses String(describing:) to get the type name, both String(describing: UserView.Public.self) and String(describing: OrganizationView.Public.self) would return "Public": https://github.com/GraphQLSwift/Graphiti/blob/927ebb174825cf8188190fb9eb38d9de84806256/Sources/Graphiti/Definition/AnyType.swift#L8-L10

The same happen for types that aren't nested but in different modules – such as two libraries/frameworks/SPM-targets – as long as the type name themselves are the same.

Proposed solution

With my limited knowledge of the Graphiti codebase, I can see two ways to fix this (always happy to discuss a better implementation with guidance from the maintainers):

1. Use String(reflecting:) to compute the hash code (which returns the fully qualified class name, i.e. "NameOfTheSwiftModule.UserView.Public").
2. Change the equality operator implementation to compare the types instead of the hash code.

I preferred solution 2 as the String(reflecting:) documentation states suitable for debugging. Therefore I avoid building production ready solutions on this.

Swift version

swift-driver version: 1.109.2 Apple Swift version 6.0 (swiftlang-6.0.0.3.300 clang-1600.0.20.10) Target: arm64-apple-macosx15.0

[paulofaria]

The preferred way to deal with this is to manually disambiguate de type names by explicitly setting the names in the initializers. Creating the name from the type name is just a convenience.

[paulofaria]

Oh, I just realized there's a crash involved. We should probably throw an error instead with a nice error message explaining what happened and suggesting the user to disambiguate the names manually by explicitly setting them through the initializers.

[abidon]

Do you mean this initializer (and its overloads) ?

https://github.com/GraphQLSwift/Graphiti/blob/927ebb174825cf8188190fb9eb38d9de84806256/Sources/Graphiti/Type/Type.swift#L109-L115

Because I specify the type name using the as name: String and it still throws, since it is not used in Type. Let me know if there's something I missed – this is an excerpt of the Schema<> object:

  Type(UserView.Public.self, as: "UserView_Public") {
    Field("id", at: \.id)
    Field("displayName", at: \.displayName)
  }

  Type(
      UserView.Private.self, as: "UserView_Private"
  ) {
      Field("id", at: \.id)
      Field("displayName", at: \.displayName)
      Field("mainEmail", at: \.mainEmail)
      Field("createdAt", at: \.createdAt)
      Field("updatedAt", at: \.updatedAt)
  }

  Type(EmailView.Private.self, as: "EmailView_Private") {
      Field("address", at: \.address)
      Field("optInNotifications", at: \.optInNotifications)
  }

It's basically the whole schema for now (if we omit for a Scalar(Date.self) and the queries/mutations).

---

The last Type(EmailView.Private.self) declaration throws the following error:

Duplicate type registration for GraphQL type name "EmailView_Private" while trying to register type Private

here in the code:

https://github.com/GraphQLSwift/Graphiti/blob/927ebb174825cf8188190fb9eb38d9de84806256/Sources/Graphiti/Definition/TypeProvider.swift#L13-L27

I placed a breakpoint on the throw statement inside the guard block, and after inspecting the value of graphQLNameMap I couldn't find EmailView_Private. See by yourself:

(lldb) po graphQLNameMap
▿ 7 elements
  ▿ 0 : 2 elements
    ▿ key : <AnyType: 0x6000007c30a0>
    - value : "String"
  ▿ 1 : 2 elements
    ▿ key : <AnyType: 0x6000007c3080>
    - value : "Float"
  ▿ 2 : 2 elements
    ▿ key : <AnyType: 0x6000007c3640>
    - value : "DateTime"
  ▿ 3 : 2 elements
    ▿ key : <AnyType: 0x6000007c30c0>
    - value : "Boolean"
  ▿ 4 : 2 elements
    ▿ key : <AnyType: 0x6000007c3660>
    - value : "UserView_Public"
  ▿ 5 : 2 elements
    ▿ key : <AnyType: 0x6000007c3680>
    - value : "UserView_Private"
  ▿ 6 : 2 elements
    ▿ key : <AnyType: 0x6000007c3060>
    - value : "Int"

The name parameter is not used as a key. The AnyType instance is. That's why no matter if I specify a name or not, it still throws.

---

I've been tempted to refactor the code to use name as the key of graphQLNameMap, or maybe forward the name to the AnyType initializer and use it in the hash computation, but it's my first dive in Graphiti's code base so... I proceed cautiously 😅

[abidon]

Oh, I just realized there's a crash involved. We should probably throw an error instead.

Sorry, my bad, there's no crash in Graphiti ! We had a try! on our side. I clarified and edited the title and description of the issue to better reflect what the problem really is.

[abidon]

Tests are all green on my local environment

Tests passing on a Mac Studio M1 Ultra running macOS 15.0 beta 1 and Xcode 16.0 beta 1

@paulofaria Is there anything specific in the CI environment that could cause the tests to crash?

I also think it might be the first PR that runs tests using Xcode 16. I checked the last merged PR checks and the CI ran with Xcode 15.4.
It shouldn't cause any trouble since I ran them successfully with latest macOS/Xcode/Swift toolchain. But maybe the GitHub action doesn't fully support it yet ?

[NeedleInAJayStack]

Ah yeah, I've validated that this CI fails on main with the same error, so it doesn't appear to be related to your changes: https://github.com/GraphQLSwift/Graphiti/actions/runs/9586143166/job/26433471298?pr=138

[NeedleInAJayStack]

OK, yeah you were exactly right on the v16 XCode version. We were using XCode latest (potentially unstable), which resulted in v16.0.0, whereas using latest-stable results in v15.4.0 and passes. I have an MR to set it to latest-stable here

From this (totally official) site, it looks like XCode 16 is using Swift 6, which would explain some significant behavior changes. We've got a bit of work to do to become Swift 6 compatible, but that's a separate effort.

Once the MR linked above is merged, feel free to rebase on it and we'll get these changes in.

Thanks for contributing!!

[NeedleInAJayStack]

Alright the linked PR is merged. You should be good to rebase!

[abidon]

@NeedleInAJayStack Rebased! 😊

[NeedleInAJayStack]

@abidon This change has been released in tag 1.15.1: https://github.com/GraphQLSwift/Graphiti/releases/tag/1.15.1