Promote count() warning to TypeError

[Girgias]

Split from #4566

[Girgias]

Will look into the test failure after I've split up the PR into separate smaller ones.

[nikic]

I'm unsure about this one. One on hand this is a pretty clear case where TypeError should be thrown. On the other hand we do know that the introduction of this warning had some major fallout (possibly one of the largest BC impacts overall in the whole 7.x series), so we might need to be more careful here.

[Girgias]

Should I raise this issue on internals and see the feedback which comes with it? I would say this is inline with your TypeError RFC.

[theodorejb]

I can see how initially introducing the warning was a large BC break, but it doesn't seem like converting it to an exception is nearly so disruptive. No more than the many other warnings being converted per the consistent type errors RFC.

[nikic]

Personally I'd just replace these @count() with isset().

[Girgias]

Replacing with isset() would change the logic of this condition as the usage of count() here is to assert that ONLY one of the sections is present otherwise the test file is bogus, same below.

[nikic]

To be clear, this is what I had in mind:

Suggested change

	if (!isset($section_text['PHPDBG']) && @count($section_text['FILE']) + @count($section_text['FILEEOF']) + @count($section_text['FILE_EXTERNAL']) != 1) {
	if (!isset($section_text['PHPDBG']) && isset($section_text['FILE']) + isset($section_text['FILEEOF']) + isset($section_text['FILE_EXTERNAL']) != 1) {

Too dirty?

[Girgias]

Hum, the intention of this is kinda lost, I'll probably have a look at it again later and try to see if I can't refactor the codebase a bit better.

[nikic]

Hm, why does this change? That seems odd.

[nikic]

Should be fixed by d266ba4.

[bwoebi]

I wouldn't change this. Just skip and return null then.

[Girgias]

ACK

[nikic]

We discussed this in R11, some points:

• The initial introduction of this count() warning caused quite a lot of fallout, including in popular libraries and frameworks.

• While the use of count(new NonCountableObject) is almost certainly a bug, count(null) is rather well-defined, and probably accounts for nearly all cases where the introduction of this warning caused issues.

• As seen from the occurrence in run-tests.php, even if code is running fine on PHP >= 7.2, it may still be affected by this change due to explicit error suppression :(

• The switch to TypeError for this error condition does not eliminate an error return value for the function. It is already count(): int, so we won't see an improvement there.

Overall, I believe the consensus was against merging this change, though not everyone agreed (in particular @markrandall). Some people we in favor of dropping the warning for count(null) entirely, though I don't think anyone is planning to actively pursue this yet.