fix: disallow AlterColumn method chaining (kysely-org#468)

* fix: disallow chaining AlterColumnBuilder operations (kysely-org#467) * chore: limit AlterColumnNode.create available params, change AlterColumnBuilder fields names
Gaspero · Oct 2, 2023 · 871d6d0 · 871d6d0
1 parent b1f3a68
commit 871d6d0
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 38 deletions.
diff --git a/src/operation-node/alter-column-node.ts b/src/operation-node/alter-column-node.ts
@@ -1,8 +1,6 @@
 import { OperationNode } from './operation-node.js'
 import { freeze } from '../util/object-utils.js'
 import { ColumnNode } from './column-node.js'
-import { DataTypeNode } from './data-type-node.js'
-import { ValueNode } from './value-node.js'
 import { RawNode } from './raw-node.js'
 
 export type AlterColumnNodeProps = Omit<AlterColumnNode, 'kind' | 'column'>
@@ -26,20 +24,11 @@ export const AlterColumnNode = freeze({
     return node.kind === 'AlterColumnNode'
   },
 
-  create(column: string): AlterColumnNode {
+  create<T extends keyof AlterColumnNodeProps>(column: string, prop: T, value: Required<AlterColumnNodeProps>[T]): AlterColumnNode {
     return freeze({
       kind: 'AlterColumnNode',
       column: ColumnNode.create(column),
-    })
-  },
-
-  cloneWith(
-    node: AlterColumnNode,
-    props: AlterColumnNodeProps
-  ): AlterColumnNode {
-    return freeze({
-      ...node,
-      ...props,
+      [prop]: value
     })
   },
 })
diff --git a/src/schema/alter-column-builder.ts b/src/schema/alter-column-builder.ts
@@ -10,49 +10,39 @@ import {
 } from '../parser/default-value-parser.js'
 
 export class AlterColumnBuilder {
-  protected readonly alterColumnNode: AlterColumnNode
+  readonly #column: string
 
-  constructor(alterColumnNode: AlterColumnNode) {
-    this.alterColumnNode = alterColumnNode
+  constructor(column: string) {
+    this.#column = column
   }
 
   setDataType(dataType: DataTypeExpression): AlteredColumnBuilder {
     return new AlteredColumnBuilder(
-      AlterColumnNode.cloneWith(this.alterColumnNode, {
-        dataType: parseDataTypeExpression(dataType),
-      })
+      AlterColumnNode.create(this.#column, 'dataType', parseDataTypeExpression(dataType))
     )
   }
 
   setDefault(value: DefaultValueExpression): AlteredColumnBuilder {
     return new AlteredColumnBuilder(
-      AlterColumnNode.cloneWith(this.alterColumnNode, {
-        setDefault: parseDefaultValueExpression(value),
-      })
+      AlterColumnNode.create(this.#column, 'setDefault', parseDefaultValueExpression(value))
     )
   }
 
   dropDefault(): AlteredColumnBuilder {
     return new AlteredColumnBuilder(
-      AlterColumnNode.cloneWith(this.alterColumnNode, {
-        dropDefault: true,
-      })
+      AlterColumnNode.create(this.#column, 'dropDefault', true)
     )
   }
 
   setNotNull(): AlteredColumnBuilder {
     return new AlteredColumnBuilder(
-      AlterColumnNode.cloneWith(this.alterColumnNode, {
-        setNotNull: true,
-      })
+      AlterColumnNode.create(this.#column, 'setNotNull', true)
     )
   }
 
   dropNotNull(): AlteredColumnBuilder {
     return new AlteredColumnBuilder(
-      AlterColumnNode.cloneWith(this.alterColumnNode, {
-        dropNotNull: true,
-      })
+      AlterColumnNode.create(this.#column, 'dropNotNull', true)
     )
   }
 
@@ -66,22 +56,30 @@ export class AlterColumnBuilder {
 }
 
 /**
- * Allows us to force consumers to do something, anything, when altering a column.
+ * Allows us to force consumers to do exactly one alteration to a column.
  *
  * Basically, deny the following:
  *
  * ```ts
  * db.schema.alterTable('person').alterColumn('age', (ac) => ac)
  * ```
+ * 
+ * ```ts
+ * db.schema.alterTable('person').alterColumn('age', (ac) => ac.dropNotNull().setNotNull())
+ * ```
  *
  * Which would now throw a compilation error, instead of a runtime error.
  */
-export class AlteredColumnBuilder
-  extends AlterColumnBuilder
-  implements OperationNodeSource
+export class AlteredColumnBuilder implements OperationNodeSource
 {
+  readonly #alterColumnNode: AlterColumnNode
+
+  constructor(alterColumnNode: AlterColumnNode) {
+    this.#alterColumnNode = alterColumnNode
+  }
+
   toOperationNode(): AlterColumnNode {
-    return this.alterColumnNode
+    return this.#alterColumnNode
   }
 }
 

diff --git a/src/schema/alter-table-builder.ts b/src/schema/alter-table-builder.ts
@@ -75,7 +75,7 @@ export class AlterTableBuilder implements ColumnAlteringInterface {
     alteration: AlterColumnBuilderCallback
   ): AlterTableColumnAlteringBuilder {
     const builder = alteration(
-      new AlterColumnBuilder(AlterColumnNode.create(column))
+      new AlterColumnBuilder(column)
     )
 
     return new AlterTableColumnAlteringBuilder({
@@ -315,7 +315,7 @@ export class AlterTableColumnAlteringBuilder
     alteration: AlterColumnBuilderCallback
   ): AlterTableColumnAlteringBuilder {
     const builder = alteration(
-      new AlterColumnBuilder(AlterColumnNode.create(column))
+      new AlterColumnBuilder(column)
     )
 
     return new AlterTableColumnAlteringBuilder({