Provide additional context in the DisallowedEmptyRule error message

[morozov]

In doctrine/dbal, the Schema and Driver APIs use array<string,mixed> parameters heavily (e.g. as connection parameters and column definitions).

While overall the value type is mixed, each array element usually has a certain type.

Since most of the array elements are optional, the boolean values are checked like this:

if (isset($field['unique']) && $field['unique']) {
    // this is a unique column 
}

Or like this:

if (empty($column['fixed'])) {
    // this is a fixed-length column
}

Before reimplementing these APIs using objects with well-defined properties, I want to suppress the error reporting of such cases.

The first case is suppressed nicely like this:

-
    message: '~^Only booleans are allowed.*mixed given~'
    paths:
        - %currentWorkingDirectory%/src/Driver/*/Driver.php
        - %currentWorkingDirectory%/src/Platforms/*Platform.php
        - %currentWorkingDirectory%/src/Schema/*SchemaManager.php

However, the second cannot be pin-pointed since the error message is generic for all cases:

Construct empty() is not allowed. Use more strict comparison.

I could rework expressions like empty($array[$key]) to isset($array[$key]) && $array[$key] and suppress them as above but I'd rather do the opposite since the empty() approach is more compact and has exactly the same meaning.

Would it be possible to include the type passed to empty() so that instead of whitelisting all such cases in certain paths, I could only whitelist the cases where a mixed like the above is passed?

[ondrejmirtes]

This would be a better use-case for local ignores using comments: https://github.com/phpstan/phpstan/issues/786

[morozov]

I don't think so. The approach in the description allows white-listing a known class of problems w/o polluting code. Furthermore, with the comments approach, maintenance of this code will require adding such comments again and again just to suppress the reporting of an already known design-level problem.

[ondrejmirtes]

Yeah, I get it, but it doesn't scale. This is a very specific situation. PHPStan can't cater to all the specific situations in the world. It shouldn't make the messages less readable just so they could be ignored.

[morozov]

PHPStan can't cater to all the specific situations in the world.

Fair point.

It shouldn't make the messages less readable just so they could be ignored.

I wouldn't say the messages would be less readable. But unnecessarily verbose, yes.