refactor: Types for BaseModel, Model and dependencies

[neznaika0]

Description
Fixes #9711
It is better to merge the develop branch for the review.

• Removed the check for a valid ID as method. It's too hard to choose a direction. Returned the deleted checks
• Added dots in comments, some formatting
• Updated PHPDoc in phpstan style
• Inherited comments have been removed
• Updated phpstan-baseline

I have re-examined the proposed ID check method. I realized that it is less useful for BaseModel. The method is useful in the Model. As long as you consider the check to be breaking changes, let everything stay as it was.

There is a note for row_array - in the case of casting, the value can be mixed. Contain any type inside, which is converted to scalars. Find the error in utils/phpstan-baseline/argument.type.neon DataConverterModelTest.php

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[michalsn]

Please update Deptrac.

[michalsn]

The point of this check is to convert a variable to an array so that the events can properly consume it. Please revert.

[neznaika0]

I adapted the old PR, lost contact with the Model. in doDelete() doUpdate() we have already checked this. That's right.

I wanted the array to be passed to the event only when it is a valid value...

        $eventData = [
            'id'    => $id, // [0 => '0'] It looks wrong
            'purge' => $purge,
        ];

I'll cancel it.

[neznaika0]

Heh, after deleting the check, we get errors in the tests. I was confused by the comment - this change was already in 4.7

To improve, you can add an array check for empty values [0 => 123, 1 => 456, 2 => null].

[michalsn]

No, there is no such code in either the develop or 4.7 branches. The diff is clear - this change was introduced in your commit.

[neznaika0]

Ahh, I'm looking at delete()

CodeIgniter4/system/BaseModel.php

Lines 1101 to 1109 in 4c88a27

	public function delete($id = null, bool $purge = false)
	{
	if (is_bool($id)) {
	throw new InvalidArgumentException('delete(): argument #1 ($id) should not be boolean.');
	}
	if (! in_array($id, [null, 0, '0'], true) && (is_numeric($id) || is_string($id))) {
	$id = [$id];
	}

BUT update() requires a similar behavior, otherwise, we will get a bad SQL. The principle is the same - you cannot update or delete the values of ID 0, null,.. Am I wrong?

UPDATE `db_job` SET `name` = 'Foo Bar'
WHERE `db_job`.`id` IN (0)
UPDATE `db_job` SET `name` = 'Foo Bar'
WHERE `db_job`.`id` IN ('')

To confirm, I can update the tests for provideUpdateThrowDatabaseExceptionWithoutWhereClause

[neznaika0]

I understand what you're talking about. Events always need an array. This can be fixed by moving the check to doUpdate() doDelete().

[neznaika0]

@michalsn @paulbalandan I have returned the previous check state to the BaseModel.
Add a method to check the isValidId($id) (Yes, I found a use for it).
Added tests for empty values for delete($id)/update($id). Is fine - we get the "Empty WHERE" exception.
But we can specify where('id', [15 => 123, 16 => null])->delete() In this case, 100% of the $id will be empty and sql does not need to be executed, but this is not an error.

Question: Do I need to check binds for an empty PK? We get SQL error '... near ")"' in "... WHERE id IN ()".
Next question is whether it is necessary to handle the case with where('id", ...)->delete('0')?

These changes should improve protection against the error of missing an ID when updating a record. Or should the user just get an SQL exception?

[michalsn]

Please do not make any changes to the behavior of checking the PK in this PR. It has to be addressed in a separate PR.

[neznaika0]

Yes, it in new PR. I want to understand whether to create it or leave the sql exception as it is?

[michalsn]

Sorry, but we won't be discussing a "virtual PR" here. I'm not sure what changes you've made there, and I needed some time to review the current state so we can choose the right direction moving forward.

[michalsn]

I think this looks good to me.

[michalsn]

Thank you @neznaika0