fix(migrator): read the pre-3.0 null column option as an alias of allowNull

changelog.d/migration-null-alias.fixed.md

@@ -0,0 +1 @@
+- Migration column functions (`t.string()` and the others, `t.column()`, `t.references()`) read `null`, the option's name before Wheels 3.0, as a deprecated alias of `allowNull` again. Since 3.0 it was ignored, so an older migration's `null=false` created a nullable column. `allowNull` wins when both are passed, and the deprecation is logged once per migration run. `wheels upgrade check` lists migrations that use `null=false` or `null=true`; check `null=false` columns for `NULL` rows before tightening them

cli/lucli/Module.cfc

@@ -9437,6 +9437,25 @@ component extends="modules.BaseModule" {
 			scanTargets: [{path: ".gitignore"}],
 			fix: "Remove the vendor line and commit vendor/, so a clone, CI run or image build has the framework and your packages. Guide: ""Commit vendor/"", #guide#"
 		});
+		// `null` was the migration column option's name before 3.0 renamed it allowNull, with no
+		// alias until 4.2. From 3.0 through 4.1 a null=false column was created nullable; 4.2 reads
+		// null=false again, so a database rebuilt on 4.2 can be stricter than one built earlier.
+		arrayAppend(arguments.checks, {
+			description: "Migrations use null=false, the column option's name before Wheels 3.0",
+			severity: "advisory",
+			checkType: "grep",
+			pattern: "[,(]\s*null\s*=\s*[""']?(false|no|0)[""']?\s*[,)]",
+			scanTargets: [{path: "app/migrator/migrations", extensions: "cfc", recurse: true}],
+			fix: "Wheels 3.0 through 4.1 ignored null=false, so a database built or rebuilt by them from these migrations has these columns NULLABLE, and they may now hold NULL rows. 4.2 reads null=false again: a database rebuilt on 4.2 gets NOT NULL columns, stricter than the one you run. Check each column for NULL rows before you tighten it with changeColumn(allowNull=false), and rename null to allowNull in the migrations. Guide: ""Migration column option null"", #guide#"
+		});
+		arrayAppend(arguments.checks, {
+			description: "Migrations use null=true, the column option's name before Wheels 3.0",
+			severity: "advisory",
+			checkType: "grep",
+			pattern: "[,(]\s*null\s*=\s*[""']?(true|yes|1)[""']?\s*[,)]",
+			scanTargets: [{path: "app/migrator/migrations", extensions: "cfc", recurse: true}],
+			fix: "4.2 reads null as a deprecated alias of allowNull and logs a warning. Rename it to allowNull. Guide: ""Migration column option null"", #guide#"
+		});
 		arrayAppend(arguments.checks, {
 			description: "findAll(returnAs=""structs"") now returns an array",
 			severity: "advisory",

cli/lucli/tests/specs/commands/UpgradeCheck4x2Spec.cfc

@@ -188,6 +188,30 @@ component extends="wheels.wheelstest.system.BaseSpec" {
 				expect(found).toBe(["app/controllers/Signups.cfc:4"]);
 			});
 
+			it("lists migrations that use null=false and null=true separately", () => {
+				var nl = chr(10);
+				put("app/migrator/migrations/20180519_Users.cfc", "component extends=""wheels.migrator.Migration"" {" & nl
+					& "function up() {" & nl
+					& "t.string(columnNames='email', default='', null=false, limit='255');" & nl
+					& "t.text(columnNames='bio', null=true);" & nl
+					& "t.integer(columnNames='age', allowNull=false);" & nl
+					& "}" & nl & "}");
+				var report = runCheck();
+				expect(matchesFor(report.advisories, "use null=false")).toBe(["app/migrator/migrations/20180519_Users.cfc:3"]);
+				expect(matchesFor(report.advisories, "use null=true")).toBe(["app/migrator/migrations/20180519_Users.cfc:4"]);
+			});
+
+			it("doesn't list allowNull or a commented-out null=", () => {
+				var nl = chr(10);
+				put("app/migrator/migrations/20180519_Users.cfc", "component extends=""wheels.migrator.Migration"" {" & nl
+					& "// t.string(columnNames='email', null=false);" & nl
+					& "function up() { t.string(columnNames='email', allowNull=false); }" & nl
+					& "}");
+				var report = runCheck();
+				expect(has(report.advisories, "use null=false")).toBeFalse();
+				expect(has(report.advisories, "use null=true")).toBeFalse();
+			});
+
 			it("doesn't flag persisted in a comment or inside another name", () => {
 				put("app/controllers/Signups.cfc", "component {#chr(10)#// if (!result.persisted) {}#chr(10)#function create() { var persistedCount = 1; }#chr(10)#}");
 				expect(has(runCheck().advisories, "reads persisted from a job result")).toBeFalse();

vendor/wheels/migrator/TableDefinition.cfc

@@ -103,6 +103,7 @@ component extends="Base" {
 		numeric precision,
 		numeric scale
 	) {
+		$applyNullAlias(arguments);
 		arguments.adapter = this.adapter;
 		arguments.name = arguments.columnName;
 		arguments.type = arguments.columnType;
@@ -124,6 +125,40 @@ component extends="Base" {
 		return this;
 	}
 
+	/**
+	 * Internal function. `null` was the column option's name before 3.0, when it became `allowNull`.
+	 * It's read again as a deprecated alias, so an older migration's `null = false` still gives a
+	 * NOT NULL column. `allowNull` wins when both are passed. Read through arguments["null"]: `null`
+	 * is a keyword on some engines.
+	 */
+	public void function $applyNullAlias(required struct args) {
+		if (!StructKeyExists(arguments.args, "null")) {
+			return;
+		}
+		if (!StructKeyExists(arguments.args, "allowNull")) {
+			arguments.args.allowNull = arguments.args["null"];
+		}
+		StructDelete(arguments.args, "null");
+		$warnNullAlias();
+	}
+
+	/**
+	 * Internal function. Logs the `null` deprecation once per request (one migration run), not once
+	 * per column. Returns whether it logged.
+	 */
+	public boolean function $warnNullAlias() {
+		if (StructKeyExists(request, "$wheelsMigratorNullAliasWarned")) {
+			return false;
+		}
+		request.$wheelsMigratorNullAliasWarned = true;
+		writeLog(
+			file = "wheels",
+			type = "warning",
+			text = "Wheels: a migration column uses `null`, the option's name before Wheels 3.0. It's read as `allowNull` for now; rename it to `allowNull`."
+		);
+		return true;
+	}
+
 	/**
 	 * Shared implementation for the typed column helpers below: resolves the
 	 * columnNames/columnName alias, stamps the column type, and adds one column
@@ -133,6 +168,7 @@ component extends="Base" {
 	 */
 	private any function $addTypedColumns(required string columnType, required struct args) {
 		$combineArguments(args = arguments.args, combine = "columnNames,columnName", required = true);
+		$applyNullAlias(arguments.args);
 		arguments.args.columnType = arguments.columnType;
 		local.columnNamesArray = ListToArray(arguments.args.columnNames);
 		local.iEnd = ArrayLen(local.columnNamesArray);
@@ -340,14 +376,18 @@ component extends="Base" {
 		string referenceNames,
 		string columnNames,
 		default,
-		boolean allowNull = "false",
+		boolean allowNull,
 		boolean polymorphic = "false",
 		boolean foreignKey = "true",
 		string onUpdate = "",
 		string onDelete = "",
 		string referenceColumn = "id"
 	) {
 		$combineArguments(args = arguments, combine = "referenceNames,columnNames", required = true);
+		$applyNullAlias(arguments);
+		if (!StructKeyExists(arguments, "allowNull")) {
+			arguments.allowNull = false;
+		}
 		local.idSuffix = $get("useUnderscoreReferenceColumns") ? "_id" : "id";
 		local.typeSuffix = $get("useUnderscoreReferenceColumns") ? "_type" : "type";
 		local.referenceNamesArray = ListToArray(arguments.referenceNames);

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

@@ -0,0 +1,74 @@
+/**
+ * `null` is the column option's 2.x name, renamed `allowNull` in 3.0. It's a deprecated alias
+ * again: column(), the typed column functions and references() read it when `allowNull` isn't
+ * passed, so a 2.x migration's `null = false` gives a NOT NULL column. `null` is a keyword on some
+ * engines, so the specs pass it through argumentCollection. No table is created.
+ */
+component extends="wheels.WheelsTest" {
+
+	function beforeAll() {
+		variables.migration = CreateObject("component", "wheels.migrator.Migration").init();
+	}
+
+	function newTable() {
+		return variables.migration.createTable(name = "c_o_r_e_nullalias", force = true, id = false);
+	}
+
+	// Column options with `null` set to `value`, plus `extra`.
+	function options(required struct extra, required boolean value) {
+		var rv = Duplicate(arguments.extra);
+		rv["null"] = arguments.value;
+		return rv;
+	}
+
+	function lastColumn(required any table) {
+		return arguments.table.columns[ArrayLen(arguments.table.columns)];
+	}
+
+	function run() {
+
+		describe("The null column option", () => {
+
+			it("sets allowNull on the typed column functions", () => {
+				var t = newTable();
+				t.string(argumentCollection = options({columnNames = "title"}, false));
+				expect(lastColumn(t).allowNull).toBeFalse();
+				t.integer(argumentCollection = options({columnNames = "views"}, true));
+				expect(lastColumn(t).allowNull).toBeTrue();
+			});
+
+			it("sets allowNull on column() and references()", () => {
+				var t = newTable();
+				t.column(argumentCollection = options({columnName = "code", columnType = "string"}, false));
+				expect(lastColumn(t).allowNull).toBeFalse();
+				t.references(argumentCollection = options({columnNames = "author"}, true));
+				expect(lastColumn(t).allowNull).toBeTrue();
+			});
+
+			it("loses to allowNull when both are passed", () => {
+				var t = newTable();
+				t.string(argumentCollection = options({columnNames = "title", allowNull = true}, false));
+				expect(lastColumn(t).allowNull).toBeTrue();
+				t.references(argumentCollection = options({columnNames = "author", allowNull = false}, true));
+				expect(lastColumn(t).allowNull).toBeFalse();
+			});
+
+			it("keeps references() NOT NULL by default", () => {
+				var t = newTable();
+				t.references(columnNames = "author");
+				expect(lastColumn(t).allowNull).toBeFalse();
+			});
+
+			it("logs the deprecation once per request", () => {
+				var t = newTable();
+				StructDelete(request, "$wheelsMigratorNullAliasWarned");
+				t.string(argumentCollection = options({columnNames = "a"}, false));
+				expect(StructKeyExists(request, "$wheelsMigratorNullAliasWarned")).toBeTrue();
+				expect(t.$warnNullAlias()).toBeFalse();
+			});
+
+		});
+
+	}
+
+}

web/sites/guides/src/content/docs/v4-2-0/upgrading/4x-1-to-4x-2.mdx

@@ -352,6 +352,16 @@ Instances that share a database now take turns running migrations ([#4134](https
 
 In a migration, `default=""` on a column that isn't string-like (`bigInteger`, `binary`, `uniqueidentifier`, `integer`, `decimal`, `float`, `date` and so on) now means "no default" ([#4248](https://github.com/wheels-dev/wheels/pull/4248)). It renders as `DEFAULT NULL` when the column allows null and as no `DEFAULT` clause on a `NOT NULL` column. In 4.1, `bigInteger` and `uniqueidentifier` emitted a bare `DEFAULT`, `binary` emitted `DEFAULT ''`, and a `NOT NULL` column got `DEFAULT NULL NOT NULL`, which MySQL rejects; those migrations failed. They now run. Migrations that already worked produce the same schema.
 
+### Migration column option `null`
+
+Before Wheels 3.0, a migration column's nullability option was called `null`. 3.0 renamed it `allowNull` and dropped the old name, so Wheels 3.0 through 4.1 ignored `null`. No `NULL` / `NOT NULL` clause was emitted, and the column got the database's default, nullable on most databases. 4.2 reads `null` again as a deprecated alias of `allowNull`: `allowNull` wins when both are passed, and a warning is logged once per migration run.
+
+This matters for `null=false`. A database built or rebuilt by 3.0 through 4.1 from such a migration has the column nullable, and it may hold `NULL` rows. A database rebuilt on 4.2 from the same migration gets a `NOT NULL` column, stricter than the one you run in production. `wheels upgrade check` lists the migrations that use `null=false` and `null=true`. For each `null=false` column:
+
+1. Check the running database for `NULL` rows in that column.
+2. Fix or backfill them, then tighten the column with a new migration (`changeColumn(..., allowNull=false)`), or change the old migration to `allowNull=true` if the column should allow `NULL`.
+3. Rename `null` to `allowNull` in the migrations.
+
 ### Unsupported migration column types fail at migrate time
 
 A migration that uses a column type the database adapter doesn't map, such as `t.column(columnName="payload", columnType="jsonb")`, now fails with `Wheels.Migrator.UnsupportedColumnType`, naming the type and the adapter ([#4095](https://github.com/wheels-dev/wheels/issues/4095)). Earlier releases emitted the column with no type. On most databases that was already a DDL error. On SQLite it created a typeless column whose model failed to load. If an existing SQLite migration relies on that, change the column to a supported type (for example `text` for JSON) before running it on a fresh database. The error's details list the supported types. Every type the `t.*()` column helpers emit is supported on every adapter.