Skip to content

Commit

Permalink
Limit @Init(default:) to single variable bindings
Browse files Browse the repository at this point in the history
Amidst tradeoffs needing careful consideration, I opted to restrict
`@Init(default:)` to single variable bindings. Enabling it for multiple
bindings in the future would be non-breaking. (Removing it in the future
would be a breaking change.)
  • Loading branch information
gohanlon committed Dec 4, 2023
1 parent 537d967 commit b058368
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 53 deletions.
35 changes: 25 additions & 10 deletions Sources/MemberwiseInitMacros/Macros/MemberwiseInitMacro.swift
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,22 @@ public struct MemberwiseInitMacro: MemberMacro {

var diagnostics = [Diagnostic]()

if let customSettings, customSettings.label?.isInvalidSwiftLabel ?? false {
diagnostics.append(customSettings.diagnosticOnLabelValue(message: .invalidSwiftLabel))
} else if let customSettings,
let label = customSettings.label,
label != "_",
variable.bindings.count > 1
{
diagnostics.append(
customSettings.diagnosticOnLabel(message: .labelAppliedToMultipleBindings)
)
if let customSettings = customSettings {
if customSettings.label?.isInvalidSwiftLabel ?? false {
diagnostics.append(customSettings.diagnosticOnLabelValue(message: .invalidSwiftLabel))
} else if let label = customSettings.label,
label != "_",
variable.bindings.count > 1
{
diagnostics.append(
customSettings.diagnosticOnLabel(message: .labelAppliedToMultipleBindings))
}

if customSettings.defaultValue != nil, variable.bindings.count > 1 {
diagnostics.append(
customSettings.diagnosticOnDefault(message: .defaultAppliedToMultipleBindings)
)
}
}

// TODO: repetition of logic for custom configuration logic
Expand Down Expand Up @@ -489,6 +495,15 @@ private struct VariableCustomSettings: Equatable {
let type: TypeSyntax?
let _syntaxNode: AttributeSyntax

func diagnosticOnDefault(message: MemberwiseInitMacroDiagnostic) -> Diagnostic {
let labelNode = self._syntaxNode
.arguments?
.as(LabeledExprListSyntax.self)?
.firstWhereLabel("default")

return diagnostic(node: labelNode ?? self._syntaxNode, message: message)
}

func diagnosticOnLabel(message: MemberwiseInitMacroDiagnostic) -> Diagnostic {
let labelNode = self._syntaxNode
.arguments?
Expand Down
7 changes: 7 additions & 0 deletions Sources/MemberwiseInitMacros/Macros/Support/Diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import SwiftDiagnostics
import SwiftSyntax

enum MemberwiseInitMacroDiagnostic: Error, DiagnosticMessage {
case defaultAppliedToMultipleBindings
case labelAppliedToMultipleBindings
case labelConflictsWithProperty(String)
case labelConflictsWithAnotherLabel(String)
Expand All @@ -12,6 +13,9 @@ enum MemberwiseInitMacroDiagnostic: Error, DiagnosticMessage {

private var rawValue: String {
switch self {
case .defaultAppliedToMultipleBindings:
".defaultAppliedToMultipleBindings"

case .labelAppliedToMultipleBindings:
".labelAppliedToMultipleBindings"

Expand Down Expand Up @@ -39,6 +43,9 @@ enum MemberwiseInitMacroDiagnostic: Error, DiagnosticMessage {

var message: String {
switch self {
case .defaultAppliedToMultipleBindings:
return "Custom 'default' can't be applied to multiple bindings"

case .labelAppliedToMultipleBindings:
return """
Custom 'label' can't be applied to multiple bindings
Expand Down
133 changes: 90 additions & 43 deletions Tests/MemberwiseInitTests/CustomInitDefaultTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import MacroTesting
import MemberwiseInitMacros
import XCTest

// TODO: Carefully consider whether to allow @Init(default:) to be applied to multiple bindings

final class CustomInitDefaultTests: XCTestCase {
override func invokeTest() {
// NB: Waiting for swift-macro-testing PR to support explicit indentationWidth: https://github.com/pointfreeco/swift-macro-testing/pull/8
Expand Down Expand Up @@ -114,21 +116,30 @@ final class CustomInitDefaultTests: XCTestCase {
@Init(default: 42) let x, y: Int
}
"""
} expansion: {
} diagnostics: {
"""
@MemberwiseInit
struct S {
@Init(default: 42) let x, y: Int
internal init(
x: Int = 42,
y: Int = 42
) {
self.x = x
self.y = y
}
┬──────────
╰─ 🛑 Custom 'default' can't be applied to multiple bindings
}
"""
}
// } expansion: {
// """
// struct S {
// @Init(default: 42) let x, y: Int
//
// internal init(
// x: Int = 42,
// y: Int = 42
// ) {
// self.x = x
// self.y = y
// }
// }
// """
}

func testVarWithMultipleBindings() {
Expand All @@ -139,21 +150,30 @@ final class CustomInitDefaultTests: XCTestCase {
@Init(default: 42) var x, y: Int
}
"""
} expansion: {
} diagnostics: {
"""
@MemberwiseInit
struct S {
@Init(default: 42) var x, y: Int
internal init(
x: Int = 42,
y: Int = 42
) {
self.x = x
self.y = y
}
┬──────────
╰─ 🛑 Custom 'default' can't be applied to multiple bindings
}
"""
}
// } expansion: {
// """
// struct S {
// @Init(default: 42) var x, y: Int
//
// internal init(
// x: Int = 42,
// y: Int = 42
// ) {
// self.x = x
// self.y = y
// }
// }
// """
}

func testLetWithFirstBindingInitialized() {
Expand All @@ -164,19 +184,28 @@ final class CustomInitDefaultTests: XCTestCase {
@Init(default: 42) let x = 0, y: Int
}
"""
} expansion: {
} diagnostics: {
"""
@MemberwiseInit
struct S {
@Init(default: 42) let x = 0, y: Int
internal init(
y: Int = 42
) {
self.y = y
}
┬──────────
╰─ 🛑 Custom 'default' can't be applied to multiple bindings
}
"""
}
// } expansion: {
// """
// struct S {
// @Init(default: 42) let x = 0, y: Int
//
// internal init(
// y: Int = 42
// ) {
// self.y = y
// }
// }
// """
}

func testVarWithFirstBindingInitialized() {
Expand All @@ -187,21 +216,30 @@ final class CustomInitDefaultTests: XCTestCase {
@Init(default: 42) var x = 0, y: Int
}
"""
} expansion: {
} diagnostics: {
"""
@MemberwiseInit
struct S {
@Init(default: 42) var x = 0, y: Int
internal init(
x: Int = 0,
y: Int = 42
) {
self.x = x
self.y = y
}
┬──────────
╰─ 🛑 Custom 'default' can't be applied to multiple bindings
}
"""
}
// } expansion: {
// """
// struct S {
// @Init(default: 42) var x = 0, y: Int
//
// internal init(
// x: Int = 0,
// y: Int = 42
// ) {
// self.x = x
// self.y = y
// }
// }
// """
}

func testLetWithRaggedBindings_SucceedsWithInvalidCode() {
Expand All @@ -212,20 +250,29 @@ final class CustomInitDefaultTests: XCTestCase {
@Init(default: 42) let x: Int, isOn: Bool
}
"""
} expansion: {
} diagnostics: {
"""
@MemberwiseInit
struct S {
@Init(default: 42) let x: Int, isOn: Bool
internal init(
x: Int = 42,
isOn: Bool = 42
) {
self.x = x
self.isOn = isOn
}
┬──────────
╰─ 🛑 Custom 'default' can't be applied to multiple bindings
}
"""
}
// } expansion: {
// """
// struct S {
// @Init(default: 42) let x: Int, isOn: Bool
//
// internal init(
// x: Int = 42,
// isOn: Bool = 42
// ) {
// self.x = x
// self.isOn = isOn
// }
// }
// """
}
}

0 comments on commit b058368

Please sign in to comment.