Skip to content

feat: add databaseInitialized attribute to @Column macro to force optional on Draft properties #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: add databaseInitialized attribute to @Column macro to force optional on Draft properties #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Sources/StructuredQueries/Macros.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ public macro Table(_ name: String? = nil) =
/// - representableType: A type that represents the property type in a query expression. For types
/// that don't have a single representation in SQL, like `Date` and `UUID`.
/// - primaryKey: The column is its table's auto-incrementing primary key.
/// - databaseInitialized: The column has a default value and is not needed in an insert statement.
@attached(accessor, names: named(willSet))
public macro Column(
Comment on lines 36 to 39
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One idea to simplify the diagnostics you added in the macro for nonsensical combinations of these two parameters would be to instead introduce an overload of @Column that simply takes the databaseInitialized parameter. That way it's impossible to invoke @Column with both parameters.

_ name: String? = nil,
as representableType: (any QueryRepresentable.Type)? = nil,
primaryKey: Bool = false
primaryKey: Bool? = nil,
databaseInitialized: Bool? = nil
Comment on lines +42 to +43
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the spirit of the above comment, I think these parameters could stay Bool and not be optional, which would further simplify diagnostics added to the macro.

) =
#externalMacro(
module: "StructuredQueriesMacros",
Expand Down
9 changes: 9 additions & 0 deletions Sources/StructuredQueriesMacros/Internal/PatternBindingSyntax.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,13 @@ extension PatternBindingSyntax {
optionalized.typeAnnotation?.type = optionalType
return optionalized
}

