Start migration to TableEditor API

[greg0ire]

The TableEditor API has no equivalent for this. Avoiding that call should allow migrating to that API.

[greg0ire]

This is why this PR is called "start migration to TableEditor API"

By making sure I end up with a table beyond this point, I make this big PR hopefully not too big.

[morozov]

I just came here to comment on this block: after it, $tableOrEditor is guaranteed to be a table, so it may be worth assigning the result to Table $table and revert the following changes that replace $table with $tableOrEditor.

[greg0ire]

Will do, it will be less confusing.

[greg0ire]

@morozov do you think this is useful? If not, should the deprecation message be improved to indicate that calling that method with e.g. foo achieves nothing and will cause an error in 5.0.x? I'm asking because ORM has a test ensuring foo is preserved:

orm/tests/Tests/ORM/Tools/SchemaToolTest.php

Lines 88 to 104 in 883c76a

	public function testAttributeOptionsArgument(): void
	{
	$em = $this->getTestEntityManager();
	$schemaTool = new SchemaTool($em);
	$schema = $schemaTool->getSchemaFromMetadata(
	[$em->getClassMetadata(TestEntityWithAttributeOptionsArgument::class)],
	);
	$table = $schema->getTable('TestEntityWithAttributeOptionsArgument');
	foreach ([$table->getOptions(), $table->getColumn('test')->getPlatformOptions()] as $options) {
	self::assertArrayHasKey('foo', $options);
	self::assertSame('bar', $options['foo']);
	self::assertArrayHasKey('baz', $options);
	self::assertSame(['key' => 'val'], $options['baz']);
	}
	}

I'm wondering if I should remove this piece of code entirely, or trigger a deprecation if we enter it.

[morozov]

@morozov do you think this is useful?

In theory it may be useful: someone may have a custom platform with an option not known to the ORM. In practice, I don't think so.

If not, should the deprecation message be improved to indicate that calling that method with e.g. foo achieves nothing and will cause an error in 5.0.x?

I'm not following. The entire Column::setPlatformOption() is deprecated and will be removed in 5.0.x for exactly that reason: ColumnEditor provides a well-typed API for supported parameters.

[greg0ire]

Oh so right now, it might achieve something, but in the future, you won't even be able to call it with foo.

[morozov]

The change looks directionally right (I've only reviewed half the PR, and mostly found just nits).

For the issues raised in the comments above, I'd step back and look at the code in question in isolation from the migration to editors, to see if the code itself looks sane (in the two cases above, it doesn't). Then I'd address those issues separately and get back to the migration — this is the process I've been following during the API rework in DBAL itself.