From 046d2c61c9a722c53b79b1ef211bbbc8a86dae06 Mon Sep 17 00:00:00 2001 From: Niklas Ausborn Date: Mon, 6 Nov 2023 14:14:51 +0100 Subject: [PATCH 1/5] Fixed failing Codable in structs with static members --- .../Attributes/KeyPath/CodedAt.swift | 3 + .../Attributes/KeyPath/CodedIn.swift | 3 + .../Attributes/RegistrationAttribute.swift | 8 ++ .../StaticVariableDeclaration.swift | 73 +++++++++++++++++++ Tests/MetaCodableTests/CodableTests.swift | 41 +++++++++++ Tests/MetaCodableTests/CodedAtTests.swift | 28 +++++++ Tests/MetaCodableTests/CodedInTests.swift | 28 +++++++ 7 files changed, 184 insertions(+) create mode 100644 Sources/CodableMacroPlugin/Diagnostics/StaticVariableDeclaration.swift diff --git a/Sources/CodableMacroPlugin/Attributes/KeyPath/CodedAt.swift b/Sources/CodableMacroPlugin/Attributes/KeyPath/CodedAt.swift index 7761d18e3..e82589a68 100644 --- a/Sources/CodableMacroPlugin/Attributes/KeyPath/CodedAt.swift +++ b/Sources/CodableMacroPlugin/Attributes/KeyPath/CodedAt.swift @@ -37,6 +37,8 @@ struct CodedAt: PropertyAttribute { /// declaration. /// * Attached declaration is not a grouped variable /// declaration. + /// * Attached declaration is not a static variable + /// declaration /// * This attribute isn't used combined with `CodedIn` /// and `IgnoreCoding` attribute. /// @@ -44,6 +46,7 @@ struct CodedAt: PropertyAttribute { func diagnoser() -> DiagnosticProducer { return AggregatedDiagnosticProducer { attachedToUngroupedVariable() + attachedToNonStaticVariable() cantDuplicate() cantBeCombined(with: CodedIn.self) cantBeCombined(with: IgnoreCoding.self) diff --git a/Sources/CodableMacroPlugin/Attributes/KeyPath/CodedIn.swift b/Sources/CodableMacroPlugin/Attributes/KeyPath/CodedIn.swift index c047af298..f5a079a5f 100644 --- a/Sources/CodableMacroPlugin/Attributes/KeyPath/CodedIn.swift +++ b/Sources/CodableMacroPlugin/Attributes/KeyPath/CodedIn.swift @@ -48,6 +48,8 @@ struct CodedIn: PropertyAttribute { /// * Attached declaration is a variable declaration. /// * Macro usage is not duplicated for the same /// declaration. + /// * Attached declaration is not a static variable + /// declaration /// * This attribute isn't used combined with `CodedAt` /// and `IgnoreCoding` attribute. /// @@ -56,6 +58,7 @@ struct CodedIn: PropertyAttribute { return AggregatedDiagnosticProducer { expect(syntax: VariableDeclSyntax.self) cantDuplicate() + attachedToNonStaticVariable() cantBeCombined(with: CodedAt.self) cantBeCombined(with: IgnoreCoding.self) } diff --git a/Sources/CodableMacroPlugin/Attributes/RegistrationAttribute.swift b/Sources/CodableMacroPlugin/Attributes/RegistrationAttribute.swift index 059f5bb1f..fe053ba09 100644 --- a/Sources/CodableMacroPlugin/Attributes/RegistrationAttribute.swift +++ b/Sources/CodableMacroPlugin/Attributes/RegistrationAttribute.swift @@ -42,6 +42,14 @@ extension RegistrationAttribute { guard let decl = member.decl.as(VariableDeclSyntax.self) else { return } + // The Macro fails to compile if the decl.modifiers.contains + // is directly used in the guard statement. Otherwise it should + // be used as a second condition in guard block above. + let isStaticVar = decl.modifiers.contains{$0.name.tokenKind == .keyword(.static)} + guard !isStaticVar else { + return + } + // builder let builder = CodingKeys(from: declaration) diff --git a/Sources/CodableMacroPlugin/Diagnostics/StaticVariableDeclaration.swift b/Sources/CodableMacroPlugin/Diagnostics/StaticVariableDeclaration.swift new file mode 100644 index 000000000..0f519fbfc --- /dev/null +++ b/Sources/CodableMacroPlugin/Diagnostics/StaticVariableDeclaration.swift @@ -0,0 +1,73 @@ +@_implementationOnly import SwiftDiagnostics +@_implementationOnly import SwiftSyntax +@_implementationOnly import SwiftSyntaxMacros + +/// A diagnostic producer type that can validate passed syntax is not a static +/// variable declaration. +/// +/// This producer can be used for macro-attributes that must be attached to +/// non static variable declarations. +struct StaticVariableDeclaration: DiagnosticProducer { + /// The attribute for which + /// validation performed. + /// + /// Uses this attribute name + /// in generated diagnostic + /// messages. + let attr: Attr + + /// Creates a static variable declaration validation instance + /// with provided attribute. + /// + /// - Parameter attr: The attribute for which + /// validation performed. + /// - Returns: Newly created diagnostic producer. + init(_ attr: Attr) { + self.attr = attr + } + + /// Validates and produces diagnostics for the passed syntax + /// in the macro expansion context provided. + /// + /// Checks whether provided syntax is a non static variable declaration, + /// for static variable declarations error diagnostics + /// are generated. + /// + /// - Parameters: + /// - syntax: The syntax to validate and produce diagnostics for. + /// - context: The macro expansion context diagnostics produced in. + /// + /// - Returns: True if syntax fails validation, false otherwise. + @discardableResult + func produce( + for syntax: some SyntaxProtocol, + in context: some MacroExpansionContext + ) -> Bool { + // The Macro fails to compile if the .modifiers.contains + // is directly used in the guard statement. + let isStaticVar = syntax.as(VariableDeclSyntax.self)? + .modifiers.contains{$0.name.tokenKind == .keyword(.static)} + guard isStaticVar ?? false + else { return false } + let message = attr.node.diagnostic( + message: + "@\(attr.name) can't be used with static variables declarations", + id: attr.misuseMessageID, + severity: .error + ) + context.diagnose(attr: attr, message: message) + return true + } +} + +extension PropertyAttribute { + /// Indicates attribute must be attached to non static variable declaration. + /// + /// The created diagnostic producer produces error diagnostic, + /// if attribute is attached to static variable declarations. + /// + /// - Returns: Static variable declaration validation diagnostic producer. + func attachedToNonStaticVariable() -> StaticVariableDeclaration { + return .init(self) + } +} diff --git a/Tests/MetaCodableTests/CodableTests.swift b/Tests/MetaCodableTests/CodableTests.swift index 5b76978a5..c14c5abad 100644 --- a/Tests/MetaCodableTests/CodableTests.swift +++ b/Tests/MetaCodableTests/CodableTests.swift @@ -72,6 +72,47 @@ final class CodableTests: XCTestCase { ) } + func testWithoutAnyCustomizationWithStaticVar() throws { + assertMacroExpansion( + """ + @Codable + struct SomeCodable { + let value: String + static let otherValue: String + public private(set) static var valueWithModifiers: String + } + """, + expandedSource: + """ + struct SomeCodable { + let value: String + static let otherValue: String + public private(set) static var valueWithModifiers: String + } + + extension SomeCodable: Decodable { + init(from decoder: any Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + self.value = try container.decode(String.self, forKey: CodingKeys.value) + } + } + + extension SomeCodable: Encodable { + func encode(to encoder: any Encoder) throws { + var container = encoder.container(keyedBy: CodingKeys.self) + try container.encode(self.value, forKey: CodingKeys.value) + } + } + + extension SomeCodable { + enum CodingKeys: String, CodingKey { + case value = "value" + } + } + """ + ) + } + func testOnlyDecodeConformance() throws { assertMacroExpansion( """ diff --git a/Tests/MetaCodableTests/CodedAtTests.swift b/Tests/MetaCodableTests/CodedAtTests.swift index 3d79cce25..7db083438 100644 --- a/Tests/MetaCodableTests/CodedAtTests.swift +++ b/Tests/MetaCodableTests/CodedAtTests.swift @@ -56,6 +56,34 @@ final class CodedAtTests: XCTestCase { ) } + func testMisuseOnStaticVariableDeclaration() throws { + assertMacroExpansion( + """ + struct SomeCodable { + @CodedAt + static let value: String + } + """, + expandedSource: + """ + struct SomeCodable { + static let value: String + } + """, + diagnostics: [ + .init( + id: CodedAt.misuseID, + message: + "@CodedAt can't be used with static variables declarations", + line: 2, column: 5, + fixIts: [ + .init(message: "Remove @CodedAt attribute") + ] + ) + ] + ) + } + func testMisuseInCombinationWithCodedInMacro() throws { assertMacroExpansion( """ diff --git a/Tests/MetaCodableTests/CodedInTests.swift b/Tests/MetaCodableTests/CodedInTests.swift index a37d25933..17f18a761 100644 --- a/Tests/MetaCodableTests/CodedInTests.swift +++ b/Tests/MetaCodableTests/CodedInTests.swift @@ -35,6 +35,34 @@ final class CodedInTests: XCTestCase { ) } + func testMisuseOnStaticVariableDeclaration() throws { + assertMacroExpansion( + """ + struct SomeCodable { + @CodedIn + static let value: String + } + """, + expandedSource: + """ + struct SomeCodable { + static let value: String + } + """, + diagnostics: [ + .init( + id: CodedIn.misuseID, + message: + "@CodedIn can't be used with static variables declarations", + line: 2, column: 5, + fixIts: [ + .init(message: "Remove @CodedIn attribute") + ] + ) + ] + ) + } + func testDuplicatedMisuse() throws { assertMacroExpansion( """ From 209a4b4368ebda81f16485e3efb23a379961e972 Mon Sep 17 00:00:00 2001 From: Niklas Ausborn Date: Mon, 6 Nov 2023 14:49:17 +0100 Subject: [PATCH 2/5] Fixed spacing and variable name --- .../Attributes/RegistrationAttribute.swift | 6 ++---- .../Diagnostics/StaticVariableDeclaration.swift | 6 +++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/Sources/CodableMacroPlugin/Attributes/RegistrationAttribute.swift b/Sources/CodableMacroPlugin/Attributes/RegistrationAttribute.swift index fe053ba09..634c66aee 100644 --- a/Sources/CodableMacroPlugin/Attributes/RegistrationAttribute.swift +++ b/Sources/CodableMacroPlugin/Attributes/RegistrationAttribute.swift @@ -45,10 +45,8 @@ extension RegistrationAttribute { // The Macro fails to compile if the decl.modifiers.contains // is directly used in the guard statement. Otherwise it should // be used as a second condition in guard block above. - let isStaticVar = decl.modifiers.contains{$0.name.tokenKind == .keyword(.static)} - guard !isStaticVar else { - return - } + let isStatic = decl.modifiers.contains { $0.name.tokenKind == .keyword(.static) } + guard !isStatic else { return } // builder let builder = diff --git a/Sources/CodableMacroPlugin/Diagnostics/StaticVariableDeclaration.swift b/Sources/CodableMacroPlugin/Diagnostics/StaticVariableDeclaration.swift index 0f519fbfc..071993ba1 100644 --- a/Sources/CodableMacroPlugin/Diagnostics/StaticVariableDeclaration.swift +++ b/Sources/CodableMacroPlugin/Diagnostics/StaticVariableDeclaration.swift @@ -45,9 +45,9 @@ struct StaticVariableDeclaration: DiagnosticProducer { ) -> Bool { // The Macro fails to compile if the .modifiers.contains // is directly used in the guard statement. - let isStaticVar = syntax.as(VariableDeclSyntax.self)? - .modifiers.contains{$0.name.tokenKind == .keyword(.static)} - guard isStaticVar ?? false + let isStatic = syntax.as(VariableDeclSyntax.self)? + .modifiers.contains { $0.name.tokenKind == .keyword(.static) } + guard isStatic ?? false else { return false } let message = attr.node.diagnostic( message: From 22be4d9b3ea4013b841a2963171b3506d77d39ec Mon Sep 17 00:00:00 2001 From: Niklas Ausborn Date: Mon, 6 Nov 2023 15:17:59 +0100 Subject: [PATCH 3/5] Add support for static diagnostic checking for Default and CodedBy --- .../Attributes/CodedBy.swift | 3 ++ .../Attributes/Default.swift | 3 ++ Tests/MetaCodableTests/CodedAtTests.swift | 50 ++++++++++++++++++- 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/Sources/CodableMacroPlugin/Attributes/CodedBy.swift b/Sources/CodableMacroPlugin/Attributes/CodedBy.swift index 6e3438c71..09f4f06eb 100644 --- a/Sources/CodableMacroPlugin/Attributes/CodedBy.swift +++ b/Sources/CodableMacroPlugin/Attributes/CodedBy.swift @@ -38,6 +38,8 @@ struct CodedBy: PropertyAttribute { /// The following conditions are checked by the /// built diagnoser: /// * Attached declaration is a variable declaration. + /// * Attached declaration is not a static variable + /// declaration /// * Macro usage is not duplicated for the same /// declaration. /// * This attribute isn't used combined with @@ -47,6 +49,7 @@ struct CodedBy: PropertyAttribute { func diagnoser() -> DiagnosticProducer { return AggregatedDiagnosticProducer { expect(syntax: VariableDeclSyntax.self) + attachedToNonStaticVariable() cantDuplicate() cantBeCombined(with: IgnoreCoding.self) } diff --git a/Sources/CodableMacroPlugin/Attributes/Default.swift b/Sources/CodableMacroPlugin/Attributes/Default.swift index 5ab4acefa..42f327b96 100644 --- a/Sources/CodableMacroPlugin/Attributes/Default.swift +++ b/Sources/CodableMacroPlugin/Attributes/Default.swift @@ -37,6 +37,8 @@ struct Default: PropertyAttribute { /// The following conditions are checked by the /// built diagnoser: /// * Attached declaration is a variable declaration. + /// * Attached declaration is not a static variable + /// declaration /// * Macro usage is not duplicated for the same /// declaration. /// * This attribute isn't used combined with @@ -46,6 +48,7 @@ struct Default: PropertyAttribute { func diagnoser() -> DiagnosticProducer { return AggregatedDiagnosticProducer { expect(syntax: VariableDeclSyntax.self) + attachedToNonStaticVariable() cantDuplicate() cantBeCombined(with: IgnoreCoding.self) } diff --git a/Tests/MetaCodableTests/CodedAtTests.swift b/Tests/MetaCodableTests/CodedAtTests.swift index 7db083438..d819c5276 100644 --- a/Tests/MetaCodableTests/CodedAtTests.swift +++ b/Tests/MetaCodableTests/CodedAtTests.swift @@ -77,7 +77,55 @@ final class CodedAtTests: XCTestCase { "@CodedAt can't be used with static variables declarations", line: 2, column: 5, fixIts: [ - .init(message: "Remove @CodedAt attribute") + .init(message: "Remove @CodedAt attribute") + ] + ) + ] + ) + } + + func testMisuseOnStaticWithCodedByDefaultVariableDeclaration() throws { + assertMacroExpansion( + """ + struct SomeCodable { + @Default("some") + @CodedBy(Since1970DateCoder()) + @CodedAt + static let value: String + } + """, + expandedSource: + """ + struct SomeCodable { + static let value: String + } + """, + diagnostics: [ + .init( + id: Default.misuseID, + message: + "@Default can't be used with static variables declarations", + line: 2, column: 5, + fixIts: [ + .init(message: "Remove @Default attribute") + ] + ), + .init( + id: CodedBy.misuseID, + message: + "@CodedBy can't be used with static variables declarations", + line: 3, column: 5, + fixIts: [ + .init(message: "Remove @CodedBy attribute") + ] + ), + .init( + id: CodedAt.misuseID, + message: + "@CodedAt can't be used with static variables declarations", + line: 4, column: 5, + fixIts: [ + .init(message: "Remove @CodedAt attribute") ] ) ] From 620490c6dda013461f09828797f3dbac576ed63a Mon Sep 17 00:00:00 2001 From: Niklas Ausborn Date: Mon, 6 Nov 2023 15:41:10 +0100 Subject: [PATCH 4/5] Fixed formatting --- .../Diagnostics/StaticVariableDeclaration.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Sources/CodableMacroPlugin/Diagnostics/StaticVariableDeclaration.swift b/Sources/CodableMacroPlugin/Diagnostics/StaticVariableDeclaration.swift index 071993ba1..00eb1abe2 100644 --- a/Sources/CodableMacroPlugin/Diagnostics/StaticVariableDeclaration.swift +++ b/Sources/CodableMacroPlugin/Diagnostics/StaticVariableDeclaration.swift @@ -47,8 +47,7 @@ struct StaticVariableDeclaration: DiagnosticProducer { // is directly used in the guard statement. let isStatic = syntax.as(VariableDeclSyntax.self)? .modifiers.contains { $0.name.tokenKind == .keyword(.static) } - guard isStatic ?? false - else { return false } + guard isStatic ?? false else { return false } let message = attr.node.diagnostic( message: "@\(attr.name) can't be used with static variables declarations", From acf371b1e1d12a1c8fdecf4b3839af5cc83c19cd Mon Sep 17 00:00:00 2001 From: Niklas Ausborn Date: Mon, 6 Nov 2023 15:45:22 +0100 Subject: [PATCH 5/5] Run swift-format --- .../Attributes/RegistrationAttribute.swift | 4 +- .../StaticVariableDeclaration.swift | 112 +++++++++--------- Tests/MetaCodableTests/CodableTests.swift | 8 +- Tests/MetaCodableTests/CodedAtTests.swift | 2 +- 4 files changed, 64 insertions(+), 62 deletions(-) diff --git a/Sources/CodableMacroPlugin/Attributes/RegistrationAttribute.swift b/Sources/CodableMacroPlugin/Attributes/RegistrationAttribute.swift index 634c66aee..f422da3db 100644 --- a/Sources/CodableMacroPlugin/Attributes/RegistrationAttribute.swift +++ b/Sources/CodableMacroPlugin/Attributes/RegistrationAttribute.swift @@ -45,7 +45,9 @@ extension RegistrationAttribute { // The Macro fails to compile if the decl.modifiers.contains // is directly used in the guard statement. Otherwise it should // be used as a second condition in guard block above. - let isStatic = decl.modifiers.contains { $0.name.tokenKind == .keyword(.static) } + let isStatic = decl.modifiers.contains { + $0.name.tokenKind == .keyword(.static) + } guard !isStatic else { return } // builder diff --git a/Sources/CodableMacroPlugin/Diagnostics/StaticVariableDeclaration.swift b/Sources/CodableMacroPlugin/Diagnostics/StaticVariableDeclaration.swift index 00eb1abe2..21caee8c3 100644 --- a/Sources/CodableMacroPlugin/Diagnostics/StaticVariableDeclaration.swift +++ b/Sources/CodableMacroPlugin/Diagnostics/StaticVariableDeclaration.swift @@ -8,65 +8,65 @@ /// This producer can be used for macro-attributes that must be attached to /// non static variable declarations. struct StaticVariableDeclaration: DiagnosticProducer { - /// The attribute for which - /// validation performed. - /// - /// Uses this attribute name - /// in generated diagnostic - /// messages. - let attr: Attr + /// The attribute for which + /// validation performed. + /// + /// Uses this attribute name + /// in generated diagnostic + /// messages. + let attr: Attr - /// Creates a static variable declaration validation instance - /// with provided attribute. - /// - /// - Parameter attr: The attribute for which - /// validation performed. - /// - Returns: Newly created diagnostic producer. - init(_ attr: Attr) { - self.attr = attr - } + /// Creates a static variable declaration validation instance + /// with provided attribute. + /// + /// - Parameter attr: The attribute for which + /// validation performed. + /// - Returns: Newly created diagnostic producer. + init(_ attr: Attr) { + self.attr = attr + } - /// Validates and produces diagnostics for the passed syntax - /// in the macro expansion context provided. - /// - /// Checks whether provided syntax is a non static variable declaration, - /// for static variable declarations error diagnostics - /// are generated. - /// - /// - Parameters: - /// - syntax: The syntax to validate and produce diagnostics for. - /// - context: The macro expansion context diagnostics produced in. - /// - /// - Returns: True if syntax fails validation, false otherwise. - @discardableResult - func produce( - for syntax: some SyntaxProtocol, - in context: some MacroExpansionContext - ) -> Bool { - // The Macro fails to compile if the .modifiers.contains - // is directly used in the guard statement. - let isStatic = syntax.as(VariableDeclSyntax.self)? - .modifiers.contains { $0.name.tokenKind == .keyword(.static) } - guard isStatic ?? false else { return false } - let message = attr.node.diagnostic( - message: - "@\(attr.name) can't be used with static variables declarations", - id: attr.misuseMessageID, - severity: .error - ) - context.diagnose(attr: attr, message: message) - return true - } + /// Validates and produces diagnostics for the passed syntax + /// in the macro expansion context provided. + /// + /// Checks whether provided syntax is a non static variable declaration, + /// for static variable declarations error diagnostics + /// are generated. + /// + /// - Parameters: + /// - syntax: The syntax to validate and produce diagnostics for. + /// - context: The macro expansion context diagnostics produced in. + /// + /// - Returns: True if syntax fails validation, false otherwise. + @discardableResult + func produce( + for syntax: some SyntaxProtocol, + in context: some MacroExpansionContext + ) -> Bool { + // The Macro fails to compile if the .modifiers.contains + // is directly used in the guard statement. + let isStatic = syntax.as(VariableDeclSyntax.self)? + .modifiers.contains { $0.name.tokenKind == .keyword(.static) } + guard isStatic ?? false else { return false } + let message = attr.node.diagnostic( + message: + "@\(attr.name) can't be used with static variables declarations", + id: attr.misuseMessageID, + severity: .error + ) + context.diagnose(attr: attr, message: message) + return true + } } extension PropertyAttribute { - /// Indicates attribute must be attached to non static variable declaration. - /// - /// The created diagnostic producer produces error diagnostic, - /// if attribute is attached to static variable declarations. - /// - /// - Returns: Static variable declaration validation diagnostic producer. - func attachedToNonStaticVariable() -> StaticVariableDeclaration { - return .init(self) - } + /// Indicates attribute must be attached to non static variable declaration. + /// + /// The created diagnostic producer produces error diagnostic, + /// if attribute is attached to static variable declarations. + /// + /// - Returns: Static variable declaration validation diagnostic producer. + func attachedToNonStaticVariable() -> StaticVariableDeclaration { + return .init(self) + } } diff --git a/Tests/MetaCodableTests/CodableTests.swift b/Tests/MetaCodableTests/CodableTests.swift index c14c5abad..334e3f058 100644 --- a/Tests/MetaCodableTests/CodableTests.swift +++ b/Tests/MetaCodableTests/CodableTests.swift @@ -72,8 +72,8 @@ final class CodableTests: XCTestCase { ) } - func testWithoutAnyCustomizationWithStaticVar() throws { - assertMacroExpansion( + func testWithoutAnyCustomizationWithStaticVar() throws { + assertMacroExpansion( """ @Codable struct SomeCodable { @@ -110,8 +110,8 @@ final class CodableTests: XCTestCase { } } """ - ) - } + ) + } func testOnlyDecodeConformance() throws { assertMacroExpansion( diff --git a/Tests/MetaCodableTests/CodedAtTests.swift b/Tests/MetaCodableTests/CodedAtTests.swift index d819c5276..64355698b 100644 --- a/Tests/MetaCodableTests/CodedAtTests.swift +++ b/Tests/MetaCodableTests/CodedAtTests.swift @@ -127,7 +127,7 @@ final class CodedAtTests: XCTestCase { fixIts: [ .init(message: "Remove @CodedAt attribute") ] - ) + ), ] ) }