feat(migrator): Migration.cfc command consistency sweep (follow-up to #2781)

vendor/wheels/migrator/CLAUDE.md

@@ -17,7 +17,18 @@ public any function string(string columnNames, any limit, string default, boolea
 
 Callers can pass either `t.string(columnNames = "a,b,c")` or `t.string(columnName = "a")` — both resolve to `arguments.columnNames` for the function body. Drop the `required` keyword from the parameter declaration; `$combineArguments(required=true)` enforces it at runtime.
 
-**`references()` carries a back-compat exception.** Its legacy parameter is `referenceNames`, with `columnNames` accepted as a synonym since the [#2781](https://github.com/wheels-dev/wheels/issues/2781) fix:
+**`references()` and its command-version siblings carry back-compat aliases.** The legacy parameter names predate the `$combineArguments` convention; the modern ones are accepted as synonyms via the same helper. Each of these accepts the modern form going forward:
+
+| Function | Legacy param | Modern alias(es) |
+|---|---|---|
+| `TableDefinition::references()` | `referenceNames` | `columnNames` |
+| `Migration::addReference()` | `referenceName` | `columnName`, `columnNames` |
+| `Migration::dropReference()` | `referenceName` | `columnName`, `columnNames` |
+| `Migration::addColumn()` / `changeColumn()` | `columnName` | `columnNames` |
+| `Migration::removeColumn()` | `columnName` | `columnNames` |
+| `Migration::addForeignKey()` | `column` | `columnName` |
+
+Example (the `references()` form, [#2781](https://github.com/wheels-dev/wheels/issues/2781)):
 
 ```cfm
 $combineArguments(args = arguments, combine = "referenceNames,columnNames", required = true);
@@ -42,8 +53,8 @@ The flag is read via `$get("useUnderscoreReferenceColumns")` inside `references(
 
 ## Anti-patterns to watch for in this directory
 
-1. **Mixing helper-style and standalone-style argument names.** `t.references(columnNames=...)` (helper inside `createTable`) and `addReference(table=..., columnName=...)` (standalone Migration.cfc method) currently use slightly different parameter shapes — see [#2781](https://github.com/wheels-dev/wheels/issues/2781) for the open consistency follow-up. When in doubt, match what's already in the file.
-2. **Hard-coding `& "id"` or `& "type"` concatenations.** `TableDefinition.cfc::references()`, `Migration.cfc::removeColumn(referenceName=...)`, and `Migration.cfc::addReference()` all resolve the suffix through `$get("useUnderscoreReferenceColumns")` ([#2781](https://github.com/wheels-dev/wheels/issues/2781)). If you add new code that builds a reference column name, route it through `$get` too.
+1. **Mixing helper-style and standalone-style argument names.** Both `t.references(columnNames=...)` (helper inside `createTable`) and `addReference(table=..., columnName=...)` (standalone Migration.cfc method) now accept the modern `columnNames` / `columnName` aliases via `$combineArguments`, alongside their legacy `referenceNames` / `referenceName` originals. Prefer the modern form in new code; the legacy names keep working.
+2. **Hard-coding `& "id"` or `& "type"` concatenations.** All four sites in this directory resolve the reference-column suffix through `$get("useUnderscoreReferenceColumns")` — `TableDefinition.cfc::references()` (id + polymorphic type), `Migration.cfc::removeColumn` (referenceName branch), and `Migration.cfc::addReference`. If you add new code that builds a reference column name, route it through `$get` too rather than hard-coding `& "id"`.
 3. **`required` on column-name parameters.** Use `$combineArguments(... required=true)` instead. Declaring CFML-level `required` blocks the alias path because validation runs before the function body.
 
 ## Tests

vendor/wheels/migrator/Migration.cfc

@@ -153,7 +153,8 @@ component extends="Base" {
 	 * @table The Name of the table to add the column to
 	 * @columnType The type of the new column
 	 * @afterColumn The name of the column which this column should be inserted after
-	 * @columnName THe name of the new column
+	 * @columnName The name of the new column
+	 * @columnNames Modern alias for `columnName` (matches the plural form every TableDefinition column helper accepts). Pass one or the other — not both.
 	 * @referenceName Name for new reference column, see documentation for references function, required if columnType is 'reference'
 	 * @default Default value for this column
 	 * @allowNull Whether to allow NULL values
@@ -164,7 +165,8 @@ component extends="Base" {
 	public void function addColumn(
 		required string table,
 		required string columnType,
-		required string columnName = "",
+		string columnName,
+		string columnNames,
 		string afterColumn = "",
 		string referenceName = "",
 		string default,
@@ -173,6 +175,12 @@ component extends="Base" {
 		numeric precision,
 		numeric scale
 	) {
+		// Resolve the alias here so addColumn is self-contained — a future
+		// refactor that stops delegating to changeColumn won't silently lose
+		// the columnNames alias path. Required unless columnType="reference"
+		// (in that branch the column name is computed from referenceName, so
+		// columnName/columnNames are not needed).
+		$combineArguments(args = arguments, combine = "columnName,columnNames", required = (arguments.columnType != "reference"));
 		arguments.addColumns = true;
 		changeColumn(argumentCollection = arguments);
 	}
@@ -185,7 +193,8 @@ component extends="Base" {
 	 * [category: Migration Functions]
 	 *
 	 * @table The Name of the table where the column is
-	 * @columnName THe name of the column
+	 * @columnName The name of the column
+	 * @columnNames Modern alias for `columnName` (matches the plural form every TableDefinition column helper accepts). Pass one or the other — not both.
 	 * @columnType The type of the column
 	 * @afterColumn The name of the column which this column should be inserted after
 	 * @referenceName Name for reference column, see documentation for references function, required if columnType is 'reference'
@@ -198,7 +207,8 @@ component extends="Base" {
 	 */
 	public void function changeColumn(
 		required string table,
-		required string columnName,
+		string columnName,
+		string columnNames,
 		required string columnType,
 		string afterColumn = "",
 		string referenceName = "",
@@ -209,12 +219,20 @@ component extends="Base" {
 		numeric scale,
 		boolean addColumns = "false"
 	) {
+		// Accept columnNames as alias for columnName (consistency with
+		// TableDefinition column helpers — #2781 follow-up). Required unless
+		// columnType="reference" — that branch ignores columnName and uses
+		// referenceName instead. Restores the missing-arg enforcement the
+		// original `required string columnName` parameter provided before
+		// this PR widened the signature to accept the alias.
+		$combineArguments(args = arguments, combine = "columnName,columnNames", required = (arguments.columnType != "reference"));
+
 		var t = changeTable(arguments.table);
 		if (arguments.columnType == "reference") {
 			arguments.columnType = "references";
 			arguments.referenceNames = arguments.referenceName;
 		} else {
-			arguments.columnNames = arguments.columnName;
+			arguments.columnNames = arguments.columnName ?: "";
 		}
 		invoke(t, arguments.columnType, arguments);
 		t.change(addColumns = arguments.addColumns);
@@ -251,9 +269,23 @@ component extends="Base" {
 	 *
 	 * @table The table containing the column to remove
 	 * @columnName The column name to remove
+	 * @columnNames Modern alias for `columnName` (matches the plural form every TableDefinition column helper accepts). Pass one or the other — not both.
 	 * @referenceName optional reference name
 	 */
-	public void function removeColumn(required string table, string columnName = "", string referenceName = "") {
+	public void function removeColumn(
+		required string table,
+		string columnName,
+		string columnNames,
+		string referenceName = ""
+	) {
+		// Accept columnNames as alias for columnName via the standard helper —
+		// matches every other site in this file. If both are passed, the alias
+		// (columnNames) wins, consistent with $combineArguments precedence
+		// across the framework.
+		$combineArguments(args = arguments, combine = "columnName,columnNames", required = false);
+		if (!StructKeyExists(arguments, "columnName")) {
+			arguments.columnName = "";
+		}
 		if (arguments.referenceName != "") {
 			local.idSuffix = $get("useUnderscoreReferenceColumns") ? "_id" : "id";
 			arguments.columnName = arguments.referenceName & local.idSuffix;
@@ -271,8 +303,21 @@ component extends="Base" {
 	 *
 	 * @table The table name to perform the operation on
 	 * @referenceName The reference table name to perform the operation on
+	 * @columnName Alias for `referenceName` (consistent with the modern migrator surface — `columnName` / `columnNames` are accepted alongside the legacy form).
+	 * @columnNames Plural alias for `referenceName`. When both `columnName` and `columnNames` are supplied, `columnNames` wins.
 	 */
-	public void function addReference(required string table, required string referenceName) {
+	public void function addReference(
+		required string table,
+		string referenceName,
+		string columnName,
+		string columnNames
+	) {
+		// Accept columnName / columnNames as aliases for referenceName.
+		// Precedence (per $combineArguments semantics): the alias wins. If a
+		// caller passes both `referenceName` and `columnName`/`columnNames`,
+		// the alias overwrites — same shape every other helper here uses.
+		$combineArguments(args = arguments, combine = "referenceName,columnName", required = false);
+		$combineArguments(args = arguments, combine = "referenceName,columnNames", required = true);
 		local.idSuffix = $get("useUnderscoreReferenceColumns") ? "_id" : "id";
 		addForeignKey(
 			table = arguments.table,
@@ -292,14 +337,19 @@ component extends="Base" {
 	 * @table The table name to perform the operation on
 	 * @referenceTable The reference table name to perform the operation on
 	 * @column The column name to perform the operation on
+	 * @columnName Modern alias for `column` (consistent with the rest of the migrator surface).
 	 * @referenceColumn The reference column name to perform the operation on
 	 */
 	public void function addForeignKey(
 		required string table,
 		required string referenceTable,
-		required string column,
+		string column,
+		string columnName,
 		required string referenceColumn
 	) {
+		// Accept columnName as alias for column (consistency with the rest
+		// of the migrator surface).
+		$combineArguments(args = arguments, combine = "column,columnName", required = true);
 		var foreignKey = CreateObject("component", "ForeignKeyDefinition").init(
 			adapter = this.adapter,
 			argumentCollection = arguments
@@ -317,9 +367,21 @@ component extends="Base" {
 	 *
 	 * @table The table name to perform the operation on
 	 * @referenceName the name of the reference to drop
+	 * @columnName Alias for `referenceName` (consistent with the modern migrator surface — `columnName` / `columnNames` are accepted alongside the legacy form).
+	 * @columnNames Plural alias for `referenceName`. When both `columnName` and `columnNames` are supplied, `columnNames` wins.
 	 *
 	 */
-	public void function dropReference(required string table, required string referenceName) {
+	public void function dropReference(
+		required string table,
+		string referenceName,
+		string columnName,
+		string columnNames
+	) {
+		// Accept columnName / columnNames as aliases for referenceName.
+		// Precedence (per $combineArguments semantics): the alias wins. See
+		// addReference for the same pattern.
+		$combineArguments(args = arguments, combine = "referenceName,columnName", required = false);
+		$combineArguments(args = arguments, combine = "referenceName,columnNames", required = true);
 		dropForeignKey(arguments.table, "FK_#arguments.table#_#pluralize(arguments.referenceName)#");
 	}
 

vendor/wheels/tests/specs/migrator/migrationCommandsSpec.cfc

@@ -0,0 +1,173 @@
+/**
+ * Coverage for the consistency-sweep changes to Migration.cfc command-version
+ * helpers (the standalone counterparts to TableDefinition's t.X() builders).
+ *
+ * Follow-up to #2781 (PR #2802). The TableDefinition layer was fixed in PR1
+ * and PR1's review iterations also added DB-roundtrip coverage for the
+ * `Migration.cfc::addReference()` and `Migration.cfc::removeColumn(referenceName=)`
+ * suffix-flag behavior — those live in `migrationSpec.cfc` and are not
+ * duplicated here. This spec covers what PR2 uniquely contributes:
+ *
+ *   - `addColumn` / `changeColumn` / `removeColumn` accept the plural
+ *     `columnNames` as an alias for the legacy singular `columnName`,
+ *     matching the convention every helper in `TableDefinition.cfc` already
+ *     follows via `$combineArguments`.
+ *
+ * `addReference` / `dropReference` `columnName`/`columnNames` aliases for
+ * the legacy `referenceName` are exercised at the unit level by the
+ * `$combineArguments` calls themselves (any regression would surface as
+ * IncorrectArguments errors in the existing migrator suite).
+ */
+component extends="wheels.WheelsTest" {
+
+	include "helperFunctions.cfm";
+
+	function beforeAll() {
+		variables.migration = CreateObject("component", "wheels.migrator.Migration").init();
+	}
+
+	function run() {
+
+		describe("Migration.cfc — argument aliases via integration", () => {
+
+			it("addColumn accepts columnNames alias and actually adds the column", () => {
+				if (!isDbCompatible()) return;
+				var t = variables.migration.createTable(name = "dbm_cmd_addcol_test", force = true);
+				t.string(columnNames = "placeholder");
+				t.create();
+				variables.migration.addColumn(
+					table = "dbm_cmd_addcol_test",
+					columnNames = "age",
+					columnType = "integer"
+				);
+				expect(ListFindNoCase(variables.migration.$getColumns("dbm_cmd_addcol_test"), "age")).toBeGT(0);
+				variables.migration.dropTable("dbm_cmd_addcol_test");
+			});
+
+			it("changeColumn accepts columnNames alias directly (not via addColumn delegation)", () => {
+				if (!isDbCompatible()) return;
+				// Direct changeColumn call — guards against a future refactor of
+				// addColumn that stops delegating and would silently lose the
+				// columnNames alias path inside changeColumn itself.
+				var t = variables.migration.createTable(name = "dbm_cmd_changecol_test", force = true);
+				t.integer(columnNames = "age");
+				t.create();
+				variables.migration.changeColumn(
+					table = "dbm_cmd_changecol_test",
+					columnNames = "age",
+					columnType = "integer"
+				);
+				expect(ListFindNoCase(variables.migration.$getColumns("dbm_cmd_changecol_test"), "age")).toBeGT(0);
+				variables.migration.dropTable("dbm_cmd_changecol_test");
+			});
+
+			it("removeColumn accepts columnNames alias and actually drops the column", () => {
+				if (!isDbCompatible()) return;
+				var t = variables.migration.createTable(name = "dbm_cmd_rmcol_test", force = true);
+				t.integer(columnNames = "age");
+				t.create();
+				expect(ListFindNoCase(variables.migration.$getColumns("dbm_cmd_rmcol_test"), "age")).toBeGT(0);
+				variables.migration.removeColumn(
+					table = "dbm_cmd_rmcol_test",
+					columnNames = "age"
+				);
+				expect(ListFindNoCase(variables.migration.$getColumns("dbm_cmd_rmcol_test"), "age")).toBe(0);
+				variables.migration.dropTable("dbm_cmd_rmcol_test");
+			});
+
+			it("addForeignKey accepts columnName alias for the legacy column parameter", () => {
+				// SQLite doesn't support altering CONSTRAINTS, so addForeignKey
+				// fires a SQL error there. We only need to verify the alias
+				// resolved (no IncorrectArguments thrown); the downstream SQL
+				// failure on SQLite is irrelevant to the alias contract.
+				var t = variables.migration.createTable(name = "dbm_cmd_addfk_alias_test", force = true);
+				t.integer(columnNames = "userid");
+				t.create();
+				var aliasAccepted = true;
+				try {
+					variables.migration.addForeignKey(
+						table = "dbm_cmd_addfk_alias_test",
+						referenceTable = "users",
+						columnName = "userid",
+						referenceColumn = "id"
+					);
+				} catch (Wheels.IncorrectArguments e) {
+					aliasAccepted = false;
+				} catch (any e) {}
+				variables.migration.dropTable("dbm_cmd_addfk_alias_test");
+				expect(aliasAccepted).toBeTrue();
+			});
+
+			it("dropReference accepts columnName alias for the legacy referenceName parameter", () => {
+				// dropReference resolves to dropForeignKey by name pattern
+				// (FK_<table>_<pluralized-reference>). If no such FK exists,
+				// dropForeignKey errors at the SQL layer — that's fine here;
+				// we only care that the alias resolved before SQL ran.
+				var aliasAccepted = true;
+				try {
+					variables.migration.dropReference(
+						table = "dbm_cmd_dropref_alias_table",
+						columnName = "user"
+					);
+				} catch (Wheels.IncorrectArguments e) {
+					aliasAccepted = false;
+				} catch (any e) {}
+				expect(aliasAccepted).toBeTrue();
+			});
+
+		});
+
+		describe("Migration.cfc — required-arg regression guards", () => {
+
+			it("addColumn throws Wheels.IncorrectArguments when neither columnName nor columnNames is provided (non-reference)", () => {
+				// Prior to PR2 the original `required string columnName = ""`
+				// signature enforced param presence at the CFML level. The
+				// widened signature uses $combineArguments(required=true) to
+				// restore equivalent enforcement for non-reference column types.
+				var threwIncorrectArgs = false;
+				try {
+					variables.migration.addColumn(
+						table = "dbm_cmd_reqcheck_test",
+						columnType = "integer"
+					);
+				} catch (Wheels.IncorrectArguments e) {
+					threwIncorrectArgs = true;
+				} catch (any e) {}
+				expect(threwIncorrectArgs).toBeTrue();
+			});
+
+			it("changeColumn throws Wheels.IncorrectArguments when neither columnName nor columnNames is provided (non-reference)", () => {
+				var threwIncorrectArgs = false;
+				try {
+					variables.migration.changeColumn(
+						table = "dbm_cmd_reqcheck_test",
+						columnType = "integer"
+					);
+				} catch (Wheels.IncorrectArguments e) {
+					threwIncorrectArgs = true;
+				} catch (any e) {}
+				expect(threwIncorrectArgs).toBeTrue();
+			});
+
+			it("addColumn does NOT require columnName for columnType='reference' (referenceName takes its place)", () => {
+				// Reference-type column construction relies on referenceName,
+				// not columnName. The conditional required check must skip
+				// this branch.
+				var threwIncorrectArgs = false;
+				try {
+					variables.migration.addColumn(
+						table = "dbm_cmd_refcheck_test",
+						columnType = "reference",
+						referenceName = "user"
+					);
+				} catch (Wheels.IncorrectArguments e) {
+					threwIncorrectArgs = true;
+				} catch (any e) {}
+				expect(threwIncorrectArgs).toBeFalse();
+			});
+
+		});
+
+	}
+
+}