From 426fb00b54a9de57ead45d4f8bac35ee2c1a18ad Mon Sep 17 00:00:00 2001 From: rcancro Date: Fri, 6 Sep 2024 15:15:29 -0700 Subject: [PATCH 1/2] [Duplicate imports] Fix issue that would cause duplicate imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the given schema file, we can see the class references itself: ``` { "id": "board_activity.json", "title": "board_activity", "description": "Schema definition of a BoardActivity", "$schema": "http://json-schema.org/schema#", "type": "object", "properties": { "ref_board_activity": { "$ref": "board_activity.json" }, }, } ``` This would cause multiple `BoardActivity` imports in `BoardActivity.m`. This was caused by calling `ObjCIR.Root.imports` `renderImplementation` method. This is how the imports were being generated: ``` case .imports(let classNames, let myName, _): return [classNames.union(Set([myName])) .sorted() .map { $0.trimmingCharacters(in: .whitespaces) } .map { ObjCIR.fileImportStmt($0, headerPrefix: params[GenerationParameterType.headerPrefix]) } .joined(separator: "\n")] ``` Using the schema above, `classNames` would include `“BoardActivity “`. Note the space. `myName` would be `”BoardActivity”`. Note the lack of space. These two get unioned together in a set. The result is `[“BoardActivity ", "BoardActivity”]`. We sort this set, then remove any trailing spaces. But at this point it is too late! We are no longer a set, we are an array which can have multiple of equivalent objects. At this point our array is `[“BoardActivity", "BoardActivity”]`. This leads to duplicate import statements. A solution would be to trim these strings BEFORE the union: ``` case .imports(let classNames, let myName, _): let strippedClassNames = Set(classNames.map { $0.trimmingCharacters(in: .whitespaces) }) return [strippedClassNames.union(Set([myName.trimmingCharacters(in: .whitespaces)])) .sorted() .map { ObjCIR.fileImportStmt($0, headerPrefix: params[GenerationParameterType.headerPrefix]) } .joined(separator: "\n")] ``` I created `strippedClassNames` which holds the trimmed `classNames`. This gets unioned that with the stripped version of `myName` to get the list of imports. Now we end up with the array only have one entry `[“BoardActivity”]`. Note: I also made a change in `ObjectiveCNSCodingExtension.swift` because the compiler was telling me that the return value was too complicated for it to figure out. --- Sources/Core/ObjectiveCIR.swift | 4 +-- .../Core/ObjectiveCNSCodingExtension.swift | 33 ++++++++++--------- Tests/CoreTests/ObjectiveCIRTests.swift | 8 +++++ 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/Sources/Core/ObjectiveCIR.swift b/Sources/Core/ObjectiveCIR.swift index 0c452adf..0c89b455 100644 --- a/Sources/Core/ObjectiveCIR.swift +++ b/Sources/Core/ObjectiveCIR.swift @@ -424,9 +424,9 @@ public struct ObjCIR { // skip macro in impl return [] case .imports(let classNames, let myName, _): - return [classNames.union(Set([myName])) + let strippedClassNames = Set(classNames.map { $0.trimmingCharacters(in: .whitespaces) }) + return [strippedClassNames.union(Set([myName.trimmingCharacters(in: .whitespaces)])) .sorted() - .map { $0.trimmingCharacters(in: .whitespaces) } .map { ObjCIR.fileImportStmt($0, headerPrefix: params[GenerationParameterType.headerPrefix]) } .joined(separator: "\n")] case .classDecl(name: let className, extends: _, methods: let methods, properties: _, protocols: let protocols): diff --git a/Sources/Core/ObjectiveCNSCodingExtension.swift b/Sources/Core/ObjectiveCNSCodingExtension.swift index a43aacc1..d48bdd9b 100644 --- a/Sources/Core/ObjectiveCNSCodingExtension.swift +++ b/Sources/Core/ObjectiveCNSCodingExtension.swift @@ -11,7 +11,7 @@ import Foundation extension ObjCModelRenderer { func renderInitWithCoder() -> ObjCIR.Method { return ObjCIR.method("- (instancetype)initWithCoder:(NSCoder *)aDecoder") { - [ + let body: [String] = [ self.isBaseClass ? ObjCIR.ifStmt("!(self = [super init])") { ["return self;"] } : "if (!(self = [super initWithCoder:aDecoder])) { return self; }", self.properties.filter { (_, schema) -> Bool in @@ -20,21 +20,22 @@ extension ObjCModelRenderer { .map(decodeStatement) .joined(separator: "\n"), ] + - self.properties.filter { (_, schema) -> Bool in - schema.schema.isBoolean() - }.map { (arg: (Parameter, SchemaObjectProperty)) -> String in - let (param, _) = arg - return "_\(booleanPropertiesIVarName).\(booleanPropertyOption(propertyName: param, className: self.className)) = [aDecoder decodeBoolForKey:\(param.objcLiteral())] & 0x1;" - } + [ - self.properties.map { (param, _) -> String in - "_\(dirtyPropertiesIVarName).\(dirtyPropertyOption(propertyName: param, className: self.className)) = [aDecoder decodeIntForKey:\((param + "_dirty_property").objcLiteral())] & 0x1;" - }.joined(separator: "\n"), - - ObjCIR.ifStmt("[self class] == [\(self.className) class]") { - [renderPostInitNotification(type: "PlankModelInitTypeDefault")] - }, - "return self;", - ] + self.properties.filter { (_, schema) -> Bool in + schema.schema.isBoolean() + }.map { (arg: (Parameter, SchemaObjectProperty)) -> String in + let (param, _) = arg + return "_\(booleanPropertiesIVarName).\(booleanPropertyOption(propertyName: param, className: self.className)) = [aDecoder decodeBoolForKey:\(param.objcLiteral())] & 0x1;" + } + [ + self.properties.map { (param, _) -> String in + "_\(dirtyPropertiesIVarName).\(dirtyPropertyOption(propertyName: param, className: self.className)) = [aDecoder decodeIntForKey:\((param + "_dirty_property").objcLiteral())] & 0x1;" + }.joined(separator: "\n"), + + ObjCIR.ifStmt("[self class] == [\(self.className) class]") { + [renderPostInitNotification(type: "PlankModelInitTypeDefault")] + }, + "return self;", + ] + return body } } diff --git a/Tests/CoreTests/ObjectiveCIRTests.swift b/Tests/CoreTests/ObjectiveCIRTests.swift index 896eb1a0..fa285c5c 100644 --- a/Tests/CoreTests/ObjectiveCIRTests.swift +++ b/Tests/CoreTests/ObjectiveCIRTests.swift @@ -188,4 +188,12 @@ class ObjectiveCIRTests: XCTestCase { XCTAssertEqual("#import \"include/ModelClass.h\"", ObjCIR.fileImportStmt("ModelClass", headerPrefix: "include/")) XCTAssertEqual("#import \"ModelClass.h\"", ObjCIR.fileImportStmt("ModelClass", headerPrefix: nil)) } + + func testImportsImplementationWithTrailingSpace() { + let imports = ObjCIR.Root.imports(classNames: Set(["MyClass "]), myName: "MyClass", parentName: nil) + let output = imports.renderImplementation(GenerationParameters()) + XCTAssertEqual(1, output.count) + XCTAssertEqual("#import \"MyClass.h\"", output[0]) + + } } From 9fa9a7e52d8e987273ba7d04afb6c6c97cb02a07 Mon Sep 17 00:00:00 2001 From: rcancro Date: Fri, 6 Sep 2024 15:42:09 -0700 Subject: [PATCH 2/2] swift format --- .../Core/ObjectiveCNSCodingExtension.swift | 30 +++++++++---------- Tests/CoreTests/ObjectiveCIRTests.swift | 3 +- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/Sources/Core/ObjectiveCNSCodingExtension.swift b/Sources/Core/ObjectiveCNSCodingExtension.swift index d48bdd9b..c284e9a1 100644 --- a/Sources/Core/ObjectiveCNSCodingExtension.swift +++ b/Sources/Core/ObjectiveCNSCodingExtension.swift @@ -20,21 +20,21 @@ extension ObjCModelRenderer { .map(decodeStatement) .joined(separator: "\n"), ] + - self.properties.filter { (_, schema) -> Bool in - schema.schema.isBoolean() - }.map { (arg: (Parameter, SchemaObjectProperty)) -> String in - let (param, _) = arg - return "_\(booleanPropertiesIVarName).\(booleanPropertyOption(propertyName: param, className: self.className)) = [aDecoder decodeBoolForKey:\(param.objcLiteral())] & 0x1;" - } + [ - self.properties.map { (param, _) -> String in - "_\(dirtyPropertiesIVarName).\(dirtyPropertyOption(propertyName: param, className: self.className)) = [aDecoder decodeIntForKey:\((param + "_dirty_property").objcLiteral())] & 0x1;" - }.joined(separator: "\n"), - - ObjCIR.ifStmt("[self class] == [\(self.className) class]") { - [renderPostInitNotification(type: "PlankModelInitTypeDefault")] - }, - "return self;", - ] + self.properties.filter { (_, schema) -> Bool in + schema.schema.isBoolean() + }.map { (arg: (Parameter, SchemaObjectProperty)) -> String in + let (param, _) = arg + return "_\(booleanPropertiesIVarName).\(booleanPropertyOption(propertyName: param, className: self.className)) = [aDecoder decodeBoolForKey:\(param.objcLiteral())] & 0x1;" + } + [ + self.properties.map { (param, _) -> String in + "_\(dirtyPropertiesIVarName).\(dirtyPropertyOption(propertyName: param, className: self.className)) = [aDecoder decodeIntForKey:\((param + "_dirty_property").objcLiteral())] & 0x1;" + }.joined(separator: "\n"), + + ObjCIR.ifStmt("[self class] == [\(self.className) class]") { + [renderPostInitNotification(type: "PlankModelInitTypeDefault")] + }, + "return self;", + ] return body } } diff --git a/Tests/CoreTests/ObjectiveCIRTests.swift b/Tests/CoreTests/ObjectiveCIRTests.swift index fa285c5c..c7295051 100644 --- a/Tests/CoreTests/ObjectiveCIRTests.swift +++ b/Tests/CoreTests/ObjectiveCIRTests.swift @@ -188,12 +188,11 @@ class ObjectiveCIRTests: XCTestCase { XCTAssertEqual("#import \"include/ModelClass.h\"", ObjCIR.fileImportStmt("ModelClass", headerPrefix: "include/")) XCTAssertEqual("#import \"ModelClass.h\"", ObjCIR.fileImportStmt("ModelClass", headerPrefix: nil)) } - + func testImportsImplementationWithTrailingSpace() { let imports = ObjCIR.Root.imports(classNames: Set(["MyClass "]), myName: "MyClass", parentName: nil) let output = imports.renderImplementation(GenerationParameters()) XCTAssertEqual(1, output.count) XCTAssertEqual("#import \"MyClass.h\"", output[0]) - } }