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

[alephao]

Discussed briefly here: #30

Context

Let's say I have a table with a default value:

CREATE TABLE my_table (
  created_at INTEGER DEFAULT(unixepoch()) NOT NULL
);

and the swift type would be:

@Table("my_table")
struct MyTable {
  @Column("created_at", as: Date.UnixTimeRepresentation.self)
  let createdAt: Date
}

The Draft API is a pretty nice way to insert a row on this table, but at the moment I need to pass a Date like so:

let draft = MyTable.Draft(createdAt: Date())

Would be great if I could annotate the property so the createdAt becomes optional on MyTable.Draft, similar to the @Column(primaryKey: true) macro. So I can use it like so:

let draft = MyTable.Draft()

One idea is to extend the @Column to accept a boolean and use it to decide if the generated property on Draft is optional or not, e.g.: databaseInitialized: Bool (@mbrandonw suggestion)

@Table("my_table")
struct MyTable {
  @Column("created_at", as: Date.UnixTimeRepresentation.self, databaseInitialized: true)
  let createdAt: Date
}

Implementation

To decide if a non-optional property should become optional in the Draft type, I did the following mapping:

 isPrimaryKey &&  databaseInitialized -> Optionalize Draft
!isPrimaryKey &&  databaseInitialized -> Optionalize Draft
 isPrimaryKey && !databaseInitialized -> Inherit
!isPrimaryKey && !databaseInitialized -> Inherit

// Note: isPrimaryKey considering both, the `id` property without the `primaryKey: false`, and a property with `primaryKey: true`

There are some invalid cases, so I added error diagnostics to them:

Invalid-1. When databaseInitialized is not literal true or false, show error:
@Column(databaseInitialized: nil) -> Error: "Argument 'databaseInitialized' must be a boolean literal"

Invalid-2. `databeseInitialized: true` attached on an optional property. Note: I'm checking other types of optional binding too, not just the one in the example below.
@Column(databaseInitialized: true) 
var foo: String? -> Error: "Can't use `databaseInitialized: true` on an optional property"

And there are some redundant cases. I added a warning diagnostics to those:

Redundant-1. When both primaryKey and databaseInitialized are true
@Column(primaryKey: true, databaseInitialized: true) -> Warning: "'databaseInitialized: true' is redundant with 'primaryKey: true'"

Redundant-2. When databaseInitialized is true on an `id` without `primaryKey: false`
@Column(databaseInitialized: true) -> Warning: "'databaseInitialized: true' is redundant for primary keys'"
let id: Int

Redundant-3. When both databaseInitialized and primaryKey are false
@Column(primaryKey: false, databaseInitialized: false) -> Warning "'databaseInitialized: false' is redundant with 'primaryKey: false'"

Redundant-4. When databaseInitialized is false on a regular property
@Column(databaseInitialized: false) -> Warning "'databaseInitialized: false' is redundant for non primary keys"
var foo: String

I added tests for all those cases

Other Changes

Updated primaryKey attribute type and default value

I had to update the attribute primaryKey: Bool = false to primaryKey: Bool? = nil because if I happen to have an id: String that should be initialized by the application (and not the database), then following code would behave in a way I'm not expecting:

@Column(databaseInitialized: false) // Previously, this would set primaryKey to false
let id: String

I'm doing the same for databaseInitialized, the attribute's argument is databaseInitialized: Bool? = nil so we don't implicitly force any behavior.

Force removing of databaseInitialized attribute from the Draft type

When creating the Draft type, it was inheriting the @Column attribute with databaseInitialized which caused all sorts of issues, so I had to add some changes to prevent the Draft property to inherit the databaseInitialized attribute

Print warning diagnostics and still run the macro

Previously, any diagnostic would prevent the macro generation, now it only stops when there is an error diagnostic, if there are warning diagnostics, it sill generates the extra swift code

[stephencelis]

@alephao Thanks so much for exploring this! We'll leave a review with some initial feedback while we think on some of the ramifications of this feature.

[stephencelis]

Some notes below!

We're excited about supporting this, but we do think there are many open questions that we're still trying to figure out.

The main one concerns the library's current design, where right now Draft is only a thing on primary-keyed tables. This means that database-initialized functionality is only available on primary-keyed-tables, which is kind of a bummer. We think it probably should be possible to have Draft types on any table with a primary key or database-initialized fields, but this is a pretty big design change to figure out.

We will hopefully have time to explore soon!

[stephencelis]

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.

[stephencelis]

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.

[stephencelis]

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

[alephao]

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.

[alephao]

@stephencelis Thanks for the quick response! I understand it's a big decision, but I'm happy to help explore the idea, even if it ends up going in a completely different direction. I'll try out your suggestions soon!

[stephencelis]

@alephao Marking this as a draft for now, and please let us know if you intend to take it any further!