DB::require_table type mismatch.

[maxime-rainville]

Module version(s) affected

Basically every recent version.

Description

The PHPDoc for DB::require_table says it expects this method signature:

    /**
     * @param string $table The name of the table
     * @param string $fieldSchema A list of the fields to create, in the same form as DataObject::$db
     * @param string $indexSchema A list of indexes to create.  The keys of the array are the names of the index.
     * @param boolean $hasAutoIncPK A flag indicating that the primary key on this table is an autoincrement type
     * @param string $options SQL statement to append to the CREATE TABLE call.
     * @param array $extensions List of extensions
     */

This is clearly wrong. $fieldSchema, $indexSchema and $options are meant to be arrays, not strings. Trying to pass string to these parameter will lead to crashes and misery.

Looking at actual usage, it clearly expects those parameters to be array:

silverstripe-framework/src/ORM/DataObject.php

Lines 3683 to 3710 in ebbd642

	public function requireTable()
	{
	// Only build the table if we've actually got fields
	$schema = static::getSchema();
	$table = $schema->tableName(static::class);
	$fields = $schema->databaseFields(static::class, false);
	$indexes = $schema->databaseIndexes(static::class, false);
	$extensions = DataObject::database_extensions(static::class);
	if (empty($table)) {
	throw new LogicException(
	"Class " . static::class . " not loaded by manifest, or no database table configured"
	);
	}
	if ($fields) {
	$hasAutoIncPK = get_parent_class($this ?? '') === DataObject::class;
	DB::require_table(
	$table,
	$fields,
	$indexes,
	$hasAutoIncPK,
	$this->config()->get('create_table_options'),
	$extensions
	);
	} else {
	DB::dont_require_table($table);
	}

The method itself is just an alias for DB::get_schema()->requireTable which has the correct signature.

How to reproduce

NA

Possible Solution

While changing the PHPDoc signature could arguably be called a "breaking change", there's no way the current stated signature could ever work. So it's clearly a mistake.

I think the only sensible fix here is to take the signature from DBSchemaManager::requireTable and apply it to DB::require_table.

Additional Context

No response

Validations

• Check that there isn't already an issue that reports the same bug
• Double check that your reproduction steps work in a fresh installation of silverstripe/installer (with any code examples you've provided)

Pull request

• DOC Document correct method signature for DB::require_table #11425

[GuySartorelli]

While changing the PHPDoc signature could arguably be called a "breaking change"

It absolutely could not. PHPDocs are just documentation of how the code works and how to use it. If the PHPDoc doesn't reflect how the code works, it's almost certainly just plain wrong (there may be some cases where the code is wrong and the PHPDoc is right - this is not one of those cases).

It's worth noting that there are a lot of incorrect PHPDoc types in the codebase unfortunately, which is a major reason why strong typing hasn't been introduced across the board yet.

Thanks for raising this and a matching PR.