phpinspectionsea
phpinspectionsea copied to clipboard
kalessil
[FR] count is missused
Description (screenshot):
Hi,
There are a lot of cases where my team do this to check if an array is empty:
if(count($strictly_an_array)){
}
If we're inside a namespace, we have an inspection that will tell us that \count($strictly_an_array) is probably better as it generate less opcodes. However,
if($strictly_an_array){
}
has the same behaviour and generate even less opcodes.
It would be cool to have an inspection to match those case and suggest to do the change.
Thanks!
It could be implemented for
strlen()andmb_strlen()for strings too.
Just checked, there is an inspection that suggest replacing strlen and mb_strlen to !== '' already, but yes, even this is not mandatory
It could be implemented for
strlen()andmb_strlen()for strings too.
No. '0' is also evaluated as false.
Yeah, that could be cleaner, but I don't think I like the syntax
if($strictly_an_array !== array()){
}
for older versions of PHP...
I think COUNT opcode (for \count) should work much faster than IS_IDENTICAL/IS_NOT_IDENTICAL opcodes to compare two arrays.
Pretty similar to https://github.com/symfony/symfony/pull/29231/files, but I would not like to promote if ($array) {}. Please vote hand up/down the issue itself, I'll pull twitter folks for votes as well.
Any expression inside if must be a boolean, let's not abuse PHP's type juggling. The only valid variants would be then:
-
if (\count($someArray) > 0) { -
if ($someArray !== []) {
Comparing to empty arrays is quite slow in PHP. I think the best of both worlds is if(empty($array)) - while empty() is generally very type-unsafe it has no magic with arrays (or at least I don't know about any yet :P)
Any expression inside
ifmust be a boolean, let's not abuse PHP's type juggling. The only valid variants would be then:1. `if (\count($someArray) > 0) {` 2. `if ($someArray !== []) {`
if ((bool)$someArray) {
Could also make sense.
if ((bool)$someArray) {
No, this is making the expression technically boolean but still misusing the truthiness/falsiness magic of PHP.
\is_array($array) && \count($array) === 0
In the example, it was given that the variable was an array so \is_array is redundant and breaks the whole purpose to simplify and optimize.
If it's not given that the variable is an array, your code change behaviour
I think it's always a misuse if you use count for something else than actual counting. If, as in example, variable is an actual strict array, empty() is the way to go.
@k0d3r1s I disagree somehow. I mean basically you're right that count() should be used for actual counting, but think about it this way: The developer wants to ensure the array has at least one element. You could even write count($array) >= 1. Well, or obviously count($array) > 0. Isn't that also counting? Or you want to express that the array doesn't contain any elements: count($array) === 0. I think it's a valid use-case for count() and at least for me preferred over empty().
To summarize, I think it's clear the clean way would be "=== []" or "\count() === 0".
I agree, even if it was not my point. I wanted to simplify a bad practice (implicit cast from int to bool via count()) into a simpler bad practice (implicit cast from array to bool). But it's not what this plugin should aim for.
Instead of my original issue, Maybe we should implement an inspection to suggest one of those: "=== []" "\count() === 0"
However, it's not a performance or a simplification anymore. It would fit more in "Code Style"
yes and no. while if(count($array) === 1) is counting (aka, you need it to have one element), if(count($array)) is not counting (aka, you don't give a damn how many items are in array). for the second one, you should't be using count
Instead of my original issue, Maybe we should implement an inspection to suggest one of those: "=== []" "\count() === 0"
I'm 👍 for this.
However, it's not a performance or a simplification anymore. It would fit more in "Code Style"
I think this is related more to type safety of the code rather than style.
yes and no. while if(count($array) === 1) is counting (aka, you need it to have one element), if(count($array)) is not counting (aka, you don't give a damn how many items are in array). for the second one, you should't be using count
But your second example is just the implicit type cast again. I think we all agree that's a bad practise. See my examples in the comment before... if(count($array) > 0) { } is counting, isn't it?
so what we want from inspection now? that you shouldn't compare it with 0 in any way and with 1 some of the time?
As I can see, there are no much benefits from having it. empty() would be an option, but it not getting many votes. Projecting this to a possible implementation, the community is mostly going to reject it, so I tend to not implementing it.
Closing, thank you all for votes and comment - that make my life so much easier!
Just for the record, I quickly checked the OPCodes using vld for the mentioned cases on PHP 7.3.3:
PHP count() variant
<?php declare(strict_types=1);
$strictly_an_array = [1, 2, 3, 4, 5];
if (\count($strictly_an_array)) echo "ARRAY";
OPCodes count() variant
Finding entry points
Branch analysis from position: 0
2 jumps found. (Code = 43) Position 1 = 6, Position 2 = 8
Branch analysis from position: 6
1 jumps found. (Code = 62) Position 1 = -2
Branch analysis from position: 8
filename: /Users/hollodotme/array1.php
function name: (null)
number of ops: 9
compiled vars: !0 = $strictly_an_array
line #* E I O op fetch ext return operands
-------------------------------------------------------------------------------------
1 0 E > NOP
3 1 EXT_STMT
2 ASSIGN !0, <array>
5 3 EXT_STMT
4 COUNT ~2 !0
5 > JMPZ ~2, ->8
6 > EXT_STMT
7 ECHO 'ARRAY'
6 8 > > RETURN 1
branch: # 0; line: 1- 5; sop: 0; eop: 5; out0: 6; out1: 8
branch: # 6; line: 5- 6; sop: 6; eop: 7; out0: 8
branch: # 8; line: 6- 6; sop: 8; eop: 8; out0: -2
path #1: 0, 6, 8,
path #2: 0, 8,
Comparison variant short array syntax
<?php declare(strict_types=1);
$strictly_an_array = [1, 2, 3, 4, 5];
if ([] !== $strictly_an_array) echo "ARRAY";
OPCodes comparison variant short array syntax
Finding entry points
Branch analysis from position: 0
2 jumps found. (Code = 43) Position 1 = 6, Position 2 = 8
Branch analysis from position: 6
1 jumps found. (Code = 62) Position 1 = -2
Branch analysis from position: 8
filename: /Users/hollodotme/array1.php
function name: (null)
number of ops: 9
compiled vars: !0 = $strictly_an_array
line #* E I O op fetch ext return operands
-------------------------------------------------------------------------------------
1 0 E > NOP
3 1 EXT_STMT
2 ASSIGN !0, <array>
5 3 EXT_STMT
4 IS_NOT_IDENTICAL ~2 !0, <array>
5 > JMPZ ~2, ->8
6 > EXT_STMT
7 ECHO 'ARRAY'
6 8 > > RETURN 1
branch: # 0; line: 1- 5; sop: 0; eop: 5; out0: 6; out1: 8
branch: # 6; line: 5- 6; sop: 6; eop: 7; out0: 8
branch: # 8; line: 6- 6; sop: 8; eop: 8; out0: -2
path #1: 0, 6, 8,
path #2: 0, 8,
Comparison variant short long syntax
<?php declare(strict_types=1);
$strictly_an_array = [1, 2, 3, 4, 5];
if (array() !== $strictly_an_array) echo "ARRAY";
OPCodes comparison variant long array syntax
Finding entry points
Branch analysis from position: 0
2 jumps found. (Code = 43) Position 1 = 6, Position 2 = 8
Branch analysis from position: 6
1 jumps found. (Code = 62) Position 1 = -2
Branch analysis from position: 8
filename: /Users/hollodotme/array1.php
function name: (null)
number of ops: 9
compiled vars: !0 = $strictly_an_array
line #* E I O op fetch ext return operands
-------------------------------------------------------------------------------------
1 0 E > NOP
3 1 EXT_STMT
2 ASSIGN !0, <array>
5 3 EXT_STMT
4 IS_NOT_IDENTICAL ~2 !0, <array>
5 > JMPZ ~2, ->8
6 > EXT_STMT
7 ECHO 'ARRAY'
6 8 > > RETURN 1
branch: # 0; line: 1- 5; sop: 0; eop: 5; out0: 6; out1: 8
branch: # 6; line: 5- 6; sop: 6; eop: 7; out0: 8
branch: # 8; line: 6- 6; sop: 8; eop: 8; out0: -2
path #1: 0, 6, 8,
path #2: 0, 8,
Comparison variant variable only
<?php declare(strict_types=1);
$strictly_an_array = [1, 2, 3, 4, 5];
if ($strictly_an_array) echo "ARRAY";
OPCodes comparison variant variable only
Finding entry points
Branch analysis from position: 0
2 jumps found. (Code = 43) Position 1 = 5, Position 2 = 7
Branch analysis from position: 5
1 jumps found. (Code = 62) Position 1 = -2
Branch analysis from position: 7
filename: /Users/hollodotme/array1.php
function name: (null)
number of ops: 8
compiled vars: !0 = $strictly_an_array
line #* E I O op fetch ext return operands
-------------------------------------------------------------------------------------
1 0 E > NOP
3 1 EXT_STMT
2 ASSIGN !0, <array>
5 3 EXT_STMT
4 > JMPZ !0, ->7
5 > EXT_STMT
6 ECHO 'ARRAY'
6 7 > > RETURN 1
branch: # 0; line: 1- 5; sop: 0; eop: 4; out0: 5; out1: 7
branch: # 5; line: 5- 6; sop: 5; eop: 6; out0: 7
branch: # 7; line: 6- 6; sop: 7; eop: 7; out0: -2
path #1: 0, 5, 7,
path #2: 0, 7,
empty() variant
<?php declare(strict_types=1);
$strictly_an_array = [1, 2, 3, 4, 5];
if (!empty($strictly_an_array)) echo "ARRAY";
OPCodes empty() variant
Finding entry points
Branch analysis from position: 0
2 jumps found. (Code = 43) Position 1 = 7, Position 2 = 9
Branch analysis from position: 7
1 jumps found. (Code = 62) Position 1 = -2
Branch analysis from position: 9
filename: /Users/hollodotme/array1.php
function name: (null)
number of ops: 10
compiled vars: !0 = $strictly_an_array
line #* E I O op fetch ext return operands
-------------------------------------------------------------------------------------
1 0 E > NOP
3 1 EXT_STMT
2 ASSIGN !0, <array>
5 3 EXT_STMT
4 ISSET_ISEMPTY_CV ~2 !0
5 BOOL_NOT ~3 ~2
6 > JMPZ ~3, ->9
7 > EXT_STMT
8 ECHO 'ARRAY'
6 9 > > RETURN 1
branch: # 0; line: 1- 5; sop: 0; eop: 6; out0: 7; out1: 9
branch: # 7; line: 5- 6; sop: 7; eop: 8; out0: 9
branch: # 9; line: 6- 6; sop: 9; eop: 9; out0: -2
path #1: 0, 7, 9,
path #2: 0, 9,
count() variant with comparison
<?php declare(strict_types=1);
$strictly_an_array = [1, 2, 3, 4, 5];
if (\count($strictly_an_array) !== 0) echo "ARRAY";
OPCodes count() variant with comparison
Finding entry points
Branch analysis from position: 0
2 jumps found. (Code = 43) Position 1 = 7, Position 2 = 9
Branch analysis from position: 7
1 jumps found. (Code = 62) Position 1 = -2
Branch analysis from position: 9
filename: /Users/hollodotme/array1.php
function name: (null)
number of ops: 10
compiled vars: !0 = $strictly_an_array
line #* E I O op fetch ext return operands
-------------------------------------------------------------------------------------
1 0 E > NOP
3 1 EXT_STMT
2 ASSIGN !0, <array>
5 3 EXT_STMT
4 COUNT ~2 !0
5 IS_NOT_IDENTICAL ~3 ~2, 0
6 > JMPZ ~3, ->9
7 > EXT_STMT
8 ECHO 'ARRAY'
6 9 > > RETURN 1
branch: # 0; line: 1- 5; sop: 0; eop: 6; out0: 7; out1: 9
branch: # 7; line: 5- 6; sop: 7; eop: 8; out0: 9
branch: # 9; line: 6- 6; sop: 9; eop: 9; out0: -2
path #1: 0, 7, 9,
path #2: 0, 9,
in_array() + count() with comparison variant
<?php declare(strict_types=1);
$strictly_an_array = [1, 2, 3, 4, 5];
if (\is_array($strictly_an_array) && \count($strictly_an_array) !== 0) echo "ARRAY";
OPCodes comparison variant variable only
Finding entry points
Branch analysis from position: 0
2 jumps found. (Code = 46) Position 1 = 6, Position 2 = 9
Branch analysis from position: 6
2 jumps found. (Code = 43) Position 1 = 10, Position 2 = 12
Branch analysis from position: 10
1 jumps found. (Code = 62) Position 1 = -2
Branch analysis from position: 12
Branch analysis from position: 9
filename: /Users/hollodotme/array1.php
function name: (null)
number of ops: 13
compiled vars: !0 = $strictly_an_array
line #* E I O op fetch ext return operands
-------------------------------------------------------------------------------------
1 0 E > NOP
3 1 EXT_STMT
2 ASSIGN !0, <array>
5 3 EXT_STMT
4 TYPE_CHECK 128 ~2 !0
5 > JMPZ_EX ~2 ~2, ->9
6 > COUNT ~3 !0
7 IS_NOT_IDENTICAL ~4 ~3, 0
8 BOOL ~2 ~4
9 > > JMPZ ~2, ->12
10 > EXT_STMT
11 ECHO 'ARRAY'
6 12 > > RETURN 1
branch: # 0; line: 1- 5; sop: 0; eop: 5; out0: 6; out1: 9
branch: # 6; line: 5- 5; sop: 6; eop: 8; out0: 9
branch: # 9; line: 5- 5; sop: 9; eop: 9; out0: 10; out1: 12
branch: # 10; line: 5- 6; sop: 10; eop: 11; out0: 12
branch: # 12; line: 6- 6; sop: 12; eop: 12; out0: -2
path #1: 0, 6, 9, 10, 12,
path #2: 0, 6, 9, 12,
path #3: 0, 9, 10, 12,
path #4: 0, 9, 12,
Personally, I use [] === $array as it is a strict type comparison and an empty check, but that is just my taste and I don't think there is the need for an inspection.
Okay, I've changed my mind (as did the more careful reading).
We'll head with if ($strictlyArray !== []), for language level 5.4+, where short array syntax is supported.
That will be readable, will have one of the lowest amounts of opcodes and the option fits my personal experience.