Merge pull request #219 from cashapp/skorulis/getter-error

Throw an error if getter-named is redundant
cashapp · Dec 13, 2024 · 7ee3dde · 7ee3dde
2 parents 4bd2a6c + 1e0544c
commit 7ee3dde
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 2 deletions.
diff --git a/Sources/KnitCodeGen/FunctionCallRegistrationParsing.swift b/Sources/KnitCodeGen/FunctionCallRegistrationParsing.swift
@@ -91,6 +91,10 @@ extension FunctionCallExprSyntax {
             return ([], [])
         }
 
+        if primaryRegistration.hasRedundantGetter {
+            throw RegistrationParsingError.redundantGetter(syntax: self)
+        }
+
         let implementsCalledMethods = calledMethods.filter { method in
             method.calledExpression.declName.baseName.text == Registration.FunctionName.implements.rawValue
         }
@@ -109,6 +113,9 @@ extension FunctionCallExprSyntax {
                 leadingTrivia: leadingTrivia,
                 functionName: .implements
             ) {
+                if forwardedRegistration.hasRedundantGetter {
+                    throw RegistrationParsingError.redundantGetter(syntax: implementsCalledMethod.calledExpression)
+                }
                 forwardedRegistrations.append(forwardedRegistration)
             }
         }
@@ -331,6 +338,7 @@ enum RegistrationParsingError: LocalizedError, SyntaxError {
     case invalidIfConfig(syntax: SyntaxProtocol, text: String)
     case nestedIfConfig(syntax: SyntaxProtocol)
     case nonAbstract(syntax: SyntaxProtocol)
+    case redundantGetter(syntax: SyntaxProtocol)
 
     var errorDescription: String? {
         switch self {
@@ -348,6 +356,8 @@ enum RegistrationParsingError: LocalizedError, SyntaxError {
             return "Nested #if statements are not supported"
         case .nonAbstract:
             return "AbstractAssemblys may only contain Abstract registrations"
+        case .redundantGetter:
+            return "getter-named matches the default accessor name and can be removed"
         }
     }
 
@@ -359,7 +369,8 @@ enum RegistrationParsingError: LocalizedError, SyntaxError {
             let .unwrappedClosureParams(syntax),
             let .invalidIfConfig(syntax, _),
             let .nestedIfConfig(syntax),
-            let .nonAbstract(syntax):
+            let .nonAbstract(syntax),
+            let .redundantGetter(syntax):
             return syntax
         }
     }

diff --git a/Sources/KnitCodeGen/Registration.swift b/Sources/KnitCodeGen/Registration.swift
@@ -59,6 +59,17 @@ public struct Registration: Equatable, Codable, Sendable {
         case service, name, accessLevel, arguments, getterConfig, functionName, concurrencyModifier, spi
     }
 
+    var namedGetterConfig: GetterConfig? {
+        getterConfig.first(where: { $0.isNamed })
+    }
+
+    var hasRedundantGetter: Bool {
+        guard let namedGetterConfig, case let GetterConfig.identifiedGetter(name) = namedGetterConfig else {
+            return false
+        }
+        return TypeNamer.computedIdentifierName(type: service) == name
+    }
+
 }
 
 extension Registration {

diff --git a/Sources/KnitCodeGen/TypeSafetySourceFile.swift b/Sources/KnitCodeGen/TypeSafetySourceFile.swift
@@ -27,7 +27,7 @@ public enum TypeSafetySourceFile {
                     if registration.getterConfig.contains(.callAsFunction) {
                         try makeResolver(registration: registration, getterType: .callAsFunction)
                     }
-                    if let namedGetter = registration.getterConfig.first(where: { $0.isNamed }) {
+                    if let namedGetter = registration.namedGetterConfig {
                         try makeResolver(registration: registration, getterType: namedGetter)
                     }
                 }

diff --git a/Tests/KnitCodeGenTests/AssemblyParsingTests.swift b/Tests/KnitCodeGenTests/AssemblyParsingTests.swift
@@ -853,6 +853,53 @@ final class AssemblyParsingTests: XCTestCase {
         )
     }
 
+    func testRedundantGetterName() throws {
+        let sourceFile: SourceFileSyntax = """
+            class TestAssembly: ModuleAssembly {
+                typealias TargetResolver = TestResolver
+                typealias ReplacedAssembly = RealAssembly
+
+                func assemble(container: Container) {
+                    // @knit getter-named("a")
+                    container.register(A.self) { }
+                }
+            }
+            """
+
+        _ = try assertParsesSyntaxTree(sourceFile, assertErrorsToPrint: { errors in
+            XCTAssertEqual(errors.count, 1)
+            if case RegistrationParsingError.redundantGetter = errors[0] {
+                // Correct
+            } else {
+                XCTFail("Incorrect error case")
+            }
+        })
+    }
+
+    func testRedundantForwardedGetterName() throws {
+        let sourceFile: SourceFileSyntax = """
+            class TestAssembly: ModuleAssembly {
+                typealias TargetResolver = TestResolver
+                typealias ReplacedAssembly = RealAssembly
+
+                func assemble(container: Container) {
+                    container.register(A.self) { }
+                        // @knit getter-named("b")
+                        .implements(B.self)
+                }
+            }
+            """
+
+        _ = try assertParsesSyntaxTree(sourceFile, assertErrorsToPrint: { errors in
+            XCTAssertEqual(errors.count, 1)
+            if case RegistrationParsingError.redundantGetter = errors[0] {
+                // Correct
+            } else {
+                XCTFail("Incorrect error case")
+            }
+        })
+    }
+
 }
 
 private func assertParsesSyntaxTree(