func isOptional() -> Bool {
// x: Optional<T> or x: T?
if self.typeAnnotation?.type.isOptionalType == true { return true }
// Missing cases
// x = Optional<T>.some(_)
// x = fnReturningOptionalType()
Comment on lines +44 to +46
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this helper mostly here to mark these cases as todos?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot to mention in the PR description, but I was wondering if it's worth it trying to do anything to figure out if a type is actually Optional or not. I was getting in a rabbit hole and left it as is to not waste too much time and get some input.

It doesn't seem worth it IMO, but I left there to inform that isOptional does not guarantee that the binding is actually optional, and left some examples of bindings that could return a false negative.

return false
}
}
129 changes: 125 additions & 4 deletions Sources/StructuredQueriesMacros/TableMacro.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ extension TableMacro: ExtensionMacro {
var columnQueryOutputType = columnQueryValueType
var isPrimaryKey = primaryKey == nil && identifier.text == "id"
var isEphemeral = false
var databaseInitialized: Bool? = nil

for attribute in property.attributes {
guard
Expand Down Expand Up @@ -182,6 +183,89 @@ extension TableMacro: ExtensionMacro {
queryValueType: columnQueryValueType
)

case let .some(label) where label.text == "databaseInitialized":
guard
let tokenKind = argument.expression.as(BooleanLiteralExprSyntax.self)?.literal
.tokenKind
else {
diagnostics.append(
Diagnostic(
node: argument.expression,
message: MacroExpansionErrorMessage(
"Argument 'databaseInitialized' must be a boolean literal")
)
)
break
}

databaseInitialized = tokenKind == .keyword(.true)

if databaseInitialized! && binding.isOptional() {
diagnostics.append(
Diagnostic(
node: argument.expression,
message: MacroExpansionErrorMessage(
"Can't use `databaseInitialized: true` on an optional property")
)
)
break
}

let hasPrimaryKeySemantics =
(isPrimaryKey || primaryKey?.identifier.text == identifier.text)

switch (hasPrimaryKeySemantics, databaseInitialized!) {
case (true, true):
var newArguments = arguments
newArguments.remove(at: argumentIndex)
if var lastArgument = newArguments.last {
lastArgument.trailingComma = nil
newArguments[newArguments.index(before: newArguments.endIndex)] = lastArgument
}
diagnostics.append(
Diagnostic(
node: argument.expression,
message: MacroExpansionWarningMessage(
isPrimaryKey
? "'databaseInitialized: true' is redundant for primary keys"
: "'databaseInitialized: true' is redundant with 'primaryKey: true'"
),
fixIt: .replace(
message: MacroExpansionFixItMessage("Remove 'databaseInitialized: true'"),
oldNode: Syntax(attribute),
newNode: Syntax(attribute.with(\.arguments, .argumentList(newArguments)))
)
)
)
break

case (false, false):
var newArguments = arguments
newArguments.remove(at: argumentIndex)
if var lastArgument = newArguments.last {
lastArgument.trailingComma = nil
newArguments[newArguments.index(before: newArguments.endIndex)] = lastArgument
}
diagnostics.append(
Diagnostic(
node: argument.expression,
message: MacroExpansionWarningMessage(
isPrimaryKey
? "'databaseInitialized: false' is redundant with 'primaryKey: false'"
: "'databaseInitialized: false' is redundant for non primary keys"
),
fixIt: .replace(
message: MacroExpansionFixItMessage("Remove 'databaseInitialized: false'"),
oldNode: Syntax(attribute),
newNode: Syntax(attribute.with(\.arguments, .argumentList(newArguments)))
)
)
)
break

default: break
}

case let argument?:
fatalError("Unexpected argument: \(argument)")
}
Expand All @@ -200,7 +284,7 @@ extension TableMacro: ExtensionMacro {
}

// NB: A compiled bug prevents us from applying the '@_Draft' macro directly
if identifier == primaryKey?.identifier {
if databaseInitialized == true || identifier == primaryKey?.identifier {
draftBindings.append((binding.optionalized(), columnQueryOutputType))
} else {
draftBindings.append((binding, columnQueryOutputType))
Expand Down Expand Up @@ -336,6 +420,7 @@ extension TableMacro: ExtensionMacro {
else { continue }
hasColumnAttribute = true
var hasPrimaryKeyArgument = false
var databaseInitializedIndex: DefaultIndices<LabeledExprListSyntax>.Element? = nil
for argumentIndex in arguments.indices {
var argument = arguments[argumentIndex]
defer { arguments[argumentIndex] = argument }
Expand All @@ -350,11 +435,17 @@ extension TableMacro: ExtensionMacro {
hasPrimaryKeyArgument = true
argument.expression = ExprSyntax(BooleanLiteralExprSyntax(false))

case "databaseInitialized":
databaseInitializedIndex = argumentIndex

default:
break
}
}
if !hasPrimaryKeyArgument {
if let databaseInitializedIndex {
arguments.remove(at: databaseInitializedIndex)
}
if !hasPrimaryKeyArgument, arguments.count > 0 {
arguments[arguments.index(before: arguments.endIndex)].trailingComma = .commaToken(
trailingTrivia: .space
)
Expand Down Expand Up @@ -389,6 +480,30 @@ extension TableMacro: ExtensionMacro {
)
)
} else {
var property = property
var binding = binding

if databaseInitialized == true, let type = binding.typeAnnotation?.type.asOptionalType() {
if let columnAttributeIdx = property.attributes.firstIndex(where: {
$0.as(AttributeSyntax.self)?.attributeName.as(IdentifierTypeSyntax.self)?.name.text
== "Column"
}),
var columnAttribute = property.attributes[columnAttributeIdx].as(AttributeSyntax.self),
case var .argumentList(arguments) = columnAttribute.arguments
{
if arguments.count <= 1 {
property.attributes.remove(at: columnAttributeIdx)
} else if let dabaseInitializeIdx = arguments.firstIndex(where: {
$0.label?.text == "databaseInitialized"
}) {
arguments.remove(at: dabaseInitializeIdx)
columnAttribute.arguments = .argumentList(arguments)
property.attributes[columnAttributeIdx] = .attribute(columnAttribute)
}
}
binding.typeAnnotation?.type = type
}
property.bindings = [binding]
draftProperties.append(
DeclSyntax(
property.trimmed
Expand Down Expand Up @@ -539,8 +654,14 @@ extension TableMacro: ExtensionMacro {
)
}

guard diagnostics.isEmpty else {
diagnostics.forEach(context.diagnose)
var hasErrorDiagnostic = false
for diagnostic in diagnostics {
context.diagnose(diagnostic)
if diagnostic.diagMessage.severity == .error {
hasErrorDiagnostic = true
}
}
guard !hasErrorDiagnostic else {
return []
}

Expand Down
Loading